Fix build failed email notification

Use comment email template instead of reverted template.

Fixes: QTBI-1640
Change-Id: I6023cbf77ea8d657c2faa1b0da64c4d41edf068b
Reviewed-by: Kari Oikarinen <kari.oikarinen@qt.io>

5 files changed, 210 insertions, 17 deletions

diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtBuildFailedSender.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtBuildFailedSender.java
new file mode 100644
index 0000000..eea7082
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtBuildFailedSender.java
@@ -0,0 +1,60 @@
+//
+// Copyright (C) 2019 The Qt Company
+//
+
+package com.googlesource.gerrit.plugins.qtcodereview;
+
+import com.google.gerrit.common.errors.EmailException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.account.ProjectWatches.NotifyType;
+import com.google.gerrit.server.mail.send.EmailArguments;
+import com.google.gerrit.server.mail.send.ReplyToChangeSender;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import java.util.ArrayList;
+import java.util.List;
+
+// Send email when build has failed
+public class QtBuildFailedSender extends ReplyToChangeSender {
+    public interface Factory {
+        QtBuildFailedSender create(Project.NameKey project, Change.Id id);
+    }
+
+    @Inject
+    public QtBuildFailedSender(EmailArguments ea,
+                               @Assisted Project.NameKey project,
+                               @Assisted Change.Id id)
+                               throws OrmException {
+        super(ea, "comment", newChangeData(ea, project, id));
+    }
+
+    @Override
+    protected void init() throws EmailException {
+        super.init();
+
+        ccAllApprovals();
+        bccStarredBy();
+        includeWatchers(NotifyType.ALL_COMMENTS);
+        removeUsersThatIgnoredTheChange();
+    }
+
+    @Override
+    protected void formatChange() throws EmailException {
+        appendText(textTemplate("Comment"));
+    }
+
+    @Override
+    protected void setupSoyContext() {
+        super.setupSoyContext();
+        List<String> emptyList = new ArrayList<>();
+        soyContext.put("commentFiles", emptyList);
+    }
+
+    @Override
+    protected boolean supportsHtml() {
+        return false;
+    }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
index f14adc1..fb8058f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
@@ -18,7 +18,6 @@ import com.google.gerrit.server.extensions.events.ChangeMerged;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.mail.send.MergedSender;
-import com.google.gerrit.server.mail.send.RevertedSender;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
@@ -50,6 +49,7 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.sql.Timestamp;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map.Entry;
 
@@ -79,7 +79,7 @@ class QtCommandBuildApprove extends SshCommand {
     private MergedSender.Factory mergedSenderFactory;
 
     @Inject
-    private RevertedSender.Factory revertedSenderFactory;
+    QtBuildFailedSender.Factory qtBuildFailedSenderFactory;
 
     @Inject
     private BatchUpdate.Factory updateFactory;
@@ -264,14 +264,14 @@ class QtCommandBuildApprove extends SshCommand {
             Change change = cd.change();
             if (passed) {
                 sendMergeEvent(cd);
+                sendMergedEmail(change.getId());
                 logger.atInfo().log("qtcodereview: staging-approve     change %s merged into %s",
                                     change, destBranchKey);
             } else {
+                sendBuildFailedEmail(change.getId());
                 logger.atInfo().log("qtcodereview: staging-approve     change %s rejected for %s",
                                     change, destBranchKey);
             }
-
-            sendResultEmail(change.getId(), passed);
         }
     }
 
@@ -309,19 +309,24 @@ class QtCommandBuildApprove extends SshCommand {
         }
     }
 
