Issue #29 resolved

PercentValue multiplied by 100 if other validation fails

Hynek Cernoch
created an issue

How to reproduce:

1) Create a form with two values: a PercentValue field and any field with a validation rule, e.g. PositiveIntegerValue.

2) Fill the form with some meaningful values: {{{ PercentValue: [10]% PositiveIntegerValue: [5] }}}

3) Try to enter a negative number to PositiveIntegerValue, press Enter. You get: {{{ PercentValue: [1000]% (x 100, without warning) PositiveIntegerValue: [-5] (Validation Err) }}}

4) Fix only the PositiveIntegerValue which had a red Validation message in the previous step. Press Enter. You get: {{{ PercentValue: [100000]% (x 10000, Error msg) PositiveIntegerValue: [5] (ok) }}}

The class PercentValue has been never used by Satchmo in the last 46 months, and not documented, so it has not been tested yet enough.

Comments (3)

  1. Hynek Cernoch reporter
    • changed status to open

    PercentValue does not work good and can not be repaired without adding aproximately three methods to PercentValue and duplicating modified parts of Django. (django.core.validators.MaxValueValidator, django.core.validators.MinValueValidator, django.form.DecimalField.init)

    Existing PercentValue is based on a cheat that edited value and python value are divided and multiplied back by 100. Edited value is in the range 0-100. Python value is in the range 0-1. Range validation is based on Python value and reports bad values for Python Value. Existing code is omitting divide by 100 between editor and python value, which produce an issue when validation of an other field fails therefore PercentValue can not save the value and a form only converts the value to python by unmodified forms.DecimalValue.to_python which is not complementar to PercentValue.to_editor. If it is fixed, the validation become incorrect and must be in the principle incorrect.

    PercentValue can be easy replaced by DecimalValue(... min_value=0, max_value=100, max_decimal_places=2) and the output value can be divided by 100 in the user application.

    please decision
    I think that PercentValue can be removed from livesettings completely because due to issue #17 nobody did use it for long time. It was never used in Satchmo. Other possibility is to remove PercentValue only from all documentation and tests and to keep its buggy class with a warning some time only for backward compatibility with people who started to use it in the last half year.

  2. Log in to comment