From 60f3d7742d6b43b2d1a95230793beca3402bf015 Mon Sep 17 00:00:00 2001
From: Aaron Gable <agable@chromium.org>
Date: Thu, 27 Apr 2017 13:00:42 -0700
Subject: [PATCH] Fall back to addByEmail for inactive CCs

When someone tries to add an inactive account as
a CC, we should simply treat it as though the user
isn't registered, instead of refusing to add them
because they used to be registered but haven't
visited in a while.

Bug: https://bugs.chromium.org/p/chromium/issues/detail?id=665263
Change-Id: I99d10bc86e7706fc96071f670d1f70c1117030bb
---
 .../acceptance/api/change/ChangeIT.java       | 27 +++++++++++++++++++
 .../gerrit/server/change/PostReviewers.java   |  3 +++
 2 files changed, 30 insertions(+)

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 2be0000880..498286c139 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -65,9 +65,11 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.api.projects.ConfigInput;
 import com.google.gerrit.extensions.client.ChangeKind;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.Comment.Range;
+import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.client.ListChangesOption;
 import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.client.Side;
@@ -1094,6 +1096,31 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(r.reviewers).isNull();
   }
 
+  @Test
+  public void addReviewerThatIsInactiveEmailFallback() throws Exception {
+    assume().that(notesMigration.enabled()).isTrue();
+
+    ConfigInput conf = new ConfigInput();
+    conf.enableReviewerByEmail = InheritableBoolean.TRUE;
+    gApi.projects().name(project.get()).config(conf);
+
+    PushOneCommit.Result result = createChange();
+
+    String username = "user@domain.com";
+    gApi.accounts().create(username).setActive(false);
+
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = username;
+    in.state = ReviewerState.CC;
+    AddReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
+
+    assertThat(r.input).isEqualTo(username);
+    assertThat(r.error).isNull();
+    // When adding by email, the reviewers field is also empty because we can't
+    // render a ReviewerInfo object for a non-account.
+    assertThat(r.reviewers).isNull();
+  }
+
   @Test
   public void addReviewer() throws Exception {
     TestTimeUtil.resetWithClockStep(1, SECONDS);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index eec0641843..13d5271591 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -240,6 +240,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
           reviewer, rsrc, ImmutableSet.of(member.getId()), null, state, notify, accountsToNotify);
     }
     if (!member.isActive()) {
+      if (allowByEmail && state == CC) {
+        return null;
+      }
       return fail(reviewer, MessageFormat.format(ChangeMessages.get().reviewerInactive, reviewer));
     }
     return fail(