Fix/test/simplify soy template for combined check state updated text emails
- A space between 'patch set' and the patch set number was missing.
- The change URL in parenthesis appeared outside of a sentence.
- Remove unused coverLetter parameter.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I989fa991bc4eb4656660a3a592bf6e447658e414
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 0851564..8967d2f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -22,11 +22,13 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
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.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.NotifyInfo;
@@ -58,9 +60,11 @@
private TestAccount bot;
private TestAccount owner;
+ private TestAccount ignoringReviewer;
private TestAccount reviewer;
private TestAccount starrer;
private TestAccount watcher;
+ private Change change;
private PatchSet.Id patchSetId;
@Before
@@ -78,7 +82,9 @@
// Create a change.
owner = admin;
- patchSetId = createChange().getPatchSetId();
+ PushOneCommit.Result result = createChange();
+ change = result.getChange().change();
+ patchSetId = result.getPatchSetId();
// Add a reviewer.
reviewer = accountCreator.create("reviewer", "reviewer@test.com", "Reviewer");
@@ -110,10 +116,10 @@
gApi.accounts().self().setWatchedProjects(ImmutableList.of(projectWatchInfo));
// Add a reviewer that ignores the change --> user doesn't get notified by checks plugin.
- TestAccount ignorer = accountCreator.create("ignorer", "ignorer@test.com", "Ignorer");
+ ignoringReviewer = accountCreator.create("ignorer", "ignorer@test.com", "Ignorer");
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(patchSetId.changeId().get()).addReviewer(ignorer.username());
- requestScopeOperations.setApiUser(ignorer.id());
+ gApi.changes().id(patchSetId.changeId().get()).addReviewer(ignoringReviewer.username());
+ requestScopeOperations.setApiUser(ignoringReviewer.id());
gApi.changes().id(patchSetId.changeId().get()).ignore(true);
// Reset request scope to admin.
@@ -539,6 +545,83 @@
}
}
+ @Test
+ public void textEmailForCombinedCheckStateUpdated() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).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(
+ "The combined check state has been updated to "
+ + CombinedCheckState.FAILED
+ + " for patch set "
+ + patchSetId.get()
+ + " of this change ( "
+ + changeUrl(change)
+ + " ).\n"
+ + emailFooterForCombinedCheckStateUpdate());
+ }
+
+ private String emailFooterForCombinedCheckStateUpdate() {
+ return "\n"
+ + "Change subject: "
+ + change.getSubject()
+ + "\n"
+ + "......................................................................\n"
+ + "-- \n"
+ + "To view, visit "
+ + changeUrl(change)
+ + "\n"
+ + "To unsubscribe, or for help writing mail filters, visit "
+ + canonicalWebUrl.get()
+ + "settings\n"
+ + "\n"
+ + "Gerrit-Project: "
+ + project.get()
+ + "\n"
+ + "Gerrit-Branch: "
+ + change.getDest().shortName()
+ + "\n"
+ + "Gerrit-Change-Id: "
+ + change.getKey().get()
+ + "\n"
+ + "Gerrit-Change-Number: "
+ + change.getChangeId()
+ + "\n"
+ + "Gerrit-PatchSet: "
+ + patchSetId.get()
+ + "\n"
+ + "Gerrit-Owner: "
+ + owner.fullName()
+ + " <"
+ + owner.email()
+ + ">\n"
+ + "Gerrit-Reviewer: "
+ + ignoringReviewer.fullName()
+ + " <"
+ + ignoringReviewer.email()
+ + ">\n"
+ + "Gerrit-Reviewer: "
+ + reviewer.fullName()
+ + " <"
+ + reviewer.email()
+ + ">\n"
+ + "Gerrit-MessageType: combinedCheckStateUpdate\n";
+ }
+
+ private String changeUrl(Change change) {
+ return canonicalWebUrl.get() + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
+ }
+
private CombinedCheckState getCombinedCheckState() throws RestApiException {
ChangeInfo changeInfo =
gApi.changes()
@@ -550,4 +633,12 @@
assertThat(infos.get(0)).isInstanceOf(ChangeCheckInfo.class);
return ((ChangeCheckInfo) infos.get(0)).combinedState;
}
+
+ private void postCheck(CheckerUuid checkerUuid, CheckState checkState) throws RestApiException {
+ requestScopeOperations.setApiUser(bot.id());
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = checkState;
+ 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 83d67b1..8666da6 100644
--- a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
@@ -21,21 +21,15 @@
* change for which the combined check state was updated.
*/
{template .CombinedCheckStateUpdated kind="text"}
- {@param change: ?}
- {@param combinedCheckState: ?}
- {@param coverLetter: ?}
{@param email: ?}
+ {@param change: ?}
{@param patchSet: ?}
- The combined check state has been updated to {$combinedCheckState} for patch set
- {$patchSet.patchSetId} of this change.
- {if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
+ {@param combinedCheckState: ?}
+ The combined check state has been updated to {$combinedCheckState} for patch set{sp}
+ {$patchSet.patchSetId} of this change
+ {if $email.changeUrl} ( {$email.changeUrl} ){/if}
+ .{\n}
{\n}
Change subject: {$change.subject}{\n}
......................................................................{\n}
- {if $coverLetter}
- {\n}
- {\n}
- {$coverLetter}
- {\n}
- {/if}
{/template}
