opendev/gerrit
Code Issues Proposed changes

Merge "Label config: Allow to configure individual votes as sticky"

Browse Source
This commit is contained in:
Martin Fick
2020-01-31 00:25:28 +00:00
committed by Gerrit Code Review
parent d421f80211 33c13a444e
commit 0879693d46
16 changed files with 152 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/config-labels.txt
View File

@@ -344,6 +344,13 @@ Code-Review labels.
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`

4
Documentation/rest-api-projects.txt
View File

@@ -3916,6 +3916,8 @@ copyAllScoresOnTrivialRebase] is set on the label.
|`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 @@ copyAllScoresOnTrivialRebase] is set on the label.
|`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.

14
java/com/google/gerrit/common/data/LabelType.java
View File

@@ -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 class LabelType {
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 @@ public class LabelType {
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 @@ public class LabelType {
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 @@ public class LabelType {
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();
}

1
java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
View File

@@ -32,6 +32,7 @@ public class LabelDefinitionInfo {
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}

1
java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
View File

@@ -31,6 +31,7 @@ public class LabelDefinitionInput extends InputWithCommitMessage {
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}

2
java/com/google/gerrit/server/ApprovalInference.java
View File

@@ -101,6 +101,8 @@ public class ApprovalInference {
return true;
} else if (type.isCopyAnyScore()) {
return true;
} else if (type.getCopyValues().contains(psa.value())) {
return true;
}
switch (kind) {
case MERGE_FIRST_PARENT_UPDATE:

1
java/com/google/gerrit/server/project/LabelDefinitionJson.java
View File

@@ -40,6 +40,7 @@ public class LabelDefinitionJson {
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;

27
java/com/google/gerrit/server/project/ProjectConfig.java
View File

@@ -109,6 +109,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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());

4
java/com/google/gerrit/server/restapi/project/CreateLabel.java
View File

@@ -191,6 +191,10 @@ public class CreateLabel
input.copyAllScoresOnMergeFirstParentUpdate);
}
if (input.copyValues != null) {
labelType.setCopyValues(input.copyValues);
}
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
}

5
java/com/google/gerrit/server/restapi/project/SetLabel.java
View File

@@ -205,6 +205,11 @@ public class SetLabel implements RestModifyView<LabelResource, LabelDefinitionIn
dirty = true;
}
if (input.copyValues != null) {
labelType.setCopyValues(input.copyValues);
dirty = true;
}
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
dirty = true;

29
javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
View File

@@ -184,6 +184,35 @@ public class StickyApprovalsIT extends AbstractDaemonTest {
}
}
@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)) {

12
javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
View File

@@ -244,6 +244,7 @@ public class CreateLabelIT extends AbstractDaemonTest {
assertThat(createdLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(createdLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(createdLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
assertThat(createdLabel.copyValues).isNull();
assertThat(createdLabel.allowPostSubmit).isTrue();
assertThat(createdLabel.ignoreSelfApproval).isNull();
}
@@ -536,6 +537,17 @@ public class CreateLabelIT extends AbstractDaemonTest {
assertThat(createdLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
}
@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();

3
javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
View File

@@ -117,6 +117,7 @@ public class GetLabelIT extends AbstractDaemonTest {
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 @@ public class GetLabelIT extends AbstractDaemonTest {
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 @@ public class GetLabelIT extends AbstractDaemonTest {
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();
}

1
javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
View File

@@ -47,6 +47,7 @@ public class LabelAssert {
assertThat(codeReviewLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(codeReviewLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(codeReviewLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
assertThat(codeReviewLabel.copyValues).isNull();
assertThat(codeReviewLabel.allowPostSubmit).isTrue();
assertThat(codeReviewLabel.ignoreSelfApproval).isNull();
}

3
javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
View File

@@ -139,6 +139,7 @@ public class ListLabelsIT extends AbstractDaemonTest {
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 @@ public class ListLabelsIT extends AbstractDaemonTest {
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 @@ public class ListLabelsIT extends AbstractDaemonTest {
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();
}

38
javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
View File

@@ -770,6 +770,44 @@ public class SetLabelIT extends AbstractDaemonTest {
.isNull();
}
@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);