Show available patch set actions as buttons below the info bar

If the user has SUBM approval type through one of their groups then
we show the action in the action bar for the current patch set.  On
sign-out we clear the bar, and on sign-in we reload the change page
so that the buttons can be seen (if available).

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-01-07 16:59:28 -08:00
parent e77f4fea70
commit c55b27bde9
9 changed files with 144 additions and 14 deletions

View File

@@ -23,8 +23,9 @@ public interface ChangeMessages extends Messages {
String changeScreenTitleId(int id);
String patchSetHeader(int id);
String patchSetAction(String action, int id);
String repoDownload(String project, int change, int ps);
String patchTableComments(@PluralCount int count);
String messageWrittenOn(String date);

View File

@@ -4,6 +4,7 @@ changesReviewableBy = Changes reviewable by {0}
changeScreenTitleId = Change {0}
patchSetHeader = Patch Set {0}
patchSetAction = {0} Patch Set {1}
repoDownload = repo download {0} {1}/{2}
patchTableComments[one] = 1 comment

View File

@@ -47,6 +47,7 @@ import java.util.List;
public class ChangeScreen extends Screen {
private Change.Id changeId;
private ChangeInfo changeInfo;
private boolean refreshOnSignIn;
private ChangeInfoBlock infoBlock;
private DisclosurePanel descriptionPanel;
@@ -87,6 +88,17 @@ public class ChangeScreen extends Screen {
return this;
}
@Override
public void onSignIn() {
if (refreshOnSignIn) {
refresh();
}
}
@Override
public void onSignOut() {
}
@Override
public void onLoad() {
if (descriptionPanel == null) {
@@ -96,6 +108,10 @@ public class ChangeScreen extends Screen {
displayTitle(changeInfo != null ? changeInfo.getSubject() : null);
super.onLoad();
refresh();
}
public void refresh() {
Util.DETAIL_SVC.changeDetail(changeId,
new ScreenLoadCallback<ChangeDetail>() {
public void onSuccess(final ChangeDetail r) {
@@ -174,13 +190,9 @@ public class ChangeScreen extends Screen {
}
private void display(final ChangeDetail detail) {
if (changeInfo == null) {
// We couldn't set the title correctly when we loaded the page
// into the browser, update it now that we have the full detail.
//
displayTitle(detail.getChange().getSubject());
}
displayTitle(detail.getChange().getSubject());
refreshOnSignIn = !detail.getChange().getStatus().isClosed();
dependencies.setAccountInfoCache(detail.getAccounts());
approvals.setAccountInfoCache(detail.getAccounts());

View File

@@ -14,15 +14,22 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.SignedInListener;
import com.google.gerrit.client.data.ApprovalType;
import com.google.gerrit.client.data.ChangeDetail;
import com.google.gerrit.client.data.PatchSetDetail;
import com.google.gerrit.client.reviewdb.PatchSet;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gwt.core.client.GWT;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.DisclosureEvent;
import com.google.gwt.user.client.ui.DisclosureHandler;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.Grid;
import com.google.gwt.user.client.ui.Panel;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
class PatchSetPanel extends Composite implements DisclosureHandler {
@@ -34,7 +41,9 @@ class PatchSetPanel extends Composite implements DisclosureHandler {
private final FlowPanel body;
private Grid infoTable;
private Panel actionsPanel;
private PatchTable patchTable;
private SignedInListener signedInListener;
PatchSetPanel(final ChangeDetail detail, final PatchSet ps) {
changeDetail = detail;
@@ -43,6 +52,22 @@ class PatchSetPanel extends Composite implements DisclosureHandler {
initWidget(body);
}
@Override
protected void onLoad() {
super.onLoad();
if (signedInListener != null) {
Gerrit.addSignedInListener(signedInListener);
}
}
@Override
protected void onUnload() {
if (signedInListener != null) {
Gerrit.removeSignedInListener(signedInListener);
}
super.onUnload();
}
public void ensureLoaded(final PatchSetDetail detail) {
infoTable = new Grid(R_CNT, 2);
infoTable.setStyleName("gerrit-InfoBlock");
@@ -60,16 +85,49 @@ class PatchSetPanel extends Composite implements DisclosureHandler {
.getChange().getDest().getParentKey().get(), changeDetail.getChange()
.getChangeId(), patchSet.getPatchSetId()));
patchTable = new PatchTable();
patchTable.setSavePointerId("patchTable "
+ changeDetail.getChange().getChangeId() + " " + patchSet.getPatchSetId());
+ changeDetail.getChange().getChangeId() + " "
+ patchSet.getPatchSetId());
patchTable.display(detail.getPatches());
patchTable.finishDisplay(false);
body.add(infoTable);
if (!changeDetail.getChange().getStatus().isClosed()
&& changeDetail.isCurrentPatchSet(detail)) {
actionsPanel = new FlowPanel();
actionsPanel.setStyleName("gerrit-PatchSetActions");
signedInListener = new SignedInListener() {
public void onSignIn() {
}
public void onSignOut() {
actionsPanel.clear();
actionsPanel.setVisible(false);
}
};
Gerrit.addSignedInListener(signedInListener);
body.add(actionsPanel);
populateActions(detail);
}
body.add(patchTable);
}
private void populateActions(final PatchSetDetail detail) {
if (changeDetail.getCurrentActions() != null
&& !changeDetail.getCurrentActions().isEmpty()) {
for (final ApprovalType at : Common.getGerritConfig().getActionTypes()) {
if (changeDetail.getCurrentActions().contains(at.getCategory().getId())) {
final Button b =
new Button(Util.M.patchSetAction(at.getCategory().getName(),
detail.getPatchSet().getPatchSetId()));
actionsPanel.add(b);
}
}
}
}
public void onOpen(final DisclosureEvent event) {
if (infoTable == null) {
Util.DETAIL_SVC.patchSetDetail(patchSet.getId(),

View File

@@ -49,6 +49,7 @@ public class ChangeDetail {
protected List<ChangeMessage> messages;
protected PatchSet.Id currentPatchSetId;
protected PatchSetDetail currentDetail;
protected Set<ApprovalCategory.Id> currentActions;
public ChangeDetail() {
}
@@ -68,16 +69,23 @@ public class ChangeDetail {
final List<ChangeApproval> allApprovals =
db.changeApprovals().byChange(change.getId()).toList();
if (!change.getStatus().isClosed()) {
final Account.Id me = Common.getAccountId();
final FunctionState fs =
new FunctionState(Common.getProjectCache().get(
change.getDest().getParentKey()), allApprovals);
missingApprovals = new HashSet<ApprovalCategory.Id>();
currentActions = new HashSet<ApprovalCategory.Id>();
for (final ApprovalType at : Common.getGerritConfig().getApprovalTypes()) {
at.getCategory().getFunction().run(at, fs);
if (!fs.isValid(at)) {
missingApprovals.add(at.getCategory().getId());
}
}
for (final ApprovalType at : Common.getGerritConfig().getActionTypes()) {
if (at.getCategory().getFunction().isValid(me, at, fs)) {
currentActions.add(at.getCategory().getId());
}
}
}
final HashMap<Account.Id, ApprovalDetail> ad =
@@ -184,6 +192,15 @@ public class ChangeDetail {
return missingApprovals;
}
public Set<ApprovalCategory.Id> getCurrentActions() {
return currentActions;
}
public boolean isCurrentPatchSet(final PatchSetDetail detail) {
return currentPatchSetId != null
&& detail.getPatchSet().getId().equals(currentPatchSetId);
}
public PatchSet getCurrentPatchSet() {
if (currentPatchSetId != null) {
// We search through the list backwards because its *very* likely

View File

@@ -33,6 +33,8 @@ public class GroupCache {
private AccountGroup.Id adminGroupId;
private AccountGroup.Id anonymousGroupId;
private AccountGroup.Id registeredGroupId;
private Set<AccountGroup.Id> anonOnly;
private final LinkedHashMap<Account.Id, Set<AccountGroup.Id>> byAccount =
new LinkedHashMap<Account.Id, Set<AccountGroup.Id>>(16, 0.75f, true) {
@@ -47,6 +49,9 @@ public class GroupCache {
adminGroupId = cfg.adminGroupId;
anonymousGroupId = cfg.anonymousGroupId;
registeredGroupId = cfg.registeredGroupId;
anonOnly =
Collections.unmodifiableSet(Collections.singleton(anonymousGroupId));
}
/**
@@ -163,7 +168,7 @@ public class GroupCache {
*/
public Set<AccountGroup.Id> getGroups(final Account.Id accountId) {
if (accountId == null) {
return Collections.emptySet();
return anonOnly;
}
Set<AccountGroup.Id> m;

View File

@@ -15,7 +15,9 @@
package com.google.gerrit.client.workflow;
import com.google.gerrit.client.data.ApprovalType;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.ApprovalCategory;
import com.google.gerrit.client.reviewdb.ProjectRight;
import com.google.gerrit.client.rpc.Common;
/**
@@ -33,12 +35,29 @@ public class AllValid extends CategoryFunction {
@Override
public void run(final ApprovalType at, final FunctionState state) {
for (final ApprovalType t : Common.getGerritConfig().getApprovalTypes()) {
if (!state.isValid(t)) {
state.valid(at, false);
return;
state.valid(at, valid(at, state));
}
@Override
public boolean isValid(final Account.Id accountId, final ApprovalType at,
final FunctionState state) {
if (valid(at, state)) {
for (final ProjectRight pr : state.getAllRights(at)) {
if (state.isMember(accountId, pr.getAccountGroupId())
&& pr.getMaxValue() > 0) {
return true;
}
}
}
state.valid(at, true);
return false;
}
private static boolean valid(final ApprovalType at, final FunctionState state) {
for (final ApprovalType t : Common.getGerritConfig().getApprovalTypes()) {
if (!state.isValid(t)) {
return false;
}
}
return true;
}
}

View File

@@ -15,8 +15,10 @@
package com.google.gerrit.client.workflow;
import com.google.gerrit.client.data.ApprovalType;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.ApprovalCategory;
import com.google.gerrit.client.reviewdb.ChangeApproval;
import com.google.gerrit.client.reviewdb.ProjectRight;
import java.util.HashMap;
import java.util.Map;
@@ -71,4 +73,15 @@ public abstract class CategoryFunction {
* the valid status into.
*/
public abstract void run(ApprovalType at, FunctionState state);
public boolean isValid(final Account.Id accountId, final ApprovalType at,
final FunctionState state) {
for (final ProjectRight pr : state.getAllRights(at)) {
if (state.isMember(accountId, pr.getAccountGroupId())
&& (pr.getMinValue() < 0 || pr.getMaxValue() > 0)) {
return true;
}
}
return false;
}
}

View File

@@ -488,6 +488,10 @@
margin-bottom: 10px;
}
.gerrit-PatchSetActions {
margin-bottom: 10px;
}
.gerrit-Change-MissingApprovalList {
margin-top: 5px;
}