Opened 10 years ago

Closed 9 years ago

#22969 closed Bug (needsinfo)

Forms show_hidden_initial not is_valid() and fix => lost changes in valid fields

Reported by: dibrovsd@… Owned by: nobody
Component: Forms Version: 1.6
Severity: Normal Keywords: Form show_hidden_initial changed_data
Cc: Adam Brenecki Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

if use show_hidden_initial as initial and have some error fields form.is_valid() == False
django set initial other (valid) fields from data
and in next step (user fix form errors) django lost changed valid fields from changed_data

from django.forms import Form, CharField

class MyForm(Form):

    req_field = CharField(show_hidden_initial=True, required=True)
    notreq_field = CharField(show_hidden_initial=True)

data = {
    'req_field': '', 'initial-req_field': '',
    'notreq_field': '12', 'initial-notreq_field': '',
}

# i can't use initial from database.
# other user can change some field and django revert unchange field back
# user 1: open form (field_1 is "1")
# user 2: change field_1 to "2"
# user 1 change field_2 to "1111" and 
# user 1 send unchange (field_1 is "1") initial value field_1 == "2" (user 2 change it)
# django mark field_1 as change from 2 to 1 back 

form = MyForm(data=data)
assert(form.is_valid(), False)  # req_field error

# show invalid form and set: initial-notreq_field = 12 (why???) 
# in next send form this field not in changed_data
for field in form.visible_fields():
    print field
# django action: set initial-notreq_field = 12
data['initial-notreq_field'] = '12'  # 

# user action: fix error
data['req_field'] = '12'
form = MyForm(data=data)
print form.is_valid()

for change_field in form.changed_data:
    print 'change', change_field, form.cleaned_data[change_field]
# oops! lost change in notreq_field

Change History (9)

comment:1 by dibrovsd@…, 10 years ago

Other example

view

def test(request):
    u""" for debug and test's """

    from django.forms import Form, CharField

    class MyForm(Form):

        req_field = CharField(show_hidden_initial=True, required=True)
        notreq_field = CharField(show_hidden_initial=True, required=False)

    if request.method == 'POST':
        form = MyForm(data=request.POST)
        if form.is_valid():
            for field_name in form.changed_data:
                print field_name
    else:
        form = MyForm()

    return render(request, 'test.html', {'form': form})

template

<form method='post'>
    {% csrf_token %} 
    {{form.as_p}}
    <input type="submit">
</form>

script:

  1. Load page, set notreq_field = '12' and submit form
  2. Fix error (set req_field = '12') and submit form
  3. console print is "req_field"

comment:2 by dibrovsd@…, 10 years ago

Has patch: set
Patch needs improvement: set

fix:
django.forms.forms.BoundField

add method

def value_for_hidden(self, widget):
    u""" 
    when form is bound, then get value from initial-field form.data (not from field)
    otherwise: if form is not valid, initial_value copy from value 
               and lost field from changed_data when send repaired form
    """
    if self.form.is_bound:
        data = widget.value_from_datadict(self.form.data, self.form.files, self.html_initial_name)
        return self.field.prepare_value(data)
    else:
        return self.value()

fix method as_widget (end of method)

    name = self.html_name
    value = self.value()
else:
    name = self.html_initial_name
    value = self.value_for_hidden(widget)

return widget.render(name, value, attrs=attrs)

comment:3 by Adam Brenecki, 10 years ago

So, just to sum up and check if I'm on the right track, correct me if I'm misunderstanding you: Given a Form with a FormField where show_hidden_initial is set, if that field is modified and the form is submitted with an error, the initial- field is updated. What you're saying is that it shouldn't be, so that in subsequent submissions of the form it shows up as a changed_field. Right?

comment:4 by Adam Brenecki, 10 years ago

So, since show_hidden_initial isn't a part of Django's public API, but rather used internally by other parts of Django, and therefore it probably shouldn't be called from user code.

Are you using this from somewhere else inside Django itself, or from third-party/user code?

comment:5 by Adam Brenecki, 10 years ago

Cc: Adam Brenecki added

in reply to:  3 comment:6 by dibrovsd@…, 10 years ago

Replying to adambrenecki:

So, just to sum up and check if I'm on the right track, correct me if I'm misunderstanding you: Given a Form with a FormField where show_hidden_initial is set, if that field is modified and the form is submitted with an error, the initial- field is updated. What you're saying is that it shouldn't be, so that in subsequent submissions of the form it shows up as a changed_field. Right?

Not right.
if 2 FormField set show_hidden_initial
and first field raise ValidationError, initial hidden input (for second field) get value from visible input (not initial- widget)
if Form(initial=None, data=request.POST)

in reply to:  4 comment:7 by dibrovsd@…, 10 years ago

Replying to adambrenecki:

So, since show_hidden_initial isn't a part of Django's public API, but rather used internally by other parts of Django, and therefore it probably shouldn't be called from user code.

Are you using this from somewhere else inside Django itself, or from third-party/user code?

This is my code. i read django source-code.
Without this parameter, it is impossible to write a serious application.
Two operators will overwrite each other's changes, when editing an object.
If they edit different fields, it should not interfere with each other.
But without show_hidden_initial, the second operator will always overwrite any changes the first statement, even if it is not explicitly changed the field.

Example:
form has 2 field.
if not use show_hidden_initial, Form see has_change() as compare initial and data params.
initial - value from database before save cleaned_data
0) field_1 = 1 and field_2 = 1
1) User 1 open form
2) User 2 open form
3) User 1 change field_1 to "2" and save form (in database field_1 = "2")
4) User 2 change field_2 to "2" and save form:
initial for field_1 == "2" (user 2 changes) and data for field_1 == "1" (this value is obsolete)
form mark field_1 as changed from "2" to "1" and restore field_1 value and override changes

It is very important
Without show_hidden_initial,
In a parallel environment, the second user will always overwrite all changes first

comment:8 by Tim Graham, 9 years ago

@dibrovsd, could you provide a patch with your proposed changes and a regression test for Django's test suite? That will be helpful in allowing us to better understand the issue.

comment:9 by Tim Graham, 9 years ago

Resolution: needsinfo
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top