#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.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/048.dbb937615feb509bd76df133f85205a6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to