Merge "CacheBasedWebSession: Remove misplaced RequestScoped annotation"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d83ef0e..be24bc7 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2818,6 +2818,53 @@
}
----
+[[check-submit-requirement]]
+=== Check Submit Requirement
+--
+'POST /changes/link:#change-id[\{change-id\}]/check.submit_requirement'
+--
+
+Tests a submit requirement and returns the result as a
+link:#submit-requirement-result-info[SubmitRequirementResultInfo]. The request
+body must contain a link:#submit-requirement-input[SubmitRequirementInput].
+
+Note that this endpoint does not modify the change resource.
+
+.Request
+----
+ POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/check.submit_requirement HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "name": "Code-Review",
+ "submittability_expression": "label:Code-Review=+2"
+ }
+----
+
+As response a link:#submit-requirement-result-info[SubmitRequirementResultInfo]
+entity is returned that describes the submit requirement result.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "name": "Code-Review",
+ "status": "SATISFIED",
+ "submittability_expression_result": {
+ "expression": "label:Code-Review=+2",
+ "fulfilled": true,
+ "passingAtoms": [
+ "label:Code-Review=+2"
+ ]
+ },
+ "is_legacy": false
+ }
+----
+
[[edit-endpoints]]
== Change Edit Endpoints
@@ -8232,6 +8279,32 @@
contains the list of predicates that are not fulfilled for the change.
|===========================
+[[submit-requirement-input]]
+=== SubmitRequirementInput
+The `SubmitRequirementInput` entity describes a submit requirement.
+
+[options="header",cols="1,^1,5"]
+|===========================
+|Field Name ||Description
+|`name`||
+The submit requirement name.
+|`description`|optional|
+Description of the submit requirement.
+|`applicability_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is then applicable for this change.
+If not specified, the submit requirement is applicable for all changes.
+|`submittability_expression`||
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is fulfilled and not blocking change submission.
+|`override_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is overridden and not blocking change submission.
+|`allow_override_in_child_projects`|optional|
+Whether this submit requirement can be overridden in child projects. Default is
+`true`.
+|===========================
+
[[submit-requirement-result-info]]
=== SubmitRequirementResultInfo
The `SubmitRequirementResultInfo` describes the result of evaluating a
@@ -8246,7 +8319,7 @@
Description of the submit requirement.
|`status`||
Status describing the result of evaluating the submit requirement. The status
-is one of (`SATISFIED`, `UNSATISFIED`, `OVERRIDDEN`, `NOT_APPLICABLE`).
+is one of (`SATISFIED`, `UNSATISFIED`, `OVERRIDDEN`, `NOT_APPLICABLE`, `ERROR`).
|`is_legacy`||
If true, this submit requirement result was created from a legacy
link:#submit-record[SubmitRecord]. Otherwise, it was created by evaluating a
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 690ba4e..2224649 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -29,6 +29,8 @@
import com.google.gerrit.extensions.common.PureRevertInfo;
import com.google.gerrit.extensions.common.RevertSubmissionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -425,6 +427,10 @@
ChangeInfo check(FixInput fix) throws RestApiException;
+ /** Returns the result of evaluating the {@link SubmitRequirementInput} input on the change. */
+ SubmitRequirementResultInfo checkSubmitRequirement(SubmitRequirementInput input)
+ throws RestApiException;
+
void index() throws RestApiException;
/** Check if this change is a pure revert of the change stored in revertOf. */
@@ -761,6 +767,12 @@
}
@Override
+ public SubmitRequirementResultInfo checkSubmitRequirement(SubmitRequirementInput input)
+ throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void index() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementInput.java b/java/com/google/gerrit/extensions/common/SubmitRequirementInput.java
new file mode 100644
index 0000000..96045d4
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementInput.java
@@ -0,0 +1,45 @@
+// Copyright (C) 2021 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.extensions.common;
+
+/** API Input describing a submit requirement entity. */
+public class SubmitRequirementInput {
+ /** Submit requirement name. */
+ public String name;
+
+ /** Submit requirement description. */
+ public String description;
+
+ /**
+ * Query expression that can be evaluated on any change. If evaluated to true on a change, the
+ * submit requirement is then applicable on this change.
+ */
+ public String applicabilityExpression;
+
+ /**
+ * Query expression that can be evaluated on any change. If evaluated to true on a change, the
+ * submit requirement is fulfilled and not blocking change submission.
+ */
+ public String submittabilityExpression;
+
+ /**
+ * Query expression that can be evaluated on any change. If evaluated to true on a change, the
+ * submit requirement is overridden and not blocking change submission.
+ */
+ public String overrideExpression;
+
+ /** Whether this submit requirement can be overridden in child projects. */
+ public Boolean allowOverrideInChildProjects;
+}
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
index d17da0a..3d50f13 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
@@ -35,7 +35,13 @@
* Submit requirement is not applicable for the change. Happens when {@code
* applicabilityExpressionResult} is not fulfilled.
*/
- NOT_APPLICABLE
+ NOT_APPLICABLE,
+
+ /**
+ * Any of the applicability, submittability or override expressions contain invalid syntax and
+ * are not parsable.
+ */
+ ERROR
}
/** Submit requirement name. */
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index cbaf49e..a49061d 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -56,6 +56,8 @@
import com.google.gerrit.extensions.common.PureRevertInfo;
import com.google.gerrit.extensions.common.RevertSubmissionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -74,6 +76,7 @@
import com.google.gerrit.server.restapi.change.ChangeIncludedIn;
import com.google.gerrit.server.restapi.change.ChangeMessages;
import com.google.gerrit.server.restapi.change.Check;
+import com.google.gerrit.server.restapi.change.CheckSubmitRequirement;
import com.google.gerrit.server.restapi.change.CreateMergePatchSet;
import com.google.gerrit.server.restapi.change.DeleteAssignee;
import com.google.gerrit.server.restapi.change.DeleteChange;
@@ -164,6 +167,7 @@
private final Provider<ListChangeDrafts> listDraftsProvider;
private final ChangeEditApiImpl.Factory changeEditApi;
private final Check check;
+ private final CheckSubmitRequirement checkSubmitRequirement;
private final Index index;
private final Move move;
private final PostPrivate postPrivate;
@@ -218,6 +222,7 @@
Provider<ListChangeDrafts> listDraftsProvider,
ChangeEditApiImpl.Factory changeEditApi,
Check check,
+ CheckSubmitRequirement checkSubmitRequirement,
Index index,
Move move,
PostPrivate postPrivate,
@@ -270,6 +275,7 @@
this.listDraftsProvider = listDraftsProvider;
this.changeEditApi = changeEditApi;
this.check = check;
+ this.checkSubmitRequirement = checkSubmitRequirement;
this.index = index;
this.move = move;
this.postPrivate = postPrivate;
@@ -708,6 +714,16 @@
}
@Override
+ public SubmitRequirementResultInfo checkSubmitRequirement(SubmitRequirementInput input)
+ throws RestApiException {
+ try {
+ return checkSubmitRequirement.apply(change, input).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot check submit requirement", e);
+ }
+ }
+
+ @Override
public void index() throws RestApiException {
try {
index.apply(change, new Input());
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index db25dc7..9a94d93 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -61,8 +61,6 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitRecord.Status;
import com.google.gerrit.entities.SubmitRequirement;
-import com.google.gerrit.entities.SubmitRequirementExpression;
-import com.google.gerrit.entities.SubmitRequirementExpressionResult;
import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.entities.SubmitTypeRecord;
import com.google.gerrit.exceptions.StorageException;
@@ -80,7 +78,6 @@
import com.google.gerrit.extensions.common.ReviewerUpdateInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.SubmitRecordInfo;
-import com.google.gerrit.extensions.common.SubmitRequirementExpressionInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.TrackingIdInfo;
import com.google.gerrit.extensions.restapi.Url;
@@ -378,11 +375,11 @@
return submitRecordInfos;
}
- private static Collection<SubmitRequirementResultInfo> submitRequirementsFor(ChangeData cd) {
+ private Collection<SubmitRequirementResultInfo> submitRequirementsFor(ChangeData cd) {
Collection<SubmitRequirementResultInfo> reqInfos = new ArrayList<>();
Map<SubmitRequirement, SubmitRequirementResult> requirements = cd.submitRequirements();
for (Map.Entry<SubmitRequirement, SubmitRequirementResult> entry : requirements.entrySet()) {
- reqInfos.add(submitRequirementToInfo(entry.getKey(), entry.getValue()));
+ reqInfos.add(SubmitRequirementsJson.toInfo(entry.getKey(), entry.getValue()));
}
return reqInfos;
}
@@ -420,39 +417,6 @@
return info;
}
- private static SubmitRequirementResultInfo submitRequirementToInfo(
- SubmitRequirement req, SubmitRequirementResult result) {
- SubmitRequirementResultInfo info = new SubmitRequirementResultInfo();
- info.name = req.name();
- info.description = req.description().orElse(null);
- if (req.applicabilityExpression().isPresent()) {
- info.applicabilityExpressionResult =
- submitRequirementExpressionToInfo(
- req.applicabilityExpression().get(), result.applicabilityExpressionResult().get());
- }
- if (req.overrideExpression().isPresent()) {
- info.overrideExpressionResult =
- submitRequirementExpressionToInfo(
- req.overrideExpression().get(), result.overrideExpressionResult().get());
- }
- info.submittabilityExpressionResult =
- submitRequirementExpressionToInfo(
- req.submittabilityExpression(), result.submittabilityExpressionResult());
- info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
- info.isLegacy = result.legacy();
- return info;
- }
-
- private static SubmitRequirementExpressionInfo submitRequirementExpressionToInfo(
- SubmitRequirementExpression expression, SubmitRequirementExpressionResult result) {
- SubmitRequirementExpressionInfo info = new SubmitRequirementExpressionInfo();
- info.expression = expression.expressionString();
- info.fulfilled = result.status().equals(SubmitRequirementExpressionResult.Status.PASS);
- info.passingAtoms = result.passingAtoms();
- info.failingAtoms = result.failingAtoms();
- return info;
- }
-
private static void finish(ChangeInfo info) {
info.id =
Joiner.on('~')
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
new file mode 100644
index 0000000..ebb0790
--- /dev/null
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -0,0 +1,63 @@
+// Copyright (C) 2021 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.server.change;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.extensions.common.SubmitRequirementExpressionInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
+
+/**
+ * Produces submit requirements related entities like {@link SubmitRequirementResultInfo}s, which
+ * are serialized to JSON afterwards.
+ */
+public class SubmitRequirementsJson {
+ private SubmitRequirementsJson() {}
+
+ public static SubmitRequirementResultInfo toInfo(
+ SubmitRequirement req, SubmitRequirementResult result) {
+ SubmitRequirementResultInfo info = new SubmitRequirementResultInfo();
+ info.name = req.name();
+ info.description = req.description().orElse(null);
+ if (req.applicabilityExpression().isPresent()) {
+ info.applicabilityExpressionResult =
+ submitRequirementExpressionToInfo(
+ req.applicabilityExpression().get(), result.applicabilityExpressionResult().get());
+ }
+ if (req.overrideExpression().isPresent()) {
+ info.overrideExpressionResult =
+ submitRequirementExpressionToInfo(
+ req.overrideExpression().get(), result.overrideExpressionResult().get());
+ }
+ info.submittabilityExpressionResult =
+ submitRequirementExpressionToInfo(
+ req.submittabilityExpression(), result.submittabilityExpressionResult());
+ info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
+ info.isLegacy = result.legacy();
+ return info;
+ }
+
+ private static SubmitRequirementExpressionInfo submitRequirementExpressionToInfo(
+ SubmitRequirementExpression expression, SubmitRequirementExpressionResult result) {
+ SubmitRequirementExpressionInfo info = new SubmitRequirementExpressionInfo();
+ info.expression = expression.expressionString();
+ info.fulfilled = result.status().equals(SubmitRequirementExpressionResult.Status.PASS);
+ info.passingAtoms = result.passingAtoms();
+ info.failingAtoms = result.failingAtoms();
+ return info;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/CheckSubmitRequirement.java b/java/com/google/gerrit/server/restapi/change/CheckSubmitRequirement.java
new file mode 100644
index 0000000..15728ce
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/CheckSubmitRequirement.java
@@ -0,0 +1,82 @@
+// Copyright (C) 2021 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.server.restapi.change;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.SubmitRequirementsJson;
+import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
+import com.google.inject.Inject;
+import java.util.Optional;
+
+/**
+ * A rest view to evaluate (test) a {@link com.google.gerrit.entities.SubmitRequirement} on a given
+ * change.
+ *
+ * <p>TODO(ghareeb): Can this class be made singleton?
+ */
+public class CheckSubmitRequirement
+ implements RestModifyView<ChangeResource, SubmitRequirementInput> {
+ private final SubmitRequirementsEvaluator evaluator;
+
+ @Inject
+ public CheckSubmitRequirement(SubmitRequirementsEvaluator evaluator) {
+ this.evaluator = evaluator;
+ }
+
+ @Override
+ public Response<SubmitRequirementResultInfo> apply(
+ ChangeResource resource, SubmitRequirementInput input)
+ throws AuthException, BadRequestException, ResourceConflictException, Exception {
+ SubmitRequirement requirement = createSubmitRequirement(input);
+ SubmitRequirementResult res =
+ evaluator.evaluateRequirement(requirement, resource.getChangeData());
+ return Response.ok(SubmitRequirementsJson.toInfo(requirement, res));
+ }
+
+ private SubmitRequirement createSubmitRequirement(SubmitRequirementInput input)
+ throws BadRequestException {
+ validateSubmitRequirementInput(input);
+ return SubmitRequirement.builder()
+ .setName(input.name)
+ .setDescription(Optional.ofNullable(input.description))
+ .setApplicabilityExpression(SubmitRequirementExpression.of(input.applicabilityExpression))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create(input.submittabilityExpression))
+ .setOverrideExpression(SubmitRequirementExpression.of(input.overrideExpression))
+ .setAllowOverrideInChildProjects(
+ input.allowOverrideInChildProjects == null ? true : input.allowOverrideInChildProjects)
+ .build();
+ }
+
+ private void validateSubmitRequirementInput(SubmitRequirementInput input)
+ throws BadRequestException {
+ if (input.name == null) {
+ throw new BadRequestException("Field 'name' is missing from input.");
+ }
+ if (input.submittabilityExpression == null) {
+ throw new BadRequestException("Field 'submittability_expression' is missing from input.");
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index 000a17e..4d955fb 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -123,6 +123,7 @@
post(CHANGE_KIND, "wip").to(SetWorkInProgress.class);
post(CHANGE_KIND, "ready").to(SetReadyForReview.class);
put(CHANGE_KIND, "message").to(PutMessage.class);
+ post(CHANGE_KIND, "check.submit_requirement").to(CheckSubmitRequirement.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestChangeReviewers.class);
child(CHANGE_KIND, "reviewers").to(Reviewers.class);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 52202d7..e657c89 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -153,6 +153,7 @@
import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.SubmitRecordInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
import com.google.gerrit.extensions.common.TrackingIdInfo;
@@ -4074,6 +4075,90 @@
}
@Test
+ public void checkSubmitRequirement_satisfied() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ SubmitRequirementInput in =
+ createSubmitRequirementInput(
+ "Code-Review", /* submittabilityExpression= */ "label:Code-Review=+2");
+
+ SubmitRequirementResultInfo result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.UNSATISFIED);
+
+ voteLabel(changeId, "Code-Review", 2);
+ result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.SATISFIED);
+ }
+
+ @Test
+ public void checkSubmitRequirement_notApplicable() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ SubmitRequirementInput in =
+ createSubmitRequirementInput(
+ "Code-Review",
+ /* applicableIf= */ "branch:non-existent",
+ /* submittableIf= */ "label:Code-Review=+2",
+ /* overrideIf= */ null);
+
+ SubmitRequirementResultInfo result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.NOT_APPLICABLE);
+
+ voteLabel(changeId, "Code-Review", 2);
+ result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.NOT_APPLICABLE);
+ }
+
+ @Test
+ public void checkSubmitRequirement_overridden() throws Exception {
+ configLabel("Override-Label", LabelFunction.NO_OP); // label function has no effect
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel("Override-Label")
+ .ref("refs/heads/master")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ SubmitRequirementInput in =
+ createSubmitRequirementInput(
+ "Code-Review",
+ /* applicableIf= */ null,
+ /* submittableIf= */ "label:Code-Review=+2",
+ /* overrideIf= */ "label:Override-Label=+1");
+
+ SubmitRequirementResultInfo result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.UNSATISFIED);
+
+ voteLabel(changeId, "Code-Review", 2);
+ result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.SATISFIED);
+
+ voteLabel(changeId, "Override-Label", 1);
+ result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.OVERRIDDEN);
+ }
+
+ @Test
+ public void checkSubmitRequirement_error() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ SubmitRequirementInput in =
+ createSubmitRequirementInput("Code-Review", /* submittabilityExpression= */ "!!!");
+
+ SubmitRequirementResultInfo result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.ERROR);
+ }
+
+ @Test
public void submitRequirement_withLabelEqualsMax() throws Exception {
configSubmitRequirement(
project,
@@ -5393,4 +5478,22 @@
return Optional.of(record);
}
}
+
+ private static SubmitRequirementInput createSubmitRequirementInput(
+ String name, String submittabilityExpression) {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = name;
+ input.submittabilityExpression = submittabilityExpression;
+ return input;
+ }
+
+ private static SubmitRequirementInput createSubmitRequirementInput(
+ String name, String applicableIf, String submittableIf, String overrideIf) {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = name;
+ input.applicabilityExpression = applicableIf;
+ input.submittabilityExpression = submittableIf;
+ input.overrideExpression = overrideIf;
+ return input;
+ }
}
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index ee579ff..905d6be 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -55,7 +55,7 @@
/**
* Represents a "generic" text range in the code (e.g. text selection)
*/
-interface TextRange {
+export declare interface TextRange {
/** first line of the range (1-based inclusive). */
start_line: number;
/** first column of the range (in the first line) (1-based inclusive). */
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
index 61785fc..e3edb5e 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -31,7 +31,6 @@
ApprovalInfo,
Reviewers,
AccountId,
- DetailedLabelInfo,
EmailAddress,
AccountDetailInfo,
isDetailedLabelInfo,
@@ -158,24 +157,19 @@
if (!change.labels) {
return NaN;
}
- const detailedLabel = change.labels[label] as DetailedLabelInfo;
- if (!detailedLabel.all) {
+ const detailedLabel = change.labels[label];
+ if (!isDetailedLabelInfo(detailedLabel) || !detailedLabel.all) {
return NaN;
}
- const detailed = detailedLabel.all
- .filter(
- (approval: ApprovalInfo) =>
- reviewer._account_id === approval._account_id
- )
- .pop();
- if (!detailed) {
+ const approvalInfo = getApprovalInfo(detailedLabel, reviewer);
+ if (!approvalInfo) {
return NaN;
}
- if (hasOwnProperty(detailed, 'permitted_voting_range')) {
- if (!detailed.permitted_voting_range) return NaN;
- return detailed.permitted_voting_range.max;
- } else if (hasOwnProperty(detailed, 'value')) {
- // If preset, user can vote on the label.
+ if (hasOwnProperty(approvalInfo, 'permitted_voting_range')) {
+ if (!approvalInfo.permitted_voting_range) return NaN;
+ return approvalInfo.permitted_voting_range.max;
+ } else if (hasOwnProperty(approvalInfo, 'value')) {
+ // If present, user can vote on the label.
return 0;
}
return NaN;
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
index d3f23ad..bf15bb5 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
@@ -26,6 +26,7 @@
import {
createAccountDetailWithId,
createChange,
+ createDetailedLabelInfo,
} from '../../../test/test-data-generators';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -423,6 +424,7 @@
...createChange(),
labels: {
Foo: {
+ ...createDetailedLabelInfo(),
all: [
{
_account_id: 7 as AccountId,
@@ -431,6 +433,7 @@
],
},
Bar: {
+ ...createDetailedLabelInfo(),
all: [
{
...createAccountDetailWithId(1),
@@ -443,6 +446,7 @@
],
},
FooBar: {
+ ...createDetailedLabelInfo(),
all: [{_account_id: 7 as AccountId, value: 0}],
},
},
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 3c9f54c..d90d173 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -120,6 +120,9 @@
margin-top: var(--spacing-m);
padding: var(--spacing-m) var(--spacing-xl) 0;
}
+ .status-placeholder {
+ visibility: hidden;
+ }
`,
];
}
@@ -155,31 +158,27 @@
private renderLabelSection() {
const labels = this.computeLabels();
+ const showLabelName = labels.length >= 2;
return html` <div class="section">
- ${labels.map(l => this.renderLabel(l))}
+ <div class="sectionIcon"></div>
+ <div class="row">
+ <!-- Hidden placeholder to be aligned as Status line above -->
+ <div class="title status-placeholder">Status</div>
+ <div>${labels.map(l => this.renderLabel(l, showLabelName))}</div>
+ </div>
</div>`;
}
- private renderLabel(label: Label) {
+ private renderLabel(label: Label, showLabelName: boolean) {
return html`
- <section class="label">
- <div class="label-title">
- <gr-limited-text
- class="name"
- limit="25"
- text="${label.labelName}"
- ></gr-limited-text>
- </div>
- <div class="label-value">
- <gr-label-info
- .change=${this.change}
- .account=${this.account}
- .mutable=${this.mutable}
- .label="${label.labelName}"
- .labelInfo="${label.labelInfo}"
- ></gr-label-info>
- </div>
- </section>
+ ${showLabelName ? html`<div>${label.labelName} votes</div>` : ''}
+ <gr-label-info
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label="${label.labelName}"
+ .labelInfo="${label.labelInfo}"
+ ></gr-label-info>
`;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
index 2df2ccb..f60b9b6 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -17,8 +17,10 @@
import '../../../styles/gr-font-styles';
import '../../../styles/gr-voting-styles';
import '../../../styles/shared-styles';
+import '../gr-vote-chip/gr-vote-chip';
import '../gr-account-label/gr-account-label';
import '../gr-account-link/gr-account-link';
+import '../gr-account-chip/gr-account-chip';
import '../gr-button/gr-button';
import '../gr-icons/gr-icons';
import '../gr-label/gr-label';
@@ -36,7 +38,12 @@
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
import {GrButton} from '../gr-button/gr-button';
-import {getVotingRangeOrDefault} from '../../../utils/label-util';
+import {
+ canVote,
+ getApprovalInfo,
+ getVotingRangeOrDefault,
+ hasNeutralStatus,
+} from '../../../utils/label-util';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
import {fontStyles} from '../../../styles/gr-font-styles';
@@ -44,6 +51,7 @@
import {votingStyles} from '../../../styles/gr-voting-styles';
import {ifDefined} from 'lit/directives/if-defined';
import {fireReload} from '../../../utils/event-util';
+import {KnownExperimentId} from '../../../services/flags/flags';
declare global {
interface HTMLElementTagNameMap {
@@ -160,28 +168,83 @@
.labelValueContainer:not(:first-of-type) td {
padding-top: var(--spacing-s);
}
+ .reviewer-row {
+ padding-top: var(--spacing-s);
+ }
+ .reviewer-row:first-of-type {
+ padding-top: 0;
+ }
+ .reviewer-row gr-account-chip,
+ .reviewer-row gr-tooltip-content {
+ display: inline-block;
+ vertical-align: top;
+ }
+ .reviewer-row .no-votes {
+ color: var(--deemphasized-text-color);
+ margin-left: var(--spacing-xs);
+ }
`,
];
}
+ private readonly flagsService = appContext.flagsService;
+
override render() {
+ if (this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)) {
+ return this.renderNewSubmitRequirements();
+ } else {
+ return this.renderOldSubmitRequirements();
+ }
+ }
+
+ private renderNewSubmitRequirements() {
+ const labelInfo = this.labelInfo;
+ if (!labelInfo) return;
+ const reviewers = (this.change?.reviewers['REVIEWER'] ?? []).filter(
+ reviewer => canVote(labelInfo, reviewer)
+ );
+ return html`<div>
+ ${reviewers.map(reviewer => this.renderReviewerVote(reviewer))}
+ </div>`;
+ }
+
+ private renderOldSubmitRequirements() {
+ const labelInfo = this.labelInfo;
return html` <p
class="placeholder ${this.computeShowPlaceholder(
- this.labelInfo,
+ labelInfo,
this.change?.labels
)}"
>
No votes
</p>
<table>
- ${this.mapLabelInfo(
- this.labelInfo,
- this.account,
- this.change?.labels
- ).map(mappedLabel => this.renderLabel(mappedLabel))}
+ ${this.mapLabelInfo(labelInfo, this.account, this.change?.labels).map(
+ mappedLabel => this.renderLabel(mappedLabel)
+ )}
</table>`;
}
+ renderReviewerVote(reviewer: AccountInfo) {
+ const labelInfo = this.labelInfo;
+ if (!labelInfo || !isDetailedLabelInfo(labelInfo)) return;
+ const approvalInfo = getApprovalInfo(labelInfo, reviewer);
+ const noVoteYet =
+ !approvalInfo || hasNeutralStatus(labelInfo, approvalInfo);
+ return html`<div class="reviewer-row">
+ <gr-account-chip .account="${reviewer}" .change="${this.change}">
+ <gr-vote-chip
+ slot="vote-chip"
+ .vote="${approvalInfo}"
+ .label="${labelInfo}"
+ ></gr-vote-chip
+ ></gr-account-chip>
+ ${noVoteYet
+ ? html`<span class="no-votes">No votes</span>`
+ : html`${this.renderRemoveVote(reviewer)}`}
+ </div>`;
+ }
+
renderLabel(mappedLabel: FormattedLabel) {
const {labelInfo, change} = this;
return html` <tr class="labelValueContainer">
@@ -201,26 +264,28 @@
.change="${change}"
></gr-account-link>
</td>
- <td>
- <gr-tooltip-content has-tooltip title="Remove vote">
- <gr-button
- link
- aria-label="Remove vote"
- @click="${this.onDeleteVote}"
- data-account-id="${ifDefined(mappedLabel.account._account_id)}"
- class="deleteBtn ${this.computeDeleteClass(
- mappedLabel.account,
- this.mutable,
- change
- )}"
- >
- <iron-icon icon="gr-icons:delete"></iron-icon>
- </gr-button>
- </gr-tooltip-content>
- </td>
+ <td>${this.renderRemoveVote(mappedLabel.account)}</td>
</tr>`;
}
+ private renderRemoveVote(reviewer: AccountInfo) {
+ return html`<gr-tooltip-content has-tooltip title="Remove vote">
+ <gr-button
+ link
+ aria-label="Remove vote"
+ @click="${this.onDeleteVote}"
+ data-account-id="${ifDefined(reviewer._account_id)}"
+ class="deleteBtn ${this.computeDeleteClass(
+ reviewer,
+ this.mutable,
+ this.change
+ )}"
+ >
+ <iron-icon icon="gr-icons:delete"></iron-icon>
+ </gr-button>
+ </gr-tooltip-content>`;
+ }
+
/**
* This method also listens on change.labels.*,
* to trigger computation when a label is removed from the change.
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index db84043..351cc13 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -102,6 +102,7 @@
import {ChangeMessage} from '../elements/change/gr-message/gr-message';
import {GenerateUrlEditViewParameters} from '../elements/core/gr-navigation/gr-navigation';
import {
+ DetailedLabelInfo,
SubmitRequirementExpressionInfo,
SubmitRequirementResultInfo,
SubmitRequirementStatus,
@@ -706,3 +707,13 @@
message: 'This is the test message.',
};
}
+
+export function createDetailedLabelInfo(): DetailedLabelInfo {
+ return {
+ values: {
+ ' 0': 'No score',
+ '+1': 'Style Verified',
+ '-1': 'Wrong Style or Formatting',
+ },
+ };
+}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 78c4132..960b02f 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -134,6 +134,16 @@
return label.all?.filter(x => x._account_id === account._account_id)[0];
}
+export function canVote(label: DetailedLabelInfo, account: AccountInfo) {
+ const approvalInfo = getApprovalInfo(label, account);
+ if (!approvalInfo) return false;
+ if (approvalInfo.permitted_voting_range) {
+ return approvalInfo.permitted_voting_range.max > 0;
+ }
+ // If value present, user can vote on the label.
+ return approvalInfo.value !== undefined;
+}
+
export function getAllUniqueApprovals(labelInfo?: LabelInfo) {
if (!labelInfo || !isDetailedLabelInfo(labelInfo)) return [];
const uniqueApprovals = (labelInfo.all ?? [])
@@ -148,8 +158,7 @@
export function hasVotes(labelInfo: LabelInfo): boolean {
if (isDetailedLabelInfo(labelInfo)) {
return (labelInfo.all ?? []).some(
- approval =>
- getLabelStatus(labelInfo, approval.value) !== LabelStatus.NEUTRAL
+ approval => !hasNeutralStatus(labelInfo, approval)
);
}
if (isQuickLabelInfo(labelInfo)) {
