Merge "PerformanceMetrics: Suppress redundant logging"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d83ef0e..0a203cc 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
@@ -7354,7 +7401,8 @@
[[included-in-info]]
=== IncludedInInfo
The `IncludedInInfo` entity contains information about the branches a
-change was merged into and tags it was tagged with.
+change was merged into and tags it was tagged with. The branch or tag
+must have 'refs/head/' or 'refs/tags/' prefix respectively.
[options="header",cols="1,^1,5"]
|=======================
@@ -8232,6 +8280,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 +8320,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/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 1605397..92e16ce 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -32,14 +32,12 @@
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.AuthConfig;
import com.google.inject.Provider;
-import com.google.inject.servlet.RequestScoped;
import java.util.EnumSet;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.http.server.GitSmartHttpTools;
-@RequestScoped
public abstract class CacheBasedWebSession extends WebSession {
@VisibleForTesting public static final String ACCOUNT_COOKIE = "GerritAccount";
protected static final long MAX_AGE_MINUTES = HOURS.toMinutes(12);
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/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
index b4f79d1..ef00b80 100644
--- a/java/com/google/gerrit/server/cache/PerThreadCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -68,7 +68,7 @@
/**
* Returns a key based on the value's class and an identifier that uniquely identify the value.
- * The identifier needs to implement {@code equals()} and {@hashCode()}.
+ * The identifier needs to implement {@code equals()} and {@code hashCode()}.
*/
public static <T> Key<T> create(Class<T> clazz, Object identifier) {
return new Key<>(clazz, ImmutableList.of(identifier));
@@ -76,7 +76,7 @@
/**
* Returns a key based on the value's class and a set of identifiers that uniquely identify the
- * value. Identifiers need to implement {@code equals()} and {@hashCode()}.
+ * value. Identifiers need to implement {@code equals()} and {@code hashCode()}.
*/
public static <T> Key<T> create(Class<T> clazz, Object... identifiers) {
return new Key<>(clazz, ImmutableList.copyOf(identifiers));
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/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 951167a..a7fea3c 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -400,7 +400,7 @@
try {
Map<String, FileDiffOutput> modifiedFiles =
diffOperations.listModifiedFilesAgainstParent(
- change.getProject(), patchSet.commitId(), /* parent= */ 0);
+ change.getProject(), patchSet.commitId(), /* parentNum= */ 0);
for (FileDiffOutput diff : modifiedFiles.values()) {
if (patchSetAttribute.files == null) {
@@ -457,7 +457,7 @@
Map<String, FileDiffOutput> modifiedFiles =
diffOperations.listModifiedFilesAgainstParent(
- change.getProject(), patchSet.commitId(), /* parent= */ 0);
+ change.getProject(), patchSet.commitId(), /* parentNum= */ 0);
for (FileDiffOutput fileDiff : modifiedFiles.values()) {
p.sizeDeletions += fileDiff.deletions();
p.sizeInsertions += fileDiff.insertions();
diff --git a/java/com/google/gerrit/server/git/validators/ValidationMessage.java b/java/com/google/gerrit/server/git/validators/ValidationMessage.java
index b5d7eb1..c743bbc 100644
--- a/java/com/google/gerrit/server/git/validators/ValidationMessage.java
+++ b/java/com/google/gerrit/server/git/validators/ValidationMessage.java
@@ -68,7 +68,8 @@
}
/**
- * Returns {@true} if this message is an error. Used to decide if the operation should be aborted.
+ * Returns {@code true} if this message is an error. Used to decide if the operation should be
+ * aborted.
*/
public boolean isError() {
return type == Type.FATAL || type == Type.ERROR;
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..55b234c
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/CheckSubmitRequirement.java
@@ -0,0 +1,79 @@
+// 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.BadRequestException;
+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 BadRequestException {
+ 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/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
index b4d9558..537c7d8 100644
--- a/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
@@ -68,8 +68,7 @@
public void changeKindPredicate_noCodeChange() throws Exception {
String change = changeKindCreator.createChange(ChangeKind.NO_CODE_CHANGE, testRepo, admin);
changeKindCreator.updateChange(change, ChangeKind.NO_CODE_CHANGE, testRepo, admin, project);
- PatchSet.Id ps1 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ PatchSet.Id ps1 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 1);
assertTrue(
queryBuilder
.parse("changekind:no-code-change")
@@ -77,8 +76,7 @@
.match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
changeKindCreator.updateChange(change, ChangeKind.TRIVIAL_REBASE, testRepo, admin, project);
- PatchSet.Id ps2 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ PatchSet.Id ps2 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 2);
assertFalse(
queryBuilder
.parse("changekind:no-code-change")
@@ -90,8 +88,7 @@
public void changeKindPredicate_trivialRebase() throws Exception {
String change = changeKindCreator.createChange(ChangeKind.TRIVIAL_REBASE, testRepo, admin);
changeKindCreator.updateChange(change, ChangeKind.TRIVIAL_REBASE, testRepo, admin, project);
- PatchSet.Id ps1 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ PatchSet.Id ps1 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 1);
assertTrue(
queryBuilder
.parse("changekind:trivial-rebase")
@@ -99,8 +96,7 @@
.match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
- PatchSet.Id ps2 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ PatchSet.Id ps2 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 2);
assertFalse(
queryBuilder
.parse("changekind:trivial-rebase")
@@ -112,8 +108,7 @@
public void changeKindPredicate_reworkAndNotRework() throws Exception {
String change = changeKindCreator.createChange(ChangeKind.REWORK, testRepo, admin);
changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
- PatchSet.Id ps1 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 1);
+ PatchSet.Id ps1 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 1);
assertTrue(
queryBuilder
.parse("changekind:rework")
@@ -121,8 +116,7 @@
.match(contextForCodeReviewLabel(/* value= */ -2, ps1, admin.id())));
changeKindCreator.updateChange(change, ChangeKind.REWORK, testRepo, admin, project);
- PatchSet.Id ps2 =
- PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* psId= */ 2);
+ PatchSet.Id ps2 = PatchSet.id(Change.id(gApi.changes().id(change).get()._number), /* id= */ 2);
assertFalse(
queryBuilder
.parse("-changekind:rework")
@@ -150,7 +144,7 @@
.match(
contextForCodeReviewLabel(
/* value= */ 2,
- PatchSet.id(pushResult.getChange().getId(), /* psId= */ 1),
+ PatchSet.id(pushResult.getChange().getId(), /* id= */ 1),
admin.id())));
// can not copy approval from patchset 2 -> 3
assertFalse(
@@ -160,7 +154,7 @@
.match(
contextForCodeReviewLabel(
/* value= */ 2,
- PatchSet.id(pushResult.getChange().getId(), /* psId= */ 2),
+ PatchSet.id(pushResult.getChange().getId(), /* id= */ 2),
admin.id())));
}
@@ -179,7 +173,7 @@
.match(
contextForCodeReviewLabel(
/* value= */ 2,
- PatchSet.id(pushResult.getChange().getId(), /* psId= */ 1),
+ PatchSet.id(pushResult.getChange().getId(), /* id= */ 1),
admin.id())));
// can not copy approval from patchset 2 -> 3
assertFalse(
@@ -189,7 +183,7 @@
.match(
contextForCodeReviewLabel(
/* value= */ 2,
- PatchSet.id(pushResult.getChange().getId(), /* psId= */ 2),
+ PatchSet.id(pushResult.getChange().getId(), /* id= */ 2),
user.id())));
}
@@ -219,7 +213,7 @@
.asMatchable()
.match(
contextForCodeReviewLabel(
- /* value= */ 2, PatchSet.id(changeId, /* psId= */ 1), admin.id())));
+ /* value= */ 2, PatchSet.id(changeId, /* id= */ 1), admin.id())));
changeOperations.change(changeId).newPatchset().file("file").delete().create();
// can not copy approval from patch-set 2 -> 3
@@ -229,7 +223,7 @@
.asMatchable()
.match(
contextForCodeReviewLabel(
- /* value= */ 2, PatchSet.id(changeId, /* psId= */ 2), admin.id())));
+ /* value= */ 2, PatchSet.id(changeId, /* id= */ 2), admin.id())));
}
@Test
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 d90d173..5feb1ae 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
@@ -16,7 +16,6 @@
*/
import '../../shared/gr-button/gr-button';
import '../../shared/gr-label-info/gr-label-info';
-import '../../shared/gr-limited-text/gr-limited-text';
import {customElement, property} from 'lit/decorators';
import {
AccountInfo,
@@ -37,7 +36,7 @@
const base = HovercardMixin(LitElement);
@customElement('gr-submit-requirement-hovercard')
-export class GrHovercardRun extends base {
+export class GrSubmitRequirementHovercard extends base {
@property({type: Object})
requirement?: SubmitRequirementResultInfo;
@@ -261,6 +260,6 @@
declare global {
interface HTMLElementTagNameMap {
- 'gr-submit-requirement-hovercard': GrHovercardRun;
+ 'gr-submit-requirement-hovercard': GrSubmitRequirementHovercard;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 004d594..40d80bd 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -14,7 +14,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+import '../../shared/gr-label-info/gr-label-info';
import '../gr-submit-requirement-hovercard/gr-submit-requirement-hovercard';
+import '../gr-trigger-vote-hovercard/gr-trigger-vote-hovercard';
import {LitElement, css, html} from 'lit';
import {customElement, property, state} from 'lit/decorators';
import {ParsedChangeInfo} from '../../../types/types';
@@ -271,6 +273,9 @@
html`<gr-trigger-vote
.label="${label}"
.labelInfo="${labels[label]}"
+ .change="${this.change}"
+ .account="${this.account}"
+ .mutable="${this.mutable ?? false}"
></gr-trigger-vote>`
)}
</section>`;
@@ -285,6 +290,15 @@
@property({type: Object})
labelInfo?: LabelInfo;
+ @property({type: Object})
+ change?: ParsedChangeInfo;
+
+ @property({type: Object})
+ account?: AccountInfo;
+
+ @property({type: Boolean})
+ mutable?: boolean;
+
static override get styles() {
return css`
:host {
@@ -309,6 +323,10 @@
--gr-vote-chip-width: 14px;
--gr-vote-chip-height: 14px;
margin-right: 0px;
+ margin-left: var(--spacing-xs);
+ }
+ gr-vote-chip:first-of-type {
+ margin-left: 0px;
}
`;
}
@@ -317,6 +335,17 @@
if (!this.labelInfo) return;
return html`
<div class="container">
+ <gr-trigger-vote-hovercard .labelName=${this.label}>
+ <gr-label-info
+ slot="label-info"
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label=${this.label}
+ .labelInfo=${this.labelInfo}
+ .showAllReviewers=${false}
+ ></gr-label-info>
+ </gr-trigger-vote-hovercard>
<span class="label">${this.label}</span>
${this.renderVotes()}
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-trigger-vote-hovercard/gr-trigger-vote-hovercard.ts b/polygerrit-ui/app/elements/change/gr-trigger-vote-hovercard/gr-trigger-vote-hovercard.ts
new file mode 100644
index 0000000..552cc69
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-trigger-vote-hovercard/gr-trigger-vote-hovercard.ts
@@ -0,0 +1,94 @@
+/**
+ * @license
+ * 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.
+ */
+import {customElement, property} from 'lit/decorators';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
+import {fontStyles} from '../../../styles/gr-font-styles';
+
+// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
+const base = HovercardMixin(LitElement);
+
+@customElement('gr-trigger-vote-hovercard')
+export class GrTriggerVoteHovercard extends base {
+ @property()
+ labelName?: string;
+
+ static override get styles() {
+ return [
+ fontStyles,
+ base.styles || [],
+ css`
+ #container {
+ min-width: 300px;
+ max-width: 300px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ .row {
+ display: flex;
+ }
+ .title {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-m);
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ align-items: flex-start;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.sectionIcon iron-icon {
+ position: relative;
+ width: 20px;
+ height: 20px;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html` <div id="container" role="tooltip" tabindex="-1">
+ <div class="section">
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>${this.labelName}</span>
+ </h3>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div class="row">
+ <div class="title">Status</div>
+ <div>
+ <slot name="label-info"></slot>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>`;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-trigger-vote-hovercard': GrTriggerVoteHovercard;
+ }
+}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index a7702d1..b47c51c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -46,7 +46,7 @@
@property({type: Boolean})
showTooltipBelow = false;
- private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
override connectedCallback() {
super.connectedCallback();
@@ -60,7 +60,7 @@
*/
setMode(newMode: DiffViewMode) {
if (this.saveOnChange && this.mode && this.mode !== newMode) {
- this.restApiService.savePreferences({diff_view: newMode});
+ this.userService.updatePreferences({diff_view: newMode});
}
this.mode = newMode;
let announcement;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
index aa97394..8b06c75 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
@@ -19,7 +19,7 @@
import './gr-diff-mode-selector';
import {GrDiffModeSelector} from './gr-diff-mode-selector';
import {DiffViewMode} from '../../../constants/constants';
-import {stubRestApi} from '../../../test/test-utils';
+import {stubUsers} from '../../../test/test-utils';
const basicFixture = fixtureFromElement('gr-diff-mode-selector');
@@ -47,7 +47,7 @@
});
test('setMode', () => {
- const saveStub = stubRestApi('savePreferences');
+ const saveStub = stubUsers('updatePreferences');
// Setting the mode initially does not save prefs.
element.saveOnChange = true;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 5b48345..c7a462e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -113,6 +113,7 @@
import {changeComments$} from '../../../services/comments/comments-model';
import {takeUntil} from 'rxjs/operators';
import {Subject} from 'rxjs';
+import {preferences$} from '../../../services/user/user-model';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const LOADING_BLAME = 'Loading blame...';
@@ -361,11 +362,20 @@
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
+ // TODO(brohlfs): This just ensures that the userService is instantiated at
+ // all. We need the service to manage the model, but we are not making any
+ // direct calls. Will need to find a better solution to this problem ...
+ assertIsDefined(appContext.userService);
+
changeComments$
.pipe(takeUntil(this.disconnected$))
.subscribe(changeComments => {
this._changeComments = changeComments;
});
+
+ preferences$.pipe(takeUntil(this.disconnected$)).subscribe(preferences => {
+ this._userPrefs = preferences;
+ });
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.cursor.replaceDiffs([this.$.diffHost]);
this._onRenderHandler = (_: Event) => {
@@ -1148,12 +1158,6 @@
promises.push(this._getDiffPreferences());
- promises.push(
- this._getPreferences().then(prefs => {
- this._userPrefs = prefs;
- })
- );
-
if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
if (!this._changeComments) this._loadComments(value.patchNum);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 2c4750a..0c7abc8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -1524,6 +1524,7 @@
assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
// User prefs but no change view state set.
+ element.changeViewState.diffMode = undefined;
element._userPrefs = {default_diff_view: 'UNIFIED_DIFF'};
assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 3533fd6..256e956 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -246,6 +246,7 @@
this.$.diffPrefs.loadData(),
];
+ // TODO(dhruvsri): move this to the service
promises.push(
this.restApiService.getPreferences().then(prefs => {
if (!prefs) {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index 8194d5b..1165f1e 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -273,7 +273,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assertMenusEqual(prefs.my, preferences.my);
assert.equal(prefs.publish_comments_on_push, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -294,7 +294,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assert.equal(prefs.publish_comments_on_push, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -315,7 +315,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assert.equal(prefs.work_in_progress_by_default, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -348,7 +348,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assertMenusEqual(prefs.my, element._localMenu);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
await element._handleSaveMenu();
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
index f808e38..8322682 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
@@ -73,6 +73,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
+
disconnected$ = new Subject();
override connectedCallback() {
@@ -106,7 +108,7 @@
if (scheme && scheme !== this.selectedScheme) {
this.set('selectedScheme', scheme);
if (this._loggedIn) {
- this.restApiService.savePreferences({
+ this.userService.updatePreferences({
download_scheme: this.selectedScheme,
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
index c932a2e..ef712ac 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
@@ -23,6 +23,7 @@
import {createPreferences} from '../../../test/test-data-generators';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {GrShellCommand} from '../gr-shell-command/gr-shell-command';
+import {createDefaultPreferences} from '../../../constants/constants';
const basicFixture = fixtureFromElement('gr-download-commands');
@@ -95,7 +96,7 @@
test('saves scheme to preferences', () => {
element._loggedIn = true;
const savePrefsStub = stubRestApi('savePreferences').returns(
- Promise.resolve(new Response())
+ Promise.resolve(createDefaultPreferences())
);
flush();
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 f60b9b6..947ef3e 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
@@ -43,6 +43,7 @@
getApprovalInfo,
getVotingRangeOrDefault,
hasNeutralStatus,
+ hasVoted,
} from '../../../utils/label-util';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
@@ -86,9 +87,21 @@
@property({type: Object})
account?: AccountInfo;
+ /**
+ * A user is able to delete a vote iff the mutable property is true and the
+ * reviewer that left the vote exists in the list of removable_reviewers
+ * received from the backend.
+ */
@property({type: Boolean})
mutable = false;
+ /**
+ * if true - show all reviewers that can vote on label
+ * if false - show only reviewers that voted on label
+ */
+ @property({type: Boolean})
+ showAllReviewers = true;
+
private readonly restApiService = appContext.restApiService;
private readonly reporting = appContext.reportingService;
@@ -201,7 +214,9 @@
const labelInfo = this.labelInfo;
if (!labelInfo) return;
const reviewers = (this.change?.reviewers['REVIEWER'] ?? []).filter(
- reviewer => canVote(labelInfo, reviewer)
+ reviewer =>
+ (this.showAllReviewers && canVote(labelInfo, reviewer)) ||
+ (!this.showAllReviewers && hasVoted(labelInfo, reviewer))
);
return html`<div>
${reviewers.map(reviewer => this.renderReviewerVote(reviewer))}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index ae2bda0..63576c2 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -682,19 +682,27 @@
});
}
- savePreferences(prefs: PreferencesInput): Promise<Response> {
+ savePreferences(
+ prefs: PreferencesInput
+ ): Promise<PreferencesInfo | undefined> {
// Note (Issue 5142): normalize the download scheme with lower case before
// saving.
if (prefs.download_scheme) {
prefs.download_scheme = prefs.download_scheme.toLowerCase();
}
- return this._restApiHelper.send({
- method: HttpMethod.PUT,
- url: '/accounts/self/preferences',
- body: prefs,
- reportUrlAsIs: true,
- });
+ return this._restApiHelper
+ .send({
+ method: HttpMethod.PUT,
+ url: '/accounts/self/preferences',
+ body: prefs,
+ reportUrlAsIs: true,
+ })
+ .then((response: Response) =>
+ this.getResponseObject(response).then(
+ obj => obj as unknown as PreferencesInfo
+ )
+ );
}
saveDiffPreferences(prefs: DiffPreferenceInput): Promise<Response> {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index 1aee75a..a60a1ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -386,7 +386,8 @@
});
test('savPreferences normalizes download scheme', () => {
- const sendStub = sinon.stub(element._restApiHelper, 'send');
+ const sendStub = sinon.stub(element._restApiHelper, 'send').returns(
+ Promise.resolve(new Response()));
element.savePreferences({download_scheme: 'HTTP'});
assert.isTrue(sendStub.called);
assert.equal(sendStub.lastCall.args[0].body.download_scheme, 'http');
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index cd3fab3..1378211 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -209,7 +209,9 @@
errFn?: ErrorCallback
): Promise<ChangeInfo | null>;
- savePreferences(prefs: PreferencesInput): Promise<Response>;
+ savePreferences(
+ prefs: PreferencesInput
+ ): Promise<PreferencesInfo | undefined>;
getDiffPreferences(): Promise<DiffPreferencesInfo | undefined>;
diff --git a/polygerrit-ui/app/services/user/user-service.ts b/polygerrit-ui/app/services/user/user-service.ts
index 0612aca..125d20c 100644
--- a/polygerrit-ui/app/services/user/user-service.ts
+++ b/polygerrit-ui/app/services/user/user-service.ts
@@ -14,7 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {AccountDetailInfo, PreferencesInfo} from '../../types/common';
+import {
+ AccountDetailInfo,
+ PreferencesInfo,
+ PreferencesInput,
+} from '../../types/common';
import {from, of} from 'rxjs';
import {account$, updateAccount, updatePreferences} from './user-model';
import {switchMap} from 'rxjs/operators';
@@ -39,4 +43,13 @@
updatePreferences(preferences ?? createDefaultPreferences());
});
}
+
+ updatePreferences(prefs: PreferencesInput) {
+ this.restApiService
+ .savePreferences(prefs)
+ .then((newPrefs: PreferencesInfo | undefined) => {
+ if (!newPrefs) return;
+ updatePreferences(newPrefs);
+ });
+ }
}
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
index 4be19fd..3bb0c34 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -83,6 +83,7 @@
import {
createDefaultDiffPrefs,
createDefaultEditPrefs,
+ createDefaultPreferences,
} from '../../constants/constants';
import {ParsedChangeInfo} from '../../types/types';
@@ -481,8 +482,8 @@
saveIncludedGroup(): Promise<GroupInfo | undefined> {
throw new Error('saveIncludedGroup() not implemented by RestApiMock.');
},
- savePreferences(): Promise<Response> {
- return Promise.resolve(new Response());
+ savePreferences(): Promise<PreferencesInfo> {
+ return Promise.resolve(createDefaultPreferences());
},
saveRepoConfig(): Promise<Response> {
return Promise.resolve(new Response());
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 1cde372..39c30ad 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -24,6 +24,7 @@
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
import {CommentsService} from '../services/comments/comments-service';
+import {UserService} from '../services/user/user-service';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise extends Promise<unknown> {
@@ -111,6 +112,10 @@
return sinon.stub(appContext.commentsService, method);
}
+export function stubUsers<K extends keyof UserService>(method: K) {
+ return sinon.stub(appContext.userService, method);
+}
+
export function stubStorage<K extends keyof StorageService>(method: K) {
return sinon.stub(appContext.storageService, method);
}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 960b02f..a8a2719 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -92,8 +92,9 @@
export function hasNeutralStatus(
label: DetailedLabelInfo,
- approvalInfo: ApprovalInfo
+ approvalInfo?: ApprovalInfo
) {
+ if (!approvalInfo) return true;
return getLabelStatus(label, approvalInfo.value) === LabelStatus.NEUTRAL;
}
@@ -134,6 +135,15 @@
return label.all?.filter(x => x._account_id === account._account_id)[0];
}
+export function hasVoted(label: LabelInfo, account: AccountInfo) {
+ if (isDetailedLabelInfo(label)) {
+ return !hasNeutralStatus(label, getApprovalInfo(label, account));
+ } else if (isQuickLabelInfo(label)) {
+ return label.approved === account || label.rejected === account;
+ }
+ return false;
+}
+
export function canVote(label: DetailedLabelInfo, account: AccountInfo) {
const approvalInfo = getApprovalInfo(label, account);
if (!approvalInfo) return false;
diff --git a/tools/BUILD b/tools/BUILD
index 46671ac..e44ae78 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -241,7 +241,7 @@
"-Xep:InstantTemporalUnit:ERROR",
"-Xep:IntLongMath:ERROR",
# "-Xep:InvalidBlockTag:WARN",
- # "-Xep:InvalidInlineTag:WARN",
+ "-Xep:InvalidInlineTag:WARN",
"-Xep:InvalidJavaTimeConstant:ERROR",
"-Xep:InvalidLink:ERROR",
# "-Xep:InvalidParam:WARN",
@@ -354,7 +354,7 @@
"-Xep:OverridesGuiceInjectableMethod:ERROR",
"-Xep:OverridesJavaxInjectableMethod:ERROR",
"-Xep:PackageInfo:ERROR",
- # "-Xep:ParameterName:WARN",
+ "-Xep:ParameterName:ERROR",
"-Xep:ParametersButNotParameterized:ERROR",
"-Xep:ParcelableCreator:ERROR",
"-Xep:PeriodFrom:ERROR",
