Migrate approval radio buttons to new REST API
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
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
index cf4b28b..7637f75 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
@@ -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 @@
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;
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
index 5341d9f..115c0c1 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
@@ -16,6 +16,7 @@
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 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();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
index e33f63d..f857169 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
@@ -68,7 +68,7 @@
}
public final Set<String> labels() {
- return Natives.keys(labels0());
+ return labels0().keySet();
}
public final native String id() /*-{ return this.id; }-*/;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 0b2a148..ecb9a04 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -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.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 @@
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 @@
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 @@
}
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 @@
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 @@
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);
}
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
index b7761c5..46e9892 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
@@ -88,21 +88,26 @@
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());
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
index 1c1cb35..e230880 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
@@ -96,6 +96,9 @@
@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 @@
changeOpen = c.open;
changeSortKey = c.sortKey;
}
+
+ public String getLabel() {
+ return label;
+ }
+
+ public void setLabel(String label) {
+ this.label = label;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
new file mode 100644
index 0000000..997f5e7
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
@@ -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);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 571439d..6586725 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -54,6 +54,7 @@
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);
