Fixed #27039 -- Fixed fallback to model field default in model forms. #7068
Conversation
timgraham
force-pushed
the
27039
branch
3 times, most recently
from
472844f to
3b78010
Compare
|
@tomchristie, could you review? |
|
Sure thing. Brief look all seems good, but will go through more thoroughly shortly. |
| @@ -481,6 +481,8 @@ def boolean_check(v): | |||
|
|
|||
|
|
|||
| class CheckboxInput(Widget): | |||
| dont_use_model_field_default_for_empty_data = True | |||
There was a problem hiding this comment.
Should we also include similar comment here?
|
Looks good. |
|
I'm experiencing a strange hiccup in the Django admin since upgrading to v1.10.1 that I think might be related to this issue. If it's not, I'm not sure what else could be causing it but it doesn't happen if I roll Django back to v1.9.9. Either way, I think you are the ones who can help demystify this for me. I have a field on a model set like so: Any time a different value is entered for this field in the Django admin and saved, the field value on the object does not change. It remains the value it was set to before. If I remove the default value, the field saves normally. Any insight as to why this might be? |
|
See #7217. |
|
Awesome, thanks much! |
| @@ -52,6 +52,11 @@ def construct_instance(form, instance, fields=None, exclude=None): | |||
| continue | |||
| if exclude and f.name in exclude: | |||
| continue | |||
| # Leave defaults for fields that aren't in POST data, except for | |||
There was a problem hiding this comment.
@timgraham This caused me a bit of a debugging headache. I expected that if I did something like this:
def clean(self):
self.cleaned_data['some_field'] = 'some_value'...that would be sufficient to set what was ultimately saved to the database. But I found that some_field was always being saved with its default value. It took me a while to figure out why, from looking at the Django source. Now I'm doing this, with success:
def __init__(self, *args, **kwargs):
super(MyForm, self).__init__(*args, **kwargs)
self.data = self.data.copy()
self.data['some_field'] = 'some_value'I'm just curious what the reasoning is behind the rule you implemented here?
There was a problem hiding this comment.
Your expectation seems reasonable. If there's a way to fix that while keeping backwards compatibility, feel free to offer a patch.
There was a problem hiding this comment.
Do Django's automated tests ensure backward compatibility? If not I probably don't have enough familiarity with Django to do so. Glad I'm not taking crazy pills, though. :)
|
If you have the tests passing that's a good start. Someone will review your patch as well. |
|
@timgraham Here we go: #11433 |
https://code.djangoproject.com/ticket/27039