From 492ed60f236d770eb9a6d56d85ff2550bb1ecfff Mon Sep 17 00:00:00 2001 From: Yash Jhunjhunwala Date: Wed, 1 Sep 2021 17:31:40 +0530 Subject: [PATCH] Fixed #33029 -- Allowed multiple popups for self-related fields in admin. --- AUTHORS | 1 + .../admin/js/admin/RelatedObjectLookups.js | 63 ++++++++++++++-- tests/admin_views/admin.py | 5 +- tests/admin_views/models.py | 6 ++ tests/admin_views/tests.py | 74 ++++++++++++++++++- 5 files changed, 140 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index 146b0be3a3..967fe06449 100644 --- a/AUTHORS +++ b/AUTHORS @@ -990,6 +990,7 @@ answer newbie questions, and generally made Django that much better: Xia Kai Yann Fouillat Yann Malet + Yash Jhunjhunwala Yasushi Masuda ye7cakf02@sneakemail.com ymasuda@ethercube.com diff --git a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js index 289e1cee26..668c56a89d 100644 --- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js +++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js @@ -4,14 +4,45 @@ 'use strict'; { const $ = django.jQuery; + let popupIndex = 0; + const relatedWindows = []; + + function dismissChildPopups() { + relatedWindows.forEach(function(win) { + if(!win.closed) { + win.dismissChildPopups(); + win.close(); + } + }); + } + + function setPopupIndex() { + if(document.getElementsByName("_popup").length > 0) { + const index = window.name.lastIndexOf("__") + 2; + popupIndex = parseInt(window.name.substring(index)); + } else { + popupIndex = 0; + } + } + + function addPopupIndex(name) { + name = name + "__" + (popupIndex + 1); + return name; + } + + function removePopupIndex(name) { + name = name.replace(new RegExp("__" + (popupIndex + 1) + "$"), ''); + return name; + } function showAdminPopup(triggeringLink, name_regexp, add_popup) { - const name = triggeringLink.id.replace(name_regexp, ''); + const name = addPopupIndex(triggeringLink.id.replace(name_regexp, '')); const href = new URL(triggeringLink.href); if (add_popup) { href.searchParams.set('_popup', 1); } const win = window.open(href, name, 'height=500,width=800,resizable=yes,scrollbars=yes'); + relatedWindows.push(win); win.focus(); return false; } @@ -21,13 +52,17 @@ } function dismissRelatedLookupPopup(win, chosenId) { - const name = win.name; + const name = removePopupIndex(win.name); const elem = document.getElementById(name); if (elem.classList.contains('vManyToManyRawIdAdminField') && elem.value) { elem.value += ',' + chosenId; } else { document.getElementById(name).value = chosenId; } + const index = relatedWindows.indexOf(win); + if (index > -1) { + relatedWindows.splice(index, 1); + } win.close(); } @@ -53,7 +88,7 @@ } function dismissAddRelatedObjectPopup(win, newId, newRepr) { - const name = win.name; + const name = removePopupIndex(win.name); const elem = document.getElementById(name); if (elem) { const elemName = elem.nodeName.toUpperCase(); @@ -74,11 +109,15 @@ SelectBox.add_to_cache(toId, o); SelectBox.redisplay(toId); } + const index = relatedWindows.indexOf(win); + if (index > -1) { + relatedWindows.splice(index, 1); + } win.close(); } function dismissChangeRelatedObjectPopup(win, objId, newRepr, newId) { - const id = win.name.replace(/^edit_/, ''); + const id = removePopupIndex(win.name.replace(/^edit_/, '')); const selectsSelector = interpolate('#%s, #%s_from, #%s_to', [id, id, id]); const selects = $(selectsSelector); selects.find('option').each(function() { @@ -93,11 +132,15 @@ this.lastChild.textContent = newRepr; this.title = newRepr; }); + const index = relatedWindows.indexOf(win); + if (index > -1) { + relatedWindows.splice(index, 1); + } win.close(); } function dismissDeleteRelatedObjectPopup(win, objId) { - const id = win.name.replace(/^delete_/, ''); + const id = removePopupIndex(win.name.replace(/^delete_/, '')); const selectsSelector = interpolate('#%s, #%s_from, #%s_to', [id, id, id]); const selects = $(selectsSelector); selects.find('option').each(function() { @@ -105,6 +148,10 @@ $(this).remove(); } }).trigger('change'); + const index = relatedWindows.indexOf(win); + if (index > -1) { + relatedWindows.splice(index, 1); + } win.close(); } @@ -115,12 +162,18 @@ window.dismissAddRelatedObjectPopup = dismissAddRelatedObjectPopup; window.dismissChangeRelatedObjectPopup = dismissChangeRelatedObjectPopup; window.dismissDeleteRelatedObjectPopup = dismissDeleteRelatedObjectPopup; + window.dismissChildPopups = dismissChildPopups; // Kept for backward compatibility window.showAddAnotherPopup = showRelatedObjectPopup; window.dismissAddAnotherPopup = dismissAddRelatedObjectPopup; + window.addEventListener('unload', function(evt) { + window.dismissChildPopups(); + }); + $(document).ready(function() { + setPopupIndex(); $("a[data-popup-opener]").on('click', function(event) { event.preventDefault(); opener.dismissRelatedLookupPopup(window, $(this).data("popup-opener")); diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index dac4517b2b..9238c7fabf 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -22,8 +22,8 @@ from .forms import MediaActionForm from .models import ( Actor, AdminOrderedAdminMethod, AdminOrderedCallable, AdminOrderedField, AdminOrderedModelMethod, Album, Answer, Answer2, Article, BarAccount, Book, - Bookmark, Category, Chapter, ChapterXtra1, Child, ChildOfReferer, Choice, - City, Collector, Color, Color2, ComplexSortedPerson, CoverLetter, + Bookmark, Box, Category, Chapter, ChapterXtra1, Child, ChildOfReferer, + Choice, City, Collector, Color, Color2, ComplexSortedPerson, CoverLetter, CustomArticle, CyclicOne, CyclicTwo, DependentChild, DooHickey, EmptyModel, EmptyModelHidden, EmptyModelMixin, EmptyModelVisible, ExplicitlyProvidedPK, ExternalSubscriber, Fabric, FancyDoodad, FieldOverridePost, @@ -1125,6 +1125,7 @@ site.register(NotReferenced) site.register(ExplicitlyProvidedPK, GetFormsetsArgumentCheckingAdmin) site.register(ImplicitlyGeneratedPK, GetFormsetsArgumentCheckingAdmin) site.register(UserProxy) +site.register(Box) # Register core models we need in our tests site.register(User, UserAdmin) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 73459e2995..c2972e0940 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -1050,3 +1050,9 @@ class ReadOnlyRelatedField(models.Model): class Héllo(models.Model): pass + + +class Box(models.Model): + title = models.CharField(max_length=100) + next_box = models.ForeignKey("self", null=True, on_delete=models.SET_NULL, blank=True) + next_box = models.ForeignKey("self", null=True, on_delete=models.SET_NULL, blank=True) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 337b5469f7..863521bee5 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -48,8 +48,8 @@ from .admin import CityAdmin, site, site2 from .models import ( Actor, AdminOrderedAdminMethod, AdminOrderedCallable, AdminOrderedField, AdminOrderedModelMethod, Album, Answer, Answer2, Article, BarAccount, Book, - Bookmark, Category, Chapter, ChapterXtra1, ChapterXtra2, Character, Child, - Choice, City, Collector, Color, ComplexSortedPerson, CoverLetter, + Bookmark, Box, Category, Chapter, ChapterXtra1, ChapterXtra2, Character, + Child, Choice, City, Collector, Color, ComplexSortedPerson, CoverLetter, CustomArticle, CyclicOne, CyclicTwo, DooHickey, Employee, EmptyModel, Fabric, FancyDoodad, FieldOverridePost, FilteredManager, FooAccount, FoodDelivery, FunkyTag, Gallery, Grommet, Inquisition, Language, Link, @@ -4983,6 +4983,76 @@ class SeleniumTests(AdminSeleniumTestCase): 50, ) + def test_related_popup_index(self): + """ + Create a chain of 'self' related objects via popups. + """ + from selenium.webdriver.support.ui import Select + self.admin_login(username='super', password='secret', login_url=reverse('admin:index')) + add_url = reverse('admin:admin_views_box_add', current_app=site.name) + self.selenium.get(self.live_server_url + add_url) + + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup() + + self.selenium.find_element_by_id('id_title').send_keys('test') + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup(num_windows=3) + + self.selenium.find_element_by_id('id_title').send_keys('test2') + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup(num_windows=4) + + self.selenium.find_element_by_id('id_title').send_keys('test3') + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.selenium.switch_to.window(self.selenium.window_handles[-1]) + select = Select(self.selenium.find_element_by_id('id_next_box')) + next_box_id = str(Box.objects.get(title="test3").id) + self.assertEqual(select.first_selected_option.get_attribute('value'), next_box_id) + + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.selenium.switch_to.window(self.selenium.window_handles[-1]) + select = Select(self.selenium.find_element_by_id('id_next_box')) + next_box_id = str(Box.objects.get(title="test2").id) + self.assertEqual(select.first_selected_option.get_attribute('value'), next_box_id) + + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.selenium.switch_to.window(self.selenium.window_handles[-1]) + select = Select(self.selenium.find_element_by_id('id_next_box')) + next_box_id = str(Box.objects.get(title="test").id) + self.assertEqual(select.first_selected_option.get_attribute('value'), next_box_id) + + def test_related_popup_incorrect_close(self): + """ + Cleanup child popups when closing a parent popup. + """ + self.admin_login(username='super', password='secret', login_url=reverse('admin:index')) + add_url = reverse('admin:admin_views_box_add', current_app=site.name) + self.selenium.get(self.live_server_url + add_url) + + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup() + + test_window = self.selenium.current_window_handle + self.selenium.find_element_by_id('id_title').send_keys('test') + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup(num_windows=3) + + test2_window = self.selenium.current_window_handle + self.selenium.find_element_by_id('id_title').send_keys('test2') + self.selenium.find_element_by_id('add_id_next_box').click() + self.wait_for_and_switch_to_popup(num_windows=4) + self.assertEqual(len(self.selenium.window_handles), 4) + + self.selenium.switch_to.window(test2_window) + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.assertEqual(len(self.selenium.window_handles), 2) + + # Close final popup to clean up test. + self.selenium.switch_to.window(test_window) + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.selenium.switch_to.window(self.selenium.window_handles[-1]) + @override_settings(ROOT_URLCONF='admin_views.urls') class ReadonlyTest(AdminFieldExtractionMixin, TestCase):