Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5234 closed (invalid)


Reported by: phil.django@… Owned by: Adrian Holovaty
Component: Forms Version: master
Severity: Keywords: prefix
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


I think there's a bug in the newforms library:

from django import newforms as forms

class F(forms.Form):

field = forms.BooleanField(label='boo')

form1 = F({'field':True}, prefix='somePrefix')
# Outputs some error:
print form1.as_p()

# Output nothing instead of True.

Patch attached, and working for me.

Attachments (1)

django_prefix_bug_patch.diff (560 bytes) - added by anonymous 11 years ago.

Download all attachments as: .zip

Change History (6)

Changed 11 years ago by anonymous

comment:1 Changed 11 years ago by Collin Grady <cgrady@…>

Resolution: invalid
Status: newclosed

That isn't a bug - if you assign a prefix to the form, it expects that prefix on the data you bind to it - it's so you can have multiple forms with matching field names, but still keep their form data separate in POST.

So you should be passing it {'somePrefix-field': True} if you want to bind that data to that prefixed form.

comment:2 Changed 11 years ago by Collin Grady <cgrady@…>

Er, that should read {'somePrefix-field': 'on'} since the data you bind to it has to match POST data, and checkboxes post through as 'on', not a python True.

comment:3 Changed 11 years ago by phil.django@…

Summary: form prefix buga

Thank you for your comments.

However please consider again the fact that adding the prefix to field names is required. This adds uncessary complexity to the code, and i don't see any reason why it should be that way. The prefix logic takes care of matching the requests with the field instances in the code, and having different Form.form instances is enough for the distinction of all the fields, i'm using this for my application.

About the second comment (boolean:"on"), this isn't required and shouldn't be neither imho (the BooleanField class takes tare of it). This would be against the principle that the logic part sould be independent of the presentation layer.

I let this ticket closed, but i still think this should be reopned. Please consider that option!

comment:4 Changed 11 years ago by Collin Grady <cgrady@…>

Adding a prefix is not required, unless you're using multiple forms.

If you are using multiple forms, then yes it's required, but it's also required to be on the data you bind to them, since that represents POST data, and will correspond to the field names.

If you're trying to set initial data instead of binding the form, then what you're actually doing wrong is passing the data in alone - you need to specify initial={'field': True} for initial data. Passing the dict in on its own means it represents POST data, which must contain the prefix to match the HTML elements generated.

Regarding the 'on' versus True, even if it manages to work right with True, it's better to stick with what POST data would actually look like, to avoid odd issues :)

comment:5 Changed 11 years ago by phil.django@…

Thank you Collin, now i see my mistake (not using the initial parameter). Sorry for the trouble!

Note: See TracTickets for help on using tickets.
Back to Top