Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#33527 closed Cleanup/optimization (fixed)

Remove unnecessary code in ModelAdmin._changeform_view().

Reported by: Maxim Danilov Owned by: Dulalet
Component: contrib.admin Version: 4.0
Severity: Normal Keywords: modeladmin, inlinemodel
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Tim Graham)

In #33111, the following code was added:

form = ModelForm(request.POST, request.FILES, instance=obj)
formsets, inline_instances = self._create_formsets(request, form.instance if add else obj, change=not add)

it is all ok, but if we have:

form = ModelForm(request.POST, request.FILES, instance=obj)

this is superabundant:

form.instance if add else obj

I think, it should be:

form = ModelForm(request.POST, request.FILES, instance=obj)
formsets, inline_instances = self._create_formsets(request, form.instance, change=not add)
# or # formsets, inline_instances = self._create_formsets(request, obj, change=not add)

The same changes is possible to made in code-block for request.GET

Change History (10)

comment:1 by Tim Graham, 4 years ago

Cc: Mariusz Felisiak removed
Description: modified (diff)

I don't see any test failures with the following patch, so if it's needed, perhaps a regression test would be useful.

  • django/contrib/admin/options.py 

    diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index b1aa610d43..64be087bf1 100644
    a b class ModelAdmin(BaseModelAdmin):  
    17871787            form = ModelForm(request.POST, request.FILES, instance=obj)
    17881788            formsets, inline_instances = self._create_formsets(
    17891789                request,
    1790                 form.instance if add else obj,
     1790                form.instance,
    17911791                change=not add,
    17921792            )
    17931793            form_validated = form.is_valid()

comment:2 by Mahdi Shirani, 4 years ago

Owner: changed from nobody to Mahdi Shirani
Status: new → assigned

comment:3 by Mahdi Shirani, 4 years ago

Resolution: → fixed
Status: assigned → closed

def get(self, request,obj):
form = ModelForm(request.POST, request.FILES, instance=obj)
if form.instance :
formsets, inline_instances = self._create_formsets(request, form.instance, change=not add)
elif obj :
formsets, inline_instances = self._create_formsets(request, obj, change=not add)

comment:4 by Tim Graham, 4 years ago

Resolution: fixed
Status: closed → new

The ticket isn't fixed until changes are merged.

comment:5 by Mariusz Felisiak, 4 years ago

Summary: remove unnecessary code from patch before in ModelAdmin inlines save. → Remove unnecessary code in ModelAdmin._changeform_view().
Triage Stage: Unreviewed → Accepted

Agreed, if add else obj is unnecessary. Maxim, would you like to prepare a patch?

comment:6 by Dulalet, 4 years ago

Owner: changed from Mahdi Shirani to Dulalet
Status: new → assigned

Hi all. I am new to Open Source and I want to make my first contribution to Django. I'd like to prepare a patch for this issue if no one minds. Thank you!

comment:7 by Mariusz Felisiak, 4 years ago

Has patch: set
Triage Stage: Accepted → Ready for checkin

​PR

comment:8 by GitHub <noreply@…>, 4 years ago

Resolution: → fixed
Status: assigned → closed

In e0442a62:

Fixed #33527 -- Removed unnecessary code in ModelAdmin._changeform_view().

Co-authored-by: Daulet1 <d.abduali@…>

comment:9 by GitHub <noreply@…>, 4 years ago

In e0442a62:

Fixed #33527 -- Removed unnecessary code in ModelAdmin._changeform_view().

Co-authored-by: Daulet1 <d.abduali@…>

comment:10 by GitHub <noreply@…>, 4 years ago

In e0442a62:

Fixed #33527 -- Removed unnecessary code in ModelAdmin._changeform_view().

Co-authored-by: Daulet1 <d.abduali@…>

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