diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java index d8c00cf00f..17c3488f7e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java @@ -71,6 +71,33 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest { } } + @Test + public void addByEmailAndById() throws Exception { + assume().that(notesMigration.readChanges()).isTrue(); + AccountInfo byEmail = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com"); + AccountInfo byId = new AccountInfo(user.id.get()); + + for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) { + PushOneCommit.Result r = createChange(); + + AddReviewerInput inputByEmail = new AddReviewerInput(); + inputByEmail.reviewer = toRfcAddressString(byEmail); + inputByEmail.state = state; + gApi.changes().id(r.getChangeId()).addReviewer(inputByEmail); + + AddReviewerInput inputById = new AddReviewerInput(); + inputById.reviewer = user.email; + inputById.state = state; + gApi.changes().id(r.getChangeId()).addReviewer(inputById); + + ChangeInfo info = + gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS)); + assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(byId, byEmail))); + // All reviewers (both by id and by email) should be removable + assertThat(info.removableReviewers).isEqualTo(ImmutableList.of(byId, byEmail)); + } + } + @Test public void removeByEmail() throws Exception { assume().that(notesMigration.readChanges()).isTrue(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 3b17eb001b..190f59bd1e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -528,14 +528,15 @@ public class ChangeJson { } out.reviewers = new HashMap<>(); - for (Map.Entry> e : - cd.reviewers().asTable().rowMap().entrySet()) { - out.reviewers.put(e.getKey().asReviewerState(), toAccountInfo(e.getValue().keySet())); - } - for (Map.Entry> e : - cd.reviewersByEmail().asTable().rowMap().entrySet()) { - out.reviewers.put( - e.getKey().asReviewerState(), toAccountInfoByEmail(e.getValue().keySet())); + for (ReviewerStateInternal state : ReviewerStateInternal.values()) { + if (state == ReviewerStateInternal.REMOVED) { + continue; + } + Collection reviewers = toAccountInfo(cd.reviewers().byState(state)); + reviewers.addAll(toAccountInfoByEmail(cd.reviewersByEmail().byState(state))); + if (!reviewers.isEmpty()) { + out.reviewers.put(state.asReviewerState(), reviewers); + } } out.removableReviewers = removableReviewers(ctl, out);