Rename copyScores to copyScoreRules in REST API copyScores is not indicative enough, so we rename it to copyScoreRules. copyScores is now deprecated and will be removed in a future commit. For now, the API remains compatible and will use the deprecated field when copyScoreRules is null. It populates both fields on the returned JSON object. Change-Id: I25ef8c1c3f336c78d95f73d8ba548795225a9ea3
diff --git a/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.html b/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.html index 79bc203..5231978 100644 --- a/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.html +++ b/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.html
@@ -107,7 +107,7 @@ <span class="value"> <input id="copyMinScore" type="checkbox" - checked="{{_copyScores.copyMinScore::change}}" + checked="{{_copyScoreRules.copyMinScore::change}}" disabled$="[[readOnly]]"> </span> </section> @@ -126,7 +126,7 @@ <span class="value"> <input id="copyMaxScore" type="checkbox" - checked="{{_copyScores.copyMaxScore::change}}" + checked="{{_copyScoreRules.copyMaxScore::change}}" disabled$="[[readOnly]]"> </span> </section> @@ -146,7 +146,7 @@ <span class="value"> <input id="copyAllScoresOnTrivialRebase" type="checkbox" - checked="{{_copyScores.copyAllScoresOnTrivialRebase::change}}" + checked="{{_copyScoreRules.copyAllScoresOnTrivialRebase::change}}" disabled$="[[readOnly]]"> </span> </section> @@ -166,7 +166,7 @@ <span class="value"> <input id="copyAllScoresIfNoCodeChange" type="checkbox" - checked="{{_copyScores.copyAllScoresIfNoCodeChange::change}}" + checked="{{_copyScoreRules.copyAllScoresIfNoCodeChange::change}}" disabled$="[[readOnly]]"> </span> </section> @@ -187,7 +187,7 @@ <span class="value"> <input id="copyAllScoresIfNoChange" type="checkbox" - checked="{{_copyScores.copyAllScoresIfNoChange::change}}" + checked="{{_copyScoreRules.copyAllScoresIfNoChange::change}}" disabled$="[[readOnly]]"> </span> </section> @@ -207,7 +207,7 @@ <span class="value"> <input id="copyAllScoresOnMergeFirstParentUpdate" type="checkbox" - checked="{{_copyScores.copyAllScoresOnMergeFirstParentUpdate::change}}" + checked="{{_copyScoreRules.copyAllScoresOnMergeFirstParentUpdate::change}}" disabled$="[[readOnly]]"> </span> </section>
diff --git a/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.js b/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.js index 7e11c39..777ff01 100644 --- a/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.js +++ b/gr-simple-submit-rules-label-config/gr-simple-submit-rules-label-config.js
@@ -45,7 +45,7 @@ type: Boolean, value: false }, - _updatingCopyScores: { + _updatingCopyScoreRules: { type: Boolean, value: false }, @@ -53,7 +53,7 @@ type: Object, computed: '_computeLabelConfig(repoConfig.labels, labelName)' }, - _copyScores: { + _copyScoreRules: { type: Object, value: {}, }, @@ -64,8 +64,8 @@ observers: [ '_observeFunctionChange(_labelConfig.function)', '_observeFunctionDescriptorChange(_negativeBlocks, _maxVoteRequired)', - '_observeCopyScoresChange(_labelConfig.copy_scores)', - '_observeCopyScoresChangeInUi(_copyScores.*)', + '_observeCopyScoreRulesChange(_labelConfig.copy_scores)', + '_observeCopyScoreRulesChangeInUi(_copyScoreRules.*)', ], _observeFunctionDescriptorChange(negativeBlocks, maxVoteRequired) { @@ -100,34 +100,34 @@ return labels[this.labelName] || {}; }, - _observeCopyScoresChange() { - if (this._updatingCopyScores) { return; } - this._updatingCopyScores = true; + _observeCopyScoreRulesChange() { + if (this._updatingCopyScoreRules) { return; } + this._updatingCopyScoreRules = true; for (let key of COPY_SCORES) { - this.set(['_copyScores', key], false); + this.set(['_copyScoreRules', key], false); } for (let value of this._labelConfig.copy_scores) { - this.set(['_copyScores', value], true); + this.set(['_copyScoreRules', value], true); } - this._updatingCopyScores = false; + this._updatingCopyScoreRules = false; }, - _observeCopyScoresChangeInUi() { - if (this._updatingCopyScores) { return; } - this._updatingCopyScores = true; + _observeCopyScoreRulesChangeInUi() { + if (this._updatingCopyScoreRules) { return; } + this._updatingCopyScoreRules = true; - let newCopyScores = []; - for (let key in this._copyScores) { - if (this._copyScores[key]) { - newCopyScores.push(key); + let newCopyScoreRules = []; + for (let key in this._copyScoreRules) { + if (this._copyScoreRules[key]) { + newCopyScoreRules.push(key); } } - this.set('_labelConfig.copy_scores', newCopyScores); + this.set('_labelConfig.copy_scores', newCopyScoreRules); - this._updatingCopyScores = false; + this._updatingCopyScoreRules = false; }, }); })();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java index 4b9d6ea..e128833 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java +++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java
@@ -23,14 +23,16 @@ public class LabelDefinition { public String function; public Boolean ignoreSelfApproval; - public Set<String> copyScores; + public Set<String> copyScoreRules; + @Deprecated public Set<String> copyScores; public LabelDefinition() {} - public LabelDefinition(String function, Boolean ignoreSelfApproval, Set<String> copyScores) { + public LabelDefinition(String function, Boolean ignoreSelfApproval, Set<String> copyScoreRules) { this.function = function; this.ignoreSelfApproval = ignoreSelfApproval; - this.copyScores = copyScores; + this.copyScoreRules = copyScoreRules; + this.copyScores = copyScoreRules; } public Optional<LabelFunction> getFunction() { @@ -39,7 +41,7 @@ @Override public int hashCode() { - return Objects.hash(function, ignoreSelfApproval, copyScores); + return Objects.hash(function, ignoreSelfApproval, copyScores, copyScoreRules); } @Override @@ -50,7 +52,8 @@ LabelDefinition other = (LabelDefinition) o; return Objects.equals(function, other.function) && Objects.equals(ignoreSelfApproval, other.ignoreSelfApproval) - && Objects.equals(copyScores, other.copyScores); + && Objects.equals(copyScores, other.copyScores) + && Objects.equals(copyScoreRules, other.copyScoreRules); } @Override @@ -59,6 +62,7 @@ .add("function", function) .add("ignoreSelfApproval", ignoreSelfApproval) .add("copyScores", copyScores) + .add("copyScoreRules", copyScoreRules) .toString(); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java index 178fadb..8e61857 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java
@@ -47,44 +47,47 @@ this.pluginName = pluginName; } - static void extractLabelCopyScores(LabelType labelType, LabelDefinition labelDefinition) { - labelDefinition.copyScores = new HashSet<>(); + static void extractLabelCopyScoreRules(LabelType labelType, LabelDefinition labelDefinition) { + labelDefinition.copyScoreRules = new HashSet<>(); if (labelType.isCopyMinScore()) { - labelDefinition.copyScores.add(ProjectConfig.KEY_COPY_MIN_SCORE); + labelDefinition.copyScoreRules.add(ProjectConfig.KEY_COPY_MIN_SCORE); } if (labelType.isCopyMaxScore()) { - labelDefinition.copyScores.add(ProjectConfig.KEY_COPY_MAX_SCORE); + labelDefinition.copyScoreRules.add(ProjectConfig.KEY_COPY_MAX_SCORE); } if (labelType.isCopyAllScoresIfNoChange()) { - labelDefinition.copyScores.add(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CHANGE); + labelDefinition.copyScoreRules.add(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CHANGE); } if (labelType.isCopyAllScoresIfNoCodeChange()) { - labelDefinition.copyScores.add(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE); + labelDefinition.copyScoreRules.add(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE); } if (labelType.isCopyAllScoresOnMergeFirstParentUpdate()) { - labelDefinition.copyScores.add( + labelDefinition.copyScoreRules.add( ProjectConfig.KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE); } if (labelType.isCopyAllScoresOnTrivialRebase()) { - labelDefinition.copyScores.add(ProjectConfig.KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE); + labelDefinition.copyScoreRules.add(ProjectConfig.KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE); } + // TODO(hiesel) Remove once caller know of the new name + labelDefinition.copyScores = labelDefinition.copyScoreRules; } - static void applyCopyScoresTo(@Nullable Collection<String> copyScores, LabelType labelType) { - if (copyScores == null) { + static void applyCopyScoreRulesTo( + @Nullable Collection<String> copyScoreRules, LabelType labelType) { + if (copyScoreRules == null) { return; } - labelType.setCopyMinScore(copyScores.contains(ProjectConfig.KEY_COPY_MIN_SCORE)); - labelType.setCopyMaxScore(copyScores.contains(ProjectConfig.KEY_COPY_MAX_SCORE)); + labelType.setCopyMinScore(copyScoreRules.contains(ProjectConfig.KEY_COPY_MIN_SCORE)); + labelType.setCopyMaxScore(copyScoreRules.contains(ProjectConfig.KEY_COPY_MAX_SCORE)); labelType.setCopyAllScoresIfNoChange( - copyScores.contains(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CHANGE)); + copyScoreRules.contains(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CHANGE)); labelType.setCopyAllScoresIfNoCodeChange( - copyScores.contains(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE)); + copyScoreRules.contains(ProjectConfig.KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE)); labelType.setCopyAllScoresOnMergeFirstParentUpdate( - copyScores.contains(ProjectConfig.KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE)); + copyScoreRules.contains(ProjectConfig.KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE)); labelType.setCopyAllScoresOnTrivialRebase( - copyScores.contains(ProjectConfig.KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE)); + copyScoreRules.contains(ProjectConfig.KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE)); } SubmitConfig convertFrom(ProjectState projectState) { @@ -159,7 +162,10 @@ } labelType.setFunction(function); } - applyCopyScoresTo(definition.copyScores, labelType); + // TODO(hiesel): Remove fallback to copyScores + applyCopyScoreRulesTo( + definition.copyScoreRules != null ? definition.copyScoreRules : definition.copyScores, + labelType); } } @@ -172,7 +178,7 @@ config.labels.put(labelType.getName(), labelDefinition); labelDefinition.function = labelType.getFunction().getFunctionName(); - extractLabelCopyScores(labelType, labelDefinition); + extractLabelCopyScoreRules(labelType, labelDefinition); labelDefinition.ignoreSelfApproval = labelType.ignoreSelfApproval(); }
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md index 33f465c..8ad2fa1 100644 --- a/src/main/resources/Documentation/about.md +++ b/src/main/resources/Documentation/about.md
@@ -14,7 +14,7 @@ The REST API provides a view of the configuration including inheritance. The PUT method only changes the current project's configuration. -The Copy Scores are handled by Core Gerrit. +The Copy Score Rules are handled by Core Gerrit. ## The REST API This plugin exposes a REST API to configure the labels and simple rules. It is available via the @@ -75,7 +75,7 @@ { "function": LabelFunction, "ignore_self_approval": boolean, - "copy_scores": CopyScoreRule[] + "copy_score_rules": CopyScoreRule[] } ```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java index 0bee0cb..fc4aefc 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
@@ -160,6 +160,27 @@ assertThat(response.labels.get("Code-Review").ignoreSelfApproval).isTrue(); } + @Test + public void copyScoreRulesApiFieldIsBackwardsCompatible() throws Exception { + LabelDefinition codeReviewNoSelfApproval = + new LabelDefinition("MaxNoBlock", true, ImmutableSet.of("copyAllScoresIfNoChange")); + codeReviewNoSelfApproval.copyScores = codeReviewNoSelfApproval.copyScoreRules; + // Use only deprecated API field, set the new one to NULL + codeReviewNoSelfApproval.copyScoreRules = null; + + SubmitConfig config = + new SubmitConfig(ImmutableMap.of("Code-Review", codeReviewNoSelfApproval), null); + postConfig(project, config); + + String currentConfig = adminRestSession.get(endpointUrl(project)).getEntityContent(); + SubmitConfig parsedConfig = newGson().fromJson(currentConfig, SubmitConfig.class); + assertThat(parsedConfig.labels.keySet()).containsExactly("Code-Review"); + + LabelDefinition result = parsedConfig.labels.get("Code-Review"); + assertThat(result.copyScoreRules).isEqualTo(result.copyScores); + assertThat(result.copyScoreRules).isEqualTo(codeReviewNoSelfApproval.copyScores); + } + private SubmitConfig postConfig(Project.NameKey project, SubmitConfig config) throws Exception { RawInput rawInput = RawInputUtil.create(newGson().toJson(config).getBytes(Charsets.UTF_8), JSON_TYPE);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java index 0528194..4b3d61c 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
@@ -70,9 +70,9 @@ LabelDefinition labelDefinition = new LabelDefinition(); functionToSet.accept(label, false); - ConfigTranslator.extractLabelCopyScores(label, labelDefinition); + ConfigTranslator.extractLabelCopyScoreRules(label, labelDefinition); - assertThat(labelDefinition.copyScores) + assertThat(labelDefinition.copyScoreRules) .named("[case %s:false]", copyScoreName) .doesNotContain(copyScoreName); } @@ -84,9 +84,9 @@ LabelDefinition labelDefinition = new LabelDefinition(); functionToSet.accept(label, true); - ConfigTranslator.extractLabelCopyScores(label, labelDefinition); + ConfigTranslator.extractLabelCopyScoreRules(label, labelDefinition); - assertThat(labelDefinition.copyScores) + assertThat(labelDefinition.copyScoreRules) .named("[case %s:true]", copyScoreName) .contains(copyScoreName); } @@ -95,7 +95,7 @@ String copyScoreName, Predicate<LabelType> functionToCheck) { LabelType label = LabelType.withDefaultValues("Verified"); - ConfigTranslator.applyCopyScoresTo(ImmutableList.of(copyScoreName), label); + ConfigTranslator.applyCopyScoreRulesTo(ImmutableList.of(copyScoreName), label); assertThat(functionToCheck.test(label)).named("[case %s:true]", copyScoreName).isTrue(); } @@ -103,7 +103,7 @@ String copyScoreName, Predicate<LabelType> functionToCheck) { LabelType label = LabelType.withDefaultValues("Verified"); - ConfigTranslator.applyCopyScoresTo(ImmutableList.of(), label); + ConfigTranslator.applyCopyScoreRulesTo(ImmutableList.of(), label); assertThat(functionToCheck.test(label)).named("[case %s:false]", copyScoreName).isFalse(); } }