Merge "Return 400 BAD_REQUEST when a user cannot be added as reviewer by email" into stable-3.9
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index e46f9e4..b0e58c5 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -71,7 +72,7 @@
ReviewerModification modification =
reviewerModifier.prepare(rsrc.getNotes(), rsrc.getUser(), input, true);
if (modification.op == null) {
- return Response.ok(modification.result);
+ return Response.withStatusCode(SC_BAD_REQUEST, modification.result);
}
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (BatchUpdate bu =
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index 647e26b..b34974c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -100,14 +100,14 @@
// Attempt to add overly large group as reviewers.
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
- ReviewerResult result = addReviewer(changeId, largeGroup);
+ ReviewerResult result = addReviewer(changeId, largeGroup, SC_BAD_REQUEST);
assertThat(result.input).isEqualTo(largeGroup);
assertThat(result.confirm).isNull();
assertThat(result.error).contains("has too many members to add them all as reviewers");
assertThat(result.reviewers).isNull();
// Attempt to add medium group without confirmation.
- result = addReviewer(changeId, mediumGroup);
+ result = addReviewer(changeId, mediumGroup, SC_BAD_REQUEST);
assertThat(result.input).isEqualTo(mediumGroup);
assertThat(result.confirm).isTrue();
assertThat(result.error)
@@ -157,6 +157,48 @@
}
@Test
+ public void addCcEmailWithoutAccount() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String testEmailAddress = "email@without.account";
+
+ // Add a reviewer
+ ReviewerInput ri = new ReviewerInput();
+ ri.reviewer = user.email();
+ ri.state = REVIEWER;
+ ReviewerResult result = addReviewer(changeId, ri);
+ assertThat(result.input).isEqualTo(user.email());
+ assertThat(result.confirm).isNull();
+ assertThat(result.error).isNull();
+ assertThat(result.reviewers).hasSize(1);
+
+ // Add an email address that has no account to CC
+ ReviewerInput ccInput = new ReviewerInput();
+ ccInput.reviewer = testEmailAddress;
+ ccInput.state = CC;
+ ReviewerResult resultCC = addReviewer(changeId, ccInput, SC_BAD_REQUEST);
+ assertThat(resultCC.error).contains("Account '" + testEmailAddress + "' not found");
+ assertThat(resultCC.error)
+ .contains(testEmailAddress + " does not identify a registered user or group");
+ }
+
+ @Test
+ public void addReviewerEmailWithoutAccount() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String testEmailAddress = "email@without.account";
+
+ // Add a reviewer without an account
+ ReviewerInput ri = new ReviewerInput();
+ ri.reviewer = testEmailAddress;
+ ri.state = REVIEWER;
+ ReviewerResult result = addReviewer(changeId, ri, SC_BAD_REQUEST);
+ assertThat(result.error).contains("Account '" + testEmailAddress + "' not found");
+ assertThat(result.error)
+ .contains(testEmailAddress + " does not identify a registered user or group");
+ }
+
+ @Test
public void addCcGroup() throws Exception {
List<TestAccount> users = createAccounts(6, "addCcGroup");
List<String> usernames = new ArrayList<>(6);
