From a85346a70a443336424ebb899bf33c742fd15225 Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Fri, 19 Jan 2018 16:56:38 -0800 Subject: [PATCH] Remove added rules from permission when cancel editing Previously, the UI removed rules correctly, but they were not removed as expected from the underlying object. If a user was to edit again, change something different, and save, they would experience an unexpected result. Change-Id: If22ed59b9cdf5b93232c2f6a69a14d18575184bf --- .../app/elements/admin/gr-permission/gr-permission.js | 5 +++++ .../elements/admin/gr-permission/gr-permission_test.html | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js index e739e318f5..24be7e5a93 100644 --- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js +++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js @@ -102,6 +102,11 @@ delete this.permission.value.deleted; this._groupFilter = ''; this._rules = this._rules.filter(rule => !rule.value.added); + for (const key of Object.keys(this.permission.value.rules)) { + if (this.permission.value.rules[key].added) { + delete this.permission.value.rules[key]; + } + } // Restore exclusive bit to original. this.set(['permission', 'value', 'exclusive'], diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html index 2c5572a5f8..3a6a9cd34e 100644 --- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html +++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html @@ -307,7 +307,7 @@ limitations under the License. }, }, }; - + element.editing = true; assert.equal(element._rules.length, 2); assert.equal(Object.keys(element._groupsWithRules).length, 2); element._handleAddRuleItem(e); @@ -316,6 +316,11 @@ limitations under the License. assert.equal(Object.keys(element._groupsWithRules).length, 3); assert.deepEqual(element.permission.value.rules['newUserGroupId'], {action: 'ALLOW', min: -2, max: 2, added: true}); + + // New rule should be removed if cancel from editing. + element.editing = false; + assert.equal(element._rules.length, 2); + assert.equal(Object.keys(element.permission.value.rules).length, 2); }); test('removing the permission', () => {