Fix faulty reviewer removal behavior
WIP mode introduced an issue in which _purgeReviewersPendingRemove was
being called as a callback to send. This is expected, but the order of
the parameters being supplied to the function was incorrect. The return
value of send() is always a truthy value, which was then supplied as the
isCancel param of _purgeReviewersPendingRemove.
Bug: Issue 6048
Change-Id: Ic2d2b825c6a83298095aa8c7f3c458c2eed9d195
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index dc40fdd..2737f18 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -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) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index ef0d9dc..86219e7 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -614,8 +614,9 @@
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 @@
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 @@
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 @@
mapReviewer(cc2),
mapReviewer(reviewer1, 'CC'),
mapReviewer(reviewer2, 'CC'),
+ {account: cc3, state: 'REMOVED'},
]);
done();
});
