[Django] #33527: remove unnecessary code from patch before in ModelAdmin inlines save.
Django
-------------------------------------+-------------------------------------
Reporter: Maxim | Owner: nobody
Danilov |
Type: | Status: new
Cleanup/optimization |
Component: | Version: 4.0
contrib.admin | Keywords: modeladmin,
Severity: Normal | inlinemodel
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
i see in Patch from https://code.djangoproject.com/ticket/33111
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
--
Ticket URL: <https://code.djangoproject.com/ticket/33527>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: modeladmin, | Triage Stage:
inlinemodel | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Mariusz, Felisiak (removed)
Old description:
> i see in Patch from https://code.djangoproject.com/ticket/33111
> 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
New description:
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
--
Comment:
I don't see any test failures with the following patch, so if it's needed,
perhaps a regression test would be useful.
{{{ #!diff
diff --git a/django/contrib/admin/options.py
b/django/contrib/admin/options.py
index b1aa610d43..64be087bf1 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -1787,7 +1787,7 @@ class ModelAdmin(BaseModelAdmin):
form = ModelForm(request.POST, request.FILES, instance=obj)
formsets, inline_instances = self._create_formsets(
request,
- form.instance if add else obj,
+ form.instance,
change=not add,
)
form_validated = form.is_valid()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: modeladmin, | Triage Stage:
inlinemodel | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => shirani98
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:2>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Keywords: modeladmin, | Triage Stage:
inlinemodel | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Shirani
Cleanup/optimization | Status: new
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
inlinemodel | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
The ticket isn't fixed until changes are merged.
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Maxim Danilov | Owner: Mahdi
Type: | Shirani
Cleanup/optimization | Status: new
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
inlinemodel |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Agreed, `if add else obj` is unnecessary. Maxim, would you like to prepare
a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: modeladmin, | Triage Stage: Accepted
inlinemodel |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Mahdi Shirani => Dulalet
* status: new => assigned
Comment:
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Maxim Danilov | Owner: Dulalet
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
inlinemodel | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/15465 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Maxim Danilov | Owner: Dulalet
Cleanup/optimization |
Component: contrib.admin | Version: 4.0
Keywords: modeladmin, | Triage Stage: Ready for
inlinemodel | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e0442a628eb480eac6a7888aed5a86f83499e299" e0442a62]:
{{{
#!CommitTicketReference repository=""
revision="e0442a628eb480eac6a7888aed5a86f83499e299"
Fixed #33527 -- Removed unnecessary code in ModelAdmin._changeform_view().
Co-authored-by: Daulet1 <d.ab...@thefactory.kz>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33527#comment:8>