[Django] #29474: BaseInlineFormset.save_new() more complicated than it needs to be?

[Django]

#29474: BaseInlineFormset.save_new() more complicated than it needs to be?
------------------------------------------------+------------------------
Reporter: ljsjl | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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

--
Ticket URL: <https://code.djangoproject.com/ticket/29474>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#29474: Simply BaseInlineFormset.save_new() with call to super()
-------------------------------------+-------------------------------------
Reporter: ljsjl | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* has_patch: 0 => 1
* stage: Unreviewed => Ready for checkin

Comment:

The simplification looks possible since
4c2f546b55c029334d22e69bb29db97f9356faa3.
[https://github.com/django/django/pull/10034 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/29474#comment:1>

[Django]

#29474: Simply BaseInlineFormset.save_new() with call to super()
-------------------------------------+-------------------------------------

Reporter: ljsjl | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"236bcfea429d6b0c6b3dcd102c09d3a4c75b84d8" 236bcfea]:
{{{
#!CommitTicketReference repository=""
revision="236bcfea429d6b0c6b3dcd102c09d3a4c75b84d8"
Fixed #29474 -- Simplified BaseInlineFormset.save_new().
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29474#comment:2>