opendev/gerrit
Code Issues Proposed changes

Display all reviewers of a closed change

Browse Source 
Readers of a closed change expect to see all reviewers, just like on
an open change. Build the table row-wise to ensure all users are
mentioned on any label that had a non-zero score stored on it at
merge time. This is an unfortunate guess the server has to make about
the state of the submit record.

Change-Id: I59b34d921dc06c64aba55eab0062a9143f0b9557
This commit is contained in:
Shawn Pearce
2013-03-08 17:52:49 -08:00
parent c5dab3f418
commit c0bcc1706b
1 changed files with 58 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -45,6 +45,7 @@ import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@@ -465,56 +466,82 @@ public class ChangeJson {
private Map<String, LabelInfo> labelsForClosedChange(ChangeData cd, private Map<String, LabelInfo> labelsForClosedChange(ChangeData cd,
boolean standard, boolean detailed) throws OrmException { boolean standard, boolean detailed) throws OrmException {
Set<Account.Id> allUsers = Sets.newHashSet();
for (PatchSetApproval psa : cd.allApprovals(db)) {
allUsers.add(psa.getAccountId());
}
Set<ApprovalCategory.Id> categories = Sets.newHashSet();
Multimap<Account.Id, PatchSetApproval> current = HashMultimap.create();
for (PatchSetApproval a : cd.currentApprovals(db)) {
if (a.getValue() != 0) {
categories.add(a.getCategoryId());
current.put(a.getAccountId(), a);
}
}
// We can only approximately reconstruct what the submit rule evaluator // We can only approximately reconstruct what the submit rule evaluator
// would have done. These should really come from a stored submit record. // would have done. These should really come from a stored submit record.
// //
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167. // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels = Map<String, LabelInfo> labels =
new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes)); new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes));
for (PatchSetApproval psa : cd.currentApprovals(db)) { for (ApprovalCategory.Id id : categories) {
LabelType type = labelTypes.byId(psa.getCategoryId().get()); LabelType type = labelTypes.byId(id.get());
if (type == null) { if (type != null) {
continue; LabelInfo li = new LabelInfo();
}
String label = type.getName();
LabelInfo li = labels.get(label);
if (li == null) {
li = new LabelInfo();
labels.put(label, li);
if (detailed) { if (detailed) {
setLabelValues(type, li); setLabelValues(type, li);
} }
labels.put(type.getName(), li);
} }
}
for (Account.Id accountId : allUsers) {
Map<String, ApprovalInfo> byLabel =
Maps.newHashMapWithExpectedSize(labels.size());
short val = psa.getValue();
if (detailed) { if (detailed) {
li.addApproval(approvalInfo(psa.getAccountId(), val)); for (String name : labels.keySet()) {
ApprovalInfo ai = approvalInfo(accountId, 0);
byLabel.put(name, ai);
labels.get(name).addApproval(ai);
}
} }
for (PatchSetApproval psa : current.get(accountId)) {
LabelType type = labelTypes.byId(psa.getCategoryId().get());
if (type == null) {
continue;
}
if (!standard || li.approved != null || li.rejected != null) { short val = psa.getValue();
continue; ApprovalInfo info = byLabel.get(type.getName());
} if (info != null) {
if (val == type.getMax().getValue()) { info.value = val;
li.approved = accountInfo(psa); }
} else if (val == type.getMin().getValue()
// A merged change can't have been rejected. LabelInfo li = labels.get(type.getName());
&& cd.getChange().getStatus() != Status.MERGED) { if (!standard || li.approved != null || li.rejected != null) {
li.rejected = accountInfo(psa); continue;
} else if (val > 0) { }
li.recommended = accountInfo(psa); if (val == type.getMax().getValue()) {
li.value = val; li.approved = accountLoader.get(accountId);
} else if (val < 0) { } else if (val == type.getMin().getValue()
li.disliked = accountInfo(psa); // A merged change can't have been rejected.
li.value = val; && cd.getChange().getStatus() != Status.MERGED) {
li.rejected = accountLoader.get(accountId);
} else if (val > 0) {
li.recommended = accountLoader.get(accountId);
li.value = val;
} else if (val < 0) {
li.disliked = accountLoader.get(accountId);
li.value = val;
}
} }
} }
return labels; return labels;
} }
private AccountInfo accountInfo(PatchSetApproval psa) {
return accountLoader.get(psa.getAccountId());
}
private ApprovalInfo approvalInfo(Account.Id id, int value) { private ApprovalInfo approvalInfo(Account.Id id, int value) {
ApprovalInfo ai = new ApprovalInfo(id); ApprovalInfo ai = new ApprovalInfo(id);
ai.value = value; ai.value = value;