From e6f68d96b17a99daa715b502dc40cadf8932936b Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Tue, 30 Jan 2018 15:52:18 -0800 Subject: [PATCH] Add the ability to add a new section in gr-repo-access Bug: Issue 8038 Change-Id: I23f7922614244cd1833602385c4cfa1657c7e8b9 --- .../gr-access-section/gr-access-section.html | 28 +-- .../gr-access-section/gr-access-section.js | 15 +- .../gr-access-section_test.html | 18 +- .../admin/gr-repo-access/gr-repo-access.html | 3 + .../admin/gr-repo-access/gr-repo-access.js | 35 ++- .../gr-repo-access/gr-repo-access_test.html | 237 ++++++++++++++++++ 6 files changed, 306 insertions(+), 30 deletions(-) diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html index 3bf4d8f5bf..a92507bc3c 100644 --- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html +++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html @@ -42,7 +42,6 @@ limitations under the License. display: flex; } .header, - .editingRef .editContainer, #deletedContainer { align-items: center; background: #f6f6f6; @@ -55,9 +54,6 @@ limitations under the License. #deletedContainer { border-bottom: 0; } - #editRefInput { - width: 70%; - } .sectionContent { padding: .7em; } @@ -67,11 +63,12 @@ limitations under the License. .deleted #mainContainer, #addPermission, #deleteBtn, - .editingRef .header, - .editContainer { + .editingRef .name, + #editRefInput { display: none; } - .editing #editBtn { + .editing #editBtn, + .editingRef #editRefInput { display: flex; } .deleted #deletedContainer { @@ -86,9 +83,6 @@ limitations under the License. #undoRemoveBtn { padding-right: .7em; } - .editingRef .editContainer { - display: flex; - }
+ on-tap="editReference"> - Remove - -
-
+ Remove +
+ Add Reference diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js index bf5ad0bcf3..65045db4cf 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js +++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js @@ -87,6 +87,7 @@ _editing: { type: Boolean, value: false, + observer: '_handleEditingChanged', }, _modified: { type: Boolean, @@ -152,6 +153,18 @@ return editing ? 'Cancel' : 'Edit'; }, + _handleEditingChanged(editing, editingOld) { + // Ignore when editing gets set initially. + if (!editingOld || editing) { return; } + // Remove any unsaved but added refs. + this._sections = this._sections.filter(p => !p.value.added); + for (const key of Object.keys(this._local)) { + if (this._local[key].added) { + delete this._local[key]; + } + } + }, + /** * @param {!Defs.projectAccessInput} addRemoveObj * @param {!Array} path @@ -202,6 +215,8 @@ for (const k in obj) { if (!obj.hasOwnProperty(k)) { return; } if (typeof obj[k] == 'object') { + const updatedId = obj[k].updatedId; + const ref = updatedId ? updatedId : k; if (obj[k].deleted) { this._updateAddRemoveObj(addRemoveObj, path.concat(k), 'remove'); @@ -209,9 +224,6 @@ } else if (obj[k].modified) { this._updateAddRemoveObj(addRemoveObj, path.concat(k), 'remove'); - - const updatedId = obj[k].updatedId; - const ref = updatedId ? updatedId : k; this._updateAddRemoveObj(addRemoveObj, path.concat(ref), 'add', obj[k]); /* Special case for ref changes because they need to be added and @@ -225,7 +237,7 @@ continue; } else if (obj[k].added) { this._updateAddRemoveObj(addRemoveObj, - path.concat(k), 'add', obj[k]); + path.concat(ref), 'add', obj[k]); continue; } this._recursivelyUpdateAddRemoveObj(obj[k], addRemoveObj, @@ -250,6 +262,21 @@ return addRemoveObj; }, + _handleCreateSection() { + let newRef = 'refs/for/*'; + // Avoid using an already used key for the placeholder, since it + // immediately gets added to an object. + while (this._local[newRef]) { + newRef = `${newRef}*`; + } + const section = {permissions: {}, added: true}; + this.push('_sections', {id: newRef, value: section}); + this.set(['_local', newRef], section); + Polymer.dom.flush(); + Polymer.dom(this.root).querySelector('gr-access-section:last-of-type') + .editReference(); + }, + _handleSaveForReview() { const addRemoveObj = this._computeAddAndRemove(); diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html index d079fcbeaf..dece99561d 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html +++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html @@ -523,6 +523,99 @@ limitations under the License. assert.deepEqual(element._computeAddAndRemove(), expectedInput); }); + test('_computeAddAndRemove new section', () => { + // Add a new permission to a section + expectedInput = { + add: { + 'refs/for/*': { + added: true, + permissions: {}, + }, + }, + remove: {}, + }; + MockInteractions.tap(element.$.addReferenceBtn); + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + expectedInput = { + add: { + 'refs/for/*': { + added: true, + permissions: { + 'label-Code-Review': { + added: true, + rules: {}, + label: 'Code-Review', + }, + }, + }, + }, + remove: {}, + }; + const newSection = Polymer.dom(element.root) + .querySelectorAll('gr-access-section')[1]; + newSection._handleAddPermission(); + flushAsynchronousOperations(); + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + // Add rule to the new permission. + expectedInput = { + add: { + 'refs/for/*': { + added: true, + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + }, + remove: {}, + }; + + newSection.$$('gr-permission')._handleAddRuleItem( + {detail: {value: {id: 'Maintainers'}}}); + + flushAsynchronousOperations(); + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + // Modify a the reference from the default value. + element._local['refs/for/*'].updatedId = 'refs/for/new'; + expectedInput = { + add: { + 'refs/for/new': { + added: true, + updatedId: 'refs/for/new', + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + }, + remove: {}, + }; + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + }); + test('_computeAddAndRemove combinations', () => { // Modify rule and delete permission that it is inside of. element._local['refs/*'].permissions.owner.rules[123].modified = true; @@ -671,6 +764,150 @@ limitations under the License. }; element._local['refs/*'].deleted = true; assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + // Add a new section. + MockInteractions.tap(element.$.addReferenceBtn); + let newSection = Polymer.dom(element.root) + .querySelectorAll('gr-access-section')[1]; + newSection._handleAddPermission(); + flushAsynchronousOperations(); + newSection.$$('gr-permission')._handleAddRuleItem( + {detail: {value: {id: 'Maintainers'}}}); + // Modify a the reference from the default value. + element._local['refs/for/*'].updatedId = 'refs/for/new'; + + expectedInput = { + add: { + 'refs/for/new': { + added: true, + updatedId: 'refs/for/new', + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + }, + remove: { + 'refs/*': { + permissions: {}, + }, + }, + }; + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + // Modify newly added rule inside new ref. + element._local['refs/for/*'].permissions['label-Code-Review']. + rules['Maintainers'].modified = true; + expectedInput = { + add: { + 'refs/for/new': { + added: true, + updatedId: 'refs/for/new', + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + modified: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + }, + remove: { + 'refs/*': { + permissions: {}, + }, + }, + }; + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + + // Add a second new section. + MockInteractions.tap(element.$.addReferenceBtn); + newSection = Polymer.dom(element.root) + .querySelectorAll('gr-access-section')[2]; + newSection._handleAddPermission(); + flushAsynchronousOperations(); + newSection.$$('gr-permission')._handleAddRuleItem( + {detail: {value: {id: 'Maintainers'}}}); + // Modify a the reference from the default value. + element._local['refs/for/**'].updatedId = 'refs/for/new2'; + expectedInput = { + add: { + 'refs/for/new': { + added: true, + updatedId: 'refs/for/new', + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + modified: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + 'refs/for/new2': { + added: true, + updatedId: 'refs/for/new2', + permissions: { + 'label-Code-Review': { + added: true, + rules: { + Maintainers: { + action: 'ALLOW', + added: true, + max: 2, + min: -2, + }, + }, + label: 'Code-Review', + }, + }, + }, + }, + remove: { + 'refs/*': { + permissions: {}, + }, + }, + }; + assert.deepEqual(element._computeAddAndRemove(), expectedInput); + }); + + test('Unsaved added refs are discarded when edit cancelled', () => { + // Unsaved changes are discarded when editing is cancelled. + MockInteractions.tap(element.$.editBtn); + assert.equal(element._sections.length, 1); + assert.equal(Object.keys(element._local).length, 1); + MockInteractions.tap(element.$.addReferenceBtn); + assert.equal(element._sections.length, 2); + assert.equal(Object.keys(element._local).length, 2); + MockInteractions.tap(element.$.editBtn); + assert.equal(element._sections.length, 1); + assert.equal(Object.keys(element._local).length, 1); }); test('_handleSaveForReview', done => {