Opened 8 months ago

Closed 3 months 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: adambrenecki 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 Changed 8 months ago by dibrovsd@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 8 months ago by dibrovsd@…

  • 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 follow-up: Changed 7 months ago by 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?

comment:4 follow-up: Changed 7 months ago by 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?

comment:5 Changed 7 months ago by adambrenecki

  • Cc adambrenecki added

comment:6 in reply to: ↑ 3 Changed 7 months ago by dibrovsd@…

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)

comment:7 in reply to: ↑ 4 Changed 7 months ago by dibrovsd@…

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 Changed 4 months ago by timgraham

@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 Changed 3 months ago by timgraham

  • Resolution set to needsinfo
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top