opendev/gerrit
Code Issues Proposed changes

Migrate approval radio buttons to new REST API

Browse Source 
PublishCommentScreen now uses two RPCs, one to the old gwtjsonrpc API
and one to the new REST API. The new style API lists verbatim the
valid approval categories and values on a per-user, per-change basis
rather than reading them from the sitewide config.

Change-Id: Iaada19112cfbe970dff26be570487b4d1ff3f6a2
This commit is contained in:
Dave Borowitz
2013-01-31 13:04:12 -08:00
parent 15130498e1
commit 9021d245c8
8 changed files with 145 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.common.data;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -123,9 +122,9 @@ public class PatchSetPublishDetail {
return null;
}
public PatchSetApproval getChangeApproval(ApprovalCategory.Id id) {
public PatchSetApproval getChangeApproval(String label) {
for (PatchSetApproval a : given) {
if (a.getCategoryId().equals(id)) {
if (a.getLabel() != null && a.getLabel().equals(label)) {
return a;
}
}

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

@@ -16,6 +16,7 @@ package com.google.gerrit.client.changes;
import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.user.client.rpc.AsyncCallback;
@@ -60,6 +61,10 @@ public class ChangeApi {
}
}
public static RestApi revision(PatchSet.Id id) {
return change(id.getParentKey().get()).view("revisions").id(id.get());
}
/** Submit a specific revision of a change. */
public static void submit(int id, String commit, AsyncCallback<SubmitInfo> cb) {
SubmitInput in = SubmitInput.create();

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

@@ -68,7 +68,7 @@ public class ChangeInfo extends JavaScriptObject {
}
public final Set<String> labels() {
return Natives.keys(labels0());
return labels0().keySet();
}
public final native String id() /*-{ return this.id; }-*/;

120
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
View File

@@ -15,11 +15,14 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo;
import com.google.gerrit.client.changes.ChangeInfo.LabelInfo;
import com.google.gerrit.client.patches.AbstractPatchContentTable;
import com.google.gerrit.client.patches.CommentEditorContainer;
import com.google.gerrit.client.patches.CommentEditorPanel;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.AccountScreen;
@@ -27,20 +30,16 @@ import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
import com.google.gerrit.common.data.PatchSetPublishDetail;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.JsArrayString;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.FormPanel;
@@ -78,6 +77,7 @@ public class PublishCommentScreen extends AccountScreen implements
private Button cancel;
private boolean saveStateOnUnload = true;
private List<CommentEditorPanel> commentEditors;
private ChangeInfo change;
public PublishCommentScreen(final PatchSet.Id psi) {
patchSetId = psi;
@@ -148,6 +148,19 @@ public class PublishCommentScreen extends AccountScreen implements
super.onLoad();
CallbackGroup cbs = new CallbackGroup();
ChangeApi.revision(patchSetId).view("review").get(cbs.add(
new AsyncCallback<ChangeInfo>() {
@Override
public void onSuccess(ChangeInfo result) {
result.init();
change = result;
}
@Override
public void onFailure(Throwable caught) {
// Handled by ScreenLoadCallback.onFailure().
}
}));
Util.DETAIL_SVC.patchSetPublishDetail(patchSetId, cbs.addGwtjsonrpc(
new ScreenLoadCallback<PatchSetPublishDetail>(this) {
@Override
@@ -230,47 +243,48 @@ public class PublishCommentScreen extends AccountScreen implements
}
private void initApprovals(final PatchSetPublishDetail r, final Panel body) {
ApprovalTypes types = Gerrit.getConfig().getApprovalTypes();
for (PermissionRange range : r.getLabels()) {
ApprovalType type = types.byLabel(range.getLabel());
if (type != null) {
// Legacy type, use radio buttons.
initApprovalType(r, body, type, range);
} else {
// TODO Newer style label.
}
for (String labelName : change.labels()) {
initLabel(r, labelName, body);
}
}
private void initApprovalType(final PatchSetPublishDetail r,
final Panel body, final ApprovalType ct, final PermissionRange range) {
body.add(new SmallHeading(ct.getCategory().getName() + ":"));
private void initLabel(PatchSetPublishDetail r, String labelName,
Panel body) {
JsArrayString nativeValues = change.permitted_values(labelName);
if (nativeValues == null || nativeValues.length() == 0) {
return;
}
List<String> values = new ArrayList<String>(nativeValues.length());
for (int i = 0; i < nativeValues.length(); i++) {
values.add(nativeValues.get(i));
}
Collections.reverse(values);
LabelInfo label = change.label(labelName);
final VerticalPanel vp = new VerticalPanel();
body.add(new SmallHeading(label.name() + ":"));
VerticalPanel vp = new VerticalPanel();
vp.setStyleName(Gerrit.RESOURCES.css().approvalCategoryList());
final List<ApprovalCategoryValue> lst =
new ArrayList<ApprovalCategoryValue>(ct.getValues());
Collections.reverse(lst);
final ApprovalCategory.Id catId = ct.getCategory().getId();
final PatchSetApproval prior = r.getChangeApproval(catId);
for (final ApprovalCategoryValue buttonValue : lst) {
if (!range.contains(buttonValue.getValue())) {
continue;
Short prior = null;
for (ApprovalInfo app : Natives.asList(label.all())) {
if (app._account_id() == Gerrit.getUserAccount().getId().get()) {
prior = app.value();
break;
}
}
ValueRadioButton b = new ValueRadioButton(ct.getCategory(), buttonValue);
SafeHtml buf = new SafeHtmlBuilder().append(buttonValue.format());
for (String value : values) {
ValueRadioButton b = new ValueRadioButton(label, value);
SafeHtml buf = new SafeHtmlBuilder().append(b.format());
buf = CommentLinkProcessor.apply(buf);
SafeHtml.set(b, buf);
if (lastState != null && patchSetId.equals(lastState.patchSetId)
&& lastState.approvals.containsKey(buttonValue.getCategoryId())) {
b.setValue(lastState.approvals.get(buttonValue.getCategoryId()).equals(
buttonValue));
&& lastState.approvals.containsKey(label.name())) {
b.setValue(lastState.approvals.get(label.name()) == value);
} else {
b.setValue(prior != null ? buttonValue.getValue() == prior.getValue()
: buttonValue.getValue() == 0);
b.setValue(b.parseValue() == (prior != null ? prior : 0));
}
approvalButtons.add(b);
@@ -366,7 +380,7 @@ public class PublishCommentScreen extends AccountScreen implements
data.init();
for (final ValueRadioButton b : approvalButtons) {
if (b.getValue()) {
data.label(b.category.getLabelName(), b.value.getValue());
data.label(b.label.name(), b.parseValue());
}
}
@@ -435,29 +449,45 @@ public class PublishCommentScreen extends AccountScreen implements
Gerrit.display(PageLinks.toChange(ck), new ChangeScreen(ck));
}
private static class ValueRadioButton extends RadioButton {
final ApprovalCategory category;
final ApprovalCategoryValue value;
private static short parseLabelValue(String value) {
if (value.charAt(0) == ' ' || value.charAt(0) == '+') {
value = value.substring(1);
}
return Short.parseShort(value);
}
ValueRadioButton(ApprovalCategory c, ApprovalCategoryValue v) {
super(c.getLabelName());
category = c;
value = v;
private static class ValueRadioButton extends RadioButton {
final LabelInfo label;
final String value;
ValueRadioButton(LabelInfo label, String value) {
super(label.name());
this.label = label;
this.value = value;
}
String format() {
return new StringBuilder().append(value).append(' ')
.append(label.value_text(value)).toString();
}
short parseValue() {
return parseLabelValue(value);
}
}
private static class SavedState {
final PatchSet.Id patchSetId;
final String message;
final Map<ApprovalCategory.Id, ApprovalCategoryValue> approvals;
final Map<String, String> approvals;
SavedState(final PublishCommentScreen p) {
patchSetId = p.patchSetId;
message = p.message.getText();
approvals = new HashMap<ApprovalCategory.Id, ApprovalCategoryValue>();
approvals = new HashMap<String, String>();
for (final ValueRadioButton b : p.approvalButtons) {
if (b.getValue()) {
approvals.put(b.value.getCategoryId(), b.value);
approvals.put(b.label.name(), b.value);
}
}
}

25
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
View File

@@ -88,21 +88,26 @@ public final class ApprovalCategoryValue {
name = n;
}
public String formatValue() {
if (getValue() < 0) {
return Short.toString(getValue());
} else if (getValue() == 0) {
public static String formatValue(short value) {
if (value < 0) {
return Short.toString(value);
} else if (value == 0) {
return " 0";
} else {
return "+" + Short.toString(getValue());
return "+" + Short.toString(value);
}
}
public String formatValue() {
return formatValue(getValue());
}
public static String format(String name, short value) {
return new StringBuilder().append(formatValue(value))
.append(' ').append(name).toString();
}
public String format() {
final StringBuilder m = new StringBuilder();
m.append(formatValue());
m.append(' ');
m.append(getName());
return m.toString();
return format(getName(), getValue());
}
}

11
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
View File

@@ -96,6 +96,9 @@ public final class PatchSetApproval {
@Column(id = 5, length = 16, notNull = false)
protected String changeSortKey;
/** Label name copied from corresponding {@link ApprovalCategory}. */
protected String label;
protected PatchSetApproval() {
}
@@ -154,4 +157,12 @@ public final class PatchSetApproval {
changeOpen = c.open;
changeSortKey = c.sortKey;
}
public String getLabel() {
return label;
}
public void setLabel(String label) {
this.label = label;
}
}

35
gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java Normal file
View File

@@ -0,0 +1,35 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.gerrit.common.changes.ListChangesOption;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
public class GetReview implements RestReadView<RevisionResource> {
private final ChangeJson json;
@Inject
GetReview(ChangeJson json) {
this.json = json.addOption(ListChangesOption.DETAILED_LABELS)
.addOption(ListChangesOption.DETAILED_ACCOUNTS);
}
@Override
public Object apply(RevisionResource resource) throws OrmException {
return json.format(resource);
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
View File

@@ -54,6 +54,7 @@ public class Module extends RestApiModule {
delete(REVIEWER_KIND).to(DeleteReviewer.class);
child(CHANGE_KIND, "revisions").to(Revisions.class);
get(REVISION_KIND, "review").to(GetReview.class);
post(REVISION_KIND, "review").to(PostReview.class);
post(REVISION_KIND, "submit").to(Submit.class);