-    private void sendResultEmail(Change.Id changeId, Boolean passed) {
+    private void sendMergedEmail(Change.Id changeId) {
         try {
-            if (passed) {
-                MergedSender mcm = mergedSenderFactory.create(projectKey, changeId);
-                mcm.setFrom(user.getAccountId());
-                mcm.send();
-            } else {
-                RevertedSender rcm = revertedSenderFactory.create(projectKey, changeId);
-                rcm.setFrom(user.getAccountId());
-                rcm.send();
-            }
+            MergedSender mcm = mergedSenderFactory.create(projectKey, changeId);
+            mcm.setFrom(user.getAccountId());
+            mcm.send();
+        } catch (Exception e) {
+            logger.atWarning().log("qtcodereview: staging-approve Merged notification not sent for %s %s", changeId, e);
+        }
+    }
+
+    private void sendBuildFailedEmail(Change.Id changeId) {
+        try {
+            QtBuildFailedSender cm = qtBuildFailedSenderFactory.create(projectKey, changeId);
+            cm.setFrom(user.getAccountId());
+            cm.setChangeMessage(message, TimeUtil.nowTs());
+            cm.send();
         } catch (Exception e) {
-            logger.atWarning().log("qtcodereview: staging-approve Cannot email notification for %s %s", changeId, e);
+            logger.atWarning().log("qtcodereview: staging-approve Build Failed not sent notification for %s %s", changeId, e);
         }
     }
 
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtModule.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtModule.java
index a3bf187..c264e3a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtModule.java
@@ -23,6 +23,7 @@ public class QtModule extends FactoryModule  {
     @Override
     protected void configure() {
 
+        factory(QtBuildFailedSender.Factory.class);
         factory(QtChangeUpdateOp.Factory.class);
         DynamicSet.bind(binder(), ChangeMessageModifier.class).to(QtChangeMessageModifier.class);
 
diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java
index 8aa2e76..6c0427e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java
@@ -48,6 +48,8 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest {
     protected static final String R_PUSH = "refs/for/";
 
     protected static final String CONTENT_DATA = "hereisjustsomecontentforthecommits";
+    protected static final String BUILD_PASS_MESSAGE = "thebuildpassed";
+    protected static final String BUILD_FAIL_MESSAGE = "thebuildfailed";
 
     @Before
     public void ReduceLogging() throws Exception {
@@ -132,7 +134,7 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest {
         commandStr += " --branch " + branch;
         commandStr += " --build-id "+ buildId;
         commandStr += " --result pass";
-        commandStr += " --message thebuildpassed";
+        commandStr += " --message " + BUILD_PASS_MESSAGE;
         String resultStr = adminSshSession.exec(commandStr);
         assertThat(adminSshSession.getError()).isNull();
     }
@@ -144,7 +146,7 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest {
         commandStr += " --branch " + branch;
         commandStr += " --build-id "+ buildId;
         commandStr += " --result fail";
-        commandStr += " --message thebuildfailed";
+        commandStr += " --message " + BUILD_FAIL_MESSAGE;
         String resultStr = adminSshSession.exec(commandStr);
         assertThat(adminSshSession.getError()).isNull();
     }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java
new file mode 100644
index 0000000..195c7cd
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java
@@ -0,0 +1,125 @@
+//
+// Copyright (C) 2019 The Qt Company
+//
+
+package com.googlesource.gerrit.plugins.qtcodereview;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.mail.Address;
+import com.google.gerrit.mail.EmailHeader;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.testing.Util;
+import com.google.gerrit.testing.FakeEmailSender;
+import org.junit.Before;
+import org.junit.Test;
+
+@TestPlugin(
+    name = "gerrit-plugin-qt-workflow",
+    sysModule = "com.googlesource.gerrit.plugins.qtcodereview.QtModule",
+    sshModule = "com.googlesource.gerrit.plugins.qtcodereview.QtSshModule"
+)
+
+@UseSsh
+public class QtEmailSendingIT extends QtCodeReviewIT {
+
+    @Before
+    public void grantPermissions() throws Exception {
+        grant(project, "refs/heads/master", Permission.QT_STAGE, false, REGISTERED_USERS);
+        grant(project, "refs/staging/*", Permission.PUSH, false, adminGroupUuid());
+        grant(project, "refs/builds/*", Permission.CREATE, false, adminGroupUuid());
+
+        ProjectConfig cfg = projectCache.get(project).getConfig();
+        Util.allow(cfg, Permission.forLabel("Code-Review"), -2, +2, REGISTERED_USERS, "refs/*");
+    }
+
+    @Test
+    public void stagedByOwnerBuildPass() throws Exception {
+        PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
+        approve(c.getChangeId());
+        QtStage(c);
+        QtNewBuild("master", "test_build_01");
+        sender.clear();
+        QtApproveBuild("master", "test_build_01");
+
+        FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "merged").get(0);
+        Address expectedTo = new Address(user.fullName, user.email);
+        assertThat(m.rcpt()).containsExactly(expectedTo);
+        assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
+          .containsExactly(expectedTo);
+        assertThat(m.body()).contains(c.getChangeId());
+    }
+
+    @Test
+    public void stagedByOwnerBuildFail() throws Exception {
+        PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
+        approve(c.getChangeId());
+        QtStage(c);
+        QtNewBuild("master", "test_build_02");
+        sender.clear();
+        QtFailBuild("master", "test_build_02");
+
+        FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "comment").get(0);
+        Address expectedTo = new Address(user.fullName, user.email);
+        assertThat(m.rcpt()).containsExactly(expectedTo);
+        assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
+            .containsExactly(expectedTo);
+        assertThat(m.body()).contains(c.getChangeId());
+        assertThat(m.body()).contains(BUILD_FAIL_MESSAGE);
+    }
+
+    @Test
+    public void stagedByOtherBuildPass() throws Exception {
+        PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
+        approve(c.getChangeId());
+        QtStageByAdmin(c);
+        QtNewBuild("master", "test_build_03");
+        sender.clear();
+        QtApproveBuild("master", "test_build_03");
+
+        FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "merged").get(0);
+        Address expectedTo = new Address(user.fullName, user.email);
+        assertThat(m.rcpt()).containsExactly(expectedTo);
+        assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
+            .containsExactly(expectedTo);
+        assertThat(m.body()).contains(c.getChangeId());
+    }
+
+    @Test
+    public void stagedByOtherBuildFail() throws Exception {
+        PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
+        approve(c.getChangeId());
+        QtStageByAdmin(c);
+        QtNewBuild("master", "test_build_04");
+        sender.clear();
+        QtFailBuild("master", "test_build_04");
+
+        FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "comment").get(0);
+        Address expectedTo = new Address(user.fullName, user.email);
+        assertThat(m.rcpt()).containsExactly(expectedTo);
+        assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
+            .containsExactly(expectedTo);
+        assertThat(m.body()).contains(c.getChangeId());
+        assertThat(m.body()).contains(BUILD_FAIL_MESSAGE);
+    }
+
+    private void QtStageByAdmin(PushOneCommit.Result c) throws Exception {
+        RestResponse response = call_REST_API_Stage_By_Admin(c.getChangeId(), c.getCommit().getName());
+        response.assertOK();
+        assertThat(c.getChange().change().getStatus()).isEqualTo(Change.Status.STAGED);
+    }
+
+    private RestResponse call_REST_API_Stage_By_Admin(String changeId, String revisionId) throws Exception {
+        String url = "/changes/" + changeId + "/revisions/" + revisionId + "/gerrit-plugin-qt-workflow~stage";
+        RestResponse response = adminRestSession.post(url);
+        return response;
+    }
+
+}