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

This reverts commit 3fc98f99b4.
The display was buggy and listed users as "Anonymous Coward"
even though they had a Full Name supplied in the Account. I
don't have time to debug this myself, but users are unhappy
the table is confusing/useless with everyone being called
an Anonymous Coward so the change goes out.

Bug: issue 1383
CC: Joe Hansche <jhansche@myyearbook.com>
Change-Id: Ibe9f70bc001df4b3a04ee2eb4dfa194f75e5b3f0

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

@@ -20,9 +20,6 @@ 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 {
@@ -31,8 +28,6 @@ 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;
 
@@ -44,23 +39,6 @@ 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,7 +29,6 @@ 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;
@@ -134,22 +133,12 @@ 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) {
-    doDisplay(detail.getChange(), detail.getApprovals(),
-        detail.getSubmitRecords());
-  }
-
-  private void doDisplay(Change change, List<ApprovalDetail> approvals,
-      List<SubmitRecord> submitRecords) {
-    changeId = change.getId();
+    changeId = detail.getChange().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) {
@@ -157,16 +146,16 @@ public class ApprovalTable extends Composite {
     }
     missing.setVisible(false);
 
-    if (submitRecords != null) {
+    if (detail.getSubmitRecords() != null) {
       HashSet<String> reportedMissing = new HashSet<String>();
 
       HashMap<Account.Id, ApprovalDetail> byUser =
           new HashMap<Account.Id, ApprovalDetail>();
-      for (ApprovalDetail ad : approvals) {
+      for (ApprovalDetail ad : detail.getApprovals()) {
         byUser.put(ad.getAccount(), ad);
       }
 
-      for (SubmitRecord rec : submitRecords) {
+      for (SubmitRecord rec : detail.getSubmitRecords()) {
         if (rec.labels == null) {
           continue;
         }
@@ -211,7 +200,7 @@ public class ApprovalTable extends Composite {
       missing.setVisible(!reportedMissing.isEmpty());
 
     } else {
-      for (ApprovalDetail ad : approvals) {
+      for (ApprovalDetail ad : rows) {
         for (PatchSetApproval psa : ad.getPatchSetApprovals()) {
           ApprovalType legacyType = types.byId(psa.getCategoryId());
           if (legacyType == null) {
@@ -247,13 +236,13 @@ public class ApprovalTable extends Composite {
       }
     }
 
-    if (approvals.isEmpty()) {
+    if (rows.isEmpty()) {
       table.setVisible(false);
     } else {
       displayHeader(columns);
-      table.resizeRows(1 + approvals.size());
-      for (int i = 0; i < approvals.size(); i++) {
-        displayRow(i + 1, approvals.get(i), change, columns);
+      table.resizeRows(1 + rows.size());
+      for (int i = 0; i < rows.size(); i++) {
+        displayRow(i + 1, rows.get(i), detail.getChange(), columns);
       }
       table.setVisible(true);
     }
@@ -261,7 +250,7 @@ public class ApprovalTable extends Composite {
     addReviewer.setVisible(Gerrit.isSignedIn());
 
     if (Gerrit.getConfig().testChangeMerge()
-        && !change.isMergeable()) {
+        && !detail.getChange().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,7 +64,6 @@ 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;
@@ -87,9 +86,6 @@ 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);
@@ -278,11 +274,6 @@ 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;
 }
 
-.approvalTable {
+.changeScreen .approvalTable {
   margin-top: 1em;
   margin-bottom: 1em;
 }

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

@@ -14,14 +14,12 @@
 
 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;
@@ -34,8 +32,6 @@ 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;
@@ -53,7 +49,6 @@ 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;
@@ -69,13 +64,11 @@ 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();
@@ -137,8 +130,6 @@ 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) {
@@ -176,60 +167,12 @@ 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());
-  }
 }