Merge "Fix check for group index in GroupCacheImpl.ByIdLoader"
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
index 4c1df8c..ee07880 100644
--- a/Documentation/dev-contributing.txt
+++ b/Documentation/dev-contributing.txt
@@ -165,7 +165,7 @@
To format Java source code, Gerrit uses the
link:https://github.com/google/google-java-format[`google-java-format`]
tool (version 1.3), and to format Bazel BUILD and WORKSPACE files the
-link:https://github.com/bazelbuild/buildifier[`buildifier`] tool (version 0.4.5).
+link:https://github.com/bazelbuild/buildifier[`buildifier`] tool (version 0.6.0).
These tools automatically apply format according to the style guides; this
streamlines code review by reducing the need for time-consuming, tedious,
and contentious discussions about trivial issues like whitespace.
diff --git a/WORKSPACE b/WORKSPACE
index afc8d9c..6a9c717 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1139,8 +1139,8 @@
bower_archive(
name = "polymer",
package = "polymer/polymer",
- sha1 = "566b5fe9a2a3eea2cf3417c67d975a6752d131eb",
- version = "1.9.3",
+ sha1 = "62ce80a5079c1b97f6c5c6ebf6b350e741b18b9c",
+ version = "1.11.0",
)
bower_archive(
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index c3cca6c..adcb57e 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -21,6 +21,8 @@
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.Util.category;
+import static com.google.gerrit.server.project.Util.value;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.HEAD;
@@ -37,6 +39,9 @@
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.LabelFunction;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.extensions.api.GerritApi;
@@ -1382,4 +1387,19 @@
return rw.parseCommit(
ObjectId.fromString(get(changeId, ListChangesOption.CURRENT_REVISION).currentRevision));
}
+
+ protected void configLabel(String label, LabelFunction func) throws Exception {
+ configLabel(
+ project, label, func, value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
+ }
+
+ protected void configLabel(
+ Project.NameKey project, String label, LabelFunction func, LabelValue... value)
+ throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ LabelType labelType = category(label, value);
+ labelType.setFunction(func);
+ cfg.getLabelSections().put(labelType.getName(), labelType);
+ saveProjectConfig(project, cfg);
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index eef9d87..7ca424d 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -69,6 +69,7 @@
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
@@ -132,6 +133,7 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.update.BatchUpdate;
@@ -3188,6 +3190,68 @@
gApi.changes().id(changeId).topic(topic);
}
+ @Test
+ public void submittableAfterLosingPermissions_MaxWithBlock() throws Exception {
+ configLabel("Label", LabelFunction.MAX_WITH_BLOCK);
+ submittableAfterLosingPermissions("Label");
+ }
+
+ @Test
+ public void submittableAfterLosingPermissions_AnyWithBlock() throws Exception {
+ configLabel("Label", LabelFunction.ANY_WITH_BLOCK);
+ submittableAfterLosingPermissions("Label");
+ }
+
+ public void submittableAfterLosingPermissions(String label) throws Exception {
+ String codeReviewLabel = "Code-Review";
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ AccountGroup.UUID registered = SystemGroupBackend.REGISTERED_USERS;
+ Util.allow(cfg, Permission.forLabel(label), -1, +1, registered, "refs/heads/*");
+ Util.allow(cfg, Permission.forLabel(codeReviewLabel), -2, +2, registered, "refs/heads/*");
+ saveProjectConfig(cfg);
+
+ setApiUser(user);
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ // Verify user's permitted range.
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertPermitted(change, label, -1, 0, 1);
+ assertPermitted(change, codeReviewLabel, -2, -1, 0, 1, 2);
+
+ ReviewInput input = new ReviewInput();
+ input.label(codeReviewLabel, 2);
+ input.label(label, 1);
+ gApi.changes().id(changeId).current().review(input);
+
+ assertThat(gApi.changes().id(changeId).current().reviewer(user.email).votes().keySet())
+ .containsExactly(codeReviewLabel, label);
+ assertThat(gApi.changes().id(changeId).current().reviewer(user.email).votes().values())
+ .containsExactly((short) 2, (short) 1);
+ assertThat(gApi.changes().id(changeId).get().submittable).isTrue();
+
+ setApiUser(admin);
+ // Remove user's permission for 'Label'.
+ Util.remove(cfg, Permission.forLabel(label), registered, "refs/heads/*");
+ // Update user's permitted range for 'Code-Review' to be -1...+1.
+ Util.remove(cfg, Permission.forLabel(codeReviewLabel), registered, "refs/heads/*");
+ Util.allow(cfg, Permission.forLabel(codeReviewLabel), -1, +1, registered, "refs/heads/*");
+ saveProjectConfig(cfg);
+
+ // Verify user's new permitted range.
+ setApiUser(user);
+ change = gApi.changes().id(changeId).get();
+ assertPermitted(change, label);
+ assertPermitted(change, codeReviewLabel, -1, 0, 1);
+
+ assertThat(gApi.changes().id(changeId).current().reviewer(user.email).votes().values())
+ .containsExactly((short) 2, (short) 1);
+ assertThat(gApi.changes().id(changeId).get().submittable).isTrue();
+
+ setApiUser(admin);
+ gApi.changes().id(changeId).current().submit();
+ }
+
private String getCommitMessage(String changeId) throws RestApiException, IOException {
return gApi.changes().id(changeId).current().file("/COMMIT_MSG").content().asString();
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
index c02b60f..1558988 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/PutUsernameIT.java
@@ -18,7 +18,8 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
-import com.google.gerrit.extensions.common.UsernameInput;
+import com.google.gerrit.extensions.api.accounts.UsernameInput;
+
import org.junit.Test;
public class PutUsernameIT extends AbstractDaemonTest {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
index 8388ed0..37d3e1e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
@@ -22,6 +22,7 @@
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.MoveInput;
@@ -33,6 +34,7 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.Util;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.PersonIdent;
@@ -227,6 +229,59 @@
move(r.getChangeId(), newBranch.get());
}
+ @Test
+ public void moveChangeOnlyKeepVetoVotes() throws Exception {
+ // A vote for a label will be kept after moving if the label's function is *WithBlock and the
+ // vote holds the minimum value.
+ createBranch(new Branch.NameKey(project, "foo"));
+
+ String codeReviewLabel = "Code-Review"; // 'Code-Review' uses 'MaxWithBlock' function.
+ String testLabelA = "Label-A";
+ String testLabelB = "Label-B";
+ String testLabelC = "Label-C";
+ configLabel(testLabelA, LabelFunction.ANY_WITH_BLOCK);
+ configLabel(testLabelB, LabelFunction.MAX_NO_BLOCK);
+ configLabel(testLabelC, LabelFunction.NO_BLOCK);
+
+ AccountGroup.UUID registered = SystemGroupBackend.REGISTERED_USERS;
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ Util.allow(cfg, Permission.forLabel(testLabelA), -1, +1, registered, "refs/heads/*");
+ Util.allow(cfg, Permission.forLabel(testLabelB), -1, +1, registered, "refs/heads/*");
+ Util.allow(cfg, Permission.forLabel(testLabelC), -1, +1, registered, "refs/heads/*");
+ saveProjectConfig(cfg);
+
+ String changeId = createChange().getChangeId();
+ gApi.changes().id(changeId).current().review(ReviewInput.reject());
+
+ amendChange(changeId);
+
+ ReviewInput input = new ReviewInput();
+ input.label(testLabelA, -1);
+ input.label(testLabelB, -1);
+ input.label(testLabelC, -1);
+ gApi.changes().id(changeId).current().review(input);
+
+ assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().keySet())
+ .containsExactly(codeReviewLabel, testLabelA, testLabelB, testLabelC);
+ assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().values())
+ .containsExactly((short) -2, (short) -1, (short) -1, (short) -1);
+
+ // Move the change to the 'foo' branch.
+ assertThat(gApi.changes().id(changeId).get().branch).isEqualTo("master");
+ move(changeId, "foo");
+ assertThat(gApi.changes().id(changeId).get().branch).isEqualTo("foo");
+
+ // 'Code-Review -2' and 'Label-A -1' will be kept.
+ assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().values())
+ .containsExactly((short) -2, (short) -1, (short) 0, (short) 0);
+
+ // Move the change back to 'master'.
+ move(changeId, "master");
+ assertThat(gApi.changes().id(changeId).get().branch).isEqualTo("master");
+ assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().values())
+ .containsExactly((short) -2, (short) -1, (short) 0, (short) 0);
+ }
+
private void move(int changeNum, String destination) throws RestApiException {
gApi.changes().id(changeNum).move(destination);
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 38ff3c7..75e7553 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -15,6 +15,10 @@
package com.google.gerrit.acceptance.server.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.common.data.LabelFunction.ANY_WITH_BLOCK;
+import static com.google.gerrit.common.data.LabelFunction.MAX_NO_BLOCK;
+import static com.google.gerrit.common.data.LabelFunction.NO_BLOCK;
+import static com.google.gerrit.common.data.LabelFunction.NO_OP;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
@@ -80,11 +84,11 @@
@Test
public void customLabelNoOp_NegativeVoteNotBlock() throws Exception {
- label.setFunctionName("NoOp");
+ label.setFunction(NO_OP);
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(new ReviewInput().label(label.getName(), -1));
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
assertThat(q.rejected).isNotNull();
@@ -93,11 +97,11 @@
@Test
public void customLabelNoBlock_NegativeVoteNotBlock() throws Exception {
- label.setFunctionName("NoBlock");
+ label.setFunction(NO_BLOCK);
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(new ReviewInput().label(label.getName(), -1));
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
assertThat(q.rejected).isNotNull();
@@ -106,11 +110,11 @@
@Test
public void customLabelMaxNoBlock_NegativeVoteNotBlock() throws Exception {
- label.setFunctionName("MaxNoBlock");
+ label.setFunction(MAX_NO_BLOCK);
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(new ReviewInput().label(label.getName(), -1));
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
assertThat(q.rejected).isNotNull();
@@ -119,11 +123,11 @@
@Test
public void customLabelAnyWithBlock_NegativeVoteBlock() throws Exception {
- label.setFunctionName("AnyWithBlock");
+ label.setFunction(ANY_WITH_BLOCK);
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(new ReviewInput().label(label.getName(), -1));
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
assertThat(q.disliked).isNull();
@@ -133,7 +137,7 @@
@Test
public void customLabelAnyWithBlock_Addreviewer_ZeroVote() throws Exception {
- P.setFunctionName("AnyWithBlock");
+ P.setFunction(ANY_WITH_BLOCK);
saveLabelConfig();
PushOneCommit.Result r = createChange();
AddReviewerInput in = new AddReviewerInput();
@@ -144,7 +148,7 @@
input.message = "foo";
revision(r).review(input);
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(P.getName());
assertThat(q.all).hasSize(2);
assertThat(q.disliked).isNull();
@@ -158,7 +162,7 @@
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(new ReviewInput().label(label.getName(), -1));
- ChangeInfo c = get(r.getChangeId());
+ ChangeInfo c = getWithLabels(r);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
assertThat(q.disliked).isNull();
@@ -168,16 +172,16 @@
@Test
public void customLabel_DisallowPostSubmit() throws Exception {
- label.setFunctionName("NoOp");
+ label.setFunction(NO_OP);
label.setAllowPostSubmit(false);
- P.setFunctionName("NoOp");
+ P.setFunction(NO_OP);
saveLabelConfig();
PushOneCommit.Result r = createChange();
revision(r).review(ReviewInput.approve());
revision(r).submit();
- ChangeInfo info = get(r.getChangeId(), ListChangesOption.DETAILED_LABELS);
+ ChangeInfo info = getWithLabels(r);
assertPermitted(info, "Code-Review", 2);
assertPermitted(info, P.getName(), 0, 1);
assertPermitted(info, label.getName());
@@ -199,4 +203,8 @@
cfg.getLabelSections().put(P.getName(), P);
saveProjectConfig(project, cfg);
}
+
+ private ChangeInfo getWithLabels(PushOneCommit.Result r) throws Exception {
+ return get(r.getChangeId(), ListChangesOption.LABELS, ListChangesOption.DETAILED_LABELS);
+ }
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelFunction.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelFunction.java
new file mode 100644
index 0000000..0ce2c29
--- /dev/null
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelFunction.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2017 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.common.data;
+
+import com.google.gerrit.common.Nullable;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * Functions for determining submittability based on label votes.
+ *
+ * <p>Only describes built-in label functions. Admins can extend the logic arbitrarily using Prolog
+ * rules, in which case the choice of function in the project config is ignored.
+ *
+ * <p>Function semantics are documented in {@code config-labels.txt}, and actual behavior is
+ * implemented in Prolog in {@code gerrit_common.pl}.
+ */
+public enum LabelFunction {
+ MAX_WITH_BLOCK("MaxWithBlock", true),
+ ANY_WITH_BLOCK("AnyWithBlock", true),
+ MAX_NO_BLOCK("MaxNoBlock", false),
+ NO_BLOCK("NoBlock", false),
+ NO_OP("NoOp", false),
+ PATCH_SET_LOCK("PatchSetLock", false);
+
+ public static final Map<String, LabelFunction> ALL;
+
+ static {
+ Map<String, LabelFunction> all = new LinkedHashMap<>();
+ for (LabelFunction f : values()) {
+ all.put(f.getFunctionName(), f);
+ }
+ ALL = Collections.unmodifiableMap(all);
+ }
+
+ public static Optional<LabelFunction> parse(@Nullable String str) {
+ return Optional.ofNullable(ALL.get(str));
+ }
+
+ private final String name;
+ private final boolean isBlock;
+
+ private LabelFunction(String name, boolean isBlock) {
+ this.name = name;
+ this.isBlock = isBlock;
+ }
+
+ /** The function name as defined in documentation and {@code project.config}. */
+ public String getFunctionName() {
+ return name;
+ }
+
+ /** Whether the label is a "block" label, meaning a minimum vote will prevent submission. */
+ public boolean isBlock() {
+ return isBlock;
+ }
+}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
index c90e1fd..7bfd22e 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
@@ -14,6 +14,7 @@
package com.google.gerrit.common.data;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.ArrayList;
@@ -22,6 +23,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
public class LabelType {
public static final boolean DEF_ALLOW_POST_SUBMIT = true;
@@ -97,7 +99,9 @@
protected String name;
+ // String rather than LabelFunction for backwards compatibility with GWT JSON interface.
protected String functionName;
+
protected boolean copyMinScore;
protected boolean copyMaxScore;
protected boolean copyAllScoresOnMergeFirstParentUpdate;
@@ -124,7 +128,7 @@
values = sortValues(valueList);
defaultValue = 0;
- functionName = "MaxWithBlock";
+ functionName = LabelFunction.MAX_WITH_BLOCK.getFunctionName();
maxNegative = Short.MIN_VALUE;
maxPositive = Short.MAX_VALUE;
@@ -154,12 +158,19 @@
return psa.getLabelId().get().equalsIgnoreCase(name);
}
- public String getFunctionName() {
- return functionName;
+ public LabelFunction getFunction() {
+ if (functionName == null) {
+ return null;
+ }
+ Optional<LabelFunction> f = LabelFunction.parse(functionName);
+ if (!f.isPresent()) {
+ throw new IllegalStateException("Unsupported functionName: " + functionName);
+ }
+ return f.get();
}
- public void setFunctionName(String functionName) {
- this.functionName = functionName;
+ public void setFunction(@Nullable LabelFunction function) {
+ this.functionName = function != null ? function.getFunctionName() : null;
}
public boolean canOverride() {
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/GpgKeysInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/GpgKeysInput.java
similarity index 93%
rename from gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/GpgKeysInput.java
rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/GpgKeysInput.java
index 95eb1c4..8fb587a 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/GpgKeysInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/GpgKeysInput.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.extensions.common;
+package com.google.gerrit.extensions.api.accounts;
import java.util.List;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SshKeyInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/SshKeyInput.java
similarity index 93%
rename from gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SshKeyInput.java
rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/SshKeyInput.java
index e04ea23..46dd858 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SshKeyInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/SshKeyInput.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.extensions.common;
+package com.google.gerrit.extensions.api.accounts;
import com.google.gerrit.extensions.restapi.RawInput;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/UsernameInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/UsernameInput.java
similarity index 93%
rename from gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/UsernameInput.java
rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/UsernameInput.java
index baff84b..f774ddc 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/UsernameInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/UsernameInput.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.extensions.common;
+package com.google.gerrit.extensions.api.accounts;
import com.google.gerrit.extensions.restapi.DefaultInput;
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
index a34243f..0df0aa9 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
@@ -15,8 +15,8 @@
package com.google.gerrit.gpg.api;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
+import com.google.gerrit.extensions.api.accounts.GpgKeysInput;
import com.google.gerrit.extensions.common.GpgKeyInfo;
-import com.google.gerrit.extensions.common.GpgKeysInput;
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index b2383ca..8014574 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -28,8 +28,8 @@
import com.google.common.collect.Sets;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.errors.EmailException;
+import com.google.gerrit.extensions.api.accounts.GpgKeysInput;
import com.google.gerrit.extensions.common.GpgKeyInfo;
-import com.google.gerrit.extensions.common.GpgKeysInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitLabels.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitLabels.java
index 60fd60f..c7309f8 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitLabels.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitLabels.java
@@ -14,6 +14,8 @@
package com.google.gerrit.pgm.init;
+import static com.google.gerrit.common.data.LabelFunction.MAX_WITH_BLOCK;
+
import com.google.gerrit.pgm.init.api.AllProjectsConfig;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitStep;
@@ -54,7 +56,7 @@
public void postRun() throws Exception {
Config cfg = allProjectsConfig.load().getConfig();
if (installVerified) {
- cfg.setString(KEY_LABEL, LABEL_VERIFIED, KEY_FUNCTION, "MaxWithBlock");
+ cfg.setString(KEY_LABEL, LABEL_VERIFIED, KEY_FUNCTION, MAX_WITH_BLOCK.getFunctionName());
cfg.setStringList(
KEY_LABEL,
LABEL_VERIFIED,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
index 6971b48..c1f89e2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
-import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -213,7 +212,7 @@
}
}
return labelNormalizer.normalize(notes, user, byUser.values()).getNormalized();
- } catch (IOException | PermissionBackendException e) {
+ } catch (IOException e) {
throw new OrmException(e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
index 79a16e7..5680b56 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
@@ -19,8 +19,8 @@
import com.google.common.io.ByteSource;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.common.errors.InvalidSshKeyException;
+import com.google.gerrit.extensions.api.accounts.SshKeyInput;
import com.google.gerrit.extensions.common.SshKeyInfo;
-import com.google.gerrit.extensions.common.SshKeyInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RawInput;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutUsername.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutUsername.java
index 335b0b4..2368913 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutUsername.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutUsername.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.account;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
+import com.google.gerrit.extensions.api.accounts.UsernameInput;
import com.google.gerrit.extensions.client.AccountFieldName;
-import com.google.gerrit.extensions.common.UsernameInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 66fb199..f36322c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
+import com.google.gerrit.extensions.api.accounts.SshKeyInput;
import com.google.gerrit.extensions.api.accounts.StatusInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
@@ -37,7 +38,6 @@
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.SshKeyInfo;
-import com.google.gerrit.extensions.common.SshKeyInput;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
index 2f3855c..27d4eb1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
@@ -21,6 +21,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.api.changes.MoveInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -31,18 +32,24 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -55,7 +62,8 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import java.util.ArrayList;
+import java.util.List;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -71,6 +79,9 @@
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeMessagesUtil cmUtil;
private final PatchSetUtil psUtil;
+ private final ApprovalsUtil approvalsUtil;
+ private final ProjectCache projectCache;
+ private final Provider<CurrentUser> userProvider;
@Inject
Move(
@@ -81,7 +92,10 @@
Provider<InternalChangeQuery> queryProvider,
ChangeMessagesUtil cmUtil,
RetryHelper retryHelper,
- PatchSetUtil psUtil) {
+ PatchSetUtil psUtil,
+ ApprovalsUtil approvalsUtil,
+ ProjectCache projectCache,
+ Provider<CurrentUser> userProvider) {
super(retryHelper);
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
@@ -90,6 +104,9 @@
this.queryProvider = queryProvider;
this.cmUtil = cmUtil;
this.psUtil = psUtil;
+ this.approvalsUtil = approvalsUtil;
+ this.projectCache = projectCache;
+ this.userProvider = userProvider;
}
@Override
@@ -138,7 +155,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
- throws OrmException, ResourceConflictException, RepositoryNotFoundException, IOException {
+ throws OrmException, ResourceConflictException, IOException {
change = ctx.getChange();
if (change.getStatus() != Status.NEW) {
throw new ResourceConflictException("Change is " + ChangeUtil.status(change));
@@ -188,10 +205,13 @@
throw new ResourceConflictException("Patch set is not current");
}
- ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
+ PatchSet.Id psId = change.currentPatchSetId();
+ ChangeUpdate update = ctx.getUpdate(psId);
update.setBranch(newDestKey.get());
change.setDest(newDestKey);
+ updateApprovals(ctx, update, psId, projectKey);
+
StringBuilder msgBuf = new StringBuilder();
msgBuf.append("Change destination moved from ");
msgBuf.append(changePrevDest.getShortName());
@@ -207,6 +227,46 @@
return true;
}
+
+ /**
+ * We have a long discussion about how to deal with its votes after moving a change from one
+ * branch to another. In the end, we think only keeping the veto votes is the best way since
+ * it's simple for us and less confusing for our users. See the discussion in the following
+ * proposal: https://gerrit-review.googlesource.com/c/gerrit/+/129171
+ */
+ private void updateApprovals(
+ ChangeContext ctx, ChangeUpdate update, PatchSet.Id psId, Project.NameKey project)
+ throws IOException, OrmException {
+ List<PatchSetApproval> approvals = new ArrayList<>();
+ for (PatchSetApproval psa :
+ approvalsUtil.byPatchSet(
+ ctx.getDb(),
+ ctx.getNotes(),
+ userProvider.get(),
+ psId,
+ ctx.getRevWalk(),
+ ctx.getRepoView().getConfig())) {
+ ProjectState projectState = projectCache.checkedGet(project);
+ LabelType type =
+ projectState.getLabelTypes(ctx.getNotes(), ctx.getUser()).byLabel(psa.getLabelId());
+ // Only keep veto votes, defined as votes where:
+ // 1- the label function allows minimum values to block submission.
+ // 2- the vote holds the minimum value.
+ if (type.isMaxNegative(psa) && type.getFunction().isBlock()) {
+ continue;
+ }
+
+ // Remove votes from NoteDb.
+ update.removeApprovalFor(psa.getAccountId(), psa.getLabel());
+ approvals.add(
+ new PatchSetApproval(
+ new PatchSetApproval.Key(psId, psa.getAccountId(), new LabelId(psa.getLabel())),
+ (short) 0,
+ ctx.getWhen()));
+ }
+ // Remove votes from ReviewDb.
+ ctx.getDb().patchSetApprovals().upsert(approvals);
+ }
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
index 6a332cd..73cda7f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
@@ -24,32 +24,26 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.permissions.LabelPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
import java.util.List;
/**
- * Normalizes votes on labels according to project config and permissions.
+ * Normalizes votes on labels according to project config.
*
* <p>Votes are recorded in the database for a user based on the state of the project at that time:
- * what labels are defined for the project, and what the user is allowed to vote on. Both of those
- * can change between the time a vote is originally made and a later point, for example when a
- * change is submitted. This class normalizes old votes against current project configuration.
+ * what labels are defined for the project. The label definition can change between the time a vote
+ * is originally made and a later point, for example when a change is submitted. This class
+ * normalizes old votes against current project configuration.
*/
@Singleton
public class LabelNormalizer {
@@ -77,33 +71,24 @@
}
}
- private final Provider<ReviewDb> db;
private final IdentifiedUser.GenericFactory userFactory;
- private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@Inject
- LabelNormalizer(
- Provider<ReviewDb> db,
- IdentifiedUser.GenericFactory userFactory,
- PermissionBackend permissionBackend,
- ProjectCache projectCache) {
- this.db = db;
+ LabelNormalizer(IdentifiedUser.GenericFactory userFactory, ProjectCache projectCache) {
this.userFactory = userFactory;
- this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
}
/**
* @param notes change containing the given approvals.
* @param approvals list of approvals.
- * @return copies of approvals normalized to the defined ranges for the label type and permissions
- * for the user. Approvals for unknown labels are not included in the output, nor are
- * approvals where the user has no permissions for that label.
+ * @return copies of approvals normalized to the defined ranges for the label type. Approvals for
+ * unknown labels are not included in the output.
* @throws OrmException
*/
public Result normalize(ChangeNotes notes, Collection<PatchSetApproval> approvals)
- throws OrmException, PermissionBackendException, IOException {
+ throws OrmException, IOException {
IdentifiedUser user = userFactory.create(notes.getChange().getOwner());
return normalize(notes, user, approvals);
}
@@ -112,13 +97,12 @@
* @param notes change notes containing the given approvals.
* @param user current user.
* @param approvals list of approvals.
- * @return copies of approvals normalized to the defined ranges for the label type and permissions
- * for the user. Approvals for unknown labels are not included in the output, nor are
- * approvals where the user has no permissions for that label.
+ * @return copies of approvals normalized to the defined ranges for the label type. Approvals for
+ * unknown labels are not included in the output.
*/
public Result normalize(
ChangeNotes notes, CurrentUser user, Collection<PatchSetApproval> approvals)
- throws PermissionBackendException, IOException {
+ throws IOException {
List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
@@ -142,9 +126,7 @@
}
PatchSetApproval copy = copy(psa);
applyTypeFloor(label, copy);
- if (!applyRightFloor(notes, label, copy)) {
- deleted.add(psa);
- } else if (copy.getValue() != psa.getValue()) {
+ if (copy.getValue() != psa.getValue()) {
updated.add(copy);
} else {
unchanged.add(psa);
@@ -157,26 +139,6 @@
return new PatchSetApproval(src.getPatchSetId(), src);
}
- private boolean applyRightFloor(ChangeNotes notes, LabelType lt, PatchSetApproval a)
- throws PermissionBackendException {
- PermissionBackend.ForChange forChange =
- permissionBackend.user(userFactory.create(a.getAccountId())).database(db).change(notes);
- // Check if the user is allowed to vote on the label at all
- try {
- forChange.check(new LabelPermission(lt.getName()));
- } catch (AuthException e) {
- return false;
- }
- // Squash vote to nearest allowed value
- try {
- forChange.check(new LabelPermission.WithValue(lt.getName(), a.getValue()));
- return true;
- } catch (AuthException e) {
- a.setValue(forChange.squashThenCheck(lt, a.getValue()));
- return true;
- }
- }
-
private void applyTypeFloor(LabelType lt, PatchSetApproval a) {
LabelValue atMin = lt.getMin();
if (atMin != null && a.getValue() < atMin.getValue()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index 5ee9c45..bb811cd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -19,11 +19,9 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner;
-import com.google.common.base.MoreObjects;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -33,6 +31,7 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
@@ -67,6 +66,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -156,9 +156,6 @@
private static final String KEY_VALUE = "value";
private static final String KEY_CAN_OVERRIDE = "canOverride";
private static final String KEY_BRANCH = "branch";
- private static final ImmutableSet<String> LABEL_FUNCTIONS =
- ImmutableSet.of(
- "MaxWithBlock", "AnyWithBlock", "MaxNoBlock", "NoBlock", "NoOp", "PatchSetLock");
private static final String REVIEWER = "reviewer";
private static final String KEY_ENABLE_REVIEWER_BY_EMAIL = "enableByEmail";
@@ -868,19 +865,20 @@
continue;
}
- String functionName =
- MoreObjects.firstNonNull(rc.getString(LABEL, name, KEY_FUNCTION), "MaxWithBlock");
- if (LABEL_FUNCTIONS.contains(functionName)) {
- label.setFunctionName(functionName);
- } else {
+ String functionName = rc.getString(LABEL, name, KEY_FUNCTION);
+ Optional<LabelFunction> function =
+ functionName != null
+ ? LabelFunction.parse(functionName)
+ : Optional.of(LabelFunction.MAX_WITH_BLOCK);
+ if (!function.isPresent()) {
error(
new ValidationError(
PROJECT_CONFIG,
String.format(
"Invalid %s for label \"%s\". Valid names are: %s",
- KEY_FUNCTION, name, Joiner.on(", ").join(LABEL_FUNCTIONS))));
- label.setFunctionName(null);
+ KEY_FUNCTION, name, Joiner.on(", ").join(LabelFunction.ALL.keySet()))));
}
+ label.setFunction(function.orElse(null));
if (!values.isEmpty()) {
short dv = (short) rc.getInt(LABEL, name, KEY_DEFAULT_VALUE, 0);
@@ -1367,7 +1365,7 @@
String name = e.getKey();
LabelType label = e.getValue();
toUnset.remove(name);
- rc.setString(LABEL, name, KEY_FUNCTION, label.getFunctionName());
+ rc.setString(LABEL, name, KEY_FUNCTION, label.getFunction().getFunctionName());
rc.setInt(LABEL, name, KEY_DEFAULT_VALUE, label.getDefaultValue());
setBooleanConfigKey(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
index 152d398..9a362d4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
@@ -45,7 +45,6 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.SubmoduleException;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -330,7 +329,7 @@
}
private void setApproval(ChangeContext ctx, IdentifiedUser user)
- throws OrmException, IOException, PermissionBackendException {
+ throws OrmException, IOException {
Change.Id id = ctx.getChange().getId();
List<SubmitRecord> records = args.commitStatus.getSubmitRecords(id);
PatchSet.Id oldPsId = toMerge.getPatchsetId();
@@ -352,7 +351,7 @@
}
private LabelNormalizer.Result approve(ChangeContext ctx, ChangeUpdate update)
- throws OrmException, IOException, PermissionBackendException {
+ throws OrmException, IOException {
PatchSet.Id psId = update.getPatchSetId();
Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
for (PatchSetApproval psa :
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 6db9357..c87e8e4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -429,65 +429,5 @@
.map((v) -> new LabelPermission.WithValue(label, v))
.collect(toSet());
}
-
- /**
- * Squash a label value to the nearest allowed value.
- *
- * <p>For multi-valued labels like Code-Review with values -2..+2 a user may try to use +2, but
- * only have permission for the -1..+1 range. The caller should have already tried:
- *
- * <pre>
- * check(new LabelPermission.WithValue("Code-Review", 2));
- * </pre>
- *
- * and caught {@link AuthException}. {@code squashThenCheck} will use {@link #test(LabelType)}
- * to determine potential values of Code-Review the user can use, and select the nearest value
- * along the same sign, e.g. -1 for -2 and +1 for +2.
- *
- * @param label definition of the label to test values of.
- * @param val previously denied value the user attempted.
- * @return nearest allowed value, or {@code 0} if no value was allowed.
- * @throws PermissionBackendException backend cannot run test or check.
- */
- public short squashThenCheck(LabelType label, short val) throws PermissionBackendException {
- short s = squashByTest(label, val);
- if (s == 0 || s == val) {
- return 0;
- }
- try {
- check(new LabelPermission.WithValue(label, s));
- return s;
- } catch (AuthException e) {
- return 0;
- }
- }
-
- /**
- * Squash a label value to the nearest allowed value using only test methods.
- *
- * <p>Tests all possible values and selects the closet available to {@code val} while matching
- * the sign of {@code val}. Unlike {@code #squashThenCheck(LabelType, short)} this method only
- * uses {@code test} methods and should not be used in contexts like a review handler without
- * checking the resulting score.
- *
- * @param label definition of the label to test values of.
- * @param val previously denied value the user attempted.
- * @return nearest likely allowed value, or {@code 0} if no value was identified.
- * @throws PermissionBackendException backend cannot run test.
- */
- public short squashByTest(LabelType label, short val) throws PermissionBackendException {
- return nearest(test(label), val);
- }
-
- private static short nearest(Iterable<LabelPermission.WithValue> possible, short wanted) {
- short s = 0;
- for (LabelPermission.WithValue v : possible) {
- if ((wanted < 0 && v.value() < 0 && wanted <= v.value() && v.value() < s)
- || (wanted > 0 && v.value() > 0 && wanted >= v.value() && v.value() > s)) {
- s = v.value();
- }
- }
- return s;
- }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 91433ee..63dd9a9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -208,7 +209,7 @@
.byLabel(ap.getLabel());
if (type != null
&& ap.getValue() == 1
- && type.getFunctionName().equalsIgnoreCase("PatchSetLock")) {
+ && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
return true;
}
}
diff --git a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
index 9bfcc61..2c76999 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
@@ -78,7 +78,7 @@
return new StructureTerm(
symLabelType,
SymbolTerm.intern(type.getName()),
- SymbolTerm.intern(type.getFunctionName()),
+ SymbolTerm.intern(type.getFunction().getFunctionName()),
min != null ? new IntegerTerm(min.getValue()) : NONE,
max != null ? new IntegerTerm(max.getValue()) : NONE);
}
diff --git a/gerrit-server/src/main/prolog/gerrit_common.pl b/gerrit-server/src/main/prolog/gerrit_common.pl
index 4671e0d..8fd0657 100644
--- a/gerrit-server/src/main/prolog/gerrit_common.pl
+++ b/gerrit-server/src/main/prolog/gerrit_common.pl
@@ -279,12 +279,12 @@
!,
max_with_block(Label, Min, Max, S).
max_with_block(Label, Min, Max, reject(Who)) :-
- check_label_range_permission(Label, Min, ok(Who)),
+ commit_label(label(Label, Min), Who),
!
.
max_with_block(Label, Min, Max, ok(Who)) :-
- \+ check_label_range_permission(Label, Min, ok(_)),
- check_label_range_permission(Label, Max, ok(Who)),
+ \+ commit_label(label(Label, Min), _),
+ commit_label(label(Label, Max), Who),
!
.
max_with_block(Label, Min, Max, need(Max)) :-
@@ -306,7 +306,7 @@
%%
any_with_block(Label, Min, reject(Who)) :-
Min < 0,
- check_label_range_permission(Label, Min, ok(Who)),
+ commit_label(label(Label, Min), Who),
!
.
any_with_block(Label, Min, may(_)).
@@ -321,7 +321,7 @@
!,
max_no_block(Label, Max, S).
max_no_block(Label, Max, ok(Who)) :-
- check_label_range_permission(Label, Max, ok(Who)),
+ commit_label(label(Label, Max), Who),
!
.
max_no_block(Label, Max, need(Max)) :-
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java
index 75b00fd..5453fad 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java
@@ -150,7 +150,7 @@
}
@Test
- public void normalizeByPermission() throws Exception {
+ public void noNormalizeByPermission() throws Exception {
ProjectConfig pc = loadAllProjects();
allow(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*");
allow(pc, forLabel("Verified"), -1, 1, REGISTERED_USERS, "refs/heads/*");
@@ -158,8 +158,7 @@
PatchSetApproval cr = psa(userId, "Code-Review", 2);
PatchSetApproval v = psa(userId, "Verified", 1);
- assertEquals(
- Result.create(list(v), list(copy(cr, 1)), list()), norm.normalize(notes, list(cr, v)));
+ assertEquals(Result.create(list(cr, v), list(), list()), norm.normalize(notes, list(cr, v)));
}
@Test
@@ -177,10 +176,10 @@
}
@Test
- public void emptyPermissionRangeOmitsResult() throws Exception {
+ public void emptyPermissionRangeKeepsResult() throws Exception {
PatchSetApproval cr = psa(userId, "Code-Review", 1);
PatchSetApproval v = psa(userId, "Verified", 1);
- assertEquals(Result.create(list(), list(), list(cr, v)), norm.normalize(notes, list(cr, v)));
+ assertEquals(Result.create(list(cr, v), list(), list()), norm.normalize(notes, list(cr, v)));
}
@Test
@@ -191,7 +190,7 @@
PatchSetApproval cr = psa(userId, "Code-Review", 0);
PatchSetApproval v = psa(userId, "Verified", 0);
- assertEquals(Result.create(list(cr), list(), list(v)), norm.normalize(notes, list(cr, v)));
+ assertEquals(Result.create(list(cr, v), list(), list()), norm.normalize(notes, list(cr, v)));
}
private ProjectConfig loadAllProjects() throws Exception {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
index 5a72d5c..6604641 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
@@ -17,6 +17,7 @@
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
@@ -47,7 +48,7 @@
public static final LabelType patchSetLock() {
LabelType label =
category("Patch-Set-Lock", value(1, "Patch Set Locked"), value(0, "Patch Set Unlocked"));
- label.setFunctionName("PatchSetLock");
+ label.setFunction(LabelFunction.PATCH_SET_LOCK);
return label;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
index 7eda3cc..3cd1696 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue;
@@ -105,7 +106,7 @@
assertThat(codeReview).isNotNull();
assertThat(codeReview.getName()).isEqualTo("Code-Review");
assertThat(codeReview.getDefaultValue()).isEqualTo(0);
- assertThat(codeReview.getFunctionName()).isEqualTo("MaxWithBlock");
+ assertThat(codeReview.getFunction()).isEqualTo(LabelFunction.MAX_WITH_BLOCK);
assertThat(codeReview.isCopyMinScore()).isTrue();
assertValueRange(codeReview, 2, 1, 0, -1, -2);
}
diff --git a/gerrit-server/src/test/resources/com/google/gerrit/rules/gerrit_common_test.pl b/gerrit-server/src/test/resources/com/google/gerrit/rules/gerrit_common_test.pl
index c993394..a7df2b9 100644
--- a/gerrit-server/src/test/resources/com/google/gerrit/rules/gerrit_common_test.pl
+++ b/gerrit-server/src/test/resources/com/google/gerrit/rules/gerrit_common_test.pl
@@ -65,7 +65,7 @@
test(default_submit_fails) :-
findall(P, default_submit(P), All),
All = [submit(C, V)],
- C = label('Code-Review', ok(test_user(alice))),
+ C = label('Code-Review', ok(_)),
V = label('Verified', need(1)).
@@ -84,7 +84,7 @@
test(can_submit_not_ready) :-
can_submit(gerrit:default_submit, S),
S = not_ready(submit(C, V)),
- C = label('Code-Review', ok(test_user(alice))),
+ C = label('Code-Review', ok(_)),
V = label('Verified', need(1)).
test(can_submit_only_verified_not_ready) :-
@@ -99,7 +99,7 @@
can_submit(gerrit:default_submit, R),
filter_submit_results(filter_out_v, [R], S),
S = [ok(submit(C))],
- C = label('Code-Review', ok(test_user(alice))).
+ C = label('Code-Review', ok(_)).
test(filter_submit_add_code_review) :-
set_commit_labels([
@@ -119,7 +119,7 @@
can_submit(gerrit:default_submit, R),
arg(1, R, S),
find_label(S, 'Code-Review', L),
- L = label('Code-Review', ok(test_user(alice))).
+ L = label('Code-Review', ok(_)).
test(find_default_verified) :-
can_submit(gerrit:default_submit, R),
@@ -133,7 +133,7 @@
test(remove_default_code_review) :-
can_submit(gerrit:default_submit, R),
arg(1, R, S),
- C = label('Code-Review', ok(test_user(alice))),
+ C = label('Code-Review', ok(_)),
remove_label(S, C, Out),
Out = submit(V),
V = label('Verified', need(1)).
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BanCommitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BanCommitCommand.java
index bdd549c..d514e2c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BanCommitCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BanCommitCommand.java
@@ -14,7 +14,7 @@
package com.google.gerrit.sshd.commands;
-import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE;
+import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
import com.google.common.base.Joiner;
import com.google.common.collect.Lists;
@@ -35,7 +35,7 @@
@CommandMetaData(
name = "ban-commit",
description = "Ban a commit from a project's repository",
- runsAt = MASTER_OR_SLAVE
+ runsAt = MASTER
)
public class BanCommitCommand extends SshCommand {
@Option(
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
index 108252e..ba99155 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
@@ -95,7 +95,6 @@
command("git-receive-pack").to(NotSupportedInSlaveModeFailureCommand.class);
command("gerrit-receive-pack").to(NotSupportedInSlaveModeFailureCommand.class);
command(git, "receive-pack").to(NotSupportedInSlaveModeFailureCommand.class);
- command(gerrit, "test-submit").to(NotSupportedInSlaveModeFailureCommand.class);
} else {
if (sshEnabled()) {
command("git-receive-pack").to(Commands.key(git, "receive-pack"));
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index 466e8f0..16cc49a 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -22,12 +22,12 @@
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.accounts.EmailInput;
+import com.google.gerrit.extensions.api.accounts.SshKeyInput;
import com.google.gerrit.extensions.common.EmailInfo;
import com.google.gerrit.extensions.common.HttpPasswordInput;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.NameInput;
import com.google.gerrit.extensions.common.SshKeyInfo;
-import com.google.gerrit.extensions.common.SshKeyInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
diff --git a/lib/js/bower_archives.bzl b/lib/js/bower_archives.bzl
index fef18aa..d3f7483 100644
--- a/lib/js/bower_archives.bzl
+++ b/lib/js/bower_archives.bzl
@@ -39,7 +39,7 @@
sha1 = "f58358ee652c67e6e721364ba50fb77a2ece1465")
bower_archive(
name = "iron-behaviors",
- package = "polymerelements/iron-behaviors",
+ package = "PolymerElements/iron-behaviors",
version = "1.0.18",
sha1 = "e231a1a02b090f5183db917639fdb96cdd0dca18")
bower_archive(
@@ -54,7 +54,7 @@
sha1 = "01c485fbf898307029bbb72ac7e132db1570a842")
bower_archive(
name = "iron-flex-layout",
- package = "polymerelements/iron-flex-layout",
+ package = "PolymerElements/iron-flex-layout",
version = "1.3.7",
sha1 = "4d4cf3232cf750a17a7df0a37476117f831ac633")
bower_archive(
@@ -139,6 +139,6 @@
sha1 = "2ba5548d36188fe54555eaad0a576de4b027661e")
bower_archive(
name = "webcomponentsjs",
- package = "webcomponentsjs",
+ package = "webcomponents/webcomponentsjs",
version = "0.7.24",
sha1 = "559227f8ee9db9bfbd81989f24510cc0c1bfc65c")
diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
index 20568e6..b851964 100644
--- a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
@@ -25,6 +25,7 @@
columnNames: {
type: Array,
value: [
+ 'Star',
'Subject',
'Status',
'Owner',
diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
index c265db87c..cabadf3 100644
--- a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
@@ -58,6 +58,7 @@
test('getComplementColumns', () => {
let columns = [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -70,6 +71,7 @@
assert.deepEqual(element.getComplementColumns(columns), []);
columns = [
+ 'Star',
'Subject',
'Status',
'Assignee',
@@ -84,6 +86,7 @@
test('isColumnHidden', () => {
const columnToCheck = 'Project';
let columnsToDisplay = [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -96,6 +99,7 @@
assert.isFalse(element.isColumnHidden(columnToCheck, columnsToDisplay));
columnsToDisplay = [
+ 'Star',
'Subject',
'Status',
'Owner',
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index a2c7853..b3598ad 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -109,7 +109,8 @@
<td class="cell keyboard">
<span class="positionIndicator">▶</span>
</td>
- <td class="cell star" hidden$="[[!showStar]]" hidden>
+ <td class="cell star"
+ hidden$="[[_computeHideStar(loggedIn, visibleChangeTableColumns)]]">
<gr-change-star change="{{change}}"></gr-change-star>
</td>
<td class="cell number" hidden$="[[!showNumber]]" hidden>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
index 79f06fe..67d01a6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -29,11 +29,8 @@
type: String,
computed: '_computeChangeURL(change)',
},
- showStar: {
- type: Boolean,
- value: false,
- },
showNumber: Boolean,
+ loggedIn: Boolean,
},
behaviors: [
@@ -115,5 +112,10 @@
if (!change.topic) { return ''; }
return Gerrit.Nav.getUrlForTopic(change.topic);
},
+
+ _computeHideStar(loggedIn, visibleChangeTableColumns) {
+ return !loggedIn ||
+ this.isColumnHidden('Star', visibleChangeTableColumns) ? true : false;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
index 243a8ab..7f1e23c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
@@ -116,6 +116,7 @@
test('no hidden columns', () => {
element.visibleChangeTableColumns = [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -136,6 +137,7 @@
test('no hidden columns', () => {
element.visibleChangeTableColumns = [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -156,6 +158,7 @@
test('project column hidden', () => {
element.visibleChangeTableColumns = [
+ 'Star',
'Subject',
'Status',
'Owner',
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
index d79dc69..99bc6d2 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
@@ -64,8 +64,7 @@
class$="[[_computeUserHeaderClass(_userId)]]"></gr-user-header>
<gr-change-list
changes="{{_changes}}"
- selected-index="{{viewState.selectedChangeIndex}}"
- show-star="[[loggedIn]]"></gr-change-list>
+ selected-index="{{viewState.selectedChangeIndex}}"></gr-change-list>
<nav>
<a id="prevArrow"
href$="[[_computeNavLink(_query, _offset, -1, _changesPerPage)]]"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
index 152ef3d..b1b0dc5 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
@@ -58,10 +58,11 @@
<table id="changeList">
<tr class="headerRow">
<th class="topHeader keyboard"></th>
- <th class="topHeader star" hidden$="[[!showStar]]" hidden></th>
+ <th class="topHeader star"
+ hidden$="[[_computeHideStar(_loggedIn, visibleChangeTableColumns)]]"></th>
<th class="topHeader number" hidden$="[[!showNumber]]" hidden>#</th>
<template is="dom-repeat" items="[[changeTableColumns]]" as="item">
- <th class$="[[_lowerCase(item)]] topHeader"
+ <th class$="topHeader [[_lowerCase(item)]]"
hidden$="[[isColumnHidden(item, visibleChangeTableColumns)]]">
[[item]]
</th>
@@ -100,8 +101,8 @@
change="[[change]]"
visible-change-table-columns="[[visibleChangeTableColumns]]"
show-number="[[showNumber]]"
- show-star="[[showStar]]"
- label-names="[[labelNames]]"></gr-change-list-item>
+ label-names="[[labelNames]]"
+ logged-in="[[_loggedIn]]"></gr-change-list-item>
</template>
</template>
</table>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 572695b..ca1f3a9 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -75,10 +75,6 @@
notify: true,
},
showNumber: Boolean, // No default value to prevent flickering.
- showStar: {
- type: Boolean,
- value: false,
- },
showReviewedState: {
type: Boolean,
value: false,
@@ -89,6 +85,10 @@
},
changeTableColumns: Array,
visibleChangeTableColumns: Array,
+ _loggedIn: {
+ type: Boolean,
+ value: false,
+ },
},
behaviors: [
@@ -137,7 +137,11 @@
_loadPreferences() {
return this._getLoggedIn().then(loggedIn => {
- this.changeTableColumns = this.columnNames;
+ this._loggedIn = loggedIn;
+
+ this.changeTableColumns = this.columnNames.filter(column => {
+ return column !== 'Star';
+ });
if (!loggedIn) {
this.showNumber = false;
@@ -314,6 +318,11 @@
return null;
},
+ _computeHideStar(loggedIn, visibleChangeTableColumns) {
+ return !loggedIn ||
+ this.isColumnHidden('Star', visibleChangeTableColumns) ? true : false;
+ },
+
_getListItems() {
return Polymer.dom(this.root).querySelectorAll('gr-change-list-item');
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index bded5f6..cf0e58f 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -301,6 +301,7 @@
legacycid_in_change_table: true,
time_format: 'HHMM_12',
change_table: [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -332,6 +333,7 @@
legacycid_in_change_table: true,
time_format: 'HHMM_12',
change_table: [
+ 'Star',
'Subject',
'Status',
'Owner',
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
index c8f606b..6c31a5b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
@@ -50,7 +50,6 @@
user-id="[[params.user]]"
class$="[[_computeUserHeaderClass(params.user)]]"></gr-user-header>
<gr-change-list
- show-star
show-reviewed-state
account="[[account]]"
selected-index="{{viewState.selectedChangeIndex}}"
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
index e1753ba..74fb370 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
@@ -42,6 +42,7 @@
sandbox = sinon.sandbox.create();
columns = [
+ 'Star',
'Subject',
'Status',
'Owner',
@@ -147,6 +148,7 @@
let checked = false;
assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
[
+ 'Star',
'Status',
'Owner',
'Assignee',
@@ -158,6 +160,7 @@
checked = true;
assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
[
+ 'Star',
'Subject',
'Status',
'Owner',
diff --git a/polygerrit-ui/app/template_test_srcs/template_test.js b/polygerrit-ui/app/template_test_srcs/template_test.js
index 88666df..401f0aa 100644
--- a/polygerrit-ui/app/template_test_srcs/template_test.js
+++ b/polygerrit-ui/app/template_test_srcs/template_test.js
@@ -22,6 +22,7 @@
'GrDiffGroup',
'GrDiffLine',
'GrDomHooks',
+ 'GrEditConstants',
'GrEtagDecorator',
'GrFileListConstants',
'GrGapiAuth',
