Merge "Display the approval table on the Publish Comments screen."

gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java

@@ -20,6 +20,9 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 public class PatchSetPublishDetail {
@@ -28,6 +31,8 @@ public class PatchSetPublishDetail {
   protected Change change;
   protected List<PatchLineComment> drafts;
   protected List<PermissionRange> labels;
+  protected List<ApprovalDetail> approvals;
+  protected List<SubmitRecord> submitRecords;
   protected List<PatchSetApproval> given;
   protected boolean canSubmit;
 
@@ -39,6 +44,23 @@ public class PatchSetPublishDetail {
     this.labels = labels;
   }
 
+  public List<ApprovalDetail> getApprovals() {
+    return approvals;
+  }
+
+  public void setApprovals(Collection<ApprovalDetail> list) {
+    approvals = new ArrayList<ApprovalDetail>(list);
+    Collections.sort(approvals, ApprovalDetail.SORT);
+  }
+
+  public void setSubmitRecords(List<SubmitRecord> all) {
+    submitRecords = all;
+  }
+
+  public List<SubmitRecord> getSubmitRecords() {
+    return submitRecords;
+  }
+
   public List<PatchSetApproval> getGiven() {
     return given;
   }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java

@@ -29,6 +29,7 @@ import com.google.gerrit.common.data.ApprovalDetail;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.ChangeDetail;
+import com.google.gerrit.common.data.PatchSetPublishDetail;
 import com.google.gerrit.common.data.ReviewerResult;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.reviewdb.client.Account;
@@ -133,12 +134,22 @@ public class ApprovalTable extends Composite {
     return AccountLink.link(accountCache, id);
   }
 
+  void display(PatchSetPublishDetail detail) {
+    doDisplay(detail.getChange(), detail.getApprovals(),
+        detail.getSubmitRecords());
+  }
+
   void display(ChangeDetail detail) {
-    changeId = detail.getChange().getId();
+    doDisplay(detail.getChange(), detail.getApprovals(),
+        detail.getSubmitRecords());
+  }
+
+  private void doDisplay(Change change, List<ApprovalDetail> approvals,
+      List<SubmitRecord> submitRecords) {
+    changeId = change.getId();
     reviewerSuggestOracle.setChange(changeId);
 
     List<String> columns = new ArrayList<String>();
-    List<ApprovalDetail> rows = detail.getApprovals();
 
     final Element missingList = missing.getElement();
     while (DOM.getChildCount(missingList) > 0) {
@@ -146,16 +157,16 @@ public class ApprovalTable extends Composite {
     }
     missing.setVisible(false);
 
-    if (detail.getSubmitRecords() != null) {
+    if (submitRecords != null) {
       HashSet<String> reportedMissing = new HashSet<String>();
 
       HashMap<Account.Id, ApprovalDetail> byUser =
           new HashMap<Account.Id, ApprovalDetail>();
-      for (ApprovalDetail ad : detail.getApprovals()) {
+      for (ApprovalDetail ad : approvals) {
         byUser.put(ad.getAccount(), ad);
       }
 
-      for (SubmitRecord rec : detail.getSubmitRecords()) {
+      for (SubmitRecord rec : submitRecords) {
         if (rec.labels == null) {
           continue;
         }
@@ -200,7 +211,7 @@ public class ApprovalTable extends Composite {
       missing.setVisible(!reportedMissing.isEmpty());
 
     } else {
-      for (ApprovalDetail ad : rows) {
+      for (ApprovalDetail ad : approvals) {
         for (PatchSetApproval psa : ad.getPatchSetApprovals()) {
           ApprovalType legacyType = types.byId(psa.getCategoryId());
           if (legacyType == null) {
@@ -236,13 +247,13 @@ public class ApprovalTable extends Composite {
       }
     }
 
-    if (rows.isEmpty()) {
+    if (approvals.isEmpty()) {
       table.setVisible(false);
     } else {
       displayHeader(columns);
-      table.resizeRows(1 + rows.size());
+      table.resizeRows(1 + approvals.size());
-      for (int i = 0; i < rows.size(); i++) {
+      for (int i = 0; i < approvals.size(); i++) {
-        displayRow(i + 1, rows.get(i), detail.getChange(), columns);
+        displayRow(i + 1, approvals.get(i), change, columns);
       }
       table.setVisible(true);
     }
@@ -250,7 +261,7 @@ public class ApprovalTable extends Composite {
     addReviewer.setVisible(Gerrit.isSignedIn());
 
     if (Gerrit.getConfig().testChangeMerge()
-        && !detail.getChange().isMergeable()) {
+        && !change.isMergeable()) {
       Element li = DOM.createElement("li");
       li.setClassName(Gerrit.RESOURCES.css().missingApproval());
       DOM.setInnerText(li, Util.C.messageNeedsRebaseOrHasDependency());

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java

@@ -64,6 +64,7 @@ public class PublishCommentScreen extends AccountScreen implements
   private final PatchSet.Id patchSetId;
   private Collection<ValueRadioButton> approvalButtons;
   private ChangeDescriptionBlock descBlock;
+  private ApprovalTable approvals;
   private Panel approvalPanel;
   private NpTextArea message;
   private FlowPanel draftsPanel;
@@ -86,6 +87,9 @@ public class PublishCommentScreen extends AccountScreen implements
     descBlock = new ChangeDescriptionBlock(null);
     add(descBlock);
 
+    approvals = new ApprovalTable();
+    add(approvals);
+
     final FormPanel form = new FormPanel();
     final FlowPanel body = new FlowPanel();
     form.setWidget(body);
@@ -274,6 +278,11 @@ public class PublishCommentScreen extends AccountScreen implements
 
     if (r.getChange().getStatus().isOpen()) {
       initApprovals(r, approvalPanel);
+
+      approvals.setAccountInfoCache(r.getAccounts());
+      approvals.display(r);
+    } else {
+      approvals.setVisible(false);
     }
 
     if (lastState != null && patchSetId.equals(lastState.patchSetId)) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css

@@ -955,7 +955,7 @@
   margin-top: 5px;
 }
 
-.changeScreen .approvalTable {
+.approvalTable {
   margin-top: 1em;
   margin-bottom: 1em;
 }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java

@@ -14,12 +14,14 @@
 
 package com.google.gerrit.httpd.rpc.changedetail;
 
+import com.google.gerrit.common.data.ApprovalDetail;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.PatchSetPublishDetail;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.httpd.rpc.Handler;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -32,6 +34,8 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.workflow.CategoryFunction;
+import com.google.gerrit.server.workflow.FunctionState;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -49,6 +53,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
 
   private final PatchSetInfoFactory infoFactory;
   private final ReviewDb db;
+  private final FunctionState.Factory functionState;
   private final ChangeControl.Factory changeControlFactory;
   private final ApprovalTypes approvalTypes;
   private final AccountInfoCacheFactory aic;
@@ -64,11 +69,13 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
   PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory,
       final ReviewDb db,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
+      final FunctionState.Factory functionState,
       final ChangeControl.Factory changeControlFactory,
       final ApprovalTypes approvalTypes,
       final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
     this.infoFactory = infoFactory;
     this.db = db;
+    this.functionState = functionState;
     this.changeControlFactory = changeControlFactory;
     this.approvalTypes = approvalTypes;
     this.aic = accountInfoCacheFactory.create();
@@ -130,6 +137,8 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
           int ok = 0;
 
           for (SubmitRecord.Label lbl : rec.labels) {
+            aic.want(lbl.appliedBy);
+
             boolean canMakeOk = false;
             PermissionRange range = rangeByName.get(lbl.label);
             if (range != null) {
@@ -167,12 +176,60 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
       if (couldSubmit && control.getRefControl().canSubmit()) {
         detail.setCanSubmit(true);
       }
+
+      detail.setSubmitRecords(submitRecords);
     }
 
     detail.setLabels(allowed);
     detail.setGiven(given);
     detail.setAccounts(aic.create());
 
+    loadApprovals(detail, control);
+
     return detail;
   }
+
+  private void loadApprovals(final PatchSetPublishDetail detail,
+      final ChangeControl control) throws OrmException {
+    final PatchSet.Id psId = detail.getChange().currentPatchSetId();
+    final Change.Id changeId = patchSetId.getParentKey();
+    final List<PatchSetApproval> allApprovals =
+        db.patchSetApprovals().byChange(changeId).toList();
+
+    if (detail.getChange().getStatus().isOpen()) {
+      final FunctionState fs = functionState.create(control, psId, allApprovals);
+
+      for (final ApprovalType at : approvalTypes.getApprovalTypes()) {
+        CategoryFunction.forCategory(at.getCategory()).run(at, fs);
+      }
+    }
+
+    final boolean canRemoveReviewers = detail.getChange().getStatus().isOpen() //
+        && control.getCurrentUser() instanceof IdentifiedUser;
+    final HashMap<Account.Id, ApprovalDetail> ad =
+        new HashMap<Account.Id, ApprovalDetail>();
+    for (PatchSetApproval ca : allApprovals) {
+      ApprovalDetail d = ad.get(ca.getAccountId());
+      if (d == null) {
+        d = new ApprovalDetail(ca.getAccountId());
+        d.setCanRemove(canRemoveReviewers);
+        ad.put(d.getAccount(), d);
+      }
+      if (d.canRemove()) {
+        d.setCanRemove(control.canRemoveReviewer(ca));
+      }
+      if (ca.getPatchSetId().equals(psId)) {
+        d.add(ca);
+      }
+    }
+
+    final Account.Id owner = detail.getChange().getOwner();
+    if (ad.containsKey(owner)) {
+      // Ensure the owner always sorts to the top of the table
+      ad.get(owner).sortFirst();
+    }
+
+    aic.want(ad.keySet());
+    detail.setApprovals(ad.values());
+  }
 }