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$


Change History (0)

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