opendev/gerrit
Code Issues Proposed changes

Show in ApprovalTable if a reviewer can't vote in a category

Browse Source 
As a change owner you can invite reviewers on the ChangeScreen and they
will be added to the ApprovalTable. Then you wait for the review but it
might be that the reviewer has actually no permissions to vote on the
change. If the reviewer has no permissions to vote in a category this
is not visible.

For a user that cannot vote in a category the cell in the category
column of the ApprovalTable will be shown with a grey background.
A tooltip explains that the user cannot vote in this category.

Change-Id: I48485e26bab7f7f990bd569df803dfd14ae5ca5c
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2012-08-24 14:28:03 +02:00
parent 96a10ed60e
commit d42555ae01
8 changed files with 54 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalDetail.java
View File

@@ -42,6 +42,7 @@ public class ApprovalDetail {
protected Account.Id account;
protected List<PatchSetApproval> approvals;
protected boolean canRemove;
private Set<String> votable;
private transient Set<String> approved;
private transient Set<String> rejected;
@@ -112,6 +113,13 @@ public class ApprovalDetail {
rejected.add(label);
}
public void votable(String label) {
if (votable == null) {
votable = new HashSet<String>();
}
votable.add(label);
}
public boolean isApproved(String label) {
return approved != null && approved.contains(label);
}
@@ -119,4 +127,8 @@ public class ApprovalDetail {
public boolean isRejected(String label) {
return rejected != null && rejected.contains(label);
}
public boolean canVote(String label) {
return votable != null && votable.contains(label);
}
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java
View File

@@ -106,4 +106,6 @@ public interface GerritConstants extends Constants {
String projectAccessError();
String projectAccessProposeForReviewHint();
String userCannotVoteToolTip();
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties
View File

@@ -89,3 +89,5 @@ jumpMineDraftComments = Go to draft comments
projectAccessError = You don't have permissions to modify the access rights for the following refs:
projectAccessProposeForReviewHint = You may propose these modifications to the project owners by clicking on 'Save for Review'.
userCannotVoteToolTip = User cannot vote in this category

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
View File

@@ -37,6 +37,7 @@ public interface GerritCss extends CssResource {
String approvalhint();
String approvalrole();
String approvalscore();
String notVotable();
String blockHeader();
String bottomheader();
String cAPPROVAL();

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
View File

@@ -397,8 +397,11 @@ public class ApprovalTable extends Composite {
for (String labelName : columns) {
fmt.setStyleName(row, col, Gerrit.RESOURCES.css().approvalscore());
if (!ad.canVote(labelName)) {
fmt.addStyleName(row, col, Gerrit.RESOURCES.css().notVotable());
fmt.getElement(row, col).setTitle(Gerrit.C.userCannotVoteToolTip());
if (ad.isRejected(labelName)) {
} else if (ad.isRejected(labelName)) {
table.setWidget(row, col, new Image(Gerrit.RESOURCES.redNot()));
} else if (ad.isApproved(labelName)) {

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
View File

@@ -871,6 +871,9 @@
.infoTable td.approvalscore {
text-align: center;
}
.infoTable td.notVotable {
background: #F5F5F5;
}
.infoTable td.negscore {
color: red;
}

15
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
View File

@@ -19,6 +19,7 @@ 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.ChangeInfo;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.httpd.rpc.Handler;
@@ -71,6 +72,8 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
private final ApprovalTypes approvalTypes;
private final ChangeControl.Factory changeControlFactory;
private final ChangeControl.GenericFactory changeControlGenericFactory;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final FunctionState.Factory functionState;
private final PatchSetDetailFactory.Factory patchSetDetail;
private final AccountInfoCacheFactory aic;
@@ -93,6 +96,8 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
final PatchSetDetailFactory.Factory patchSetDetail, final ReviewDb db,
final GitRepositoryManager repoManager,
final ChangeControl.Factory changeControlFactory,
final ChangeControl.GenericFactory changeControlGenericFactory,
final IdentifiedUser.GenericFactory identifiedUserFactory,
final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
final AnonymousUser anonymousUser,
final MergeOp.Factory opFactory,
@@ -104,6 +109,8 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
this.db = db;
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
this.identifiedUserFactory = identifiedUserFactory;
this.anonymousUser = anonymousUser;
this.aic = accountInfoCacheFactory.create();
@@ -241,6 +248,14 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
if (ca.getPatchSetId().equals(psId)) {
d.add(ca);
}
final ChangeControl chgCtrl =
changeControlGenericFactory.controlFor(detail.getChange(),
identifiedUserFactory.create(ca.getAccountId()));
for (PermissionRange pr : chgCtrl.getLabelRanges()) {
if (pr.getMin() != 0 || pr.getMax() != 0) {
d.votable(pr.getLabel());
}
}
}
final Account.Id owner = detail.getChange().getOwner();

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

@@ -55,6 +55,8 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
private final ReviewDb db;
private final FunctionState.Factory functionState;
private final ChangeControl.Factory changeControlFactory;
private final ChangeControl.GenericFactory changeControlGenericFactory;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ApprovalTypes approvalTypes;
private final AccountInfoCacheFactory aic;
private final IdentifiedUser user;
@@ -71,12 +73,16 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
final FunctionState.Factory functionState,
final ChangeControl.Factory changeControlFactory,
final ChangeControl.GenericFactory changeControlGenericFactory,
final IdentifiedUser.GenericFactory identifiedUserFactory,
final ApprovalTypes approvalTypes,
final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
this.infoFactory = infoFactory;
this.db = db;
this.functionState = functionState;
this.changeControlFactory = changeControlFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
this.identifiedUserFactory = identifiedUserFactory;
this.approvalTypes = approvalTypes;
this.aic = accountInfoCacheFactory.create();
this.user = user;
@@ -190,7 +196,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
}
private void loadApprovals(final PatchSetPublishDetail detail,
final ChangeControl control) throws OrmException {
final ChangeControl control) throws OrmException, NoSuchChangeException {
final PatchSet.Id psId = detail.getChange().currentPatchSetId();
final Change.Id changeId = patchSetId.getParentKey();
final List<PatchSetApproval> allApprovals =
@@ -221,6 +227,14 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
if (ca.getPatchSetId().equals(psId)) {
d.add(ca);
}
final ChangeControl chgCtrl =
changeControlGenericFactory.controlFor(detail.getChange(),
identifiedUserFactory.create(ca.getAccountId()));
for (PermissionRange pr : chgCtrl.getLabelRanges()) {
if (pr.getMin() != 0 || pr.getMax() != 0) {
d.votable(pr.getLabel());
}
}
}
final Account.Id owner = detail.getChange().getOwner();