Normalize the change approvals on unclosed changes

If a change hasn't been closed yet its approvals are still in flux.
We normalize the value prior to display based on the current values
in the ProjectRights entities for the destination project.

Any approval category not at its max is listed in the new member of
ChangeDetail so clients can determine what status is missing from
this change.

Signed-off-by: Shawn O. Pearce <sop@google.com>

appjar/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java

@@ -24,25 +24,37 @@ import com.google.gerrit.client.reviewdb.ApprovalCategoryValue;
 import com.google.gerrit.client.reviewdb.ChangeApproval;
 import com.google.gerrit.client.ui.AccountDashboardLink;
 import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Grid;
+import com.google.gwt.user.client.ui.Label;
+import com.google.gwt.user.client.ui.Panel;
 import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /** Displays a table of {@link ApprovalDetail} objects for a change record. */
 public class ApprovalTable extends Composite {
   private final List<ApprovalType> types;
   private final Grid table;
+  private final Panel missing;
   private AccountInfoCache accountCache = AccountInfoCache.empty();
 
   public ApprovalTable() {
+
     types = Gerrit.getGerritConfig().getApprovalTypes();
     table = new Grid(1, 3 + types.size());
     table.addStyleName("gerrit-InfoTable");
     displayHeader();
 
-    initWidget(table);
+    missing = new FlowPanel();
+    missing.setStyleName("gerrit-Change-MissingApprovalList");
+
+    final FlowPanel fp = new FlowPanel();
+    fp.add(table);
+    fp.add(missing);
+    initWidget(fp);
   }
 
   private void displayHeader() {
@@ -86,7 +98,8 @@ public class ApprovalTable extends Composite {
     return AccountDashboardLink.link(accountCache, id);
   }
 
-  public void display(final List<ApprovalDetail> rows) {
+  public void display(final Set<ApprovalCategory.Id> need,
+      final List<ApprovalDetail> rows) {
     final int oldcnt = table.getRowCount();
     table.resizeRows(1 + rows.size());
     if (oldcnt < 1 + rows.size()) {
@@ -99,6 +112,20 @@ public class ApprovalTable extends Composite {
     for (int i = 0; i < rows.size(); i++) {
       displayRow(i + 1, rows.get(i));
     }
+
+    missing.clear();
+    missing.setVisible(false);
+    if (need != null) {
+      for (final ApprovalType at : types) {
+        if (need.contains(at.getCategory().getId())) {
+          final Label l =
+              new Label(Util.M.needApproval(at.getCategory().getName()));
+          l.setStyleName("gerrit-Change-MissingApproval");
+          missing.add(l);
+          missing.setVisible(true);
+        }
+      }
+    }
   }
 
   private void displayRow(final int row, final ApprovalDetail ad) {

appjar/src/main/java/com/google/gerrit/client/changes/ChangeDetailServiceImpl.java

@@ -16,6 +16,7 @@ package com.google.gerrit.client.changes;
 
 import com.google.gerrit.client.data.AccountInfoCacheFactory;
 import com.google.gerrit.client.data.ChangeDetail;
+import com.google.gerrit.client.data.GerritConfig;
 import com.google.gerrit.client.data.GroupCache;
 import com.google.gerrit.client.data.PatchSetDetail;
 import com.google.gerrit.client.reviewdb.Change;
@@ -23,6 +24,7 @@ import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.BaseServiceImplementation;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
+import com.google.gerrit.client.workflow.RightRule;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.SchemaFactory;
@@ -30,11 +32,13 @@ import com.google.gwtorm.client.SchemaFactory;
 public class ChangeDetailServiceImpl extends BaseServiceImplementation
     implements ChangeDetailService {
   private final GroupCache groupCache;
+  private final GerritConfig config;
 
   public ChangeDetailServiceImpl(final SchemaFactory<ReviewDb> rdf,
-      final GroupCache groups) {
+      final GroupCache groups, final GerritConfig c) {
     super(rdf);
     groupCache = groups;
+    config = c;
   }
 
   public void changeDetail(final Change.Id id,
@@ -46,8 +50,9 @@ public class ChangeDetailServiceImpl extends BaseServiceImplementation
           throw new Failure(new NoSuchEntityException());
         }
 
+        final RightRule rules = new RightRule(config, groupCache, db);
         final ChangeDetail d = new ChangeDetail();
-        d.load(db, new AccountInfoCacheFactory(db), change);
+        d.load(db, new AccountInfoCacheFactory(db), rules, change);
         return d;
       }
     });

appjar/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java

@@ -32,4 +32,6 @@ public interface ChangeMessages extends Messages {
   String renamedFrom(String sourcePath);
   String copiedFrom(String sourcePath);
   String otherFrom(String sourcePath);
+
+  String needApproval(String categoryName);
 }

appjar/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties

@@ -14,3 +14,5 @@ messageWrittenOn = on {0}
 renamedFrom = renamed from {0}
 copiedFrom = copied from {0}
 otherFrom = from {0}
+
+needApproval = Need {0}

appjar/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java

@@ -186,7 +186,7 @@ public class ChangeScreen extends Screen {
     description.setText(detail.getDescription());
     dependsOn.display(detail.getDependsOn());
     neededBy.display(detail.getNeededBy());
-    approvals.display(detail.getApprovals());
+    approvals.display(detail.getMissingApprovals(), detail.getApprovals());
 
     addPatchSets(detail);
     addMessages(detail);

appjar/src/main/java/com/google/gerrit/client/data/ApprovalDetail.java

@@ -15,14 +15,18 @@
 package com.google.gerrit.client.data;
 
 import com.google.gerrit.client.reviewdb.Account;
+import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.ChangeApproval;
+import com.google.gerrit.client.reviewdb.ProjectRight;
 
 import java.sql.Timestamp;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 public class ApprovalDetail {
   static final Timestamp EG_0 = new Timestamp(0);
@@ -62,4 +66,27 @@ public class ApprovalDetail {
       sortOrder = g;
     }
   }
+
+  void applyProjectRights(final GroupCache groupCache,
+      final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights) {
+    final Set<AccountGroup.Id> groups = groupCache.getGroups(account);
+    for (final ChangeApproval a : approvals) {
+      Collection<ProjectRight> l = rights.get(a.getCategoryId());
+      short min = 0, max = 0;
+      if (l != null) {
+        for (final ProjectRight r : l) {
+          if (groups.contains(r.getAccountGroupId())) {
+            min = (short) Math.min(min, r.getMinValue());
+            max = (short) Math.max(max, r.getMaxValue());
+          }
+        }
+      }
+
+      if (a.getValue() < min) {
+        a.setValue(min);
+      } else if (a.getValue() > max) {
+        a.setValue(max);
+      }
+    }
+  }
 }

appjar/src/main/java/com/google/gerrit/client/data/ChangeDetail.java

@@ -16,6 +16,7 @@ package com.google.gerrit.client.data;
 
 import com.google.gerrit.client.changes.ChangeScreen;
 import com.google.gerrit.client.reviewdb.Account;
+import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.ChangeApproval;
 import com.google.gerrit.client.reviewdb.ChangeMessage;
@@ -23,6 +24,7 @@ import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.PatchSetAncestor;
 import com.google.gerrit.client.reviewdb.RevId;
 import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gerrit.client.workflow.RightRule;
 import com.google.gwtorm.client.OrmException;
 
 import java.util.ArrayList;
@@ -32,6 +34,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /** Detail necessary to display {@link ChangeScreen}. */
 public class ChangeDetail {
@@ -41,6 +44,7 @@ public class ChangeDetail {
   protected List<ChangeInfo> neededBy;
   protected List<PatchSet> patchSets;
   protected List<ApprovalDetail> approvals;
+  protected Set<ApprovalCategory.Id> missingApprovals;
   protected List<ChangeMessage> messages;
   protected PatchSet.Id currentPatchSetId;
   protected PatchSetDetail currentDetail;
@@ -49,9 +53,10 @@ public class ChangeDetail {
   }
 
   public void load(final ReviewDb db, final AccountInfoCacheFactory acc,
-      final Change c) throws OrmException {
+      final RightRule rules, final Change c) throws OrmException {
     change = c;
-    acc.want(change.getOwner());
+    final Account.Id owner = change.getOwner();
+    acc.want(owner);
 
     patchSets = db.patchSets().byChange(change.getId()).toList();
     messages = db.changeMessages().byChange(change.getId()).toList();
@@ -59,15 +64,15 @@ public class ChangeDetail {
       acc.want(m.getAuthor());
     }
 
+    final List<ChangeApproval> allApprovals =
+        db.changeApprovals().byChange(change.getId()).toList();
+    if (!change.getStatus().isClosed()) {
+      missingApprovals =
+          rules.apply(change.getDest().getParentKey(), allApprovals);
+    }
     final HashMap<Account.Id, ApprovalDetail> ad =
         new HashMap<Account.Id, ApprovalDetail>();
-    {
-      final ApprovalDetail d = new ApprovalDetail(change.getOwner());
-      d.sortOrder = ApprovalDetail.EG_0;
-      // TODO Mark self-approved, self-verified if permitted.
-      ad.put(d.getAccount(), d);
-    }
-    for (ChangeApproval ca : db.changeApprovals().byChange(change.getId())) {
+    for (ChangeApproval ca : allApprovals) {
       ApprovalDetail d = ad.get(ca.getAccountId());
       if (d == null) {
         d = new ApprovalDetail(ca.getAccountId());
@@ -165,6 +170,10 @@ public class ChangeDetail {
     return approvals;
   }
 
+  public Set<ApprovalCategory.Id> getMissingApprovals() {
+    return missingApprovals;
+  }
+
   public PatchSet getCurrentPatchSet() {
     if (currentPatchSetId != null) {
       // We search through the list backwards because its *very* likely

appjar/src/main/java/com/google/gerrit/client/reviewdb/ChangeApproval.java

@@ -84,6 +84,7 @@ public final class ChangeApproval {
   public ChangeApproval(final ChangeApproval.Key k, final short v) {
     key = k;
     setValue(v);
+    setGranted();
   }
 
   public Change.Id getChangeId() {
@@ -104,7 +105,6 @@ public final class ChangeApproval {
 
   public void setValue(final short v) {
     value = v;
-    granted = new Timestamp(System.currentTimeMillis());
   }
 
   public void clear() {
@@ -114,4 +114,8 @@ public final class ChangeApproval {
   public Timestamp getGranted() {
     return granted;
   }
+
+  public void setGranted() {
+    granted = new Timestamp(System.currentTimeMillis());
+  }
 }

appjar/src/main/java/com/google/gerrit/client/workflow/RightRule.java

@@ -0,0 +1,152 @@
+// Copyright 2008 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+
+package com.google.gerrit.client.workflow;
+
+import com.google.gerrit.client.data.ApprovalType;
+import com.google.gerrit.client.data.GerritConfig;
+import com.google.gerrit.client.data.GroupCache;
+import com.google.gerrit.client.reviewdb.AccountGroup;
+import com.google.gerrit.client.reviewdb.ApprovalCategory;
+import com.google.gerrit.client.reviewdb.ApprovalCategoryValue;
+import com.google.gerrit.client.reviewdb.ChangeApproval;
+import com.google.gerrit.client.reviewdb.Project;
+import com.google.gerrit.client.reviewdb.ProjectRight;
+import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gwtorm.client.OrmException;
+import com.google.gwtorm.client.Transaction;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+public class RightRule {
+  private final GerritConfig config;
+  private final GroupCache groupCache;
+  private final ReviewDb db;
+
+  public RightRule(final GerritConfig c, final GroupCache gc, final ReviewDb d) {
+    config = c;
+    groupCache = gc;
+    db = d;
+  }
+
+  public Set<ApprovalCategory.Id> apply(final Project.NameKey projectName,
+      final Collection<ChangeApproval> approvals) throws OrmException {
+    final Project project = db.projects().get(projectName);
+    final Project.Id projectId = project != null ? project.getId() : null;
+    return apply(projectId, approvals);
+  }
+
+  public Set<ApprovalCategory.Id> apply(final Project.Id projectId,
+      final Collection<ChangeApproval> approvals) throws OrmException {
+    final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights;
+    final Map<ApprovalCategory.Id, ChangeApproval> max;
+
+    rights = loadRights(projectId);
+    max = new HashMap<ApprovalCategory.Id, ChangeApproval>();
+    for (final ChangeApproval a : approvals) {
+      normalize(rights, a);
+      final ChangeApproval m = max.get(a.getCategoryId());
+      if (m == null || m.getValue() < a.getValue()) {
+        max.put(a.getCategoryId(), a);
+      }
+    }
+
+    final Set<ApprovalCategory.Id> missing = new HashSet<ApprovalCategory.Id>();
+    for (final ApprovalType at : config.getApprovalTypes()) {
+      final ChangeApproval m = max.get(at.getCategory().getId());
+      final ApprovalCategoryValue n = at.getMax();
+      if (m == null || n == null || m.getValue() < n.getValue()) {
+        missing.add(at.getCategory().getId());
+      }
+    }
+    return missing;
+  }
+
+  public void normalize(final Project.NameKey projectName,
+      final Collection<ChangeApproval> approvals, final Transaction txn)
+      throws OrmException {
+    final Project project = db.projects().get(projectName);
+    final Project.Id projectId = project != null ? project.getId() : null;
+    normalize(projectId, approvals, txn);
+  }
+
+  public void normalize(final Project.Id projectId,
+      final Collection<ChangeApproval> approvals, final Transaction txn)
+      throws OrmException {
+    final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights;
+
+    rights = loadRights(projectId);
+    for (final ChangeApproval a : approvals) {
+      if (normalize(rights, a)) {
+        db.changeApprovals().update(Collections.singleton(a), txn);
+      }
+    }
+  }
+
+  private boolean normalize(
+      final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights,
+      final ChangeApproval a) {
+    short min = 0, max = 0;
+    final Collection<ProjectRight> l = rights.get(a.getCategoryId());
+    if (l != null) {
+      final Set<AccountGroup.Id> gs = groupCache.getGroups(a.getAccountId());
+      for (final ProjectRight r : l) {
+        if (gs.contains(r.getAccountGroupId())) {
+          min = (short) Math.min(min, r.getMinValue());
+          max = (short) Math.max(max, r.getMaxValue());
+        }
+      }
+    }
+    if (a.getValue() < min) {
+      a.setValue(min);
+      return true;
+    } else if (a.getValue() > max) {
+      a.setValue(max);
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  private Map<ApprovalCategory.Id, Collection<ProjectRight>> loadRights(
+      final Project.Id projectId) throws OrmException {
+    final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights =
+        new HashMap<ApprovalCategory.Id, Collection<ProjectRight>>();
+    if (projectId != null) {
+      loadRights(rights, projectId);
+    }
+    loadRights(rights, ProjectRight.WILD_PROJECT);
+    return rights;
+  }
+
+  private void loadRights(
+      final Map<ApprovalCategory.Id, Collection<ProjectRight>> rights,
+      final Project.Id projectId) throws OrmException {
+    for (final ProjectRight p : db.projectRights().byProject(projectId)) {
+      Collection<ProjectRight> l = rights.get(p.getApprovalCategoryId());
+      if (l == null) {
+        l = new ArrayList<ProjectRight>();
+        rights.put(p.getApprovalCategoryId(), l);
+      }
+      l.add(p);
+    }
+  }
+}

appjar/src/main/java/com/google/gerrit/public/Gerrit.css

@@ -480,6 +480,15 @@
   margin-bottom: 10px;
 }
 
+.gerrit-Change-MissingApprovalList {
+	margin-top: 5px;
+}
+.gerrit-Change-MissingApproval {
+	margin-left: 10px;
+	font-weight: bold;
+	white-space: nowrap;
+}
+
 
 /** SideBySideScreen **/
 .gerrit-SideBySideScreen-SideBySideTable {

appjar/src/main/java/com/google/gerrit/server/ChangeDetailServiceSrv.java

@@ -15,12 +15,19 @@
 package com.google.gerrit.server;
 
 import com.google.gerrit.client.changes.ChangeDetailServiceImpl;
+import com.google.gerrit.client.data.GerritConfig;
+import com.google.gerrit.client.data.GroupCache;
+import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gwtorm.client.SchemaFactory;
 
 /** Publishes {@link ChangeDetailServiceImpl} over JSON. */
 public class ChangeDetailServiceSrv extends GerritJsonServlet {
   @Override
   protected Object createServiceHandle() throws Exception {
     final GerritServer gs = GerritServer.getInstance();
-    return new ChangeDetailServiceImpl(gs.getDatabase(), gs.getGroupCache());
+    final SchemaFactory<ReviewDb> rdf = gs.getDatabase();
+    final GroupCache groups = gs.getGroupCache();
+    final GerritConfig config = gs.getGerritConfig();
+    return new ChangeDetailServiceImpl(rdf, groups, config);
   }
 }