Merge changes from topic 'draft-reviewers'

* changes:
  Use reviewers from ChangeInfo when rendering reviewers
  ChangeJson: Fix approvals for draft changes

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DraftChangeIT.java

@@ -16,14 +16,19 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.testutil.GerritServerTests.isNoteDbTestEnabled;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.RestSession;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.testutil.ConfigSuite;
@@ -33,6 +38,7 @@ import org.eclipse.jgit.lib.Config;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Collection;
 
 public class DraftChangeIT extends AbstractDaemonTest {
   @ConfigSuite.Config
@@ -109,6 +115,38 @@ public class DraftChangeIT extends AbstractDaemonTest {
     r.assertErrorStatus("draft workflow is disabled");
   }
 
+  @Test
+  public void listApprovalsOnDraftChange() throws Exception {
+    assume().that(isAllowDrafts()).isTrue();
+    PushOneCommit.Result result = createDraftChange();
+    result.assertOkStatus();
+    String changeId = result.getChangeId();
+    String triplet = project.get() + "~master~" + changeId;
+
+    gApi.changes().id(triplet).addReviewer(user.fullName);
+
+    ChangeInfo info = get(triplet);
+    LabelInfo label = info.labels.get("Code-Review");
+    assertThat(label.all).hasSize(1);
+    assertThat(label.all.get(0)._accountId).isEqualTo(user.id.get());
+    assertThat(label.all.get(0).value).isEqualTo(0);
+
+    ReviewerState rs = isNoteDbTestEnabled()
+        ? ReviewerState.REVIEWER : ReviewerState.CC;
+    Collection<AccountInfo> ccs = info.reviewers.get(rs);
+    assertThat(ccs).hasSize(1);
+    assertThat(ccs.iterator().next()._accountId).isEqualTo(user.id.get());
+
+    setApiUser(user);
+    gApi.changes().id(triplet).current().review(ReviewInput.recommend());
+    setApiUser(admin);
+
+    label = get(triplet).labels.get("Code-Review");
+    assertThat(label.all).hasSize(1);
+    assertThat(label.all.get(0)._accountId).isEqualTo(user.id.get());
+    assertThat(label.all.get(0).value).isEqualTo(1);
+  }
+
   private PushOneCommit.Result createDraftChange() throws Exception {
     return pushTo("refs/drafts/master");
   }

gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java

@@ -19,6 +19,7 @@ import com.google.gerrit.client.rpc.NativeString;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.common.data.LabelValue;
 import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -29,10 +30,13 @@ import com.google.gwt.core.client.JsArrayString;
 import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
 
 import java.sql.Timestamp;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
@@ -133,6 +137,23 @@ public class ChangeInfo extends JavaScriptObject {
   public final native JsArray<AccountInfo> removableReviewers()
   /*-{ return this.removable_reviewers; }-*/;
 
+  private final native NativeMap<JsArray<AccountInfo>> _reviewers()
+  /*-{ return this.reviewers; }-*/;
+  public final Map<ReviewerState, List<AccountInfo>> reviewers() {
+    NativeMap<JsArray<AccountInfo>> reviewers = _reviewers();
+    Map<ReviewerState, List<AccountInfo>> result = new HashMap<>();
+    for (String k : reviewers.keySet()) {
+      ReviewerState state = ReviewerState.valueOf(k.toUpperCase());
+      List<AccountInfo> accounts = result.get(state);
+      if (accounts == null) {
+        accounts = new ArrayList<>();
+        result.put(state, accounts);
+      }
+      accounts.addAll(Natives.asList(reviewers.get(k)));
+    }
+    return result;
+  }
+
   public final native boolean hasActions() /*-{ return this.hasOwnProperty('actions') }-*/;
   public final native NativeMap<ActionInfo> actions() /*-{ return this.actions; }-*/;
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java

@@ -29,6 +29,7 @@ import com.google.gerrit.client.rpc.NativeMap;
 import com.google.gerrit.client.rpc.NativeString;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.ui.RemoteSuggestBox;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.core.client.JavaScriptObject;
@@ -52,7 +53,9 @@ import com.google.gwt.user.client.ui.UIObject;
 import com.google.gwtexpui.safehtml.client.SafeHtml;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -205,15 +208,9 @@ public class Reviewers extends Composite {
   }
 
   private void display(ChangeInfo info) {
-    Map<Integer, AccountInfo> r = new HashMap<>();
+    Map<ReviewerState, List<AccountInfo>> reviewers = info.reviewers();
-    Map<Integer, AccountInfo> cc = new HashMap<>();
+    Map<Integer, AccountInfo> r = byAccount(reviewers, ReviewerState.REVIEWER);
-    for (LabelInfo label : Natives.asList(info.allLabels().values())) {
+    Map<Integer, AccountInfo> cc = byAccount(reviewers, ReviewerState.CC);
-      if (label.all() != null) {
-        for (ApprovalInfo ai : Natives.asList(label.all())) {
-          (ai.value() != 0 ? r : cc).put(ai._accountId(), ai);
-        }
-      }
-    }
     for (Integer i : r.keySet()) {
       cc.remove(i);
     }
@@ -237,6 +234,19 @@ public class Reviewers extends Composite {
     }
   }
 
+  private static Map<Integer, AccountInfo> byAccount(
+      Map<ReviewerState, List<AccountInfo>> reviewers, ReviewerState state) {
+    List<AccountInfo> accounts = reviewers.get(state);
+    if (accounts == null) {
+      return Collections.emptyMap();
+    }
+    Map<Integer, AccountInfo> result = new HashMap<>();
+    for (AccountInfo a : accounts) {
+      result.put(a._accountId(), a);
+    }
+    return result;
+  }
+
   private static Map<Integer, VotableInfo> votable(ChangeInfo change) {
     Map<Integer, VotableInfo> d = new HashMap<>();
     for (String name : change.labels()) {

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java

@@ -158,6 +158,7 @@ public class ChangeJson {
   private final GpgApiAdapter gpgApi;
 
   private AccountLoader accountLoader;
+  private Map<Change.Id, List<SubmitRecord>> submitRecords;
   private FixInput fix;
 
   @AssistedInject
@@ -485,14 +486,22 @@ public class ChangeJson {
   }
 
   private List<SubmitRecord> submitRecords(ChangeData cd) throws OrmException {
-    if (cd.getSubmitRecords() != null) {
+    // Maintain our own cache rather than using cd.getSubmitRecords(),
-      return cd.getSubmitRecords();
+    // since the latter may not have used the same values for
+    // fastEvalLabels/allowDraft/etc.
+    // TODO(dborowitz): Handle this better at the ChangeData level.
+    if (submitRecords == null) {
+      submitRecords = new HashMap<>();
     }
-    cd.setSubmitRecords(new SubmitRuleEvaluator(cd)
+    List<SubmitRecord> records = submitRecords.get(cd.getId());
+    if (records == null) {
+      records = new SubmitRuleEvaluator(cd)
         .setFastEvalLabels(true)
         .setAllowDraft(true)
-        .evaluate());
+        .evaluate();
-    return cd.getSubmitRecords();
+      submitRecords.put(cd.getId(), records);
+    }
+    return records;
   }
 
   private Map<String, LabelInfo> labelsFor(ChangeControl ctl,