Merge "Label config: Allow to configure individual votes as sticky"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 193a96f..a3b9d0b 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -344,6 +344,13 @@
Defaults to true.
+[[label_copyValue]]
+=== `label.Label-Name.copyValue`
+
+Value that should be copied forward when a new patch set is uploaded.
+This can be used to enable sticky votes. Can be specified multiple
+times. By default not set.
+
[[label_canOverride]]
=== `label.Label-Name.canOverride`
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 2f788be..a4e27b3 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3916,6 +3916,8 @@
|`copy_all_scores_on_merge_first_parent_update`|`false` if not set|
Whether link:config-labels.html#label_copyAllScoresOnMergeFirstParentUpdate[
copyAllScoresOnMergeFirstParentUpdate] is set on the label.
+|`copy_values` |optional|
+List of values that should be copied forward when a new patch set is uploaded.
|`allow_post_submit`|`false` if not set|
Whether link:config-labels.html#label_allowPostSubmit[allowPostSubmit] is set
on the label.
@@ -3981,6 +3983,8 @@
|`copy_all_scores_on_merge_first_parent_update`|optional|
Whether link:config-labels.html#label_copyAllScoresOnMergeFirstParentUpdate[
copyAllScoresOnMergeFirstParentUpdate] is set on the label.
+|`copy_values` |optional|
+List of values that should be copied forward when a new patch set is uploaded.
|`allow_post_submit`|optional|
Whether link:config-labels.html#label_allowPostSubmit[allowPostSubmit] is set
on the label.
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 14b8310..964cf67 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -14,14 +14,17 @@
package com.google.gerrit.common.data;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toList;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSetApproval;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -37,6 +40,7 @@
public static final boolean DEF_COPY_ANY_SCORE = false;
public static final boolean DEF_COPY_MAX_SCORE = false;
public static final boolean DEF_COPY_MIN_SCORE = false;
+ public static final ImmutableList<Short> DEF_COPY_VALUES = ImmutableList.of();
public static final boolean DEF_IGNORE_SELF_APPROVAL = false;
public static LabelType withDefaultValues(String name) {
@@ -104,6 +108,7 @@
protected boolean copyAllScoresOnTrivialRebase;
protected boolean copyAllScoresIfNoCodeChange;
protected boolean copyAllScoresIfNoChange;
+ protected ImmutableList<Short> copyValues;
protected boolean allowPostSubmit;
protected boolean ignoreSelfApproval;
protected short defaultValue;
@@ -144,6 +149,7 @@
setCopyAnyScore(DEF_COPY_ANY_SCORE);
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
+ setCopyValues(DEF_COPY_VALUES);
setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
setIgnoreSelfApproval(DEF_IGNORE_SELF_APPROVAL);
@@ -298,6 +304,14 @@
this.copyAllScoresIfNoChange = copyAllScoresIfNoChange;
}
+ public ImmutableList<Short> getCopyValues() {
+ return copyValues;
+ }
+
+ public void setCopyValues(Collection<Short> copyValues) {
+ this.copyValues = copyValues.stream().sorted().collect(toImmutableList());
+ }
+
public boolean isMaxNegative(PatchSetApproval ca) {
return maxNegative == ca.value();
}
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
index 64c3997..f552566 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
@@ -32,6 +32,7 @@
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
+ public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
index 0523f61..23d5df1 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
@@ -31,6 +31,7 @@
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
+ public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/ApprovalInference.java
index 44b0529..a4c9f2a 100644
--- a/java/com/google/gerrit/server/ApprovalInference.java
+++ b/java/com/google/gerrit/server/ApprovalInference.java
@@ -101,6 +101,8 @@
return true;
} else if (type.isCopyAnyScore()) {
return true;
+ } else if (type.getCopyValues().contains(psa.value())) {
+ return true;
}
switch (kind) {
case MERGE_FIRST_PARENT_UPDATE:
diff --git a/java/com/google/gerrit/server/project/LabelDefinitionJson.java b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
index 2ecd8c2..0452d0b 100644
--- a/java/com/google/gerrit/server/project/LabelDefinitionJson.java
+++ b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
@@ -40,6 +40,7 @@
label.copyAllScoresOnTrivialRebase = toBoolean(labelType.isCopyAllScoresOnTrivialRebase());
label.copyAllScoresOnMergeFirstParentUpdate =
toBoolean(labelType.isCopyAllScoresOnMergeFirstParentUpdate());
+ label.copyValues = labelType.getCopyValues().isEmpty() ? null : labelType.getCopyValues();
label.allowPostSubmit = toBoolean(labelType.allowPostSubmit());
label.ignoreSelfApproval = toBoolean(labelType.ignoreSelfApproval());
return label;
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index fa877af..4ab583d 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -109,6 +109,7 @@
public static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
public static final String KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = "copyAllScoresIfNoCodeChange";
public static final String KEY_COPY_ALL_SCORES_IF_NO_CHANGE = "copyAllScoresIfNoChange";
+ public static final String KEY_COPY_VALUE = "copyValue";
public static final String KEY_VALUE = "value";
public static final String KEY_CAN_OVERRIDE = "canOverride";
public static final String KEY_BRANCH = "branch";
@@ -1014,6 +1015,27 @@
name,
KEY_COPY_ALL_SCORES_IF_NO_CHANGE,
LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE));
+ Set<Short> copyValues = new HashSet<>();
+ for (String value : rc.getStringList(LABEL, name, KEY_COPY_VALUE)) {
+ try {
+ short copyValue = Shorts.checkedCast(PermissionRule.parseInt(value));
+ if (!copyValues.add(copyValue)) {
+ error(
+ new ValidationError(
+ PROJECT_CONFIG,
+ String.format(
+ "Duplicate %s \"%s\" for label \"%s\"", KEY_COPY_VALUE, value, name)));
+ }
+ } catch (IllegalArgumentException notValue) {
+ error(
+ new ValidationError(
+ PROJECT_CONFIG,
+ String.format(
+ "Invalid %s \"%s\" for label \"%s\": %s",
+ KEY_COPY_VALUE, value, name, notValue.getMessage())));
+ }
+ }
+ label.setCopyValues(copyValues);
label.setCanOverride(
rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, LabelType.DEF_CAN_OVERRIDE));
label.setRefPatterns(getStringListOrNull(rc, LABEL, name, KEY_BRANCH));
@@ -1492,6 +1514,11 @@
KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE,
label.isCopyAllScoresOnMergeFirstParentUpdate(),
LabelType.DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
+ rc.setStringList(
+ LABEL,
+ name,
+ KEY_COPY_VALUE,
+ label.getCopyValues().stream().map(LabelValue::formatValue).collect(toList()));
setBooleanConfigKey(
rc, LABEL, name, KEY_CAN_OVERRIDE, label.canOverride(), LabelType.DEF_CAN_OVERRIDE);
List<String> values = new ArrayList<>(label.getValues().size());
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index 5d51527..a85ad39 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -191,6 +191,10 @@
input.copyAllScoresOnMergeFirstParentUpdate);
}
+ if (input.copyValues != null) {
+ labelType.setCopyValues(input.copyValues);
+ }
+
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index 824b4ed..0a35865 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -205,6 +205,11 @@
dirty = true;
}
+ if (input.copyValues != null) {
+ labelType.setCopyValues(input.copyValues);
+ dirty = true;
+ }
+
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
dirty = true;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 7e69251..923b66f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -185,6 +185,35 @@
}
@Test
+ public void stickyOnCopyValues() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .getLabelSections()
+ .get("Code-Review")
+ .setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ u.save();
+ }
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = createChange(changeKind);
+ vote(admin, changeId, -1, 1);
+ vote(user, changeId, -2, -1);
+ vote(user2, changeId, 1, -1);
+
+ updateChange(changeId, changeKind);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, -1, 0, changeKind);
+ assertVotes(c, user, 0, 0, changeKind);
+ assertVotes(c, user2, 1, 0, changeKind);
+ }
+ }
+
+ @Test
public void stickyOnTrivialRebase() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresOnTrivialRebase(true);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
index 57a1e56..e5587a9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
@@ -244,6 +244,7 @@
assertThat(createdLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(createdLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(createdLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(createdLabel.copyValues).isNull();
assertThat(createdLabel.allowPostSubmit).isTrue();
assertThat(createdLabel.ignoreSelfApproval).isNull();
}
@@ -537,6 +538,17 @@
}
@Test
+ public void createWithCopyValues() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.copyValues = ImmutableList.of((short) -1, (short) 1);
+
+ LabelDefinitionInfo createdLabel =
+ gApi.projects().name(project.get()).label("foo").create(input).get();
+ assertThat(createdLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
+ }
+
+ @Test
public void createWithAllowPostSubmit() throws Exception {
LabelDefinitionInput input = new LabelDefinitionInput();
input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
index 9f98490..940fae5 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
@@ -117,6 +117,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(fooLabel.copyValues).isNull();
assertThat(fooLabel.allowPostSubmit).isNull();
assertThat(fooLabel.ignoreSelfApproval).isNull();
}
@@ -134,6 +135,7 @@
labelType.setCopyAllScoresIfNoCodeChange(true);
labelType.setCopyAllScoresOnTrivialRebase(true);
labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
labelType.setIgnoreSelfApproval(true);
u.getConfig().getLabelSections().put(labelType.getName(), labelType);
u.save();
@@ -148,6 +150,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isTrue();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isTrue();
+ assertThat(fooLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
assertThat(fooLabel.allowPostSubmit).isTrue();
assertThat(fooLabel.ignoreSelfApproval).isTrue();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java b/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
index 7998ecb..65e352b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
@@ -47,6 +47,7 @@
assertThat(codeReviewLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(codeReviewLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(codeReviewLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(codeReviewLabel.copyValues).isNull();
assertThat(codeReviewLabel.allowPostSubmit).isTrue();
assertThat(codeReviewLabel.ignoreSelfApproval).isNull();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
index d2539e5..ef08079 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
@@ -139,6 +139,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(fooLabel.copyValues).isNull();
assertThat(fooLabel.allowPostSubmit).isNull();
assertThat(fooLabel.ignoreSelfApproval).isNull();
}
@@ -156,6 +157,7 @@
labelType.setCopyAllScoresIfNoCodeChange(true);
labelType.setCopyAllScoresOnTrivialRebase(true);
labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
labelType.setIgnoreSelfApproval(true);
u.getConfig().getLabelSections().put(labelType.getName(), labelType);
u.save();
@@ -173,6 +175,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isTrue();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isTrue();
+ assertThat(fooLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
assertThat(fooLabel.allowPostSubmit).isTrue();
assertThat(fooLabel.ignoreSelfApproval).isTrue();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
index 97b795f..b08c72b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
@@ -771,6 +771,44 @@
}
@Test
+ public void setCopyValues() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNull();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyValues = ImmutableList.of((short) -1, (short) 1);
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
+
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues)
+ .containsExactly((short) -1, (short) 1)
+ .inOrder();
+ }
+
+ @Test
+ public void unsetCopyValues() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ LabelType labelType = u.getConfig().getLabelSections().get("foo");
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.save();
+ }
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNotEmpty();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyValues = ImmutableList.of();
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyValues).isNull();
+
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNull();
+ }
+
+ @Test
public void setAllowPostSubmit() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
