opendev/gerrit
Code Issues Proposed changes

Merge "Fix faulty reviewer removal behavior"

Browse Source
This commit is contained in:
Logan Hanks
2017-05-13 00:04:29 +00:00
committed by Gerrit Code Review
parent 47027bf0a0 7591ceb1ec
commit d18bf04340
2 changed files with 14 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
View File

@@ -457,7 +457,7 @@
}
for (const entry of change[key]) {
if (entry._account_id === owner._account_id) {
return;
continue;
}
switch (key) {
case 'REVIEWER':
@@ -537,11 +537,14 @@
.then(() => {
return this.send(this._includeComments);
})
.then(this._purgeReviewersPendingRemove.bind(this));
.then(keepReviewers => {
this._purgeReviewersPendingRemove(false, keepReviewers);
});
return;
}
this.send(this._includeComments)
.then(this._purgeReviewersPendingRemove.bind(this));
this.send(this._includeComments).then(keepReviewers => {
this._purgeReviewersPendingRemove(false, keepReviewers);
});
},
_saveReview(review, opt_errFn) {

10
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
View File

@@ -614,8 +614,9 @@ limitations under the License.
const reviewer2 = makeAccount();
const cc1 = makeAccount();
const cc2 = makeAccount();
const cc3 = makeAccount();
element._reviewers = [reviewer1, reviewer2];
element._ccs = [cc1, cc2];
element._ccs = [cc1, cc2, cc3];
const mutations = [];
@@ -633,6 +634,7 @@ limitations under the License.
reviewers.fire('remove', {account: reviewer1});
ccs.$.entry.fire('add', {value: {account: reviewer1}});
ccs.fire('remove', {account: cc1});
ccs.fire('remove', {account: cc3});
reviewers.$.entry.fire('add', {value: {account: cc1}});
// Add to other field without removing from former field.
@@ -647,9 +649,10 @@ limitations under the License.
return result;
};
// Send and purge and verify moves without deletions.
// Send and purge and verify moves, delete cc3.
element.send()
.then(element._purgeReviewersPendingRemove.bind(element))
.then(keepReviewers =>
element._purgeReviewersPendingRemove(false, keepReviewers))
.then(() => {
assert.deepEqual(
mutations, [
@@ -657,6 +660,7 @@ limitations under the License.
mapReviewer(cc2),
mapReviewer(reviewer1, 'CC'),
mapReviewer(reviewer2, 'CC'),
{account: cc3, state: 'REMOVED'},
]);
done();
});