From 95f61c6db050c82580ff65b95a4fd20eccaf5fc0 Mon Sep 17 00:00:00 2001
From: Paladox none <thomasmulhall410@yahoo.com>
Date: Mon, 26 Nov 2018 22:53:54 +0000
Subject: [PATCH 1/3] Fix assignee getting the owner's status on change list

Change-Id: I243cf476eb6881522c08686bff1deb3c633a5479
---
 .../change-list/gr-change-list-item/gr-change-list-item.html   | 3 ++-
 .../gr-change-list-item/gr-change-list-item_test.html          | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index 9a3fc03dc4..8c152b6ce6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -165,8 +165,9 @@ limitations under the License.
         hidden$="[[isColumnHidden('Assignee', visibleChangeTableColumns)]]">
       <template is="dom-if" if="[[change.assignee]]">
         <gr-account-link
+            id="assigneeAccountLink"
             account="[[change.assignee]]"
-            additional-text="[[_computeAccountStatusString(change.owner)]]"></gr-account-link>
+            additional-text="[[_computeAccountStatusString(change.assignee)]]"></gr-account-link>
       </template>
       <template is="dom-if" if="[[!change.assignee]]">
         <span class="placeholder">--</span>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
index aaad362785..3637653d53 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
@@ -182,10 +182,13 @@ limitations under the License.
       element.change = {
         assignee: {
           name: 'test',
+          status: 'test',
         },
       };
       flushAsynchronousOperations();
       assert.isOk(element.$$('.assignee gr-account-link'));
