Allow configuration of a default value for a label.
This change allows a project admin or owner to define a default value
(or score) for a label. The default value is set in the project configurations
file with a 'defaultValue' key. The defaultValue must be within the range of
valid label values. It is an optional label setting, if not defined the
defaultValue for the label will be 0. When a defaultValue is defined, that
value will get set in the Reply dialog by default. A defaultValue can be set
to a score that is outside of the permissible range for a user. In that case
the score that will get set in the Reply box will be the next closes score
to the defaultValue.
feature: Issue 2041
Change-Id: I12dece29b043e09eaab62cdcf56146a1380041bc
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index fc25f22..0fe12ca 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -169,6 +169,20 @@
optional leading `+`.
+[[label_defaultValue]]
+=== `label.Label-Name.defaultValue`
+
+The default value (or score) for the label. The defaultValue must be
+within the range of valid label values. It is an optional label setting,
+if not defined the defaultValue for the label will be 0. When a
+defaultValue is defined, that value will get set in the Reply dialog
+by default.
+
+A defaultValue can be set to a score that is outside of the permissible
+range for a user. In that case the score that will get set in the Reply
+box will be either the lowest or highest score in the permissible range.
+
+
[[label_abbreviation]]
=== `label.Label-Name.abbreviation`
@@ -297,6 +311,32 @@
copyright` will block submit, while `+1 Copyright clear` is required to
enable submit.
+=== Default Value Example
+
+This example attempts to describe how a label default value works with the
+user permissions. Assume the configuration below.
+
+====
+ [access "refs/heads/*"]
+ label-Snarky-Review = -3..+3 group Administrators
+ label-Snarky-Review = -2..+2 group Project Owners
+ label-Snarky-Review = -1..+1 group Registered Users
+ [label "Snarky-Review"]
+ value = -3 Ohh, hell no!
+ value = -2 Hmm, I'm not a fan
+ value = -1 I'm not sure I like this
+ value = 0 No score
+ value = +1 I like, but need another to like it as well
+ value = +2 Hmm, this is pretty nice
+ value = +3 Ohh, hell yes!
+ defaultValue = -3
+====
+
+Upon clicking the Reply button:
+* Administrators have all scores (-3..+3) available, -3 is set as the default.
+* Project Owners have limited scores (-2..+2) available, -2 is set as the default.
+* Registered Users have limited scores (-1..+1) available, -1 is set as the default.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 8f6b8a4..c172781 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3093,6 +3093,8 @@
|`value` |optional|The voting value of the user who
recommended/disliked this label on the change if it is not
"`+1`"/"`-1`".
+|`default_value`|optional|The default voting value for the label.
+This value may be outside the range specified in permitted_labels.
|===========================
==== Fields set by `DETAILED_LABELS`
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
index f3db988..7240036 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
@@ -69,6 +69,7 @@
codeReview.setCopyMaxScore(false);
codeReview.setCopyAllScoresOnTrivialRebase(false);
codeReview.setCopyAllScoresIfNoCodeChange(false);
+ codeReview.setDefaultValue((short)-1);
saveProjectConfig(cfg);
}
@@ -264,6 +265,7 @@
// through JSON instead of querying the DB directly.
ChangeInfo c = get(r.getChangeId());
LabelInfo cr = c.labels.get("Code-Review");
+ assertEquals(-1, (int) cr.defaultValue);
assertEquals(1, cr.all.size());
assertEquals("Administrator", cr.all.get(0).name);
assertEquals(expected, cr.all.get(0).value.intValue());
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 d4f5b7b..565b2dc 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
@@ -112,6 +112,7 @@
protected boolean copyMaxScore;
protected boolean copyAllScoresOnTrivialRebase;
protected boolean copyAllScoresIfNoCodeChange;
+ protected short defaultValue;
protected List<LabelValue> values;
protected short maxNegative;
@@ -129,6 +130,7 @@
this.name = checkName(name);
canOverride = true;
values = sortValues(valueList);
+ defaultValue = 0;
abbreviation = defaultAbbreviation(name);
functionName = "MaxWithBlock";
@@ -204,6 +206,14 @@
return v.getValue() > 0 ? v : null;
}
+ public short getDefaultValue() {
+ return defaultValue;
+ }
+
+ public void setDefaultValue(short defaultValue) {
+ this.defaultValue = defaultValue;
+ }
+
public boolean isCopyMinScore() {
return copyMinScore;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/LabelInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/LabelInfo.java
index fd6008f..1e4edcd 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/LabelInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/LabelInfo.java
@@ -25,6 +25,7 @@
public List<ApprovalInfo> all;
public Map<String, String> values;
public Short value;
+ public Short defaultValue;
public Boolean optional;
public Boolean blocking;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
index dc6d710..4e77041 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
@@ -322,10 +322,23 @@
}
}
+ private Short normalizeDefaultValue(Short defaultValue, Set<Short> permittedValues) {
+ Short pmin = Collections.min(permittedValues);
+ Short pmax = Collections.max(permittedValues);
+ Short dv = defaultValue;
+ if (dv > pmax) {
+ dv = pmax;
+ } else if (dv < pmin) {
+ dv = pmin;
+ }
+ return dv;
+ }
+
private void renderRadio(int row,
List<Short> columns,
LabelAndValues lv) {
String id = lv.info.name();
+ Short dv = normalizeDefaultValue(lv.info.defaultValue(), lv.permitted);
labelHelpColumn = 1 + columns.size();
labelsTable.setText(row, 0, id);
@@ -345,9 +358,10 @@
if (lv.permitted.contains(v)) {
String text = lv.info.value_text(LabelValue.formatValue(v));
LabelRadioButton b = new LabelRadioButton(group, text, v);
- if ((self != null && v == self.value()) || (self == null && v == 0)) {
+ if ((self != null && v == self.value()) || (self == null && v.equals(dv))) {
b.setValue(true);
group.select(b);
+ in.label(group.label, v);
labelsTable.setText(row, labelHelpColumn, b.text);
}
group.buttons.add(b);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
index a627725..ed6f046 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
@@ -161,6 +161,7 @@
public final native boolean optional() /*-{ return this.optional ? true : false; }-*/;
public final native boolean blocking() /*-{ return this.blocking ? true : false; }-*/;
+ public final native short defaultValue() /*-{ return this.default_value; }-*/;
final native short _value()
/*-{
if (this.value) return this.value;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java
index 9d813fc..b82328c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java
@@ -116,6 +116,7 @@
lo.recommended = fromAcountInfo(li.recommended);
lo.disliked = fromAcountInfo(li.disliked);
lo.value = li.value;
+ lo.defaultValue = li.defaultValue;
lo.optional = li.optional;
lo.blocking = li.blocking;
lo.values = li.values;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 5955fde..5321acd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -631,6 +631,7 @@
}
private void setLabelValues(LabelType type, LabelInfo label) {
+ label.defaultValue = type.getDefaultValue();
label.values = Maps.newLinkedHashMap();
for (LabelValue v : type.getValues()) {
label.values.put(v.formatValue(), v.getText());
@@ -1007,6 +1008,7 @@
public Map<String, String> values;
public Short value;
+ public Short defaultValue;
public Boolean optional;
public Boolean blocking;
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 4971f84..f1eeaed 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
@@ -129,6 +129,7 @@
private static final String LABEL = "label";
private static final String KEY_ABBREVIATION = "abbreviation";
private static final String KEY_FUNCTION = "function";
+ private static final String KEY_DEFAULT_VALUE = "defaultValue";
private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
private static final String KEY_COPY_MAX_SCORE = "copyMaxScore";
private static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
@@ -665,6 +666,15 @@
KEY_FUNCTION, name, Joiner.on(", ").join(LABEL_FUNCTIONS))));
label.setFunctionName(null);
}
+
+ short dv = (short) rc.getInt(LABEL, name, KEY_DEFAULT_VALUE, 0);
+ if (isInRange(dv, values)) {
+ label.setDefaultValue(dv);
+ } else {
+ error(new ValidationError(PROJECT_CONFIG, String.format(
+ "Invalid %s \"%s\" for label \"%s\"",
+ KEY_DEFAULT_VALUE, dv, name)));
+ }
label.setCopyMinScore(
rc.getBoolean(LABEL, name, KEY_COPY_MIN_SCORE, false));
label.setCopyMaxScore(
@@ -680,6 +690,15 @@
}
}
+ private boolean isInRange(short value, List<LabelValue> labelValues) {
+ for (LabelValue lv : labelValues) {
+ if (lv.getValue() == value) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private List<String> getStringListOrNull(Config rc, String section,
String subSection, String name) {
String[] ac = rc.getStringList(section, subSection, name);
@@ -1018,6 +1037,8 @@
} else {
rc.unset(LABEL, name, KEY_ABBREVIATION);
}
+
+ rc.setInt(LABEL, name, KEY_DEFAULT_VALUE, label.getDefaultValue());
if (label.isCopyMinScore()) {
rc.setBoolean(LABEL, name, KEY_COPY_MIN_SCORE, true);
} else {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
index 1a9f74b..7b3ede2 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
@@ -21,9 +21,11 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import com.google.common.collect.Iterables;
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.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -47,6 +49,7 @@
import java.io.IOException;
import java.util.Collections;
+import java.util.Map;
public class ProjectConfigTest extends LocalDiskRepositoryTestCase {
private final GroupReference developers = new GroupReference(
@@ -115,6 +118,60 @@
}
@Test
+ public void testReadConfigLabelDefaultValue() throws Exception {
+ RevCommit rev = util.commit(util.tree( //
+ util.file("groups", util.blob(group(developers))), //
+ util.file("project.config", util.blob(""//
+ + "[label \"CustomLabel\"]\n" //
+ + " value = -1 Negative\n" //
+ + " value = 0 No Score\n" //
+ + " value = 1 Positive\n")) //
+ ));
+
+ ProjectConfig cfg = read(rev);
+ Map<String, LabelType> labels = cfg.getLabelSections();
+ Short dv = labels.entrySet().iterator().next().getValue().getDefaultValue();
+ assertEquals(0, (int) dv);
+ }
+
+ @Test
+ public void testReadConfigLabelDefaultValueInRange() throws Exception {
+ RevCommit rev = util.commit(util.tree( //
+ util.file("groups", util.blob(group(developers))), //
+ util.file("project.config", util.blob(""//
+ + "[label \"CustomLabel\"]\n" //
+ + " value = -1 Negative\n" //
+ + " value = 0 No Score\n" //
+ + " value = 1 Positive\n" //
+ + " defaultValue = -1\n")) //
+ ));
+
+ ProjectConfig cfg = read(rev);
+ Map<String, LabelType> labels = cfg.getLabelSections();
+ Short dv = labels.entrySet().iterator().next().getValue().getDefaultValue();
+ assertEquals(-1, (int) dv);
+ }
+
+ @Test
+ public void testReadConfigLabelDefaultValueNotInRange() throws Exception {
+ RevCommit rev = util.commit(util.tree( //
+ util.file("groups", util.blob(group(developers))), //
+ util.file("project.config", util.blob(""//
+ + "[label \"CustomLabel\"]\n" //
+ + " value = -1 Negative\n" //
+ + " value = 0 No Score\n" //
+ + " value = 1 Positive\n" //
+ + " defaultValue = -2\n")) //
+ ));
+
+ ProjectConfig cfg = read(rev);
+ assertEquals(1, cfg.getValidationErrors().size());
+ assertEquals("project.config: Invalid defaultValue \"-2\" "
+ + "for label \"CustomLabel\"",
+ Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage());
+ }
+
+ @Test
public void testEditConfig() throws Exception {
RevCommit rev = util.commit(util.tree( //
util.file("groups", util.blob(group(developers))), //
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 4756390..15a66ab 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
@@ -129,6 +129,7 @@
assertNotNull(codeReview);
assertEquals("Code-Review", codeReview.getName());
assertEquals("CR", codeReview.getAbbreviation());
+ assertEquals(0, codeReview.getDefaultValue());
assertEquals("MaxWithBlock", codeReview.getFunctionName());
assertTrue(codeReview.isCopyMinScore());
assertValueRange(codeReview, 2, 1, 0, -1, -2);
