opendev/gerrit
Code Issues Proposed changes

Add the ability to add a new section in gr-repo-access

Browse Source 
Bug: Issue 8038
Change-Id: I23f7922614244cd1833602385c4cfa1657c7e8b9
This commit is contained in:
Becky Siegel
2018-01-30 15:52:18 -08:00
parent e5c06d622c
commit e6f68d96b1
6 changed files with 306 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
View File

@@ -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;
}
</style>
<style include="gr-form-styles"></style>
<fieldset id="section"
@@ -101,23 +95,21 @@ limitations under the License.
id="editBtn"
link
class$="[[_computeEditBtnClass(section.id)]]"
on-tap="_handleEditReference">
on-tap="editReference">
<iron-icon id="icon" icon="gr-icons:create"></iron-icon>
</gr-button>
</div>
<gr-button
link
id="deleteBtn"
on-tap="_handleRemoveReference">Remove</gr-button>
</div><!-- end header -->
<div class="editContainer">
<input
id="editRefInput"
bind-value="{{section.id}}"
is="iron-input"
type="text"
on-input="_handleValueChange">
</div><!-- end editContainer -->
<gr-button
link
id="deleteBtn"
on-tap="_handleRemoveReference">Remove</gr-button>
</div><!-- end header -->
<div class="sectionContent">
<template
is="dom-repeat"

11
polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.js
View File

@@ -78,11 +78,15 @@
},
_handleValueChange() {
if (!this.section.value.added) {
this.section.value.modified = this.section.id !== this._originalId;
this.section.value.updatedId = this.section.id;
// Allows overall access page to know a change has been made.
// For a new section, this is not fired because new permissions and
// rules have to be added in order to save, modifying the ref is not
// enough.
this.dispatchEvent(new CustomEvent('access-modified', {bubbles: true}));
}
this.section.value.updatedId = this.section.id;
},
_handleEditingChanged(editing, editingOld) {
@@ -187,8 +191,9 @@
delete this.section.value.deleted;
},
_handleEditReference() {
editReference() {
this._editingRef = true;
this.$.editRefInput.focus();
},
_computeSectionClass(editing, editingRef, deleted) {

18
polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.html
View File

@@ -264,8 +264,8 @@ limitations under the License.
assert.isFalse(element._editingRef);
});
test('_handleEditReference', () => {
element._handleEditReference();
test('editReference', () => {
element.editReference();
assert.isTrue(element._editingRef);
});
@@ -456,6 +456,7 @@ limitations under the License.
});
test('_handleValueChange', () => {
// For an exising section.
const modifiedHandler = sandbox.stub();
element.section = {id: 'refs/for/bar', value: {permissions: {}}};
assert.notOk(element.section.value.updatedId);
@@ -465,10 +466,21 @@ limitations under the License.
element._handleValueChange();
assert.equal(element.section.value.updatedId, 'refs/for/baz');
assert.isTrue(element.section.value.modified);
assert.isTrue(modifiedHandler.called);
assert.equal(modifiedHandler.callCount, 1);
element.section.id = 'refs/for/bar';
element._handleValueChange();
assert.isFalse(element.section.value.modified);
assert.equal(modifiedHandler.callCount, 2);
// For a new section.
element.section.value.added = true;
element._handleValueChange();
assert.isFalse(element.section.value.modified);
assert.equal(modifiedHandler.callCount, 2);
element.section.id = 'refs/for/bar';
element._handleValueChange();
assert.isFalse(element.section.value.modified);
assert.equal(modifiedHandler.callCount, 2);
});
test('remove section', () => {

3
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.html
View File

@@ -80,6 +80,9 @@ limitations under the License.
editing="[[_editing]]"
groups="[[_groups]]"></gr-access-section>
</template>
<gr-button id="addReferenceBtn"
class$="[[_computeShowSaveClass(_editing)]]"
on-tap="_handleCreateSection">Add Reference</gr-button>
</main>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>

35
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
View File

@@ -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();

237
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
View File

@@ -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 => {