Opened 6 years ago

Closed 6 years ago

#29474 closed Cleanup/optimization (fixed)

Simply BaseInlineFormset.save_new() with call to super()

Reported by: ljsjl Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Looking at save_new() in BaseInlineFormset it seems it can be replaced with e.g.

--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -940,17 +940,7 @@ class BaseInlineFormSet(BaseModelFormSet):
         # form (it may have been saved after the formset was originally
         # instantiated).
         setattr(form.instance, self.fk.name, self.instance)
-        # Use commit=False so we can assign the parent key afterwards, then
-        # save the object.
-        obj = form.save(commit=False)
-        pk_value = getattr(self.instance, self.fk.remote_field.field_name)
-        setattr(obj, self.fk.get_attname(), getattr(pk_value, 'pk', pk_value))
-        if commit:
-            obj.save()
-        # form.save_m2m() can be called via the formset later on if commit=False
-        if commit and hasattr(form, 'save_m2m'):
-            form.save_m2m()
-        return obj
+        return super().save_new(form, commit=commit)
 
     def add_fields(self, form, index):
         super().add_fields(form, index)

This passes the existing test suite so suggests yes, or that the test suite is missing important cases.

From my shaky grasp of model field internals the extra lines set the value of the field attname (e.g. user_id) attribute, but this is already set by the earlier setattr call updating form.instance. I feel like I'm missing something here but it seems to work so...?

Cheers
LJS

Change History (2)

comment:1 by Tim Graham, 6 years ago

Has patch: set
Summary: BaseInlineFormset.save_new() more complicated than it needs to be?Simply BaseInlineFormset.save_new() with call to super()
Triage Stage: UnreviewedReady for checkin

The simplification looks possible since 4c2f546b55c029334d22e69bb29db97f9356faa3. PR

comment:2 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 236bcfea:

Fixed #29474 -- Simplified BaseInlineFormset.save_new().

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