Merge "A11y - improve announcing"
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 07cb04f..bf00d27 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -132,9 +133,15 @@
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
-
+ String ccOrReviewer =
+ approvalsUtil
+ .getReviewers(ctx.getNotes())
+ .byState(ReviewerStateInternal.CC)
+ .contains(reviewerId)
+ ? "cc"
+ : "reviewer";
StringBuilder msg = new StringBuilder();
- msg.append("Removed reviewer " + reviewer.account().fullName());
+ msg.append(String.format("Removed %s %s", ccOrReviewer, reviewer.account().fullName()));
StringBuilder removedVotesMsg = new StringBuilder();
removedVotesMsg.append(" with the following votes:\n\n");
boolean votesRemoved = false;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index f59fba0..b7f1ef0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2258,6 +2258,10 @@
assertThat(message.body()).contains("Removed reviewer " + user.fullName() + ".");
assertThat(message.body()).doesNotContain("with the following votes");
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed reviewer " + user.fullName());
+
// Make sure the reviewer can still be added again.
gApi.changes().id(changeId).addReviewer(user.id().toString());
c = gApi.changes().id(changeId).get();
@@ -2273,6 +2277,31 @@
}
@Test
+ public void removeCC() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ // Add a cc
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = CC;
+ addReviewerInput.reviewer = user.id().toString();
+ gApi.changes().id(changeId).addReviewer(addReviewerInput);
+
+ // Remove a cc
+ sender.clear();
+ gApi.changes().id(changeId).reviewer(user.id().toString()).remove();
+ assertThat(gApi.changes().id(changeId).get().reviewers).isEmpty();
+
+ // Make sure the email for removing a cc is correct.
+ assertThat(sender.getMessages()).hasSize(1);
+ Message message = sender.getMessages().get(0);
+ assertThat(message.body()).contains("Removed cc " + user.fullName() + ".");
+
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed cc " + user.fullName());
+ }
+
+ @Test
public void removeReviewer() throws Exception {
testRemoveReviewer(true);
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 51ce9b4..20fbc32 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -42,6 +42,7 @@
import {
fetchChangeUpdates,
patchNumEquals,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {
changeIsOpen,
@@ -1591,7 +1592,7 @@
if (!labels) {
return Promise.resolve(undefined);
}
- return this.$.restAPI.saveChangeReview(newChangeId, 'current', {labels});
+ return this.$.restAPI.saveChangeReview(newChangeId, CURRENT, {labels});
}
_handleResponse(action: UIActionInfo, response?: Response) {
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 683d887..ef5be9fd 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -23,6 +23,7 @@
getParentIndex,
isMergeParent,
patchNumEquals,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {
@@ -612,7 +613,7 @@
}
getPortedComments(changeNum: NumericChangeId, revision?: RevisionId) {
- if (!revision) revision = 'current';
+ if (!revision) revision = CURRENT;
return Promise.all([
this.$.restAPI.getPortedComments(changeNum, revision),
this.$.restAPI.getPortedDrafts(changeNum, revision),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index bb0ed61..5a6113c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -48,6 +48,7 @@
computeLatestPatchNum,
patchNumEquals,
PatchSet,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {
addUnmodifiedFiles,
@@ -1047,7 +1048,7 @@
const portedCommentsPromise = this.$.commentAPI.getPortedComments(
value.changeNum,
- value.patchNum || 'current'
+ value.patchNum || CURRENT
);
const promises: Promise<unknown>[] = [];
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
index da2881e..0b4e577 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
@@ -46,6 +46,7 @@
} from '../../../utils/attention-set-util';
import {ReviewerState} from '../../../constants/constants';
import {isRemovableReviewer} from '../../../utils/change-util';
+import {CURRENT} from '../../../utils/patch-set-util';
export interface GrHovercardAccount {
$: {
@@ -186,7 +187,7 @@
];
this.$.restAPI
- .saveChangeReview(this.change._number, 'current', reviewInput)
+ .saveChangeReview(this.change._number, CURRENT, reviewInput)
.then(response => {
if (!response || !response.ok) {
throw new Error(
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index d75c186..b86fcff 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -22,6 +22,7 @@
import {ListChangesOption} from '../../../utils/change-util.js';
import {appContext} from '../../../services/app-context.js';
import {createChange} from '../../../test/test-data-generators.js';
+import {CURRENT} from '../../../utils/patch-set-util.js';
const basicFixture = fixtureFromElement('gr-rest-api-interface');
@@ -1350,7 +1351,7 @@
sinon.stub(element._restApiHelper, 'fetchJSON').returns(Promise.resolve({
ok: false}));
- element.getPortedComments(change._number, 'current');
+ element.getPortedComments(change._number, CURRENT);
assert.isFalse(dispatchStub.called);
});
@@ -1361,7 +1362,7 @@
const getChangeURLAndFetchStub = sinon.stub(element,
'_getChangeURLAndFetch');
- element.getPortedDrafts(change._number, 'current');
+ element.getPortedDrafts(change._number, CURRENT);
assert.isFalse(getChangeURLAndFetchStub.called);
});
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index 8974af8..d063168 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -45,6 +45,8 @@
PARENT: 'PARENT',
};
+export const CURRENT = 'current';
+
export interface PatchSet {
num: PatchSetNum;
desc: string | undefined;
