Refactor gr-repo-access Use recursive approach to find modifications and deletions. This is keeping current functionality- only modifying, removing rules. Change-Id: I30fec5ccb7de8973e99b7c8311b3a989d2dfd959
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 8efdd10..2110aaa 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
@@ -97,52 +97,6 @@ 'access-modified': '_handleAccessModified', }, - /** - * Gets an array of gr-acces-section Polymer elements. - * - * @return {!Array} - */ - _getSections() { - return Polymer.dom(this.root).querySelectorAll('gr-access-section'); - }, - - /** - * Gets an array of the gr-permission polymer elements for a particular - * access section. - * - * @param {!Object} section - * @return {!Array} - */ - _getPermissionsForSection(section) { - return Polymer.dom(section.root).querySelectorAll('gr-permission'); - }, - - /** - * Gets an array of the gr-rule-editor polymer elements for a particular - * permission (within a section). - * - * @param {!Object} permission - * @return {!Array} - */ - _getRulesForPermission(permission) { - return Polymer.dom(permission.root).querySelectorAll('gr-rule-editor'); - }, - - /** - * Gets an array of all rules for the entire project access. - * - * @return {!Array} - */ - _getAllRules() { - let rules = []; - for (const section of this._getSections()) { - for (const permission of this._getPermissionsForSection(section)) { - rules = rules.concat(this._getRulesForPermission(permission)); - } - } - return rules; - }, - _handleAccessModified() { this._modified = true; }, @@ -191,38 +145,49 @@ }, /** - * Returns whether or not a given permission exists in the remove object - * - * @param {!Object} remove - * @param {string} sectionId - * @param {string} permissionId - * @return {boolean} - */ - _permissionInRemove(remove, sectionId, permissionId) { - return !!(remove[sectionId] && - remove[sectionId].permissions[permissionId]); - }, - - /** - * Returns a projectAccessInput object that contains new permission Objects - * for a given permission in a section. - * * @param {!Defs.projectAccessInput} addRemoveObj - * @param {string} sectionId - * @param {string} permissionId - * + * @param {!Array} path + * @param {string} type add or remove + * @param {!Object=} opt_value value to add if the type is 'add' * @return {!Defs.projectAccessInput} */ - _generatePermissionObject(addRemoveObj, sectionId, permissionId) { - const permissionObjAdd = {}; - const permissionObjRemove = {}; - permissionObjAdd[permissionId] = {rules: {}}; - permissionObjRemove[permissionId] = {rules: {}}; - addRemoveObj.add[sectionId] = {permissions: permissionObjAdd}; - addRemoveObj.remove[sectionId] = {permissions: permissionObjRemove}; + _updateAddRemoveObj(addRemoveObj, path, type, opt_value) { + let curPos = addRemoveObj[type]; + for (const item of path) { + if (!curPos[item]) { + if (item === path[path.length - 1] && type === 'remove') { + curPos[item] = null; + } else if (item === path[path.length - 1] && type === 'add') { + curPos[item] = opt_value; + } else { + curPos[item] = {}; + } + } + curPos = curPos[item]; + } return addRemoveObj; }, + _recursivelyUpdateAddRemoveObj(obj, addRemoveObj, path = []) { + for (const k in obj) { + if (!obj.hasOwnProperty(k)) { return; } + if (typeof obj[k] == 'object') { + if (obj[k].deleted) { + this._updateAddRemoveObj(addRemoveObj, + path.concat(k), 'remove'); + continue; + } else if (obj[k].modified) { + this._updateAddRemoveObj(addRemoveObj, + path.concat(k), 'remove'); + this._updateAddRemoveObj(addRemoveObj, + path.concat(k), 'add', obj[k]); + } + this._recursivelyUpdateAddRemoveObj(obj[k], addRemoveObj, + path.concat(k)); + } + } + }, + /** * Returns an object formatted for saving or submitting access changes for * review @@ -230,46 +195,12 @@ * @return {!Defs.projectAccessInput} */ _computeAddAndRemove() { - let addRemoveObj = { + const addRemoveObj = { add: {}, remove: {}, }; - const sections = this._getSections(); - for (const section of sections) { - const sectionId = section.section.id; - const permissions = this._getPermissionsForSection(section); - for (const permission of permissions) { - const permissionId = permission.permission.id; - const rules = this._getRulesForPermission(permission); - for (const rule of rules) { - // Find all rules that are changed. In the event that it has been - // modified. - if (!rule.modified && !rule.deleted) { continue; } - const ruleId = rule.rule.id; - const ruleValue = rule.rule.value; - // If the rule's parent permission has already been added to the - // remove object (don't need to check add, as they are always - // done to both at the same time). If it doesn't exist yet, it needs - // to be created. - if (!this._permissionInRemove(addRemoveObj.remove, sectionId, - permissionId)) { - addRemoveObj = this._generatePermissionObject(addRemoveObj, - sectionId, permissionId); - } - - // Remove the rule with a value of null - addRemoveObj.remove[sectionId].permissions[permissionId] - .rules[ruleId] = null; - // Add the rule with a value of the updated rule value, unless the - // rule has been removed. - if (!rule.deleted) { - addRemoveObj.add[sectionId].permissions[permissionId] - .rules[ruleId] = ruleValue; - } - } - } - } + this._recursivelyUpdateAddRemoveObj(this._local, addRemoveObj); return addRemoveObj; },
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 d789612..cfa20d9 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
@@ -82,7 +82,7 @@ permissions: { owner: { rules: { - 123: {action: 'DENY'}, + 123: {action: 'DENY', modified: true}, }, }, }, @@ -102,15 +102,7 @@ }; const repoAccessInputRemoved = { - add: { - 'refs/*': { - permissions: { - owner: { - rules: {}, - }, - }, - }, - }, + add: {}, remove: { 'refs/*': { permissions: { @@ -256,20 +248,6 @@ flushAsynchronousOperations(); }); - test('_getAllRules', () => { - assert.equal(element._getAllRules().length, 3); - }); - - test('_getSections, _getPermissionsForSection, _getRulesForPermission', - () => { - const sections = element._getSections(); - assert.equal(sections.length, 1); - const permissions = element._getPermissionsForSection(sections[0]); - assert.equal(permissions.length, 2); - assert.equal(element._getRulesForPermission(permissions[0]).length, - 2); - }); - test('button visibility for non admin', () => { assert.equal(getComputedStyle(element.$.saveBtn).display, 'none'); assert.equal(getComputedStyle(element.$.editBtn).display, 'none'); @@ -302,44 +280,15 @@ assert.isTrue(element._handleAccessModified.called); }); - test('_permissionInRemove', () => { - let toRemove = {}; - assert.isFalse(element._permissionInRemove(toRemove, 'refs/*', - 'owner')); - toRemove = { - 'refs/*': { - permissions: { - owner: {}, - }, - }, - }; - assert.isTrue(element._permissionInRemove(toRemove, 'refs/*', 'owner')); - }); - - test('_generatePermissionObject', () => { - const addRemoveObj = {add: {}, remove: {}}; - - const expectedResult = { - add: {'refs/*': { - permissions: {owner: {rules: {}}}}, - }, - remove: {'refs/*': { - permissions: {owner: {rules: {}}}}, - }, - }; - assert.deepEqual(element._generatePermissionObject( - addRemoveObj, 'refs/*', 'owner'), expectedResult); - }); - test('_computeAddAndRemove', () => { // With nothing modified + element._local = accessRes.local; assert.deepEqual(element._computeAddAndRemove(), {add: {}, remove: {}}); - const rules = element._getAllRules(); - rules[0].modified = true; - assert.deepEqual(element._computeAddAndRemove(), repoAccessInput); - rules[0].modified = false; - rules[0].deleted = true; + element._local['refs/*'].permissions.owner.rules[123].deleted = true; assert.deepEqual(element._computeAddAndRemove(), repoAccessInputRemoved); + delete element._local['refs/*'].permissions.owner.rules[123].deleted; + element._local['refs/*'].permissions.owner.rules[123].modified = true; + assert.deepEqual(element._computeAddAndRemove(), repoAccessInput); }); test('_handleSaveForReview', done => {