Display the approval table on the Publish Comments screen.
The ApprovalTable (displayed only on the ChangeDetail screen currently) has been added to the PublishCommentScreen, in order to see all existing votes, and which other users are set as reviewers of the change. ApprovalTable also gives hints as to which votes are still required before the change can be submitted, and it gives the reviewer the option to add additional reviewers directly from the comment page. Bug: Issue 1383 Change-Id: Ie42f86dc1509749f8cfa4fcea08e2ebf5a793ede
This commit is contained in:
@@ -20,6 +20,9 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
|
|||||||
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
||||||
import com.google.gerrit.reviewdb.client.PatchSetInfo;
|
import com.google.gerrit.reviewdb.client.PatchSetInfo;
|
||||||
|
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.Collection;
|
||||||
|
import java.util.Collections;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
public class PatchSetPublishDetail {
|
public class PatchSetPublishDetail {
|
||||||
@@ -28,6 +31,8 @@ public class PatchSetPublishDetail {
|
|||||||
protected Change change;
|
protected Change change;
|
||||||
protected List<PatchLineComment> drafts;
|
protected List<PatchLineComment> drafts;
|
||||||
protected List<PermissionRange> labels;
|
protected List<PermissionRange> labels;
|
||||||
|
protected List<ApprovalDetail> approvals;
|
||||||
|
protected List<SubmitRecord> submitRecords;
|
||||||
protected List<PatchSetApproval> given;
|
protected List<PatchSetApproval> given;
|
||||||
protected boolean canSubmit;
|
protected boolean canSubmit;
|
||||||
|
|
||||||
@@ -39,6 +44,23 @@ public class PatchSetPublishDetail {
|
|||||||
this.labels = labels;
|
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() {
|
public List<PatchSetApproval> getGiven() {
|
||||||
return given;
|
return given;
|
||||||
}
|
}
|
||||||
|
@@ -29,6 +29,7 @@ import com.google.gerrit.common.data.ApprovalDetail;
|
|||||||
import com.google.gerrit.common.data.ApprovalType;
|
import com.google.gerrit.common.data.ApprovalType;
|
||||||
import com.google.gerrit.common.data.ApprovalTypes;
|
import com.google.gerrit.common.data.ApprovalTypes;
|
||||||
import com.google.gerrit.common.data.ChangeDetail;
|
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.ReviewerResult;
|
||||||
import com.google.gerrit.common.data.SubmitRecord;
|
import com.google.gerrit.common.data.SubmitRecord;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
@@ -133,12 +134,22 @@ public class ApprovalTable extends Composite {
|
|||||||
return AccountLink.link(accountCache, id);
|
return AccountLink.link(accountCache, id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void display(PatchSetPublishDetail detail) {
|
||||||
|
doDisplay(detail.getChange(), detail.getApprovals(),
|
||||||
|
detail.getSubmitRecords());
|
||||||
|
}
|
||||||
|
|
||||||
void display(ChangeDetail detail) {
|
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);
|
reviewerSuggestOracle.setChange(changeId);
|
||||||
|
|
||||||
List<String> columns = new ArrayList<String>();
|
List<String> columns = new ArrayList<String>();
|
||||||
List<ApprovalDetail> rows = detail.getApprovals();
|
|
||||||
|
|
||||||
final Element missingList = missing.getElement();
|
final Element missingList = missing.getElement();
|
||||||
while (DOM.getChildCount(missingList) > 0) {
|
while (DOM.getChildCount(missingList) > 0) {
|
||||||
@@ -146,16 +157,16 @@ public class ApprovalTable extends Composite {
|
|||||||
}
|
}
|
||||||
missing.setVisible(false);
|
missing.setVisible(false);
|
||||||
|
|
||||||
if (detail.getSubmitRecords() != null) {
|
if (submitRecords != null) {
|
||||||
HashSet<String> reportedMissing = new HashSet<String>();
|
HashSet<String> reportedMissing = new HashSet<String>();
|
||||||
|
|
||||||
HashMap<Account.Id, ApprovalDetail> byUser =
|
HashMap<Account.Id, ApprovalDetail> byUser =
|
||||||
new HashMap<Account.Id, ApprovalDetail>();
|
new HashMap<Account.Id, ApprovalDetail>();
|
||||||
for (ApprovalDetail ad : detail.getApprovals()) {
|
for (ApprovalDetail ad : approvals) {
|
||||||
byUser.put(ad.getAccount(), ad);
|
byUser.put(ad.getAccount(), ad);
|
||||||
}
|
}
|
||||||
|
|
||||||
for (SubmitRecord rec : detail.getSubmitRecords()) {
|
for (SubmitRecord rec : submitRecords) {
|
||||||
if (rec.labels == null) {
|
if (rec.labels == null) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
@@ -197,7 +208,7 @@ public class ApprovalTable extends Composite {
|
|||||||
missing.setVisible(!reportedMissing.isEmpty());
|
missing.setVisible(!reportedMissing.isEmpty());
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
for (ApprovalDetail ad : rows) {
|
for (ApprovalDetail ad : approvals) {
|
||||||
for (PatchSetApproval psa : ad.getPatchSetApprovals()) {
|
for (PatchSetApproval psa : ad.getPatchSetApprovals()) {
|
||||||
ApprovalType legacyType = types.byId(psa.getCategoryId());
|
ApprovalType legacyType = types.byId(psa.getCategoryId());
|
||||||
if (legacyType == null) {
|
if (legacyType == null) {
|
||||||
@@ -231,13 +242,13 @@ public class ApprovalTable extends Composite {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (rows.isEmpty()) {
|
if (approvals.isEmpty()) {
|
||||||
table.setVisible(false);
|
table.setVisible(false);
|
||||||
} else {
|
} else {
|
||||||
displayHeader(columns);
|
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);
|
table.setVisible(true);
|
||||||
}
|
}
|
||||||
@@ -245,7 +256,7 @@ public class ApprovalTable extends Composite {
|
|||||||
addReviewer.setVisible(Gerrit.isSignedIn());
|
addReviewer.setVisible(Gerrit.isSignedIn());
|
||||||
|
|
||||||
if (Gerrit.getConfig().testChangeMerge()
|
if (Gerrit.getConfig().testChangeMerge()
|
||||||
&& !detail.getChange().isMergeable()) {
|
&& !change.isMergeable()) {
|
||||||
Element li = DOM.createElement("li");
|
Element li = DOM.createElement("li");
|
||||||
li.setClassName(Gerrit.RESOURCES.css().missingApproval());
|
li.setClassName(Gerrit.RESOURCES.css().missingApproval());
|
||||||
DOM.setInnerText(li, Util.C.messageNeedsRebaseOrHasDependency());
|
DOM.setInnerText(li, Util.C.messageNeedsRebaseOrHasDependency());
|
||||||
|
@@ -64,6 +64,7 @@ public class PublishCommentScreen extends AccountScreen implements
|
|||||||
private final PatchSet.Id patchSetId;
|
private final PatchSet.Id patchSetId;
|
||||||
private Collection<ValueRadioButton> approvalButtons;
|
private Collection<ValueRadioButton> approvalButtons;
|
||||||
private ChangeDescriptionBlock descBlock;
|
private ChangeDescriptionBlock descBlock;
|
||||||
|
private ApprovalTable approvals;
|
||||||
private Panel approvalPanel;
|
private Panel approvalPanel;
|
||||||
private NpTextArea message;
|
private NpTextArea message;
|
||||||
private FlowPanel draftsPanel;
|
private FlowPanel draftsPanel;
|
||||||
@@ -86,6 +87,9 @@ public class PublishCommentScreen extends AccountScreen implements
|
|||||||
descBlock = new ChangeDescriptionBlock();
|
descBlock = new ChangeDescriptionBlock();
|
||||||
add(descBlock);
|
add(descBlock);
|
||||||
|
|
||||||
|
approvals = new ApprovalTable();
|
||||||
|
add(approvals);
|
||||||
|
|
||||||
final FormPanel form = new FormPanel();
|
final FormPanel form = new FormPanel();
|
||||||
final FlowPanel body = new FlowPanel();
|
final FlowPanel body = new FlowPanel();
|
||||||
form.setWidget(body);
|
form.setWidget(body);
|
||||||
@@ -274,6 +278,11 @@ public class PublishCommentScreen extends AccountScreen implements
|
|||||||
|
|
||||||
if (r.getChange().getStatus().isOpen()) {
|
if (r.getChange().getStatus().isOpen()) {
|
||||||
initApprovals(r, approvalPanel);
|
initApprovals(r, approvalPanel);
|
||||||
|
|
||||||
|
approvals.setAccountInfoCache(r.getAccounts());
|
||||||
|
approvals.display(r);
|
||||||
|
} else {
|
||||||
|
approvals.setVisible(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (lastState != null && patchSetId.equals(lastState.patchSetId)) {
|
if (lastState != null && patchSetId.equals(lastState.patchSetId)) {
|
||||||
|
@@ -952,7 +952,7 @@
|
|||||||
margin-top: 5px;
|
margin-top: 5px;
|
||||||
}
|
}
|
||||||
|
|
||||||
.changeScreen .approvalTable {
|
.approvalTable {
|
||||||
margin-top: 1em;
|
margin-top: 1em;
|
||||||
margin-bottom: 1em;
|
margin-bottom: 1em;
|
||||||
}
|
}
|
||||||
|
@@ -14,12 +14,14 @@
|
|||||||
|
|
||||||
package com.google.gerrit.httpd.rpc.changedetail;
|
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.ApprovalType;
|
||||||
import com.google.gerrit.common.data.ApprovalTypes;
|
import com.google.gerrit.common.data.ApprovalTypes;
|
||||||
import com.google.gerrit.common.data.PatchSetPublishDetail;
|
import com.google.gerrit.common.data.PatchSetPublishDetail;
|
||||||
import com.google.gerrit.common.data.PermissionRange;
|
import com.google.gerrit.common.data.PermissionRange;
|
||||||
import com.google.gerrit.common.data.SubmitRecord;
|
import com.google.gerrit.common.data.SubmitRecord;
|
||||||
import com.google.gerrit.httpd.rpc.Handler;
|
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.Change;
|
||||||
import com.google.gerrit.reviewdb.client.PatchLineComment;
|
import com.google.gerrit.reviewdb.client.PatchLineComment;
|
||||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
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.patch.PatchSetInfoNotAvailableException;
|
||||||
import com.google.gerrit.server.project.ChangeControl;
|
import com.google.gerrit.server.project.ChangeControl;
|
||||||
import com.google.gerrit.server.project.NoSuchChangeException;
|
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.gwtorm.server.OrmException;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
import com.google.inject.assistedinject.Assisted;
|
import com.google.inject.assistedinject.Assisted;
|
||||||
@@ -49,6 +53,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
|
|||||||
|
|
||||||
private final PatchSetInfoFactory infoFactory;
|
private final PatchSetInfoFactory infoFactory;
|
||||||
private final ReviewDb db;
|
private final ReviewDb db;
|
||||||
|
private final FunctionState.Factory functionState;
|
||||||
private final ChangeControl.Factory changeControlFactory;
|
private final ChangeControl.Factory changeControlFactory;
|
||||||
private final ApprovalTypes approvalTypes;
|
private final ApprovalTypes approvalTypes;
|
||||||
private final AccountInfoCacheFactory aic;
|
private final AccountInfoCacheFactory aic;
|
||||||
@@ -64,11 +69,13 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
|
|||||||
PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory,
|
PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory,
|
||||||
final ReviewDb db,
|
final ReviewDb db,
|
||||||
final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
|
final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
|
||||||
|
final FunctionState.Factory functionState,
|
||||||
final ChangeControl.Factory changeControlFactory,
|
final ChangeControl.Factory changeControlFactory,
|
||||||
final ApprovalTypes approvalTypes,
|
final ApprovalTypes approvalTypes,
|
||||||
final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
|
final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
|
||||||
this.infoFactory = infoFactory;
|
this.infoFactory = infoFactory;
|
||||||
this.db = db;
|
this.db = db;
|
||||||
|
this.functionState = functionState;
|
||||||
this.changeControlFactory = changeControlFactory;
|
this.changeControlFactory = changeControlFactory;
|
||||||
this.approvalTypes = approvalTypes;
|
this.approvalTypes = approvalTypes;
|
||||||
this.aic = accountInfoCacheFactory.create();
|
this.aic = accountInfoCacheFactory.create();
|
||||||
@@ -130,6 +137,8 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
|
|||||||
int ok = 0;
|
int ok = 0;
|
||||||
|
|
||||||
for (SubmitRecord.Label lbl : rec.labels) {
|
for (SubmitRecord.Label lbl : rec.labels) {
|
||||||
|
aic.want(lbl.appliedBy);
|
||||||
|
|
||||||
boolean canMakeOk = false;
|
boolean canMakeOk = false;
|
||||||
PermissionRange range = rangeByName.get(lbl.label);
|
PermissionRange range = rangeByName.get(lbl.label);
|
||||||
if (range != null) {
|
if (range != null) {
|
||||||
@@ -166,12 +175,60 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
|
|||||||
if (couldSubmit && control.getRefControl().canSubmit()) {
|
if (couldSubmit && control.getRefControl().canSubmit()) {
|
||||||
detail.setCanSubmit(true);
|
detail.setCanSubmit(true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
detail.setSubmitRecords(submitRecords);
|
||||||
}
|
}
|
||||||
|
|
||||||
detail.setLabels(allowed);
|
detail.setLabels(allowed);
|
||||||
detail.setGiven(given);
|
detail.setGiven(given);
|
||||||
detail.setAccounts(aic.create());
|
detail.setAccounts(aic.create());
|
||||||
|
|
||||||
|
loadApprovals(detail, control);
|
||||||
|
|
||||||
return detail;
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user