opendev/gerrit
Code Issues Proposed changes

Do not normalize approval scores on closed changes in dashboard

Browse Source 
If a change is open, we want to normalize the current score objects
to the actual allowed range for the user, in case their permissions
were modified since they made the approval.

However, if a change is closed, the scores for the current patch set
were normalized and frozen in the database during submit.  We should
not be normalized them further.  If we did normalize them again we
might convert a "Code Review +2" that was valid at time of submit
into a "Code Review +1", if the user has lost their +2 rights in
that project after the change was submitted.

Change-Id: I8f19ba3a276071437d027da9f78f4b7c53771b2a
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-09-03 15:35:42 -07:00
parent 0760b34199
commit 99c665d397
1 changed files with 12 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/main/java/com/google/gerrit/server/rpc/patch/PatchDetailServiceImpl.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.rpc.patch;
import com.google.gerrit.client.data.AccountInfo;
import com.google.gerrit.client.data.ApprovalSummary;
import com.google.gerrit.client.data.ApprovalSummarySet;
import com.google.gerrit.client.data.ApprovalType;
@@ -59,7 +58,6 @@ import com.google.inject.Provider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -347,9 +345,11 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
for (final PatchSetApproval ca : db.patchSetApprovals()
.byPatchSetUser(ps_id, aid)) {
fs.normalize(approvalTypes.getApprovalType(ca.getCategoryId()),
ca);
psas.put(ca.getCategoryId(), ca);
final ApprovalCategory.Id category = ca.getCategoryId();
if (change.getStatus().isOpen()) {
fs.normalize(approvalTypes.getApprovalType(category), ca);
}
psas.put(category, ca);
}
approvals.put(id, new ApprovalSummary(psas.values()));
@@ -386,11 +386,13 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
for (PatchSetApproval ca : db.patchSetApprovals()
.byPatchSet(ps_id)) {
fs.normalize(approvalTypes.getApprovalType(ca.getCategoryId()),
ca);
final ApprovalCategory.Id category = ca.getCategoryId();
if (change.getStatus().isOpen()) {
fs.normalize(approvalTypes.getApprovalType(category), ca);
}
boolean keep = true;
if (psas.containsKey(ca.getCategoryId())) {
final short oldValue = psas.get(ca.getCategoryId()).getValue();
if (psas.containsKey(category)) {
final short oldValue = psas.get(category).getValue();
final short newValue = ca.getValue();
keep = (Math.abs(oldValue) < Math.abs(newValue))
|| ((Math.abs(oldValue) == Math.abs(newValue)
@@ -398,7 +400,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
}
if (keep) {
aicFactory.want(ca.getAccountId());
psas.put(ca.getCategoryId(), ca);
psas.put(category, ca);
}
}