Include details of failed check into checks notification email
If a check fails and posting it causes the combined check state to be
updated
a) from anything to FAILED
b) from anything but FAILED to WARNING
the user likely wants to know which check failed, hence the details of
this check should be included into the combined check state updated
email notifications.
This means check details are always included if the combined check state
is updated to a less good value (from anything to FAILED, from anything
but FAILED to WARNING). An update of the combined check state from
FAILED to WARNING is never caused by posting a failed check (but e.g, by
rerunning a FAILED required checker or updating the check state for a
required checker from FAILED to SUCCESSFUL). Hence in this case no check
details are included.
The logic to include check details into the notification emails is fully
implemented in the soy templates. For this we provide more data (old
combined check state, checker data, check data) to the template. This
has the advantage that everyone can easily modify and adapt the email
contents.
Since checkers and checks have some optional fields (checker URL, check
message, check URL), which should be included into the email if they are
present, the conditional logic in the soy templates is a little more
complex, hence there are dedeciated tests for these optional fields that
cover this logic.
Bug: Issue 11730
Change-Id: I797d895d9c12ed7d82b3d7d55c81d5117762b998
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
index ab50f70..ecfcd76 100644
--- a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
@@ -54,13 +54,14 @@
ChecksUpdate createWithServerIdent();
}
+ private final ChecksStorageUpdate checksStorageUpdate;
private final CombinedCheckStateCache combinedCheckStateCache;
private final CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory;
private final ChangeNotes.Factory notesFactory;
private final PatchSetUtil psUtil;
+ private final Checkers checkers;
private final NotifyResolver notifyResolver;
private final Optional<IdentifiedUser> currentUser;
- private final ChecksStorageUpdate checksStorageUpdate;
@AssistedInject
ChecksUpdate(
@@ -69,15 +70,17 @@
CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
ChangeNotes.Factory notesFactory,
PatchSetUtil psUtil,
+ Checkers checkers,
NotifyResolver notifyResolver,
@Assisted IdentifiedUser currentUser) {
+ this.checksStorageUpdate = checksStorageUpdate;
this.combinedCheckStateCache = combinedCheckStateCache;
this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
this.notesFactory = notesFactory;
this.psUtil = psUtil;
+ this.checkers = checkers;
this.notifyResolver = notifyResolver;
this.currentUser = Optional.of(currentUser);
- this.checksStorageUpdate = checksStorageUpdate;
}
@AssistedInject
@@ -87,14 +90,16 @@
CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
ChangeNotes.Factory notesFactory,
PatchSetUtil psUtil,
+ Checkers checkers,
NotifyResolver notifyResolver) {
+ this.checksStorageUpdate = checksStorageUpdate;
this.combinedCheckStateCache = combinedCheckStateCache;
this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
this.notesFactory = notesFactory;
this.psUtil = psUtil;
+ this.checkers = checkers;
this.notifyResolver = notifyResolver;
this.currentUser = Optional.empty();
- this.checksStorageUpdate = checksStorageUpdate;
}
public Check createCheck(
@@ -110,9 +115,8 @@
CombinedCheckState newCombinedCheckState =
combinedCheckStateCache.get(key.repository(), key.patchSet());
- if (oldCombinedCheckState != newCombinedCheckState) {
- sendEmail(notifyHandling, notifyDetails, key, newCombinedCheckState);
- }
+ maybeSendEmail(
+ notifyHandling, notifyDetails, check, oldCombinedCheckState, newCombinedCheckState);
return check;
}
@@ -130,28 +134,35 @@
CombinedCheckState newCombinedCheckState =
combinedCheckStateCache.get(key.repository(), key.patchSet());
- if (oldCombinedCheckState != newCombinedCheckState) {
- sendEmail(notifyHandling, notifyDetails, key, newCombinedCheckState);
- }
+ maybeSendEmail(
+ notifyHandling, notifyDetails, check, oldCombinedCheckState, newCombinedCheckState);
return check;
}
- private void sendEmail(
+ private void maybeSendEmail(
@Nullable NotifyHandling notifyHandling,
@Nullable Map<RecipientType, NotifyInfo> notifyDetails,
- CheckKey checkKey,
- CombinedCheckState combinedCheckState)
+ Check updatedCheck,
+ CombinedCheckState oldCombinedCheckState,
+ CombinedCheckState newCombinedCheckState)
throws BadRequestException, IOException, ConfigInvalidException {
+ if (oldCombinedCheckState == newCombinedCheckState) {
+ // do not send an email if the combined check state was not updated
+ return;
+ }
+
notifyHandling =
notifyHandling != null
? notifyHandling
- : combinedCheckState == CombinedCheckState.SUCCESSFUL
- || combinedCheckState == CombinedCheckState.NOT_RELEVANT
+ : newCombinedCheckState == CombinedCheckState.SUCCESSFUL
+ || newCombinedCheckState == CombinedCheckState.NOT_RELEVANT
? NotifyHandling.ALL
: NotifyHandling.OWNER;
NotifyResolver.Result notify = notifyResolver.resolve(notifyHandling, notifyDetails);
+ CheckKey checkKey = updatedCheck.key();
+
try {
CombinedCheckStateUpdatedSender sender =
combinedCheckStateUpdatedSenderFactory.create(
@@ -165,8 +176,17 @@
notesFactory.create(checkKey.repository(), checkKey.patchSet().changeId());
PatchSet patchSet = psUtil.get(changeNotes, checkKey.patchSet());
sender.setPatchSet(patchSet);
-
- sender.setCombinedCheckState(combinedCheckState);
+ sender.setCombinedCheckState(oldCombinedCheckState, newCombinedCheckState);
+ sender.setCheck(
+ checkers
+ .getChecker(checkKey.checkerUuid())
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "checker %s of check %s not found",
+ checkKey.checkerUuid(), checkKey))),
+ updatedCheck);
sender.setNotify(notify);
sender.send();
} catch (Exception e) {
diff --git a/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
index 9e117be..b4adcfd 100644
--- a/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
+++ b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
@@ -14,9 +14,14 @@
package com.google.gerrit.plugins.checks.email;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.EmailException;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.Checker;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import com.google.gerrit.server.mail.send.ChangeEmail;
@@ -24,6 +29,8 @@
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.util.HashMap;
+import java.util.Map;
/** Send notice about an update of the combined check state of a change. */
public class CombinedCheckStateUpdatedSender extends ReplyToChangeSender {
@@ -32,7 +39,10 @@
CombinedCheckStateUpdatedSender create(Project.NameKey project, Change.Id changeId);
}
- private CombinedCheckState combinedCheckState;
+ private CombinedCheckState oldCombinedCheckState;
+ private CombinedCheckState newCombinedCheckState;
+ private Checker checker;
+ private Check check;
@Inject
public CombinedCheckStateUpdatedSender(
@@ -50,16 +60,59 @@
removeUsersThatIgnoredTheChange();
}
- public void setCombinedCheckState(CombinedCheckState combinedCheckState) {
- this.combinedCheckState = combinedCheckState;
+ public void setCombinedCheckState(
+ CombinedCheckState oldCombinedCheckState, CombinedCheckState newCombinedCheckState) {
+ this.oldCombinedCheckState = requireNonNull(oldCombinedCheckState);
+ this.newCombinedCheckState = requireNonNull(newCombinedCheckState);
+ }
+
+ public void setCheck(Checker checker, Check check) {
+ requireNonNull(check, "check is missing");
+ requireNonNull(checker, "checker is missing");
+ checkState(
+ check.key().checkerUuid().equals(checker.getUuid()),
+ "checker %s doesn't match check %s",
+ checker.getUuid(),
+ check.key());
+
+ this.checker = checker;
+ this.check = check;
}
@Override
protected void setupSoyContext() {
super.setupSoyContext();
- if (combinedCheckState != null) {
- soyContext.put("combinedCheckState", combinedCheckState.name());
+ if (oldCombinedCheckState != null) {
+ soyContext.put("oldCombinedCheckState", oldCombinedCheckState.name());
+ }
+
+ if (newCombinedCheckState != null) {
+ soyContext.put("newCombinedCheckState", newCombinedCheckState.name());
+ }
+
+ if (checker != null) {
+ Map<String, String> checkerData = new HashMap<>();
+ checkerData.put("uuid", checker.getUuid().get());
+ checkerData.put("name", checker.getName());
+ checkerData.put("repository", checker.getRepository().get());
+ checker
+ .getDescription()
+ .ifPresent(description -> checkerData.put("description", description));
+ checker.getUrl().ifPresent(url -> checkerData.put("url", url));
+ soyContext.put("checker", checkerData);
+ }
+
+ if (check != null) {
+ Map<String, Object> checkData = new HashMap<>();
+ checkData.put("checkerUuid", check.key().checkerUuid().get());
+ checkData.put("change", check.key().patchSet().changeId().get());
+ checkData.put("patchSet", check.key().patchSet().get());
+ checkData.put("repository", check.key().repository().get());
+ checkData.put("state", check.state().name());
+ check.message().ifPresent(message -> checkData.put("message", message));
+ check.url().ifPresent(url -> checkData.put("url", url));
+ soyContext.put("check", checkData);
}
}
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 480fbf8..a7377ee 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -546,12 +547,38 @@
}
@Test
- public void verifyMessageBodiesForCombinedCheckStateUpdatedEmail() throws Exception {
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedDefaultEmail() throws Exception {
CheckerUuid checkerUuid =
checkerOperations.newChecker().repository(project).required().create();
assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
sender.clear();
+ postCheck(checkerUuid, CheckState.SUCCESSFUL);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.SUCCESSFUL);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.SUCCESSFUL)
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.SUCCESSFUL)
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToFailedEmail() throws Exception {
+ String checkerName = "My Checker";
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name(checkerName).required().create();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
postCheck(checkerUuid, CheckState.FAILED);
assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
@@ -561,27 +588,238 @@
Message message = messages.get(0);
assertThat(message.body())
.isEqualTo(
- "The combined check state has been updated to "
- + CombinedCheckState.FAILED
- + " for patch set "
- + patchSetId.get()
- + " of this change ( "
- + changeUrl(change)
+ combinedCheckStateUpdatedText(CombinedCheckState.FAILED)
+ + "\n"
+ + "Checker "
+ + checkerName
+ + " updated the check state to "
+ + CheckState.FAILED
+ + ".\n"
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.FAILED)
+ + "<p>Checker <strong>"
+ + checkerName
+ + "</strong> updated the check state to "
+ + CheckState.FAILED
+ + ".</p>"
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToFailedEmailWithCheckerUrl()
+ throws Exception {
+ String checkerName = "My Checker";
+ String checkerUrl = "http://my-checker/";
+ CheckerUuid checkerUuid =
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .name(checkerName)
+ .url(checkerUrl)
+ .required()
+ .create();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
+ postCheck(checkerUuid, CheckState.FAILED);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.FAILED)
+ + "\n"
+ + "Checker "
+ + checkerName
+ + " ( "
+ + checkerUrl
+ + " ) updated the check state to "
+ + CheckState.FAILED
+ + ".\n"
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.FAILED)
+ + "<p>Checker <a href=\""
+ + checkerUrl
+ + "\">"
+ + checkerName
+ + "</a> updated the check state to "
+ + CheckState.FAILED
+ + ".</p>"
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToFailedEmailWithCheckMessage()
+ throws Exception {
+ String checkerName = "My Checker";
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name(checkerName).required().create();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
+ String checkMessage = "foo bar baz";
+ postCheck(checkerUuid, CheckState.FAILED, checkMessage);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.FAILED)
+ + "\n"
+ + "Checker "
+ + checkerName
+ + " updated the check state to "
+ + CheckState.FAILED
+ + ":\n"
+ + checkMessage
+ + "\n"
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.FAILED)
+ + "<p>Checker <strong>"
+ + checkerName
+ + "</strong> updated the check state to "
+ + CheckState.FAILED
+ + ":<br>"
+ + checkMessage
+ + "</p>"
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToFailedEmailWithCheckUrl()
+ throws Exception {
+ String checkerName = "My Checker";
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name(checkerName).required().create();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
+ String checkUrl = "http://my-checker/12345";
+ postCheck(checkerUuid, CheckState.FAILED, null, checkUrl);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.FAILED)
+ + "\n"
+ + "Checker "
+ + checkerName
+ + " updated the check state to "
+ + CheckState.FAILED
+ + " ( "
+ + checkUrl
+ " ).\n"
+ textEmailFooterForCombinedCheckStateUpdate());
assertThat(message.htmlBody())
.isEqualTo(
- "<p>The combined check state has been updated to <strong>"
- + CombinedCheckState.FAILED
- + "</strong> for patch set "
- + patchSetId.get()
- + " of this <a href=\""
- + changeUrl(change)
- + "\">change</a>.</p>"
+ combinedCheckStateUpdatedHtml(CombinedCheckState.FAILED)
+ + "<p>Checker <strong>"
+ + checkerName
+ + "</strong> updated the check state to <a href=\""
+ + checkUrl
+ + "\">"
+ + CheckState.FAILED
+ + "</a>.</p>"
+ htmlEmailFooterForCombinedCheckStateUpdate());
}
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToWarningEmail() throws Exception {
+ String checkerName = "My Checker";
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name(checkerName).create();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
+ postCheck(checkerUuid, CheckState.FAILED);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.WARNING);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.WARNING)
+ + "\n"
+ + "Checker "
+ + checkerName
+ + " updated the check state to "
+ + CheckState.FAILED
+ + ".\n"
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.WARNING)
+ + "<p>Checker <strong>"
+ + checkerName
+ + "</strong> updated the check state to "
+ + CheckState.FAILED
+ + ".</p>"
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ @Test
+ public void verifyMessageBodiesForCombinedCheckStateUpdatedToWarningFromFailedEmail()
+ throws Exception {
+ CheckerUuid checkerUuidRequired =
+ checkerOperations.newChecker().repository(project).required().create();
+ postCheck(checkerUuidRequired, CheckState.FAILED);
+
+ CheckerUuid checkerUuidOptional = checkerOperations.newChecker().repository(project).create();
+ postCheck(checkerUuidOptional, CheckState.FAILED);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ sender.clear();
+ postCheck(checkerUuidRequired, CheckState.SUCCESSFUL);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.WARNING);
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.body())
+ .isEqualTo(
+ combinedCheckStateUpdatedText(CombinedCheckState.WARNING)
+ + textEmailFooterForCombinedCheckStateUpdate());
+
+ assertThat(message.htmlBody())
+ .isEqualTo(
+ combinedCheckStateUpdatedHtml(CombinedCheckState.WARNING)
+ + htmlEmailFooterForCombinedCheckStateUpdate());
+ }
+
+ private String combinedCheckStateUpdatedText(CombinedCheckState combinedCheckState) {
+ return "The combined check state has been updated to "
+ + combinedCheckState
+ + " for patch set "
+ + patchSetId.get()
+ + " of this change ( "
+ + changeUrl(change)
+ + " ).\n";
+ }
+
private String textEmailFooterForCombinedCheckStateUpdate() {
return "\n"
+ "Change subject: "
@@ -629,6 +867,16 @@
+ "Gerrit-MessageType: combinedCheckStateUpdate\n";
}
+ private String combinedCheckStateUpdatedHtml(CombinedCheckState combinedCheckState) {
+ return "<p>The combined check state has been updated to <strong>"
+ + combinedCheckState
+ + "</strong> for patch set "
+ + patchSetId.get()
+ + " of this <a href=\""
+ + changeUrl(change)
+ + "\">change</a>.</p>";
+ }
+
private String htmlViewChangeButton() {
return "<p><a href=\"" + changeUrl(change) + "\">View Change</a></p>";
}
@@ -698,10 +946,26 @@
}
private void postCheck(CheckerUuid checkerUuid, CheckState checkState) throws RestApiException {
+ postCheck(checkerUuid, checkState, null);
+ }
+
+ private void postCheck(CheckerUuid checkerUuid, CheckState checkState, @Nullable String message)
+ throws RestApiException {
+ postCheck(checkerUuid, checkState, message, null);
+ }
+
+ private void postCheck(
+ CheckerUuid checkerUuid,
+ CheckState checkState,
+ @Nullable String message,
+ @Nullable String url)
+ throws RestApiException {
requestScopeOperations.setApiUser(bot.id());
CheckInput input = new CheckInput();
input.checkerUuid = checkerUuid.get();
input.state = checkState;
+ input.message = message;
+ input.url = url;
checksApiFactory.revision(patchSetId).create(input).get();
}
}
diff --git a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
index 8666da6..978146c 100644
--- a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
@@ -24,11 +24,29 @@
{@param email: ?}
{@param change: ?}
{@param patchSet: ?}
- {@param combinedCheckState: ?}
- The combined check state has been updated to {$combinedCheckState} for patch set{sp}
+ {@param checker: ?}
+ {@param check: ?}
+ {@param oldCombinedCheckState: ?}
+ {@param newCombinedCheckState: ?}
+ The combined check state has been updated to {$newCombinedCheckState} for patch set{sp}
{$patchSet.patchSetId} of this change
- {if $email.changeUrl} ( {$email.changeUrl} ){/if}
+ {if $email.changeUrl}{sp}( {$email.changeUrl} ){/if}
.{\n}
+ {if $check and $checker and $check.state == 'FAILED'
+ and ($newCombinedCheckState == 'FAILED'
+ or ($newCombinedCheckState == 'WARNING' and $oldCombinedCheckState != 'FAILED'))}
+ {\n}
+ Checker {$checker.name}
+ {if $checker.url}{sp}( {$checker.url} ){/if}
+ {sp}updated the check state to {$check.state}
+ {if $check.url}{sp}( {$check.url} ){/if}
+ {if $check.message}
+ :{\n}
+ {$check.message}{\n}
+ {else}
+ .{\n}
+ {/if}
+ {/if}
{\n}
Change subject: {$change.subject}{\n}
......................................................................{\n}
diff --git a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
index 4d23ffd..692b074 100644
--- a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
@@ -23,16 +23,42 @@
{template .CombinedCheckStateUpdatedHtml}
{@param email: ?}
{@param patchSet: ?}
- {@param combinedCheckState: ?}
+ {@param checker: ?}
+ {@param check: ?}
+ {@param oldCombinedCheckState: ?}
+ {@param newCombinedCheckState: ?}
<p>
- The combined check state has been updated to <strong>{$combinedCheckState}</strong> for patch
- set {$patchSet.patchSetId} of this{sp}
+ The combined check state has been updated to <strong>{$newCombinedCheckState}</strong> for{sp}
+ patch set {$patchSet.patchSetId} of this{sp}
{if $email.changeUrl}
<a href="{$email.changeUrl}">change</a>
{else}
change
{/if}.
</p>
+ {if $check and $checker and $check.state == 'FAILED'
+ and ($newCombinedCheckState == 'FAILED'
+ or ($newCombinedCheckState == 'WARNING' and $oldCombinedCheckState != 'FAILED'))}
+ <p>
+ Checker{sp}
+ {if $checker.url}
+ <a href="{$checker.url}">{$checker.name}</a>
+ {else}
+ <strong>{$checker.name}</strong>
+ {/if}
+ {sp}updated the check state to{sp}
+ {if $check.url}
+ <a href="{$check.url}">{$check.state}</a>
+ {else}
+ {$check.state}
+ {/if}
+ {if $check.message}
+ :<br>{$check.message}
+ {else}
+ .
+ {/if}
+ </p>
+ {/if}
{if $email.changeUrl}
<p>
