Opened 10 years ago
Last modified 10 years ago
#24395 closed Bug
Fixing #24325 (Cannot reference FK relation from inline ModelForm.save()) — at Initial Version
Reported by: | Stanislas Guerra | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.8alpha1 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi devs,
This is directly related to the closed ticket #24235 by Tim.
The problem is that the instance of the related object (an Author
instance in my example) in the formset's forms instances (BookForm
) is not up-to-date.
In the admin, the call stack is:
# django.contrib.admin.options.py ModelAdmin.changeform_view(): ... new_object = self.save_form(request, form, change=not add) ... self.save_model(request, new_object, form, not add) self.save_related(...) [1] def save_related(): self.save_formset(request, form, formset, change=change) [2] def save_formset(self, request, form, formset, change): formset.save() [3]
At this point [1], the main form has already been saved and the new_object
(an Author
instance) is in the database.
This new_object
instance is correctly bound to the formset(s) instance(s) but not to the formsets' forms.
IMAO this is a bug.
Since the new_object
is saved in the database and all the forms have been validated, there is no reason to have an old instance attached to the ModelForm.instance attribute (which is a Book
instance in my example).
A basic patch could be something like that:
diff --git a/django/forms/models.py b/django/forms/models.py index 98f84a0..40217f4 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -649,6 +649,7 @@ class BaseModelFormSet(BaseFormSet): for form in self.saved_forms: form.save_m2m() self.save_m2m = save_m2m + self.set_forms_related_object(self.instance) return self.save_existing_objects(commit) + self.save_new_objects(commit) save.alters_data = True @@ -656,6 +657,11 @@ class BaseModelFormSet(BaseFormSet): def clean(self): self.validate_unique() + def set_forms_related_object(self, obj): + rel_name = self.fk.name + for form in self.forms: + setattr(form.instance, rel_name, obj) + def validate_unique(self): # Collect unique_checks and date_checks to run from all the forms. all_unique_checks = set()
What do you guys think ?
This patch passed most of admin tests. But I got a segfault with the last master checkout either I use the patch or not.
stan@stanislrrasimac:tests$ PYTHONPATH=..:$PYTHONPATH python runtests.py admin_views -v 2 Testing against Django installed in '/Users/stan/src/Django/repos/django/django/django' Importing application admin_views Creating test database for alias 'default' (':memory:')... ... test_logout_and_password_change_URLs (admin_views.tests.AdminViewBasicTest) ... ok test_multiple_sort_same_field (admin_views.tests.AdminViewBasicTest) ... ok test_named_group_field_choices_change_list (admin_views.tests.AdminViewBasicTest) ... ok test_named_group_field_choices_filter (admin_views.tests.AdminViewBasicTest) ... ok test_popup_add_POST (admin_views.tests.AdminViewBasicTest) ... ok test_popup_dismiss_related (admin_views.tests.AdminViewBasicTest) ... ok test_proxy_model_content_type_is_used_for_log_entries (admin_views.tests.AdminViewBasicTest) ... ok test_relation_spanning_filters (admin_views.tests.AdminViewBasicTest) ... ok test_resolve_admin_views (admin_views.tests.AdminViewBasicTest) ... ok test_sort_indicators_admin_order (admin_views.tests.AdminViewBasicTest) ... ok Segmentation fault: 11 stan@stanislrrasimac:tests$