commit ebb24b8c0e0d722e6db3903afc32ec9d76f7cb73
author Edwin Kempin <ekempin@google.com> Tue Oct 15 09:51:23 2019 +0200
committer Edwin Kempin <ekempin@google.com> Wed Oct 16 09:35:21 2019 +0200
tree 46982610fc2996d293e9fda118ed698e5d0cf591
parent 0f370e7f076662a2171d30e6f653fbec6523fef6

Fix/test/simplify soy template for combined check state updated html emails

- Make text 'change' a link to the change.
- Remove unused coverLetter parameter.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I7a4b471eb085c2b49d50e9823018305496e6df0c

javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java

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 8967d2f..480fbf8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -546,7 +546,7 @@
   }
 
   @Test
-  public void textEmailForCombinedCheckStateUpdated() throws Exception {
+  public void verifyMessageBodiesForCombinedCheckStateUpdatedEmail() throws Exception {
     CheckerUuid checkerUuid =
         checkerOperations.newChecker().repository(project).required().create();
     assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
@@ -568,10 +568,21 @@
                 + " of this change ( "
                 + changeUrl(change)
                 + " ).\n"
-                + emailFooterForCombinedCheckStateUpdate());
+                + 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>"
+                + htmlEmailFooterForCombinedCheckStateUpdate());
   }
 
-  private String emailFooterForCombinedCheckStateUpdate() {
+  private String textEmailFooterForCombinedCheckStateUpdate() {
     return "\n"
         + "Change subject: "
         + change.getSubject()
@@ -618,6 +629,58 @@
         + "Gerrit-MessageType: combinedCheckStateUpdate\n";
   }
 
+  private String htmlViewChangeButton() {
+    return "<p><a href=\"" + changeUrl(change) + "\">View Change</a></p>";
+  }
+
+  private String htmlEmailFooterForCombinedCheckStateUpdate() {
+    return htmlViewChangeButton()
+        + "<p>To view, visit <a href=\""
+        + changeUrl(change)
+        + "\">change "
+        + change.getChangeId()
+        + "</a>."
+        + " To unsubscribe, or for help writing mail filters, visit <a href=\""
+        + canonicalWebUrl.get()
+        + "settings\">settings</a>.</p>"
+        + "<div itemscope itemtype=\"http://schema.org/EmailMessage\">"
+        + "<div itemscope itemprop=\"action\" itemtype=\"http://schema.org/ViewAction\">"
+        + "<link itemprop=\"url\" href=\""
+        + changeUrl(change)
+        + "\"/>"
+        + "<meta itemprop=\"name\" content=\"View Change\"/>"
+        + "</div>"
+        + "</div>\n\n"
+        + "<div style=\"display:none\"> Gerrit-Project: "
+        + project.get()
+        + " </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Branch: "
+        + change.getDest().shortName()
+        + " </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Change-Id: "
+        + change.getKey().get()
+        + " </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Change-Number: "
+        + change.getChangeId()
+        + " </div>\n"
+        + "<div style=\"display:none\"> Gerrit-PatchSet: "
+        + patchSetId.get()
+        + " </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Owner: Administrator &lt;admin@example.com&gt; </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Reviewer: "
+        + ignoringReviewer.fullName()
+        + " &lt;"
+        + ignoringReviewer.email()
+        + "&gt; </div>\n"
+        + "<div style=\"display:none\"> Gerrit-Reviewer: "
+        + reviewer.fullName()
+        + " &lt;"
+        + reviewer.email()
+        + "&gt; </div>\n"
+        + "<div style=\"display:none\"> Gerrit-MessageType: combinedCheckStateUpdate </div>\n"
+        + "\n";
+  }
+
   private String changeUrl(Change change) {
     return canonicalWebUrl.get() + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
   }

resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy

diff --git a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
index 8e7e5ff..4d23ffd 100644
--- a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
@@ -21,13 +21,17 @@
  * a change for which the combined check state was updated.
  */
 {template .CombinedCheckStateUpdatedHtml}
-  {@param combinedCheckState: ?}
-  {@param coverLetter: ?}
   {@param email: ?}
   {@param patchSet: ?}
+  {@param combinedCheckState: ?}
   <p>
     The combined check state has been updated to <strong>{$combinedCheckState}</strong> for patch
-    set {$patchSet.patchSetId} of this change.
+    set {$patchSet.patchSetId} of this{sp}
+    {if $email.changeUrl}
+      <a href="{$email.changeUrl}">change</a>
+    {else}
+      change
+    {/if}.
   </p>
 
   {if $email.changeUrl}
@@ -35,8 +39,4 @@
       {call .ViewChangeButton data="all" /}
     </p>
   {/if}
-
-  {if $coverLetter}
-    <div style="white-space:pre-wrap">{$coverLetter}</div>
-  {/if}
 {/template}