+      assert.equal(Polymer.dom(element.root)
+          .querySelector('#assigneeAccountLink').additionalText, '(test)');
     });
 
     test('_computeAccountStatusString', () => {

From da8a8ef4be0b3ac59ae02db6bd03a485a0792486 Mon Sep 17 00:00:00 2001
From: Sabari Ajay Kumar <sabarik@codeaurora.org>
Date: Sat, 24 Nov 2018 23:40:57 -0800
Subject: [PATCH 2/3] BugFix: Add documentation for 'read as' capability

An earlier change that added the 'read as' capability was missing
documentation.

Change-Id: Id9d75621599177997728df4594f46a5e61f3c2c3
---
 Documentation/access-control.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index d3f5d77e28..05c27312b5 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -1356,6 +1356,12 @@ This limit applies not only to the link:cmd-query.html[`gerrit query`]
 command, but also to the web UI results pagination size.
 
 
+[[capability_readAs]]
+=== Read As
+
+Allow users to impersonate any user to see which refs they can read.
+
+
 [[capability_runAs]]
 === Run As
 

From a4a21a69c1cb76c55bb89dc6895b96e4033492f8 Mon Sep 17 00:00:00 2001
From: Edwin Kempin <ekempin@google.com>
Date: Thu, 29 Nov 2018 10:09:47 +0100
Subject: [PATCH 3/3] Fix IAE when listing reviewers of change that has
 reviewers by email

Reviewers by email don't have an account ID but formatting reviewers as
JSON tried to get account IDs of all reviewers and trying to get an
account ID from a reviewer by email fails with an
IllegalArgumentException.

Reviewers by email only have an email and optionally a name. Since they
don't have an account we must not invoke the account loader for them. In
addition reviewers by email cannot have approvals so we can skip the
approval format step.

Listing reviewers of a change is not available through the extension API.
This is why the test needs to make a REST call to test this
functionality.

Change-Id: I97226ddce397f2ae9cc3c3526fbc1c4861e4ee58
Signed-off-by: Edwin Kempin <ekempin@google.com>
(cherry picked from commit b7a24723c121fc159962780b2f1ce0e176fe8e90)
---
 .../gerrit/server/change/ReviewerJson.java    | 30 ++++++++++-------
 .../rest/change/ChangeReviewersByEmailIT.java | 33 +++++++++++++++++--
 2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java
index 650256915b..ef2c926393 100644
--- a/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -23,6 +23,7 @@ import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.mail.Address;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -77,12 +78,15 @@ public class ReviewerJson {
       if (cd == null || !cd.getId().equals(rsrc.getChangeId())) {
         cd = changeDataFactory.create(db.get(), rsrc.getChangeResource().getNotes());
       }
-      ReviewerInfo info =
-          format(
-              new ReviewerInfo(rsrc.getReviewerUser().getAccountId().get()),
-              rsrc.getReviewerUser().getAccountId(),
-              cd);
-      loader.put(info);
+      ReviewerInfo info;
+      if (rsrc.isByEmail()) {
+        Address address = rsrc.getReviewerByEmail();
+        info = ReviewerInfo.byEmail(address.getName(), address.getEmail());
+      } else {
+        Account.Id reviewerAccountId = rsrc.getReviewerUser().getAccountId();
+        info = format(new ReviewerInfo(reviewerAccountId.get()), reviewerAccountId, cd);
+        loader.put(info);
+      }
       infos.add(info);
     }
     loader.fill();
@@ -94,19 +98,21 @@ public class ReviewerJson {
     return format(ImmutableList.<ReviewerResource>of(rsrc));
   }
 
-  public ReviewerInfo format(ReviewerInfo out, Account.Id reviewer, ChangeData cd)
+  public ReviewerInfo format(ReviewerInfo out, Account.Id reviewerAccountId, ChangeData cd)
       throws OrmException, PermissionBackendException {
     PatchSet.Id psId = cd.change().currentPatchSetId();
     return format(
         out,
-        reviewer,
+        reviewerAccountId,
         cd,
-        approvalsUtil.byPatchSetUser(
-            db.get(), cd.notes(), psId, new Account.Id(out._accountId), null, null));
+        approvalsUtil.byPatchSetUser(db.get(), cd.notes(), psId, reviewerAccountId, null, null));
   }
 
   public ReviewerInfo format(
-      ReviewerInfo out, Account.Id reviewer, ChangeData cd, Iterable<PatchSetApproval> approvals)
+      ReviewerInfo out,
+      Account.Id reviewerAccountId,
+      ChangeData cd,
+      Iterable<PatchSetApproval> approvals)
       throws OrmException, PermissionBackendException {
     LabelTypes labelTypes = cd.getLabelTypes();
 
@@ -123,7 +129,7 @@ public class ReviewerJson {
     PatchSet ps = cd.currentPatchSet();
     if (ps != null) {
       PermissionBackend.ForChange perm =
-          permissionBackend.absentUser(reviewer).database(db).change(cd);
+          permissionBackend.absentUser(reviewerAccountId).database(db).change(cd);
 
       for (SubmitRecord rec : submitRuleEvaluator.evaluate(cd)) {
         if (rec.labels == null) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index 257c88b4a0..dc71c1fffe 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -22,8 +22,8 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -35,11 +35,12 @@ import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.mail.Address;
 import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gson.reflect.TypeToken;
+import java.lang.reflect.Type;
 import java.util.List;
 import org.junit.Before;
 import org.junit.Test;
 
-@NoHttpd
 public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
 
   @Before
@@ -95,6 +96,34 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void listReviewersByEmail() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput input = new AddReviewerInput();
+      input.reviewer = toRfcAddressString(acc);
+      input.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(input);
+
+      RestResponse restResponse =
+          adminRestSession.get("/changes/" + r.getChangeId() + "/reviewers/");
+      restResponse.assertOK();
+      Type type = new TypeToken<List<ReviewerInfo>>() {}.getType();
+      List<ReviewerInfo> reviewers = newGson().fromJson(restResponse.getReader(), type);
+      restResponse.consume();
+
+      assertThat(reviewers).hasSize(1);
+      ReviewerInfo reviewerInfo = Iterables.getOnlyElement(reviewers);
+      assertThat(reviewerInfo._accountId).isNull();
+      assertThat(reviewerInfo.name).isEqualTo(acc.name);
+      assertThat(reviewerInfo.email).isEqualTo(acc.email);
+    }
+  }
+
   @Test
   public void removeByEmail() throws Exception {
     assume().that(notesMigration.readChanges()).isTrue();