Opened 17 years ago
Closed 17 years ago
#6302 closed (fixed)
FileField/ImageField still requires a value when ModelForm is called when instance != None
| Reported by: | Owned by: | Brian Rosner | |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Keywords: | ModelForm FileField required | |
| Cc: | tom@… | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
This would make the users upload a new picture every time they change the form. A sensible default would be valued, and a custom handler to decide specific behavior is, I think, a must.
Attachments (9)
Change History (27)
comment:1 by , 17 years ago
comment:2 by , 17 years ago
Ok, I have verified this is occuring and it is not user error. The FieldField model field tries to set its formfield to require=False when there is initial data. Well, that worked well for for_for_instance , but now that ModelForm calls formfield before it gets the initial data from the model instance it will never be marked as False. I will have a patch whipped up shortly.
comment:3 by , 17 years ago
| Owner: | changed from to |
|---|
comment:4 by , 17 years ago
| Status: | new → assigned |
|---|
comment:5 by , 17 years ago
The discussion on django-dev: http://groups.google.com/group/django-developers/browse_frm/thread/68bd9110af6ab591
by , 17 years ago
| Attachment: | newforms-admin_filefield_initial_fix.diff added |
|---|
Possible fix, not sure if this is the correct way to do this
comment:6 by , 17 years ago
I haven't looked at the functionality but from oyvind's I would just say it should probably check that initial isn't none before setting required to false.
comment:7 by , 17 years ago
Just noticed that myself, but testing for empty values makes it fail for some reason.
by , 17 years ago
| Attachment: | pass_initial_through_messy_example.diff added |
|---|
Patch against newforms-admin, don't like all parts of it
comment:8 by , 17 years ago
Forgot to say, latest patch is against trunk.
Seems to be working
Should value_from_datadict handle initial too instead of making a own value_from_initial on the !Input! widget??
by , 17 years ago
| Attachment: | pass_initial_through_with_tests_2.diff added |
|---|
this might be overkill, but initial data is now passed to field.clean. should FileField be special cased?
by , 17 years ago
| Attachment: | pass_initial_through_with_tests_3.diff added |
|---|
special cased to FileField
comment:9 by , 17 years ago
Like the spechialcased solution, less backwardas compatibility issues too.
by , 17 years ago
| Attachment: | pass_initial_through_with_tests_4.diff added |
|---|
Testing for takes_initial instead of using isinstance for the FileField
by , 17 years ago
| Attachment: | pass_initial_through_with_tests_5.diff added |
|---|
Fixed a problem with combofield and filefield
comment:10 by , 17 years ago
Latest patch does allow FileFields with MultiValueField , I and brosner think this is a edge case.
Since takes_initial can be added to fields this is left as a exercise to the user.
by , 17 years ago
| Attachment: | pass_initial_through_with_tests_mulitvaluefield.diff added |
|---|
MultiValueField works too, is this overkill?
comment:12 by , 17 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
I'm going to check in http://code.djangoproject.com/attachment/ticket/6302/pass_initial_through_with_tests_3.diff mostly as is. Please open another ticket for fixing ComboField, etc. I'm not entirely sure about the takes_initial thing yet. It seems like over-engineering, but more isinstance checks make me cringe as well.
comment:13 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:14 by , 17 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
This fix appears not to work with ImageFields; I'm writing a test and fix.
comment:15 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
comment:16 by , 17 years ago
Ah, good catch Jacob. I looked at the code you committed and could have sworn that was in my patch, but turns out for some reason I removed it from my new patch that special cased FileField. Should have had a test for ImageField. :)
comment:17 by , 17 years ago
| Cc: | added |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
Still doesn't work for me in latest SVN. I am using RemovableImageField from the following code:
http://www.djangosnippets.org/snippets/469/
In RemovableFileFormField's init method, kwargs doesn't contain an initial attribute.
This worked perfectly with form_for_instance().
comment:18 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
If you looked at the code that was committed you would have seen that the initial data is not passed in through the kwargs in __init__. It is passed to the field clean method. Any further problems should really be put in a new ticket as this ticket as described was fixed. Thanks.
Can you please provide more detailed information on how actually this is not working? It is probably a user error in this case and should be brought up in django-users. As long as the
FileFieldorImageFieldhas a value it will be marked as not required.