Merge "Prevent diff view nav links from overflowing the header"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index a1fa859..306ba33 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7908,7 +7908,7 @@
Notify handling that defines to whom email notifications should be sent
after the change is created. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
-If not set, the default is `ALL`.
+If not set, the default is `OWNER` for WIP changes and `ALL` otherwise.
|`notify_details` |optional|
Additional information about whom to notify about the change creation
as a map of link:user-notify.html#recipient-types[recipient type] to
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 5d40e2d..fadbad9 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -724,6 +724,22 @@
Valid relations are >=, >, \<=, <, or no relation, which will match if the number of unresolved
comments is exactly equal.
+[[unmet_requirement]]
+unmet_requirement:'SUBMIT_REQUIREMENT_NAME'::
++
+Matches changes where the given submit requirement is evaluated and unmet (its result is UNSATISFIED, ERROR, or TIMEOUT). The requirement name comparison is case-insensitive.
+
+[[unsatisfied_requirement_count]]
+unsatisfied_requirement_count:'RELATION''NUMBER'::
++
+True if the number of unsatisfied submit requirements satisfies the given relation for the given number.
++
+For example, unsatisfied_requirement_count:>0 will be true for any change which has at least one unsatisfied
+submit requirement while unsatisfied_requirement_count:0 will be true for any change which has all submit requirements resolved.
++
+Valid relations are >=, >, \<=, <, or no relation, which will match if the number of unsatisfied
+submit requirements is exactly equal.
+
== Argument Quoting
Operator values that are not bare words (roughly A-Z, a-z, 0-9, @,
diff --git a/java/com/google/gerrit/entities/converter/ChangeInputProtoConverter.java b/java/com/google/gerrit/entities/converter/ChangeInputProtoConverter.java
index 7270af8..053c077 100644
--- a/java/com/google/gerrit/entities/converter/ChangeInputProtoConverter.java
+++ b/java/com/google/gerrit/entities/converter/ChangeInputProtoConverter.java
@@ -98,7 +98,9 @@
if (changeInput.author != null) {
builder.setAuthor(accountInputConverter.toProto(changeInput.author));
}
- builder.setNotify(Entities.NotifyHandling.forNumber(changeInput.notify.getValue()));
+ if (changeInput.notify != null) {
+ builder.setNotify(Entities.NotifyHandling.forNumber(changeInput.notify.getValue()));
+ }
List<ListChangesOption> responseFormatOptions = changeInput.responseFormatOptions;
if (responseFormatOptions != null) {
@@ -166,7 +168,9 @@
}
}
- changeInput.notify = NotifyHandling.valueOf(proto.getNotify().name());
+ if (proto.hasNotify()) {
+ changeInput.notify = NotifyHandling.valueOf(proto.getNotify().name());
+ }
if (proto.getNotifyDetailsCount() > 0) {
changeInput.notifyDetails = new HashMap<>();
diff --git a/java/com/google/gerrit/extensions/common/ChangeInput.java b/java/com/google/gerrit/extensions/common/ChangeInput.java
index 2e2b9ca..07289a2 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInput.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInput.java
@@ -62,8 +62,12 @@
this.subject = subject;
}
- /** Who to send email notifications to after change is created. */
- public NotifyHandling notify = NotifyHandling.ALL;
+ /**
+ * Who to send email notifications to after change is created. If not specified, defaults to
+ * {@link NotifyHandling#OWNER} if the change is created as work-in-progress, or {@link
+ * NotifyHandling#ALL} otherwise.
+ */
+ public NotifyHandling notify;
public Map<RecipientType, NotifyInfo> notifyDetails;
}
diff --git a/java/com/google/gerrit/server/change/AddReviewersOp.java b/java/com/google/gerrit/server/change/AddReviewersOp.java
index 239fa7a..43598e4 100644
--- a/java/com/google/gerrit/server/change/AddReviewersOp.java
+++ b/java/com/google/gerrit/server/change/AddReviewersOp.java
@@ -82,6 +82,7 @@
private final Collection<Address> addresses;
private final ReviewerState state;
private final boolean forGroup;
+ private boolean allowDowngradeToCc = true;
// Unlike addedCCs, addedReviewers is a PatchSetApproval because the ReviewerResult returned
// via the REST API is supposed to include vote information.
@@ -118,6 +119,10 @@
this.forGroup = forGroup;
}
+ public void setAllowDowngradeToCc(boolean allowDowngradeToCc) {
+ this.allowDowngradeToCc = allowDowngradeToCc;
+ }
+
@Override
public boolean updateChange(ChangeContext ctx) throws RestApiException, IOException {
change = ctx.getChange();
@@ -130,7 +135,10 @@
if (state == CC) {
addedCCs =
approvalsUtil.addCcs(
- ctx.getNotes(), ctx.getUpdate(change.currentPatchSetId()), accountIds, forGroup);
+ ctx.getNotes(),
+ ctx.getUpdate(change.currentPatchSetId()),
+ accountIds,
+ forGroup || !allowDowngradeToCc);
} else {
addedReviewers =
approvalsUtil.addReviewers(
diff --git a/java/com/google/gerrit/server/change/ReviewerModifier.java b/java/com/google/gerrit/server/change/ReviewerModifier.java
index 92dd9db..8fdb888 100644
--- a/java/com/google/gerrit/server/change/ReviewerModifier.java
+++ b/java/com/google/gerrit/server/change/ReviewerModifier.java
@@ -130,6 +130,13 @@
/** Whether the visibility check for the reviewer account should be skipped. */
public boolean skipVisibilityCheck = false;
+
+ /**
+ * Whether an existing REVIEWER should be downgraded to CC if the input state is CC.
+ *
+ * <p>If false, and the account is already a REVIEWER, the state will remain REVIEWER.
+ */
+ public boolean allowDowngradeToCc = true;
}
public static InternalReviewerInput newReviewerInput(
@@ -161,6 +168,10 @@
in.state = CC;
in.notify = notify;
in.otherFailureBehavior = FailureBehavior.IGNORE_ALL;
+ // Automatic addition of author/committer as CC is an implicit system action on push.
+ // Preserve existing REVIEWER status so implicit auto-CC does not demote explicitly assigned
+ // reviewers.
+ in.allowDowngradeToCc = false;
return Optional.of(in);
}
@@ -527,6 +538,9 @@
this.reviewersByEmail,
state(),
forGroup);
+ if (input instanceof InternalReviewerInput internalInput) {
+ ((AddReviewersOp) op).setAllowDowngradeToCc(internalInput.allowDowngradeToCc);
+ }
}
this.exactMatchFound = exactMatchFound;
}
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 6209854..2b3bdba 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -995,11 +995,11 @@
private void setThreadName(String oldThreadName) {
try {
Thread.currentThread().setName(oldThreadName + "[" + this + "]");
- } catch (Exception e) {
+ } catch (RuntimeException e) {
logger.atWarning().withCause(e).log("Cannot describe task");
try {
Thread.currentThread().setName(oldThreadName + "[" + runnable.getClass().getName() + "]");
- } catch (Exception e2) {
+ } catch (RuntimeException e2) {
logger.atWarning().withCause(e2).log("Cannot get runnable class name");
Thread.currentThread().setName(oldThreadName + "[unknown task]");
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 6c11800..9737d4d 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2639,7 +2639,8 @@
if (idList.isEmpty()) {
messages.add(
- new ValidationMessage("warning: pushing without Change-Id is deprecated", false));
+ new ValidationMessage(
+ "pushing without Change-Id is deprecated", ValidationMessage.Type.WARNING));
break;
}
}
@@ -3566,11 +3567,12 @@
if (messageEq && parentsEq && authorEq) {
addMessage(
String.format(
- "warning: no changes between prior commit %s and new commit %s",
- abbreviateName(priorCommit, reader), abbreviateName(newCommit, reader)));
+ "no changes between prior commit %s and new commit %s",
+ abbreviateName(priorCommit, reader), abbreviateName(newCommit, reader)),
+ ValidationMessage.Type.WARNING);
} else {
StringBuilder msg = new StringBuilder();
- msg.append("warning: ").append(abbreviateName(newCommit, reader));
+ msg.append("commit ").append(abbreviateName(newCommit, reader));
msg.append(":");
msg.append(" no files changed");
if (!authorEq) {
@@ -3582,7 +3584,7 @@
if (!parentsEq) {
msg.append(", was rebased");
}
- addMessage(msg.toString());
+ addMessage(msg.toString(), ValidationMessage.Type.WARNING);
}
}
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index e26165a..f7f1ce1 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -1674,6 +1674,51 @@
STORED_SUBMIT_REQUIREMENTS_SPEC =
STORED_SUBMIT_REQUIREMENTS_FIELD.storedOnly("full_submit_requirements");
+ /**
+ * Names of submit requirements that are not fulfilled for a change (i.e., whose evaluation status
+ * is {@link com.google.gerrit.entities.SubmitRequirementResult.Status#UNSATISFIED}, {@link
+ * com.google.gerrit.entities.SubmitRequirementResult.Status#ERROR}, or {@link
+ * com.google.gerrit.entities.SubmitRequirementResult.Status#TIMEOUT}).
+ *
+ * <p>Note that if evaluating a submit requirement results in an ERROR or TIMEOUT, it is
+ * considered unfulfilled and its name will be included in this field.
+ */
+ public static final IndexedField<ChangeData, Iterable<String>> UNMET_REQUIREMENT_FIELD =
+ IndexedField.<ChangeData>iterableStringBuilder("UnmetRequirement")
+ .build(
+ cd ->
+ cd.submitRequirementsIncludingLegacy().values().stream()
+ .filter(sr -> !sr.fulfilled())
+ .map(sr -> sr.submitRequirement().name().toLowerCase(Locale.US))
+ .collect(toImmutableSet()));
+
+ public static final IndexedField<ChangeData, Iterable<String>>.SearchSpec UNMET_REQUIREMENT_SPEC =
+ UNMET_REQUIREMENT_FIELD.exact("unmet_requirement");
+
+ /**
+ * The number of submit requirements that are not fulfilled for a change (i.e., whose evaluation
+ * status is {@link com.google.gerrit.entities.SubmitRequirementResult.Status#UNSATISFIED}, {@link
+ * com.google.gerrit.entities.SubmitRequirementResult.Status#ERROR}, or {@link
+ * com.google.gerrit.entities.SubmitRequirementResult.Status#TIMEOUT}).
+ *
+ * <p>Note that if evaluating a submit requirement results in an ERROR or TIMEOUT, it is
+ * considered unfulfilled and is counted here.
+ */
+ public static final IndexedField<ChangeData, Integer> UNSATISFIED_REQUIREMENT_COUNT_FIELD =
+ IndexedField.<ChangeData>integerBuilder("UnsatisfiedRequirementCount")
+ .stored()
+ .build(
+ cd ->
+ (int)
+ cd.submitRequirementsIncludingLegacy().values().stream()
+ .filter(sr -> !sr.fulfilled())
+ .count(),
+ (cd, field) -> cd.setUnsatisfiedRequirementCount(field));
+
+ public static final IndexedField<ChangeData, Integer>.SearchSpec
+ UNSATISFIED_REQUIREMENT_COUNT_SPEC =
+ UNSATISFIED_REQUIREMENT_COUNT_FIELD.integerRange("unsatisfied_requirement_count");
+
private static void parseSubmitRequirements(
Iterable<Cache.SubmitRequirementResultProto> values, ChangeData out) {
out.setSubmitRequirements(
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index aaa7535..809dd0a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -274,6 +274,7 @@
@Deprecated static final Schema<ChangeData> V87 = schema(V86);
/** Add REVIEWERS_COUNT_FIELD */
+ @Deprecated
static final Schema<ChangeData> V88 =
new Schema.Builder<ChangeData>()
.add(V87)
@@ -281,6 +282,16 @@
.addSearchSpecs(ChangeField.REVIEWER_COUNT_SPEC)
.build();
+ /** Add met and unmet requirement tracking fields */
+ static final Schema<ChangeData> V89 =
+ new Schema.Builder<ChangeData>()
+ .add(V88)
+ .addIndexedFields(
+ ChangeField.UNMET_REQUIREMENT_FIELD, ChangeField.UNSATISFIED_REQUIREMENT_COUNT_FIELD)
+ .addSearchSpecs(
+ ChangeField.UNMET_REQUIREMENT_SPEC, ChangeField.UNSATISFIED_REQUIREMENT_COUNT_SPEC)
+ .build();
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index a3b1e3e..6dee3eb 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -443,6 +443,7 @@
Maps.newLinkedHashMapWithExpectedSize(1);
private Map<SubmitRequirement, SubmitRequirementResult> submitRequirements;
+ private Integer unsatisfiedRequirementCount;
private StorageConstraint storageConstraint = StorageConstraint.NOTEDB_ONLY;
private Change change;
@@ -1307,6 +1308,14 @@
projectConfigReqs, legacyReqs, this);
}
+ public Integer unsatisfiedRequirementCount() {
+ return unsatisfiedRequirementCount;
+ }
+
+ public void setUnsatisfiedRequirementCount(Integer count) {
+ this.unsatisfiedRequirementCount = count;
+ }
+
/**
* Get all evaluated submit requirements for this change, including those from parent projects.
* For closed changes, submit requirements are read from the change notes. For active changes,
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index fdaa061..6331323 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -242,6 +242,12 @@
ChangeField.HASHTAG_SPEC, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase(Locale.US));
}
+ /** Returns a predicate that matches changes that have an unmet submit requirement. */
+ public static Predicate<ChangeData> unmetRequirement(String requirementName) {
+ return new ChangeIndexPredicate(
+ ChangeField.UNMET_REQUIREMENT_SPEC, requirementName.toLowerCase(Locale.US));
+ }
+
/** Returns a predicate that matches changes tagged with the provided {@code hashtag}. */
public static Predicate<ChangeData> fuzzyHashtag(String hashtag) {
// Use toLowerCase without locale to match behavior in ChangeField.
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 6311c65..68530ef 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -1460,6 +1460,21 @@
}
@Operator
+ public Predicate<ChangeData> unmet_requirement(String requirementName)
+ throws QueryParseException {
+ checkFieldAvailable(ChangeField.UNMET_REQUIREMENT_SPEC, "unmet_requirement");
+ return ChangePredicates.unmetRequirement(requirementName);
+ }
+
+ @Operator
+ public Predicate<ChangeData> unsatisfied_requirement_count(String value)
+ throws QueryParseException {
+ checkFieldAvailable(
+ ChangeField.UNSATISFIED_REQUIREMENT_COUNT_SPEC, "unsatisfied_requirement_count");
+ return new UnsatisfiedRequirementCountPredicate(value);
+ }
+
+ @Operator
public Predicate<ChangeData> cc(String who)
throws QueryParseException, IOException, ConfigInvalidException {
return reviewerByState(who, ReviewerStateInternal.CC, false);
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index 26ccb2c..4ca53c5 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -131,6 +131,21 @@
}
@Override
+ public Predicate<ChangeData> unmet_requirement(String requirementName)
+ throws QueryParseException {
+ throw new QueryParseException(
+ "Operator 'unmet_requirement' cannot be used in submit requirement expressions.");
+ }
+
+ @Override
+ public Predicate<ChangeData> unsatisfied_requirement_count(String value)
+ throws QueryParseException {
+ throw new QueryParseException(
+ "Operator 'unsatisfied_requirement_count' cannot be used in submit requirement"
+ + " expressions.");
+ }
+
+ @Override
public Predicate<ChangeData> has(String value) throws QueryParseException {
if (value.toLowerCase(Locale.US).startsWith(SUBMODULE_UPDATE_HAS_ARG)) {
List<String> args = SUBMODULE_UPDATE_SPLITTER.splitToList(value);
diff --git a/java/com/google/gerrit/server/query/change/UnsatisfiedRequirementCountPredicate.java b/java/com/google/gerrit/server/query/change/UnsatisfiedRequirementCountPredicate.java
new file mode 100644
index 0000000..827558e
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/UnsatisfiedRequirementCountPredicate.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2026 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.query.change;
+
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.index.change.ChangeField;
+
+public class UnsatisfiedRequirementCountPredicate extends IntegerRangeChangePredicate {
+ public UnsatisfiedRequirementCountPredicate(String value) throws QueryParseException {
+ super(ChangeField.UNSATISFIED_REQUIREMENT_COUNT_SPEC, value);
+ }
+
+ @Override
+ protected Integer getValueInt(ChangeData changeData) {
+ Integer count = changeData.unsatisfiedRequirementCount();
+ if (count != null) {
+ return count;
+ }
+ return ChangeField.UNSATISFIED_REQUIREMENT_COUNT_SPEC.get(changeData);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index fa4296b..9205a46 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -559,11 +559,18 @@
ins.setCustomKeyedValues(customKeyedValues.build());
}
+ // Default to NotifyHandling.OWNER for WIP changes to avoid emailing project watchers
+ // before the change is marked ready for review (matching ReceiveCommits and CommitUtil).
+ // Private changes do not change default notify handling; they are filtered by visibility
+ // ACLs.
+ boolean isWorkInProgress =
+ Boolean.TRUE.equals(input.workInProgress) || !c.getFilesWithGitConflicts().isEmpty();
+ NotifyHandling defaultNotify = isWorkInProgress ? NotifyHandling.OWNER : NotifyHandling.ALL;
try (BatchUpdate bu = updateFactory.create(projectState.getNameKey(), me, now)) {
bu.setRepository(git, rw, oi);
bu.setNotify(
notifyResolver.resolve(
- firstNonNull(input.notify, NotifyHandling.ALL), input.notifyDetails));
+ firstNonNull(input.notify, defaultNotify), input.notifyDetails));
bu.insertChange(ins);
bu.execute();
}
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
index 06c2037..305e476 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
@@ -40,6 +40,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
@@ -329,12 +330,29 @@
String.join(
" OR ",
attributes.refPatterns().stream()
- .map(b -> "branch:\\\"" + b + "\\\"")
+ .map(MigrateLabelFunctionsToSubmitRequirement::toApplicableIfExpression)
.collect(Collectors.toList()))));
}
return builder.build();
}
+ private static String toApplicableIfExpression(String branchRef) {
+ // Reqex -> migrate as it is.
+ if (branchRef.startsWith("^")) {
+ return "branch:" + branchRef;
+ }
+ // Wildcard -> needs to converted into gerrit regex.
+ if (branchRef.endsWith("/*")) {
+ String prefix = branchRef.substring(0, branchRef.length() - 1);
+ String regex = "^" + Pattern.quote(prefix) + ".*";
+ return "branch:" + regex;
+ }
+ // If branch with " -> need to escape "
+ branchRef = branchRef.replace("\"", "\\\"");
+ // Other cases e.g. branch with # or " -> needs to be quoted
+ return "branch:\"" + branchRef + "\"";
+ }
+
private static boolean isBlockingOrRequiredLabel(String function) {
return function.equals("AnyWithBlock")
|| function.equals("MaxWithBlock")
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index 5bf1a40..b8d06ef 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -2783,6 +2783,58 @@
}
@Test
+ public void submitRequirement_disallowsUnsatisfiedRequirementCountInExpression()
+ throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Wrong-Req")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("unsatisfied_requirement_count:0"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ SubmitRequirementResultInfo srResult =
+ change.submitRequirements.stream()
+ .filter(sr -> sr.name.equals("Wrong-Req"))
+ .collect(MoreCollectors.onlyElement());
+ assertThat(srResult.status).isEqualTo(Status.ERROR);
+ assertThat(srResult.submittabilityExpressionResult.errorMessage)
+ .isEqualTo(
+ "Operator 'unsatisfied_requirement_count' cannot be used in submit requirement"
+ + " expressions.");
+ }
+
+ @Test
+ public void submitRequirement_disallowsUnmetRequirementInExpression() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Wrong-Req")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("unmet_requirement:Code-Review"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ SubmitRequirementResultInfo srResult =
+ change.submitRequirements.stream()
+ .filter(sr -> sr.name.equals("Wrong-Req"))
+ .collect(MoreCollectors.onlyElement());
+ assertThat(srResult.status).isEqualTo(Status.ERROR);
+ assertThat(srResult.submittabilityExpressionResult.errorMessage)
+ .isEqualTo(
+ "Operator 'unmet_requirement' cannot be used in submit requirement expressions.");
+ }
+
+ @Test
public void submitRequirements_forcedByDirectSubmission() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 3e82712..9f2610f 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -37,6 +37,8 @@
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_ACCOUNTS;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import static com.google.gerrit.extensions.common.testing.EditInfoSubject.assertThat;
import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
@@ -499,7 +501,7 @@
.committer(new PersonIdent(admin.newIdent(), testRepo.getInstant()))
.create();
PushResult result = pushHead(testRepo, "refs/for/master");
- assertThat(result.getMessages()).contains("warning: pushing without Change-Id is deprecated");
+ assertThat(result.getMessages()).contains("WARNING: pushing without Change-Id is deprecated");
}
@Test
@@ -805,6 +807,30 @@
}
@Test
+ public void authorRemainsReviewerOnNewPatchSet() throws Exception {
+ PushOneCommit.Result r = pushTo("refs/for/master");
+ String changeId = r.getChangeId();
+
+ TestAccount user = accountCreator.user1();
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ PushOneCommit push =
+ pushFactory.create(user.newIdent(), testRepo, "Subject", "file.txt", "content", changeId);
+ r = push.to("refs/for/master");
+ r.assertOkStatus();
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ Collection<AccountInfo> reviewers = changeInfo.reviewers.get(REVIEWER);
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewers.stream().anyMatch(a -> a._accountId == user.id().get())).isTrue();
+
+ Collection<AccountInfo> ccs = changeInfo.reviewers.get(CC);
+ if (ccs != null) {
+ assertThat(ccs.stream().anyMatch(a -> a._accountId == user.id().get())).isFalse();
+ }
+ }
+
+ @Test
public void pushForMasterWithReviewerByEmail() throws Exception {
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
@@ -3027,7 +3053,7 @@
assertPushOk(pr, r);
assertThat(pr.getMessages())
.contains(
- "warning: no changes between prior commit "
+ "WARNING: no changes between prior commit "
+ abbreviateName(c)
+ " and new commit "
+ abbreviateName(amended));
@@ -3056,7 +3082,8 @@
pr = pushHead(testRepo, r, false);
assertPushOk(pr, r);
assertThat(pr.getMessages())
- .contains("warning: " + abbreviateName(amended) + ": no files changed, message updated");
+ .contains(
+ "WARNING: commit " + abbreviateName(amended) + ": no files changed, message updated");
}
@Test
@@ -3080,7 +3107,8 @@
pr = pushHead(testRepo, r, false);
assertPushOk(pr, r);
assertThat(pr.getMessages())
- .contains("warning: " + abbreviateName(amended) + ": no files changed, author changed");
+ .contains(
+ "WARNING: commit " + abbreviateName(amended) + ": no files changed, author changed");
}
@Test
@@ -3107,7 +3135,7 @@
pr = pushHead(testRepo, r, false);
assertPushOk(pr, r);
assertThat(pr.getMessages())
- .contains("warning: " + abbreviateName(amended) + ": no files changed, was rebased");
+ .contains("WARNING: commit " + abbreviateName(amended) + ": no files changed, was rebased");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
index 5a6c5c5..e695254 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
@@ -277,7 +277,7 @@
assertExistentSr(
/* srName */ "Foo",
- /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\"",
+ /* applicabilityExpression= */ "branch:\"refs/heads/master\"",
/* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
/* canOverride= */ true);
assertLabelFunction("Foo", "NoBlock");
@@ -299,8 +299,52 @@
assertExistentSr(
/* srName */ "Foo",
- /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
- + "OR branch:\\\"refs/heads/develop\\\"",
+ /* applicabilityExpression= */ "branch:\"refs/heads/master\" "
+ + "OR branch:\"refs/heads/develop\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withQuotesInBranchNameAttribute() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/gerr\"it"));
+
+ assertNonExistentSr(/* srName= */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName= */ "Foo",
+ /* applicabilityExpression= */ "branch:\"refs/heads/gerr\\\"it\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withHashInBranchNameAttribute() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/gerr#it"));
+
+ assertNonExistentSr(/* srName= */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName= */ "Foo",
+ /* applicabilityExpression= */ "branch:\"refs/heads/gerr#it\"",
/* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
/* canOverride= */ true);
assertLabelFunction("Foo", "NoBlock");
@@ -321,8 +365,30 @@
assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
assertExistentSr(
- /* srName */ "Foo",
- /* applicabilityExpression= */ "branch:\\\"^refs/heads/main-.*\\\"",
+ /* srName= */ "Foo",
+ /* applicabilityExpression= */ "branch:^refs/heads/main-.*",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withWildcardBranchAttribute() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/release/*"));
+
+ assertNonExistentSr(/* srName= */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName= */ "Foo",
+ /* applicabilityExpression= */ "branch:^\\Qrefs/heads/release/\\E.*",
/* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
/* canOverride= */ true);
assertLabelFunction("Foo", "NoBlock");
@@ -344,8 +410,8 @@
assertExistentSr(
/* srName */ "Foo",
- /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
- + "OR branch:\\\"^refs/heads/main-.*\\\"",
+ /* applicabilityExpression= */ "branch:\"refs/heads/master\" "
+ + "OR branch:^refs/heads/main-.*",
/* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
/* canOverride= */ true);
assertLabelFunction("Foo", "NoBlock");
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index 5accd00..2f85c88 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -27,11 +27,17 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Address;
+import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.NotifyConfig;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewerInput;
+import com.google.gerrit.extensions.client.InheritableBoolean;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.inject.Inject;
@@ -716,4 +722,143 @@
// assert that there was no email notification for user
assertThat(sender.getMessages()).isEmpty();
}
+
+ // Unlike private changes (which notify users with VIEW_PRIVATE_CHANGES permission as tested
+ // in watchProjectNotifyOnPrivateChange), WIP changes are visible to all readers but suppress
+ // notifications by defaulting notify handling to NotifyHandling.OWNER unless overridden.
+ @Test
+ public void watchProjectNoNotificationForWipChange_createdViaRest() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ ChangeInput input = new ChangeInput();
+ input.project = watchedProject;
+ input.branch = "master";
+ input.subject = "wip change";
+ input.workInProgress = true;
+ gApi.changes().create(input);
+
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void watchProjectNoNotificationForWipChange_pushedWithoutNotify() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(Project.nameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "wip change", "a", "a1")
+ .to("refs/for/master%wip");
+ r.assertOkStatus();
+
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void watchProjectNotificationForWipChange_createdViaRestWithNotifyAll() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ ChangeInput input = new ChangeInput();
+ input.project = watchedProject;
+ input.branch = "master";
+ input.subject = "wip change";
+ input.workInProgress = true;
+ input.notify = NotifyHandling.ALL;
+ gApi.changes().create(input);
+
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(user.getNameEmail());
+ }
+
+ @Test
+ public void watchProjectNoNotificationForWipChange_projectWipByDefault() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ try (ProjectConfigUpdate u = updateProject(Project.nameKey(watchedProject))) {
+ u.getConfig()
+ .updateProject(
+ b ->
+ b.setBooleanConfig(
+ BooleanProjectConfig.WORK_IN_PROGRESS_BY_DEFAULT, InheritableBoolean.TRUE));
+ u.save();
+ }
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ ChangeInput input = new ChangeInput();
+ input.project = watchedProject;
+ input.branch = "master";
+ input.subject = "default wip change";
+ gApi.changes().create(input);
+
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void watchProjectNoNotificationForWipChange_addedAsReviewerOrCc() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ ChangeInput input = new ChangeInput();
+ input.project = watchedProject;
+ input.branch = "master";
+ input.subject = "wip change";
+ input.workInProgress = true;
+ String changeId = gApi.changes().create(input).get().id;
+ assertThat(sender.getMessages()).isEmpty();
+
+ // Adding user as a reviewer on the WIP change does not send watch/reviewer notifications
+ gApi.changes().id(changeId).addReviewer(user.email());
+ assertThat(sender.getMessages()).isEmpty();
+
+ // Adding user2 as CC on the WIP change also does not send notifications
+ TestAccount user2 = accountCreator.user2();
+ requestScopeOperations.setApiUser(user2.id());
+ watch(watchedProject);
+ requestScopeOperations.setApiUser(admin.id());
+ ReviewerInput ccInput = new ReviewerInput();
+ ccInput.reviewer = user2.email();
+ ccInput.state = ReviewerState.CC;
+ gApi.changes().id(changeId).addReviewer(ccInput);
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void watchProjectNoNotificationForWipChange_newPatchsetWhenReviewer() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+ watch(watchedProject);
+
+ requestScopeOperations.setApiUser(admin.id());
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(Project.nameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "wip change", "a", "a1")
+ .to("refs/for/master%wip");
+ r.assertOkStatus();
+ gApi.changes().id(r.getChangeId()).addReviewer(user.email());
+ assertThat(sender.getMessages()).isEmpty();
+
+ // Pushing a new patchset on the WIP change does not notify user (who is both watcher and
+ // reviewer)
+ PushOneCommit.Result r2 =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "wip change", "a", "a2", r.getChangeId())
+ .to("refs/for/master%wip");
+ r2.assertOkStatus();
+ assertThat(sender.getMessages()).isEmpty();
+ }
}
diff --git a/javatests/com/google/gerrit/entities/converter/ChangeInputProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/ChangeInputProtoConverterTest.java
index e64a9e5..05ffa4d 100644
--- a/javatests/com/google/gerrit/entities/converter/ChangeInputProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/ChangeInputProtoConverterTest.java
@@ -177,7 +177,6 @@
.setProject("test-project")
.setBranch("test-branch")
.setSubject("test-subject")
- .setNotify(Entities.NotifyHandling.ALL)
.build();
assertThat(proto).isEqualTo(expectedProto);
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index 53431d1..6fcbf5d 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -105,6 +105,29 @@
}
@Test
+ public void unmetRequirementField() {
+ SubmitRequirementResult sr1 =
+ submitRequirementResult(
+ "Code-Review", "label:CR=+1", SubmitRequirementExpressionResult.Status.PASS);
+ SubmitRequirementResult sr2 =
+ submitRequirementResult(
+ "Verified", "label:V=+1", SubmitRequirementExpressionResult.Status.FAIL);
+
+ assertThat(sr1.fulfilled()).isTrue();
+ assertThat(sr2.fulfilled()).isFalse();
+
+ ChangeData cd = org.mockito.Mockito.mock(ChangeData.class);
+ org.mockito.Mockito.when(cd.submitRequirementsIncludingLegacy())
+ .thenReturn(
+ com.google.common.collect.ImmutableMap.of(
+ sr1.submitRequirement(), sr1,
+ sr2.submitRequirement(), sr2));
+
+ assertThat(ChangeField.UNMET_REQUIREMENT_FIELD.get(cd)).containsExactly("verified");
+ assertThat(ChangeField.UNSATISFIED_REQUIREMENT_COUNT_FIELD.get(cd)).isEqualTo(1);
+ }
+
+ @Test
public void storedSubmitRecords() {
assertStoredRecordRoundTrip(record(SubmitRecord.Status.CLOSED));
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 8d1e3dd..059be1b 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -4559,6 +4559,35 @@
}
@Test
+ public void bySubmitRequirement_unmet() throws Exception {
+ assume().that(getSchema().hasField(ChangeField.UNMET_REQUIREMENT_SPEC)).isTrue();
+ Project.NameKey project = Project.nameKey("repo");
+ repo = createAndOpenProject(project);
+ Change change1 = insert(project, newChange(repo));
+ assertQuery("unmet_requirement:Code-Review", change1);
+
+ approve(change1);
+ assertQuery("unmet_requirement:Code-Review");
+ }
+
+ @Test
+ public void bySubmitRequirement_unsatisfiedCount() throws Exception {
+ assume().that(getSchema().hasField(ChangeField.UNSATISFIED_REQUIREMENT_COUNT_SPEC)).isTrue();
+ Project.NameKey project = Project.nameKey("repo");
+ repo = createAndOpenProject(project);
+ Change change1 = insert(project, newChange(repo));
+ assertQuery("unsatisfied_requirement_count:>0", change1);
+ assertQuery("unsatisfied_requirement_count:1", change1);
+ assertQuery("unsatisfied_requirement_count:<=1", change1);
+ assertQuery("unmet_requirement:Code-Review", change1);
+
+ approve(change1);
+ assertQuery("unsatisfied_requirement_count:0", change1);
+ assertQuery("unsatisfied_requirement_count:<1", change1);
+ assertQuery("unmet_requirement:Code-Review");
+ }
+
+ @Test
public void byUrlEncodedProject() throws Exception {
Project.NameKey project = Project.nameKey("repo+foo");
repo = createAndOpenProject(project);
diff --git a/tools/BUILD b/tools/BUILD
index 8b3429d..539aad5 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -306,6 +306,13 @@
"-Xep:NonOverridingEquals:ERROR",
"-Xep:NonRuntimeAnnotation:ERROR",
"-Xep:NullOptional:ERROR",
+ # Demoted to a warning: false positive on Guava methods like
+ # Iterables.getFirst(iterable, null) when javac cannot read the
+ # @Nullable bound of <T extends @Nullable Object> from class
+ # files. Bazel's remotejdk_21 (Zulu 21.0.9) lacks the
+ # JDK-8341779 backport; JDK 25 is unaffected. See the commit
+ # message introducing this line for the full reference trail.
+ "-Xep:NullArgumentForNonNullParameter:WARN",
"-Xep:NullTernary:ERROR",
"-Xep:NullableConstructor:ERROR",
"-Xep:NullablePrimitive:ERROR",
diff --git a/tools/js/eslint.bzl b/tools/js/eslint.bzl
index 737d01f..473330d 100644
--- a/tools/js/eslint.bzl
+++ b/tools/js/eslint.bzl
@@ -33,6 +33,7 @@
],
extensions = [".ts"],
plugins = [
+ "//:node_modules/@typescript-eslint/eslint-plugin",
"//:node_modules/eslint-config-google",
"//:node_modules/eslint-plugin-html",
"//:node_modules/eslint-plugin-import",
