From 24e540fbd71bd2b0843e751bde61ad0052a811b3 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 24 Oct 2019 16:37:55 +0200 Subject: [PATCH] Fixed #29087 -- Added delete buttons for unsaved admin inlines on validation error. --- AUTHORS | 1 + .../contrib/admin/static/admin/css/base.css | 7 +- .../contrib/admin/static/admin/js/inlines.js | 45 +++++- .../admin/static/admin/js/inlines.min.js | 21 +-- .../templates/admin/edit_inline/tabular.html | 2 +- js_tests/admin/inlines.test.js | 128 +++++++++++++++++- js_tests/tests.html | 46 ++++++- tests/admin_inlines/models.py | 10 ++ tests/admin_inlines/tests.py | 94 ++++++++++++- 9 files changed, 327 insertions(+), 27 deletions(-) diff --git a/AUTHORS b/AUTHORS index 8e126b2537..983e656ae0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -293,6 +293,7 @@ answer newbie questions, and generally made Django that much better: flavio.curella@gmail.com Florian Apolloner Florian Moussous + Fran Hrženjak Francisco Albarran Cristobal Francisco Couzo François Freitag diff --git a/django/contrib/admin/static/admin/css/base.css b/django/contrib/admin/static/admin/css/base.css index fd011a3f9a..3e27dd41f0 100644 --- a/django/contrib/admin/static/admin/css/base.css +++ b/django/contrib/admin/static/admin/css/base.css @@ -284,11 +284,14 @@ tr.alt { background: #f6f6f6; } -.row1 { +.row1, .row-form-errors { background: #fff; } -.row2 { +.row2, +.row2 .errorlist, +.row1 + .row-form-errors, +.row1 + .row-form-errors .errorlist { background: #f9f9f9; } diff --git a/django/contrib/admin/static/admin/js/inlines.js b/django/contrib/admin/static/admin/js/inlines.js index 460b6f33b5..c91b853a6b 100644 --- a/django/contrib/admin/static/admin/js/inlines.js +++ b/django/contrib/admin/static/admin/js/inlines.js @@ -37,6 +37,7 @@ var totalForms = $("#id_" + options.prefix + "-TOTAL_FORMS").prop("autocomplete", "off"); var nextIndex = parseInt(totalForms.val(), 10); var maxForms = $("#id_" + options.prefix + "-MAX_NUM_FORMS").prop("autocomplete", "off"); + var minForms = $("#id_" + options.prefix + "-MIN_NUM_FORMS").prop("autocomplete", "off"); var addButton; /** @@ -79,6 +80,9 @@ if ((maxForms.val() !== '') && (maxForms.val() - totalForms.val()) <= 0) { addButton.parent().hide(); } + // Show the remove buttons if there are more than min_num. + toggleDeleteButtonVisibility(row.closest('.inline-group')); + // Pass the new form to the post-add callback, if provided. if (options.added) { options.added(row); @@ -112,7 +116,13 @@ e1.preventDefault(); var deleteButton = $(e1.target); var row = deleteButton.closest('.' + options.formCssClass); - // Remove the parent form containing this button: + var inlineGroup = row.closest('.inline-group'); + // Remove the parent form containing this button, + // and also remove the relevant row with non-field errors: + var prevRow = row.prev(); + if (prevRow.length && prevRow.hasClass('row-form-errors')) { + prevRow.remove(); + } row.remove(); nextIndex -= 1; // Pass the deleted form to the post-delete callback, if provided. @@ -127,6 +137,8 @@ if ((maxForms.val() === '') || (maxForms.val() - forms.length) > 0) { addButton.parent().show(); } + // Hide the remove buttons if at min_num. + toggleDeleteButtonVisibility(inlineGroup); // Also, update names and ids for all remaining form controls so // they remain in sequence: var i, formCount; @@ -139,18 +151,37 @@ } }; - // Show the add button if we are allowed to add more items. - // Note that max_num = None translates to a blank string. - var showAddButton = maxForms.val() === '' || (maxForms.val() - totalForms.val()) > 0; + var toggleDeleteButtonVisibility = function(inlineGroup) { + if ((minForms.val() !== '') && (minForms.val() - totalForms.val()) >= 0) { + inlineGroup.find('.inline-deletelink').hide(); + } else { + inlineGroup.find('.inline-deletelink').show(); + } + }; + $this.each(function(i) { $(this).not("." + options.emptyCssClass).addClass(options.formCssClass); }); - // Create the add button. + // Create the delete buttons for all unsaved inlines: + $this.filter('.' + options.formCssClass + ':not(.has_original):not(.' + options.emptyCssClass + ')').each(function() { + addInlineDeleteButton($(this)); + }); + toggleDeleteButtonVisibility($this); + + // Create the add button, initially hidden. addButton = options.addButton; + addInlineAddButton(); + + // Show the add button if allowed to add more items. + // Note that max_num = None translates to a blank string. + var showAddButton = maxForms.val() === '' || (maxForms.val() - totalForms.val()) > 0; if ($this.length && showAddButton) { - addInlineAddButton(); + addButton.parent().show(); + } else { + addButton.parent().hide(); } + return this; }; @@ -314,7 +345,7 @@ $(selector).stackedFormset(selector, inlineOptions.options); break; case "tabular": - selector = inlineOptions.name + "-group .tabular.inline-related tbody:first > tr"; + selector = inlineOptions.name + "-group .tabular.inline-related tbody:first > tr.form-row"; $(selector).tabularFormset(selector, inlineOptions.options); break; } diff --git a/django/contrib/admin/static/admin/js/inlines.min.js b/django/contrib/admin/static/admin/js/inlines.min.js index e2d61af0bd..f5f951c824 100644 --- a/django/contrib/admin/static/admin/js/inlines.min.js +++ b/django/contrib/admin/static/admin/js/inlines.min.js @@ -1,10 +1,11 @@ -(function(b){b.fn.formset=function(c){var a=b.extend({},b.fn.formset.defaults,c),d=b(this),h=d.parent(),l=function(a,e,k){var f=new RegExp("("+e+"-(\\d+|__prefix__))");e=e+"-"+k;b(a).prop("for")&&b(a).prop("for",b(a).prop("for").replace(f,e));a.id&&(a.id=a.id.replace(f,e));a.name&&(a.name=a.name.replace(f,e))},g=b("#id_"+a.prefix+"-TOTAL_FORMS").prop("autocomplete","off"),e=parseInt(g.val(),10),k=b("#id_"+a.prefix+"-MAX_NUM_FORMS").prop("autocomplete","off");c=function(){if(null===m)if("TR"===d.prop("tagName")){var b= -d.eq(-1).children().length;h.append(''+a.addText+"");m=h.find("tr:last a")}else d.filter(":last").after('
'+a.addText+"
"),m=d.filter(":last").next().find("a");m.on("click",n)};var n=function(f){f.preventDefault();f=b("#"+a.prefix+"-empty");var c=f.clone(!0);c.removeClass(a.emptyCssClass).addClass(a.formCssClass).attr("id",a.prefix+"-"+e);p(c);c.find("*").each(function(){l(this, -a.prefix,g.val())});c.insertBefore(b(f));b(g).val(parseInt(g.val(),10)+1);e+=1;""!==k.val()&&0>=k.val()-g.val()&&m.parent().hide();a.added&&a.added(c);b(document).trigger("formset:added",[c,a.prefix])},p=function(b){b.is("tr")?b.children(":last").append('"):b.is("ul")||b.is("ol")?b.append('
  • '+a.deleteText+"
    • "):b.children(":first").append(''+ -a.deleteText+"");b.find("a."+a.deleteCssClass).on("click",q.bind(this))},q=function(c){c.preventDefault();c=b(c.target).closest("."+a.formCssClass);c.remove();--e;a.removed&&a.removed(c);b(document).trigger("formset:removed",[c,a.prefix]);c=b("."+a.formCssClass);b("#id_"+a.prefix+"-TOTAL_FORMS").val(c.length);(""===k.val()||0 tr",b(c).tabularFormset(c,a.options)}})})})(django.jQuery); +(function(b){b.fn.formset=function(d){var a=b.extend({},b.fn.formset.defaults,d),c=b(this),k=c.parent(),n=function(a,e,l){var g=new RegExp("("+e+"-(\\d+|__prefix__))");e=e+"-"+l;b(a).prop("for")&&b(a).prop("for",b(a).prop("for").replace(g,e));a.id&&(a.id=a.id.replace(g,e));a.name&&(a.name=a.name.replace(g,e))},h=b("#id_"+a.prefix+"-TOTAL_FORMS").prop("autocomplete","off"),e=parseInt(h.val(),10),l=b("#id_"+a.prefix+"-MAX_NUM_FORMS").prop("autocomplete","off"),q=b("#id_"+a.prefix+"-MIN_NUM_FORMS").prop("autocomplete", +"off"),t=function(g){g.preventDefault();g=b("#"+a.prefix+"-empty");var f=g.clone(!0);f.removeClass(a.emptyCssClass).addClass(a.formCssClass).attr("id",a.prefix+"-"+e);r(f);f.find("*").each(function(){n(this,a.prefix,h.val())});f.insertBefore(b(g));b(h).val(parseInt(h.val(),10)+1);e+=1;""!==l.val()&&0>=l.val()-h.val()&&m.parent().hide();p(f.closest(".inline-group"));a.added&&a.added(f);b(document).trigger("formset:added",[f,a.prefix])},r=function(b){b.is("tr")?b.children(":last").append('"):b.is("ul")||b.is("ol")?b.append('
  • '+a.deleteText+"
    • "):b.children(":first").append(''+a.deleteText+"");b.find("a."+a.deleteCssClass).on("click",u.bind(this))},u=function(g){g.preventDefault();var f=b(g.target).closest("."+a.formCssClass);g=f.closest(".inline-group");var d=f.prev();d.length&&d.hasClass("row-form-errors")&&d.remove(); +f.remove();--e;a.removed&&a.removed(f);b(document).trigger("formset:removed",[f,a.prefix]);f=b("."+a.formCssClass);b("#id_"+a.prefix+"-TOTAL_FORMS").val(f.length);(""===l.val()||0'+a.addText+"");m=k.find("tr:last a")}else c.filter(":last").after('
    '+a.addText+"
    "),m=c.filter(":last").next().find("a");m.on("click",t)})();d=""===l.val()|| +0 tr.form-row",b(d).tabularFormset(d,a.options)}})})})(django.jQuery); diff --git a/django/contrib/admin/templates/admin/edit_inline/tabular.html b/django/contrib/admin/templates/admin/edit_inline/tabular.html index 531d7b6a21..29a2af1089 100644 --- a/django/contrib/admin/templates/admin/edit_inline/tabular.html +++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html @@ -23,7 +23,7 @@ {% for inline_admin_form in inline_admin_formset %} {% if inline_admin_form.form.non_field_errors %} - {{ inline_admin_form.form.non_field_errors }} + {{ inline_admin_form.form.non_field_errors }} {% endif %} diff --git a/js_tests/admin/inlines.test.js b/js_tests/admin/inlines.test.js index 433ea0672a..582ede1465 100644 --- a/js_tests/admin/inlines.test.js +++ b/js_tests/admin/inlines.test.js @@ -11,7 +11,7 @@ QUnit.module('admin.inlines: tabular formsets', { $('#qunit-fixture').append($('#tabular-formset').text()); this.table = $('table.inline'); this.inlineRow = this.table.find('tr'); - that.inlineRow.tabularFormset('table.inline tr', { + this.inlineRow.tabularFormset('table.inline tr.form-row', { prefix: 'first', addText: that.addText, deleteText: 'Remove' @@ -31,6 +31,13 @@ QUnit.test('add form', function(assert) { assert.ok(this.table.find('#first-1').hasClass('row2')); }); +QUnit.test('added form has remove button', function(assert) { + var addButton = this.table.find('.add-row a'); + assert.equal(addButton.text(), this.addText); + addButton.click(); + assert.equal(this.table.find('#first-1.row2 .inline-deletelink').length, 1); +}); + QUnit.test('add/remove form events', function(assert) { assert.expect(6); var $ = django.jQuery; @@ -49,7 +56,7 @@ QUnit.test('add/remove form events', function(assert) { assert.equal(true, $row.is(deletedRow)); assert.equal(formsetName, 'first'); }); - deleteLink.click(); + deleteLink.trigger($.Event('click', {target: deleteLink})); }); QUnit.test('existing add button', function(assert) { @@ -69,3 +76,120 @@ QUnit.test('existing add button', function(assert) { addButton.click(); assert.ok(this.table.find('#first-1').hasClass('row2')); }); + + +QUnit.module('admin.inlines: tabular formsets with validation errors', { + beforeEach: function() { + var $ = django.jQuery; + + $('#qunit-fixture').append($('#tabular-formset-with-validation-error').text()); + this.table = $('table.inline'); + this.inlineRows = this.table.find('tr.form-row'); + this.inlineRows.tabularFormset('table.inline tr.form-row', { + prefix: 'second' + }); + } +}); + +QUnit.test('first form has delete checkbox and no button', function(assert) { + var tr = this.inlineRows.slice(0, 1); + assert.ok(tr.hasClass('dynamic-second')); + assert.ok(tr.hasClass('has_original')); + assert.equal(tr.find('td.delete input').length, 1); + assert.equal(tr.find('td.delete .inline-deletelink').length, 0); +}); + +QUnit.test('dynamic form has remove button', function(assert) { + var tr = this.inlineRows.slice(1, 2); + assert.ok(tr.hasClass('dynamic-second')); + assert.notOk(tr.hasClass('has_original')); + assert.equal(tr.find('.inline-deletelink').length, 1); +}); + +QUnit.test('dynamic template has nothing', function(assert) { + var tr = this.inlineRows.slice(2, 3); + assert.ok(tr.hasClass('empty-form')); + assert.notOk(tr.hasClass('dynamic-second')); + assert.notOk(tr.hasClass('has_original')); + assert.equal(tr.find('td.delete')[0].innerHTML, ''); +}); + +QUnit.test('removing a form-row also removed related row with non-field errors', function(assert) { + var $ = django.jQuery; + assert.ok(this.table.find('.row-form-errors').length); + var tr = this.inlineRows.slice(1, 2); + var trWithErrors = tr.prev(); + assert.ok(trWithErrors.hasClass('row-form-errors')); + var deleteLink = tr.find('a.inline-deletelink'); + deleteLink.trigger($.Event('click', {target: deleteLink})); + assert.notOk(this.table.find('.row-form-errors').length); +}); + +QUnit.test('removing and adding a row keeps cycling row1 and row2 classes', function(assert) { + var $ = django.jQuery; + var tr = this.inlineRows.slice(1, 2); + var deleteLink = tr.find('a.inline-deletelink'); + var addLink = this.table.find('.add-row > td > a'); + assert.ok(this.table.find('tr.form-row:even').hasClass('row1')); + assert.ok(this.table.find('tr.form-row:odd').hasClass('row2')); + deleteLink.trigger($.Event('click', {target: deleteLink})); + assert.ok(this.table.find('tr.form-row:even').hasClass('row1')); + assert.ok(this.table.find('tr.form-row:odd').hasClass('row2')); + addLink.trigger($.Event('click', {target: addLink})); + assert.ok(this.table.find('tr.form-row:even').hasClass('row1')); + assert.ok(this.table.find('tr.form-row:odd').hasClass('row2')); +}); + + +QUnit.module('admin.inlines: tabular formsets with max_num', { + beforeEach: function() { + var $ = django.jQuery; + $('#qunit-fixture').append($('#tabular-formset-with-validation-error').text()); + this.table = $('table.inline'); + this.maxNum = $('input.id_second-MAX_NUM_FORMS'); + this.maxNum.val(2); + this.inlineRows = this.table.find('tr.form-row'); + this.inlineRows.tabularFormset('table.inline tr.form-row', { + prefix: 'second' + }); + } +}); + +QUnit.test('does not show the add button if already at max_num', function(assert) { + var addButton = this.table.find('tr.add_row > td > a'); + assert.notOk(addButton.is(':visible')); +}); + +QUnit.test('make addButton visible again', function(assert) { + var $ = django.jQuery; + var addButton = this.table.find('tr.add_row > td > a'); + var removeButton = this.table.find('tr.form-row:first').find('a.inline-deletelink'); + removeButton.trigger($.Event( "click", { target: removeButton } )); + assert.notOk(addButton.is(':visible')); +}); + + +QUnit.module('admin.inlines: tabular formsets with min_num', { + beforeEach: function() { + var $ = django.jQuery; + $('#qunit-fixture').append($('#tabular-formset-with-validation-error').text()); + this.table = $('table.inline'); + this.minNum = $('input#id_second-MIN_NUM_FORMS'); + this.minNum.val(2); + this.inlineRows = this.table.find('tr.form-row'); + this.inlineRows.tabularFormset('table.inline tr.form-row', { + prefix: 'second' + }); + } +}); + +QUnit.test('does not show the remove buttons if already at min_num', function(assert) { + assert.notOk(this.table.find('.inline-deletelink:visible').length); +}); + +QUnit.test('make removeButtons visible again', function(assert) { + var $ = django.jQuery; + var addButton = this.table.find('tr.add-row > td > a'); + addButton.trigger($.Event( "click", { target: addButton } )); + assert.equal(this.table.find('.inline-deletelink:visible').length, 2); +}); diff --git a/js_tests/tests.html b/js_tests/tests.html index a2342883ec..988b7e3a4c 100644 --- a/js_tests/tests.html +++ b/js_tests/tests.html @@ -28,18 +28,56 @@ - + - +
    - +
    - +
    + diff --git a/tests/admin_inlines/models.py b/tests/admin_inlines/models.py index a42e2588e9..1a705c55c7 100644 --- a/tests/admin_inlines/models.py +++ b/tests/admin_inlines/models.py @@ -110,11 +110,21 @@ class Inner4Stacked(models.Model): dummy = models.IntegerField(help_text="Awesome stacked help text is awesome.") holder = models.ForeignKey(Holder4, models.CASCADE) + class Meta: + constraints = [ + models.UniqueConstraint(fields=['dummy', 'holder'], name='unique_stacked_dummy_per_holder') + ] + class Inner4Tabular(models.Model): dummy = models.IntegerField(help_text="Awesome tabular help text is awesome.") holder = models.ForeignKey(Holder4, models.CASCADE) + class Meta: + constraints = [ + models.UniqueConstraint(fields=['dummy', 'holder'], name='unique_tabular_dummy_per_holder') + ] + # Models for #12749 diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py index fe0d913b0d..a43c43560f 100644 --- a/tests/admin_inlines/tests.py +++ b/tests/admin_inlines/tests.py @@ -145,7 +145,7 @@ class TestInline(TestDataMixin, TestCase): # Here colspan is "4": two fields (title1 and title2), one hidden field and the delete checkbox. self.assertContains( response, - '
      ' + '
        ' '
      • The two titles must be the same
      ' ) @@ -907,8 +907,100 @@ class SeleniumTests(AdminSeleniumTestCase): self.assertEqual(rows_length(), 5, msg="sanity check") for delete_link in self.selenium.find_elements_by_css_selector('%s .inline-deletelink' % inline_id): delete_link.click() + with self.disable_implicit_wait(): + self.assertEqual(rows_length(), 0) + + def test_delete_invalid_stacked_inlines(self): + from selenium.common.exceptions import NoSuchElementException + self.admin_login(username='super', password='secret') + self.selenium.get(self.live_server_url + reverse('admin:admin_inlines_holder4_add')) + + inline_id = '#inner4stacked_set-group' + + def rows_length(): + return len(self.selenium.find_elements_by_css_selector('%s .dynamic-inner4stacked_set' % inline_id)) self.assertEqual(rows_length(), 3) + add_button = self.selenium.find_element_by_link_text( + 'Add another Inner4 stacked') + add_button.click() + add_button.click() + self.assertEqual(len(self.selenium.find_elements_by_css_selector('#id_inner4stacked_set-4-dummy')), 1) + + # Enter some data and click 'Save'. + self.selenium.find_element_by_name('dummy').send_keys('1') + self.selenium.find_element_by_name('inner4stacked_set-0-dummy').send_keys('100') + self.selenium.find_element_by_name('inner4stacked_set-1-dummy').send_keys('101') + self.selenium.find_element_by_name('inner4stacked_set-2-dummy').send_keys('222') + self.selenium.find_element_by_name('inner4stacked_set-3-dummy').send_keys('103') + self.selenium.find_element_by_name('inner4stacked_set-4-dummy').send_keys('222') + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.wait_page_loaded() + + self.assertEqual(rows_length(), 5, msg="sanity check") + errorlist = self.selenium.find_element_by_css_selector( + '%s .dynamic-inner4stacked_set .errorlist li' % inline_id + ) + self.assertEqual('Please correct the duplicate values below.', errorlist.text) + delete_link = self.selenium.find_element_by_css_selector('#inner4stacked_set-4 .inline-deletelink') + delete_link.click() + self.assertEqual(rows_length(), 4) + with self.disable_implicit_wait(), self.assertRaises(NoSuchElementException): + self.selenium.find_element_by_css_selector('%s .dynamic-inner4stacked_set .errorlist li' % inline_id) + + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.wait_page_loaded() + + # The objects have been created in the database. + self.assertEqual(Inner4Stacked.objects.all().count(), 4) + + def test_delete_invalid_tabular_inlines(self): + from selenium.common.exceptions import NoSuchElementException + self.admin_login(username='super', password='secret') + self.selenium.get(self.live_server_url + reverse('admin:admin_inlines_holder4_add')) + + inline_id = '#inner4tabular_set-group' + + def rows_length(): + return len(self.selenium.find_elements_by_css_selector('%s .dynamic-inner4tabular_set' % inline_id)) + self.assertEqual(rows_length(), 3) + + add_button = self.selenium.find_element_by_link_text( + 'Add another Inner4 tabular') + add_button.click() + add_button.click() + self.assertEqual(len(self.selenium.find_elements_by_css_selector('#id_inner4tabular_set-4-dummy')), 1) + + # Enter some data and click 'Save'. + self.selenium.find_element_by_name('dummy').send_keys('1') + self.selenium.find_element_by_name('inner4tabular_set-0-dummy').send_keys('100') + self.selenium.find_element_by_name('inner4tabular_set-1-dummy').send_keys('101') + self.selenium.find_element_by_name('inner4tabular_set-2-dummy').send_keys('222') + self.selenium.find_element_by_name('inner4tabular_set-3-dummy').send_keys('103') + self.selenium.find_element_by_name('inner4tabular_set-4-dummy').send_keys('222') + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.wait_page_loaded() + + self.assertEqual(rows_length(), 5, msg="sanity check") + + # Non-field errorlist is in its own just before + # tr#inner4tabular_set-3: + errorlist = self.selenium.find_element_by_css_selector( + '%s #inner4tabular_set-3 + .row-form-errors .errorlist li' % inline_id + ) + self.assertEqual('Please correct the duplicate values below.', errorlist.text) + delete_link = self.selenium.find_element_by_css_selector('#inner4tabular_set-4 .inline-deletelink') + delete_link.click() + self.assertEqual(rows_length(), 4) + with self.disable_implicit_wait(), self.assertRaises(NoSuchElementException): + self.selenium.find_element_by_css_selector('%s .dynamic-inner4tabular_set .errorlist li' % inline_id) + + self.selenium.find_element_by_xpath('//input[@value="Save"]').click() + self.wait_page_loaded() + + # The objects have been created in the database. + self.assertEqual(Inner4Tabular.objects.all().count(), 4) + def test_add_inlines(self): """ The "Add another XXX" link correctly adds items to the inline form.