summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomasz Zarna2013-08-08 07:49:25 (EDT)
committerTomasz Zarna2013-08-13 05:25:18 (EDT)
commit42ea6a7628ca5f346177723ff0774e9509631a9d (patch)
tree2cad3dea28f746f81cf6b023163f1fa975e7662c
parent6504ce308ecc761fd5717f67c798e72566be313e (diff)
downloadorg.eclipse.mylyn.reviews-42ea6a7628ca5f346177723ff0774e9509631a9d.zip
org.eclipse.mylyn.reviews-42ea6a7628ca5f346177723ff0774e9509631a9d.tar.gz
org.eclipse.mylyn.reviews-42ea6a7628ca5f346177723ff0774e9509631a9d.tar.bz2
414647: Cannot vote on a review in Gerrit 2.6refs/changes/42/15242/3
Bug: 414647 Change-Id: I502cf6dc3c5d9ae43dd3fbab47027d8818304fc0 Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=414647 Signed-off-by: Tomasz Zarna <tomasz.zarna@tasktop.com>
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java17
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PatchSetPublishDetailX.java5
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PermissionLabel.java16
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java39
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ChangeInfoTest.java48
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java48
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/ChangeInfo_CodeReviewMinusOne.json15
-rw-r--r--org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/factories/PublishUiFactory.java3
-rw-r--r--org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/GerritOperationDialog.java6
-rw-r--r--org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/PublishDialog.java85
10 files changed, 196 insertions, 86 deletions
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
index 3c1cb11..343d1aa 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
@@ -440,13 +440,16 @@ public class GerritClient extends ReviewsClient {
public PatchSetPublishDetailX getPatchSetPublishDetail(final PatchSet.Id id, IProgressMonitor monitor)
throws GerritException {
- PatchSetPublishDetailX publishDetail = execute(monitor,
- new Operation<org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchSetPublishDetailX>() {
- @Override
- public void execute(IProgressMonitor monitor) throws GerritException {
- getChangeDetailService(monitor).patchSetPublishDetailX(id, this);
- }
- });
+ PatchSetPublishDetailX publishDetail = execute(monitor, new Operation<PatchSetPublishDetailX>() {
+ @Override
+ public void execute(IProgressMonitor monitor) throws GerritException {
+ getChangeDetailService(monitor).patchSetPublishDetailX(id, this);
+ }
+ });
+ if (publishDetail.getLabels() == null && isVersion26OrLater(monitor)) {
+ ChangeInfo changeInfo = getChangeInfo(id.getParentKey().get(), monitor);
+ publishDetail.setLabels(changeInfo.convertToPermissionLabels());
+ }
return publishDetail;
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PatchSetPublishDetailX.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PatchSetPublishDetailX.java
index a2fbb1f..52be0e5 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PatchSetPublishDetailX.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PatchSetPublishDetailX.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011 Tasktop Technologies and others.
+ * Copyright (c) 2011, 2013 Tasktop Technologies and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -32,4 +32,7 @@ public class PatchSetPublishDetailX extends com.google.gerrit.common.data.PatchS
return labels;
}
+ public void setLabels(List<PermissionLabel> labels) {
+ this.labels = labels;
+ }
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PermissionLabel.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PermissionLabel.java
index 94fc87f..52a7adc 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PermissionLabel.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/PermissionLabel.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011 Tasktop Technologies and others.
+ * Copyright (c) 2011, 2013 Tasktop Technologies and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -33,15 +33,27 @@ public class PermissionLabel {
return max;
}
+ public void setMax(int max) {
+ this.max = max;
+ }
+
public int getMin() {
return min;
}
+ public void setMin(int min) {
+ this.min = min;
+ }
+
public String getName() {
return name;
}
- public String toLabelName(String identifier) {
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ public static String toLabelName(String identifier) {
return "label-" + ApprovalUtil.toNameWithDash(identifier); //$NON-NLS-1$
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
index e856435..17042e7 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
@@ -20,6 +20,8 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import org.eclipse.mylyn.internal.gerrit.core.client.compat.PermissionLabel;
+
import com.google.gerrit.common.data.ApprovalDetail;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.reviewdb.Account;
@@ -67,6 +69,14 @@ public class ChangeInfo {
private AccountInfo owner;
+ private Map<String/*Label*/, LabelInfo> labels;
+
+ private String current_revision;
+
+ private Map<String/*commit ID*/, RevisionInfo> revisions;
+
+ private Map<String/*Label*/, String[]> permitted_labels;
+
// e.g. "0023412400000f7d"
@SuppressWarnings("unused")
private String _sortkey;
@@ -122,12 +132,6 @@ public class ChangeInfo {
return owner;
}
- private Map<String/*Label*/, LabelInfo> labels;
-
- private String current_revision;
-
- private Map<String/*commit ID*/, RevisionInfo> revisions;
-
public Map<String, LabelInfo> getLabels() {
return labels;
}
@@ -140,6 +144,10 @@ public class ChangeInfo {
return revisions;
}
+ public Map<String, String[]> getPermittedLabels() {
+ return permitted_labels;
+ }
+
private PatchSet.Id getCurrentPatchSetId() {
Change.Id changeId = new Change.Id(_number);
int patchSetId = revisions.get(current_revision).getNumber();
@@ -188,4 +196,23 @@ public class ChangeInfo {
return result;
}
+ public List<PermissionLabel> convertToPermissionLabels() {
+ if (permitted_labels == null) {
+ return null;
+ }
+ List<PermissionLabel> result = new ArrayList<PermissionLabel>(permitted_labels.size());
+ for (Entry<String, String[]> entry : permitted_labels.entrySet()) {
+ List<Short> values = new ArrayList<Short>(entry.getValue().length);
+ for (String value : entry.getValue()) {
+ values.add(ApprovalUtil.parseShort(value));
+ }
+ PermissionLabel label = new PermissionLabel();
+ label.setName(PermissionLabel.toLabelName(entry.getKey()));
+ label.setMin(Collections.min(values));
+ label.setMax(Collections.max(values));
+ result.add(label);
+ }
+ return result;
+ }
+
}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ChangeInfoTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ChangeInfoTest.java
index 5ae1cea..e866a3c 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ChangeInfoTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ChangeInfoTest.java
@@ -12,6 +12,8 @@
package org.eclipse.mylyn.gerrit.tests.core.client.rest;
import static org.eclipse.mylyn.gerrit.tests.core.client.rest.IsEmpty.empty;
+import static org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil.CRVW;
+import static org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil.toNameWithDash;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
@@ -36,9 +38,9 @@ import junit.framework.TestCase;
import org.eclipse.mylyn.commons.sdk.util.CommonTestUtil;
import org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport;
+import org.eclipse.mylyn.internal.gerrit.core.client.compat.PermissionLabel;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.AccountInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalInfo;
-import org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ChangeInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.LabelInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.RevisionInfo;
@@ -135,6 +137,7 @@ public class ChangeInfoTest extends TestCase {
assertThat(changeInfo.getLabels().get("Code-Review").getAll(), nullValue());
assertThat(changeInfo.convertToApprovalDetails(), empty());
assertHasCodeReviewApprovalType(changeInfo.convertToApprovalTypes());
+ // no permitted labels
assertHasRevisions(changeInfo, 1);
}
@@ -146,6 +149,7 @@ public class ChangeInfoTest extends TestCase {
assertHasApprovalInfo(changeInfo.getLabels().get("Code-Review").getAll(), -1);
assertHasApprovalDetail(changeInfo.convertToApprovalDetails(), -1);
assertHasCodeReviewApprovalType(changeInfo.convertToApprovalTypes());
+ assertHasCodeReviewPermissionLabels(changeInfo);
assertHasRevisions(changeInfo, 1);
}
@@ -157,6 +161,7 @@ public class ChangeInfoTest extends TestCase {
assertHasApprovalInfo(changeInfo.getLabels().get("Code-Review").getAll(), 0);
assertHasApprovalDetail(changeInfo.convertToApprovalDetails(), 0);
assertHasCodeReviewApprovalType(changeInfo.convertToApprovalTypes());
+ // no permission labels
assertHasRevisions(changeInfo, 2);
}
@@ -176,11 +181,11 @@ public class ChangeInfoTest extends TestCase {
assertThat(values, Matchers.<String, String> hasKey(" 0"));
assertThat(values, Matchers.<String, String> hasKey("+1"));
assertThat(values, Matchers.<String, String> hasKey("+2"));
- assertThat(values.get("-2"), equalTo(ApprovalUtil.CRVW.getValue((short) -2).getName()));
- assertThat(values.get("-1"), equalTo(ApprovalUtil.CRVW.getValue((short) -1).getName()));
- assertThat(values.get(" 0"), equalTo(ApprovalUtil.CRVW.getValue((short) 0).getName()));
- assertThat(values.get("+1"), equalTo(ApprovalUtil.CRVW.getValue((short) 1).getName()));
- assertThat(values.get("+2"), equalTo(ApprovalUtil.CRVW.getValue((short) 2).getName()));
+ assertThat(values.get("-2"), equalTo(CRVW.getValue((short) -2).getName()));
+ assertThat(values.get("-1"), equalTo(CRVW.getValue((short) -1).getName()));
+ assertThat(values.get(" 0"), equalTo(CRVW.getValue((short) 0).getName()));
+ assertThat(values.get("+1"), equalTo(CRVW.getValue((short) 1).getName()));
+ assertThat(values.get("+2"), equalTo(CRVW.getValue((short) 2).getName()));
}
private static Timestamp timestamp(String date) throws ParseException {
@@ -201,10 +206,10 @@ public class ChangeInfoTest extends TestCase {
assertThat(approvalTypes, not(empty()));
assertThat(approvalTypes.size(), is(1));
ApprovalType approvalType = approvalTypes.iterator().next();
- assertThat(approvalType.getCategory().getId(), equalTo(ApprovalUtil.CRVW.getCategory().getId()));
+ assertThat(approvalType.getCategory().getId(), equalTo(CRVW.getCategory().getId()));
assertThat(approvalType.getCategory().getName(), is("Code Review"));
assertThat(approvalType.getValues().size(), is(5));
- for (ApprovalCategoryValue approvalCategoryValue : ApprovalUtil.CRVW.getValues()) {
+ for (ApprovalCategoryValue approvalCategoryValue : CRVW.getValues()) {
assertHasItem(approvalType.getValues(), ApprovalCategoryValueComparator.INSTANCE, approvalCategoryValue);
}
}
@@ -225,11 +230,34 @@ public class ChangeInfoTest extends TestCase {
assertThat(approvalDetail, notNullValue());
Map<Id, PatchSetApproval> approvalMap = approvalDetail.getApprovalMap();
assertThat(approvalMap, notNullValue());
- assertThat(approvalMap, Matchers.<Id, PatchSetApproval> hasKey(ApprovalUtil.CRVW.getCategory().getId()));
- PatchSetApproval patchSetApproval = approvalMap.get(ApprovalUtil.CRVW.getCategory().getId());
+ assertThat(approvalMap, Matchers.<Id, PatchSetApproval> hasKey(CRVW.getCategory().getId()));
+ PatchSetApproval patchSetApproval = approvalMap.get(CRVW.getCategory().getId());
assertThat(patchSetApproval.getValue(), is((short) value));
}
+ public static void assertHasCodeReviewPermissionLabels(ChangeInfo changeInfo) {
+ Map<String, String[]> permittedLabels = changeInfo.getPermittedLabels();
+
+ assertThat(permittedLabels, notNullValue());
+ assertThat(permittedLabels, not(empty()));
+ assertThat(permittedLabels.size(), is(1));
+ String[] crvwPermittedLabels = permittedLabels.get(toNameWithDash(CRVW.getCategory().getName()));
+ assertThat(crvwPermittedLabels, not(empty()));
+ assertThat(crvwPermittedLabels.length, is(3));
+ assertThat(crvwPermittedLabels[0], is(CRVW.getValue((short) -1).formatValue()));
+ assertThat(crvwPermittedLabels[1], is(CRVW.getValue((short) 0).formatValue()));
+ assertThat(crvwPermittedLabels[2], is(CRVW.getValue((short) 1).formatValue()));
+
+ List<PermissionLabel> permissionLabels = changeInfo.convertToPermissionLabels();
+ assertThat(permissionLabels, notNullValue());
+ assertThat(permissionLabels, not(empty()));
+ assertThat(permissionLabels.size(), is(1));
+ PermissionLabel crvwAllowed = permissionLabels.get(0);
+ assertThat(crvwAllowed.getName(), is(PermissionLabel.toLabelName(toNameWithDash(CRVW.getCategory().getName()))));
+ assertThat(crvwAllowed.getMin(), is(-1));
+ assertThat(crvwAllowed.getMax(), is(1));
+ }
+
private static void assertHasRevisions(ChangeInfo changeInfo, int patchSetNr) {
assertThat(changeInfo, notNullValue());
String currentRevision = changeInfo.getCurrentRevision();
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
index e864535..b60fe5e 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
@@ -14,6 +14,7 @@ package org.eclipse.mylyn.internal.gerrit.core.remote;
import static org.eclipse.mylyn.gerrit.tests.core.client.rest.IsEmpty.empty;
import static org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil.CRVW;
import static org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil.VRIF;
+import static org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil.toNameWithDash;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.is;
@@ -21,6 +22,7 @@ import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.Matchers.startsWith;
+import static org.hamcrest.core.IsNot.not;
import static org.junit.Assert.assertThat;
import java.util.Arrays;
@@ -36,13 +38,13 @@ import org.apache.commons.lang.StringUtils;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jgit.api.CommitCommand;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.ChangeInfoTest;
-import org.eclipse.mylyn.gerrit.tests.support.GerritFixture;
import org.eclipse.mylyn.gerrit.tests.support.GerritProject.CommitResult;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritChange;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritVersion;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.ChangeDetailX;
-import org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil;
+import org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchSetPublishDetailX;
+import org.eclipse.mylyn.internal.gerrit.core.client.compat.PermissionLabel;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ChangeInfo;
import org.eclipse.mylyn.reviews.core.model.IApprovalType;
import org.eclipse.mylyn.reviews.core.model.IChange;
@@ -57,15 +59,16 @@ import org.eclipse.mylyn.reviews.core.model.RequirementStatus;
import org.eclipse.mylyn.reviews.core.model.ReviewStatus;
import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
import org.junit.Test;
-import org.osgi.framework.Version;
import com.google.gerrit.common.data.ApprovalDetail;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.ReviewerResult;
import com.google.gerrit.reviewdb.ApprovalCategory;
import com.google.gerrit.reviewdb.ApprovalCategoryValue;
+import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.Change.Status;
import com.google.gerrit.reviewdb.ChangeMessage;
+import com.google.gerrit.reviewdb.PatchSet;
import com.google.gerrit.reviewdb.PatchSetApproval;
/**
@@ -380,7 +383,7 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
assertThat(approvalMap.isEmpty(), is(false));
assertThat(approvalMap.size(), is(1));
- PatchSetApproval crvw = approvalMap.get(ApprovalUtil.CRVW.getCategory().getId());
+ PatchSetApproval crvw = approvalMap.get(CRVW.getCategory().getId());
assertThat(crvw, notNullValue());
assertThat(crvw.getAccountId().get(), is(1000001));
assertThat(crvw.getValue(), is((short) 0));
@@ -394,14 +397,6 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
assertThat(getReview().getReviewerApprovals().get(0), nullValue());
}
- private static Version getCurrentVersion() {
- String version = GerritFixture.current().getSimpleInfo();
- if (version.indexOf('/') != -1) {
- version = version.substring(0, version.indexOf('/'));
- }
- return GerritVersion.parseGerritVersion(version);
- }
-
@Test
public void testCannotRebaseChangeAlreadyUpToDate() throws Exception {
try {
@@ -438,12 +433,9 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
public void testUnpermittedApproval() throws Exception {
String approvalMessage = "approval, time: " + System.currentTimeMillis();
try {
- reviewHarness.client.publishComments(
- reviewHarness.shortId,
- 1,
- approvalMessage,
- new HashSet<ApprovalCategoryValue.Id>(Collections.singleton(ApprovalUtil.CRVW.getValue((short) 2)
- .getId())), new NullProgressMonitor());
+ reviewHarness.client.publishComments(reviewHarness.shortId, 1, approvalMessage,
+ new HashSet<ApprovalCategoryValue.Id>(Collections.singleton(CRVW.getValue((short) 2).getId())),
+ new NullProgressMonitor());
fail("Expected to fail when trying to vote +2 when it's not permitted");
} catch (GerritException e) {
if (isVersion26OrLater()) {
@@ -455,6 +447,26 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
}
}
+ @Test
+ public void testGetPatchSetPublishDetail() throws Exception {
+ int reviewId = Integer.parseInt(reviewHarness.shortId);
+ PatchSet.Id id = new PatchSet.Id(new Change.Id(reviewId), 1);
+
+ PatchSetPublishDetailX patchSetDetail = reviewHarness.client.getPatchSetPublishDetail(id,
+ new NullProgressMonitor());
+
+ assertThat(patchSetDetail, notNullValue());
+ List<PermissionLabel> allowed = patchSetDetail.getLabels();
+ assertThat(allowed, notNullValue());
+ assertThat(allowed, not(empty()));
+ assertThat(allowed.size(), is(1));
+ PermissionLabel crvwAllowed = allowed.get(0);
+ assertThat(crvwAllowed.matches(CRVW.getCategory()), is(true));
+ assertThat(crvwAllowed.getName(), is(PermissionLabel.toLabelName(toNameWithDash(CRVW.getCategory().getName()))));
+ assertThat(crvwAllowed.getMin(), is(-1));
+ assertThat(crvwAllowed.getMax(), is(1));
+ }
+
private boolean isVersion26OrLater() throws GerritException {
return GerritVersion.isVersion26OrLater(reviewHarness.client.getVersion(new NullProgressMonitor()));
}
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/ChangeInfo_CodeReviewMinusOne.json b/org.eclipse.mylyn.gerrit.tests/testdata/ChangeInfo_CodeReviewMinusOne.json
index 9004a19..7ef8432 100644
--- a/org.eclipse.mylyn.gerrit.tests/testdata/ChangeInfo_CodeReviewMinusOne.json
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/ChangeInfo_CodeReviewMinusOne.json
@@ -34,8 +34,19 @@
}
}
},
- "permitted_labels": {},
- "removable_reviewers": [],
+ "permitted_labels": {
+ "Code-Review": [
+ "-1",
+ " 0",
+ "+1"
+ ]
+ },
+ "removable_reviewers": [
+ {
+ "_account_id": 1000001,
+ "email": "tests@mylyn.eclipse.org"
+ }
+ ],
"current_revision": "d164521a077df962fdd674ba0fa8574d1c5eaa47",
"revisions": {
"d164521a077df962fdd674ba0fa8574d1c5eaa47": {
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/factories/PublishUiFactory.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/factories/PublishUiFactory.java
index bfddc9b..f41259a 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/factories/PublishUiFactory.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/factories/PublishUiFactory.java
@@ -45,7 +45,8 @@ public class PublishUiFactory extends AbstractPatchSetUiFactory {
TaskAttribute comment = getTaskData().getRoot().getAttribute(TaskAttribute.COMMENT_NEW);
String editorCommentText = comment != null ? comment.getValue() : "";
- int open = new PublishDialog(getShell(), getTask(), publishDetail, getModelObject(), editorCommentText).open(getEditor());
+ int open = new PublishDialog(getShell(), getTask(), getChange(), publishDetail, getModelObject(),
+ editorCommentText).open(getEditor());
if (open == Window.OK && comment != null) {
comment.clearValues();
if (getTaskEditorPage() != null) {
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/GerritOperationDialog.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/GerritOperationDialog.java
index 4b8a75b..3d5ce3c 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/GerritOperationDialog.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/GerritOperationDialog.java
@@ -149,15 +149,13 @@ public abstract class GerritOperationDialog extends ProgressDialog {
Control control = super.createDialogArea(parent);
if (needsConfig()) {
GerritConfig config = getOperationFactory().getClient(getTask()).getGerritConfig();
- if (config != null) {
- doRefresh(config);
- } else {
+ if (config == null) {
GerritOperation<GerritConfiguration> operation = getOperationFactory().createRefreshConfigOperation(
getTask(), new RefreshConfigRequest());
performOperation(operation);
config = operation.getOperationResult().getGerritConfig();
- doRefresh(config);
}
+ doRefresh(config);
}
return control;
}
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/PublishDialog.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/PublishDialog.java
index 0984e42..167ece6 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/PublishDialog.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/operations/PublishDialog.java
@@ -12,6 +12,7 @@
package org.eclipse.mylyn.internal.gerrit.ui.operations;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
@@ -19,6 +20,7 @@ import java.util.List;
import java.util.Set;
import org.eclipse.jface.layout.GridDataFactory;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritChange;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchSetPublishDetailX;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.PermissionLabel;
import org.eclipse.mylyn.internal.gerrit.core.operations.GerritOperation;
@@ -51,6 +53,8 @@ public class PublishDialog extends GerritOperationDialog {
private static final String KEY_ID = "ApprovalCategoryValue.Id"; //$NON-NLS-1$
+ private final GerritChange gerritChange;
+
private final PatchSetPublishDetail publishDetail;
private RichTextEditor messageEditor;
@@ -63,9 +67,10 @@ public class PublishDialog extends GerritOperationDialog {
private final IReviewItemSet set;
- public PublishDialog(Shell parentShell, ITask task, PatchSetPublishDetail publishDetail, IReviewItemSet set,
- String editorCommentText) {
+ public PublishDialog(Shell parentShell, ITask task, GerritChange gerritChange, PatchSetPublishDetail publishDetail,
+ IReviewItemSet set, String editorCommentText) {
super(parentShell, task);
+ this.gerritChange = gerritChange;
this.publishDetail = publishDetail;
this.set = set;
this.editorCommentText = editorCommentText;
@@ -138,47 +143,57 @@ public class PublishDialog extends GerritOperationDialog {
return;
}
- if (config.getApprovalTypes() != null && config.getApprovalTypes().getApprovalTypes() != null) {
- for (ApprovalType approvalType : config.getApprovalTypes().getApprovalTypes()) {
- Set<ApprovalCategoryValue.Id> allowed = getAllowed(publishDetail, approvalType);
- if (allowed != null && allowed.size() > 0) {
- Group group = new Group(approvalComposite, SWT.NONE);
- GridDataFactory.fillDefaults().grab(true, false).applyTo(group);
- group.setText(approvalType.getCategory().getName());
- group.setLayout(new RowLayout(SWT.VERTICAL));
-
- int givenValue = 0;
- if (publishDetail.getGiven() != null) {
- // Gerrit 2.1
- PatchSetApproval approval = publishDetail.getGiven().get(approvalType.getCategory().getId());
- if (approval != null) {
- givenValue = approval.getValue();
- }
- }
+ Collection<ApprovalType> approvalTypes = getApprovalTypes(config);
+ if (approvalTypes == null) {
+ approvalTypes = gerritChange.getChangeDetail().getApprovalTypes();
+ }
- List<ApprovalCategoryValue.Id> allowedList = new ArrayList<ApprovalCategoryValue.Id>(allowed);
- Collections.sort(allowedList, new Comparator<ApprovalCategoryValue.Id>() {
- public int compare(ApprovalCategoryValue.Id o1, ApprovalCategoryValue.Id o2) {
- return o2.get() - o1.get();
- }
- });
- for (ApprovalCategoryValue.Id valueId : allowedList) {
- ApprovalCategoryValue approvalValue = approvalType.getValue(valueId.get());
-
- Button button = new Button(group, SWT.RADIO);
- button.setText(approvalValue.format());
- if (approvalValue.getValue() == givenValue) {
- button.setSelection(true);
- }
+ for (ApprovalType approvalType : approvalTypes) {
+ Set<ApprovalCategoryValue.Id> allowed = getAllowed(publishDetail, approvalType);
+ if (allowed != null && allowed.size() > 0) {
+ Group group = new Group(approvalComposite, SWT.NONE);
+ GridDataFactory.fillDefaults().grab(true, false).applyTo(group);
+ group.setText(approvalType.getCategory().getName());
+ group.setLayout(new RowLayout(SWT.VERTICAL));
+
+ int givenValue = 0;
+ if (publishDetail.getGiven() != null) {
+ // Gerrit 2.1
+ PatchSetApproval approval = publishDetail.getGiven().get(approvalType.getCategory().getId());
+ if (approval != null) {
+ givenValue = approval.getValue();
+ }
+ }
- button.setData(KEY_ID, valueId);
- approvalButtons.add(button);
+ List<ApprovalCategoryValue.Id> allowedList = new ArrayList<ApprovalCategoryValue.Id>(allowed);
+ Collections.sort(allowedList, new Comparator<ApprovalCategoryValue.Id>() {
+ public int compare(ApprovalCategoryValue.Id o1, ApprovalCategoryValue.Id o2) {
+ return o2.get() - o1.get();
+ }
+ });
+ for (ApprovalCategoryValue.Id valueId : allowedList) {
+ ApprovalCategoryValue approvalValue = approvalType.getValue(valueId.get());
+
+ Button button = new Button(group, SWT.RADIO);
+ button.setText(approvalValue.format());
+ if (approvalValue.getValue() == givenValue) {
+ button.setSelection(true);
}
+
+ button.setData(KEY_ID, valueId);
+ approvalButtons.add(button);
}
}
}
}
+ private Collection<ApprovalType> getApprovalTypes(GerritConfig config) {
+ if (config.getApprovalTypes() != null && config.getApprovalTypes().getApprovalTypes() != null) {
+ return config.getApprovalTypes().getApprovalTypes();
+ }
+ return null;
+ }
+
private Set<ApprovalCategoryValue.Id> getAllowed(PatchSetPublishDetail publishDetail, ApprovalType approvalType) {
if (publishDetail.getAllowed() != null) {
// Gerrit 2.1