Opened 11 years ago
Last modified 11 years ago
#24395 closed Bug
Cannot reference FK relation from inline ModelForm.save() — at Version 1
| 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 (last modified by )
Hi devs,
This is directly related to the closed ticket #24325 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$
Change History (1)
comment:1 by , 11 years ago
| Description: | modified (diff) |
|---|---|
| Severity: | Normal → Release blocker |
| Summary: | Fixing #24325 (Cannot reference FK relation from inline ModelForm.save()) → Cannot reference FK relation from inline ModelForm.save() |
| Triage Stage: | Unreviewed → Accepted |
| Version: | master → 1.8alpha1 |
Do you have time to add a test now?
tests/model_formsetsis probably a good place. If not, I can pick this up.