[Django] #33029: Django Admin Popup
Django
---------------------------------------------+------------------------
Reporter: Yash Jhunjhunwala | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 1 |
---------------------------------------------+------------------------
Django Admin shows popup for adding/editing a related field object. The
popup doesn't always a new popup but instead replaces the current one when
opening multiple popups.
Steps to recreate the problem:
For E.g.
{{{
class A(models.Model):
foo = models.ForeignKey('self',
on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
True",related_name="foo_set",)
bar = models.ForeignKey('self',
on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
True",related_name="bar_set",)
}}}
For the above Model A :
Create a new instance of model A using the admin. Open a relatedmodel
popup for foo by clicking on the add sign.
In the new pop up click on the add sign for foo again and it will replace
the current popup (instead of opening up another popup) whereas if you
click on the add sign for bar it will open another popup - which is the
expected behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------------+--------------------------------------
Reporter: Yash Jhunjhunwala | Owner: nobody
Component: contrib.admin | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Old description:
> Django Admin shows popup for adding/editing a related field object. The
> popup doesn't always a new popup but instead replaces the current one
> when opening multiple popups.
>
> Steps to recreate the problem:
>
> For E.g.
>
> {{{
> class A(models.Model):
> foo = models.ForeignKey('self',
> on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
> True",related_name="foo_set",)
> bar = models.ForeignKey('self',
> on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
> True",related_name="bar_set",)
> }}}
>
> For the above Model A :
>
> Create a new instance of model A using the admin. Open a relatedmodel
> popup for foo by clicking on the add sign.
> In the new pop up click on the add sign for foo again and it will replace
> the current popup (instead of opening up another popup) whereas if you
> click on the add sign for bar it will open another popup - which is the
> expected behavior.
New description:
Django Admin shows popup for adding/editing a related field object. The
popup doesn't always a new popup but instead replaces the current one when
opening multiple popups.
Steps to recreate the problem:
For E.g.
{{{
class A(models.Model):
foo = models.ForeignKey('self',
on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
True",related_name="foo_set",)
bar = models.ForeignKey('self',
on_delete=models.SET_NULL,null=True,blank=True,verbose_name="If
True",related_name="bar_set",)
}}}
For the above Model A :
Create a new instance of model A using the admin. Open a relatedmodel
popup for foo by clicking on the add sign.
In the new pop up click on the add sign for foo again and it will replace
the current popup (instead of opening up another popup) whereas if you
click on the add sign for bar it will open another popup - which is the
expected behavior.
--
Comment:
Hi. Thanks for the report.
I'm going to say wontfix here.
In the normal case this works as expected. I'm adding a new object for a
foreign key; I click the same + button; I can't add two objects to the
same FK, so the popup is replaced; meanwhile, if I click the + for a
second related field a new popup is created (since I can add two different
objects to two different FKs).
The case for `self` is more complex, since in principle I could add a
chain of models for the same FK, but each on a different instance.
However, without a candidate implementation it's not clear it's worth
additional complexity to the popup implementation to handle that case.
(You can simply build the chain in the other direction.)
Happy to reopen if you want to work on an implementation that's not too
complex though!
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:1>
Django
-----------------------------------+--------------------------------------
Component: contrib.admin | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:2>
Django
-----------------------------------+--------------------------------------
Component: contrib.admin | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Yash Jhunjhunwala):
Replying to [comment:1 Carlton Gibson]:
> Happy to reopen if you want to work on an implementation that's not too
complex though!
What if we add an index of sorts to the window name. for e.g. if the field
name is foo then the instead of window name being "id_foo", we can use
"id_foo_1". Also pass the index to popup.
> I click the same + button; I can't add two objects to the same FK, so
the popup is replaced;
This would work as is since the + will always open window of the same
index. But would solve the problem for the chain of models.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:3>
Django
-----------------------------------+--------------------------------------
Component: contrib.admin | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* status: closed => new
* resolution: wontfix =>
Comment:
Created a draft pull request https://github.com/django/django/pull/14804
Need to add tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:4>
Django
-----------------------------------+------------------------------------
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* version: 2.2 => 3.2
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Hi Yash. OK — the draft PR looks small enough to consider. I didn't yet
check that it functions fully as expected but if that's all that it would
take...
> Need to add tests.
So for these you'll need to use Selenium. See the section in the
contributing guide on
[https://docs.djangoproject.com/en/3.2/internals/contributing/writing-code
/unit-tests#s-running-the-selenium-tests running the selenium tests].
If you create an admin for a self referential model and check with a test
case that a chain (say 3?) can be opened and filled in in order, and that
they correctly function (and populate the right form back up the chain)
then that shouldn't be too far off.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:5>
Django
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* owner: nobody => Yash Jhunjhunwala
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 1
Comment (by Yash Jhunjhunwala):
Added the tests and submitted a new pull request
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* needs_better_patch: 1 => 0
Comment:
Fixed Pre Commit Linter issues
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Yash Jhunjhunwala):
Replying to [comment:10 Mariusz Felisiak]:
Running pre-commit shows all tests passed but linter workflow fails
(isort) on GitHub
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
Just some small tweaks to the behaviour and tests still needed. But patch
look sound generally.
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Yash Jhunjhunwala):
* needs_better_patch: 1 => 0
Comment:
Updated the patch as per the changes suggested by Carlton
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by Carlton Gibson <carlton.gibson@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"492ed60f236d770eb9a6d56d85ff2550bb1ecfff" 492ed60]:
{{{
#!CommitTicketReference repository=""
revision="492ed60f236d770eb9a6d56d85ff2550bb1ecfff"
Fixed #33029 -- Allowed multiple popups for self-related fields in admin.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Yash Jhunjhunwala | Owner: Yash
| Jhunjhunwala
Type: New feature | Status: closed
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Comment (by Carlton Gibson <carlton@…>):
In [changeset:"402ae37873974afa5093e6d6149175a118979cd9" 402ae378]:
{{{
#!CommitTicketReference repository=""
revision="402ae37873974afa5093e6d6149175a118979cd9"
Refs #33029 -- Fixed popups Selenium tests in headless mode.
Co-authored-by: Yash Jhunjhunwala <ya...@jhunjhunwalaadvisors.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33029#comment:17>