Revert "Disallow adding inactive reviewer"

This reverts commit e84b33e5b345b31a130890a80ebd55451f335fa7.

Reason for revert: the following use-cases were broken by the original change:

1. Revert a change where one of reviewers/cc is inactive.
2. Removing an inactive reviewer from the "Reply" dialog.

Release-Notes: skip
Google-Bug-Id: b/277053099
Change-Id: I7ff68c37e78c32a4dc9ce07208918862ebf118c1
(cherry picked from commit 17022251627494d22d0429b44ed8e9c226723cbc)

2 files changed, 22 insertions, 24 deletions

diff --git a/java/com/google/gerrit/server/change/ReviewerModifier.java b/java/com/google/gerrit/server/change/ReviewerModifier.java
index 49bd28acbf..b5e01810a5 100644
--- a/java/com/google/gerrit/server/change/ReviewerModifier.java
+++ b/java/com/google/gerrit/server/change/ReviewerModifier.java
@@ -113,7 +113,6 @@ public class ReviewerModifier {
 
   private enum FailureType {
     NOT_FOUND,
-    INACTIVE,
     OTHER;
   }
 
@@ -233,9 +232,6 @@ public class ReviewerModifier {
 
       ReviewerModification byAccountId = byAccountId(input, notes, user);
 
-      if (byAccountId.isFailure() && byAccountId.failureType.equals(FailureType.INACTIVE)) {
-        return byAccountId;
-      }
       ReviewerModification wholeGroup = null;
       if (!byAccountId.exactMatchFound) {
         wholeGroup = addWholeGroup(input, notes, user, confirmed, allowGroup, allowByEmail);
@@ -289,13 +285,6 @@ public class ReviewerModifier {
       } else {
         reviewerUser = accountResolver.resolveIncludeInactive(input.reviewer).asUniqueUser();
       }
-      if (reviewerUser.getAccount().inactive()) {
-        return fail(
-            input,
-            FailureType.INACTIVE,
-            String.format(
-                "Cannot add inactive account %s, %s", reviewerUser.getAccountId(), input.reviewer));
-      }
       if (input.reviewer.equalsIgnoreCase(reviewerUser.getName())
           || input.reviewer.equals(String.valueOf(reviewerUser.getAccountId()))) {
         exactMatchFound = true;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index b5c9aab09a..118b31b9d9 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -125,6 +125,7 @@ import com.google.gerrit.extensions.api.changes.RevertInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
 import com.google.gerrit.extensions.api.changes.ReviewResult;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.api.changes.ReviewerInput;
 import com.google.gerrit.extensions.api.changes.ReviewerResult;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
@@ -1370,23 +1371,26 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void addReviewerThatIsInactiveByUsername_notAllowed() throws Exception {
+  public void addReviewerThatIsInactiveByUsername() throws Exception {
     PushOneCommit.Result result = createChange();
 
     String username = name("new-user");
-    Account.Id accountId = accountOperations.newAccount().username(username).inactive().create();
+    Account.Id id = accountOperations.newAccount().username(username).inactive().create();
 
     ReviewerInput in = new ReviewerInput();
     in.reviewer = username;
     ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
 
     assertThat(r.input).isEqualTo(in.reviewer);
-    assertThat(r.error)
-        .isEqualTo(String.format("Cannot add inactive account %s, %s", accountId, username));
+    assertThat(r.error).isNull();
+    assertThat(r.reviewers).hasSize(1);
+    ReviewerInfo reviewer = r.reviewers.get(0);
+    assertThat(reviewer._accountId).isEqualTo(id.get());
+    assertThat(reviewer.username).isEqualTo(username);
   }
 
   @Test
-  public void addReviewerThatIsInactiveById_notAllowed() throws Exception {
+  public void addReviewerThatIsInactiveById() throws Exception {
     PushOneCommit.Result result = createChange();
 
     String username = name("new-user");
@@ -1397,28 +1401,33 @@ public class ChangeIT extends AbstractDaemonTest {
     ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
 
     assertThat(r.input).isEqualTo(in.reviewer);
-
-    assertThat(r.error)
-        .isEqualTo(String.format("Cannot add inactive account %s, %s", id, id.get()));
+    assertThat(r.error).isNull();
+    assertThat(r.reviewers).hasSize(1);
+    ReviewerInfo reviewer = r.reviewers.get(0);
+    assertThat(reviewer._accountId).isEqualTo(id.get());
+    assertThat(reviewer.username).isEqualTo(username);
   }
 
   @Test
-  public void addReviewerThatIsInactiveByEmail_notAllowed() throws Exception {
+  public void addReviewerThatIsInactiveByEmail() throws Exception {
     ConfigInput conf = new ConfigInput();
     conf.enableReviewerByEmail = InheritableBoolean.TRUE;
     gApi.projects().name(project.get()).config(conf);
     PushOneCommit.Result result = createChange();
     String username = "user@domain.com";
-    Account.Id accountId = accountOperations.newAccount().username(username).inactive().create();
+    Account.Id id = accountOperations.newAccount().username(username).inactive().create();
 
     ReviewerInput in = new ReviewerInput();
     in.reviewer = username;
     in.state = ReviewerState.CC;
     ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
 
-    assertThat(r.input).isEqualTo(in.reviewer);
-    assertThat(r.error)
-        .isEqualTo(String.format("Cannot add inactive account %s, %s", accountId, username));
+    assertThat(r.input).isEqualTo(username);
+    assertThat(r.error).isNull();
+    assertThat(r.ccs).hasSize(1);
+    AccountInfo reviewer = r.ccs.get(0);
+    assertThat(reviewer._accountId).isEqualTo(id.get());
+    assertThat(reviewer.username).isEqualTo(username);
   }
 
   @Test