Merge "Remove _legacyUndefinedCheck"
diff --git a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
index 4d6cc54..63df867 100644
--- a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
@@ -159,6 +159,14 @@
return;
}
+ CheckKey checkKey = updatedCheck.key();
+ ChangeNotes changeNotes =
+ notesFactory.create(checkKey.repository(), checkKey.patchSet().changeId());
+ if (!checkKey.patchSet().equals(changeNotes.getCurrentPatchSet().id())) {
+ // do not send an email for non-current patch sets
+ return;
+ }
+
notifyHandling =
notifyHandling != null
? notifyHandling
@@ -168,8 +176,6 @@
: NotifyHandling.OWNER;
NotifyResolver.Result notify = notifyResolver.resolve(notifyHandling, notifyDetails);
- CheckKey checkKey = updatedCheck.key();
-
try {
CombinedCheckStateUpdatedSender sender =
combinedCheckStateUpdatedSenderFactory.create(
@@ -179,8 +185,6 @@
sender.setFrom(currentUser.get().getAccountId());
}
- ChangeNotes changeNotes =
- notesFactory.create(checkKey.repository(), checkKey.patchSet().changeId());
PatchSet patchSet = psUtil.get(changeNotes, checkKey.patchSet());
sender.setPatchSet(patchSet);
sender.setCombinedCheckState(oldCombinedCheckState, newCombinedCheckState);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
index c3c5fd1..2edc6a6 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -1065,6 +1065,30 @@
+ htmlEmailFooterForCombinedCheckStateUpdate());
}
+ @Test
+ public void noEmailForCombinedCheckStateUpdatesOfNonCurrentPatchSet() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().name("My Checker").repository(project).create();
+
+ // push a new patch set
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT,
+ PushOneCommit.FILE_NAME,
+ "new content " + System.nanoTime(),
+ change.getKey().get());
+ push.to("refs/for/master").assertOkStatus();
+
+ sender.clear();
+
+ // post check on old patch set
+ postCheck(checkerUuid, CheckState.SUCCESSFUL);
+
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
private String combinedCheckStateUpdatedText(CombinedCheckState combinedCheckState) {
return "The combined check state has been updated to "
+ combinedCheckState
diff --git a/resources/Documentation/rest-api-checks.md b/resources/Documentation/rest-api-checks.md
index b64b32f..0d35ac8 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -204,16 +204,16 @@
| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
-| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to posting this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `ALL` if the combined check state is updated to either `SUCCESSFUL` or `NOT_RELEVANT`, otherwise the default is `OWNER`.
-| `notify_details`| optional | Additional information about whom to notify when the combined check state changes due to posting this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity.
+| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to posting this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `ALL` if the combined check state is updated to either `SUCCESSFUL` or `NOT_RELEVANT`, otherwise the default is `OWNER`. Regardless of this setting there are no email notifications for posting checks on non-current patch sets.
+| `notify_details`| optional | Additional information about whom to notify when the combined check state changes due to posting this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity. Regardless of this setting there are no email notifications for posting checks on non-current patch sets.
### <a id="rerun-input"> RerunInput
The `RerunInput` entity contains information for rerunning a check.
| Field Name | | Description |
| --------------- | -------- | ----------- |
-| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to rerunning this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `OWNER`.
-| `notify_details`| optional | Additional information about whom to notify when the combined check state changes due to rerunning this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity.
+| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to rerunning this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `OWNER`. Regardless of this setting there are no email notifications for rerunning checks on non-current patch sets.
+| `notify_details`| optional | Additional information about whom to notify when the combined check state changes due to rerunning this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity. Regardless of this setting there are no email notifications for rerunning checks on non-current patch sets.
### <a id="check-state"> CheckState (enum)
The `CheckState` enum can have the following values: `NOT_STARTED`, `FAILED`,
