Skip to content

Commit dd41d9e

Browse files
jujokinijujokini
committed
Fix build failed email notification
Use comment email template instead of reverted template. Fixes: QTBI-1640 Change-Id: I6023cbf77ea8d657c2faa1b0da64c4d41edf068b Reviewed-by: Kari Oikarinen <[email protected]>
1 parent 5ccbe44 commit dd41d9e

File tree

5 files changed

+210
-17
lines changed

5 files changed

+210
-17
lines changed

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtBuildFailedSender.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//
2+
// Copyright (C) 2019 The Qt Company
3+
//
4+
5+
package com.googlesource.gerrit.plugins.qtcodereview;
6+
7+
import com.google.gerrit.common.errors.EmailException;
8+
import com.google.gerrit.reviewdb.client.Change;
9+
import com.google.gerrit.reviewdb.client.Project;
10+
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
11+
import com.google.gerrit.server.mail.send.EmailArguments;
12+
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
13+
import com.google.gwtorm.server.OrmException;
14+
import com.google.inject.Inject;
15+
import com.google.inject.assistedinject.Assisted;
16+
17+
import java.util.ArrayList;
18+
import java.util.List;
19+
20+
// Send email when build has failed
21+
public class QtBuildFailedSender extends ReplyToChangeSender {
22+
public interface Factory {
23+
QtBuildFailedSender create(Project.NameKey project, Change.Id id);
24+
}
25+
26+
@Inject
27+
public QtBuildFailedSender(EmailArguments ea,
28+
@Assisted Project.NameKey project,
29+
@Assisted Change.Id id)
30+
throws OrmException {
31+
super(ea, "comment", newChangeData(ea, project, id));
32+
}
33+
34+
@Override
35+
protected void init() throws EmailException {
36+
super.init();
37+
38+
ccAllApprovals();
39+
bccStarredBy();
40+
includeWatchers(NotifyType.ALL_COMMENTS);
41+
removeUsersThatIgnoredTheChange();
42+
}
43+
44+
@Override
45+
protected void formatChange() throws EmailException {
46+
appendText(textTemplate("Comment"));
47+
}
48+
49+
@Override
50+
protected void setupSoyContext() {
51+
super.setupSoyContext();
52+
List<String> emptyList = new ArrayList<>();
53+
soyContext.put("commentFiles", emptyList);
54+
}
55+
56+
@Override
57+
protected boolean supportsHtml() {
58+
return false;
59+
}
60+
}

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
1919
import com.google.gerrit.server.git.GitRepositoryManager;
2020
import com.google.gerrit.server.mail.send.MergedSender;
21-
import com.google.gerrit.server.mail.send.RevertedSender;
2221
import com.google.gerrit.server.permissions.PermissionBackend;
2322
import com.google.gerrit.server.permissions.PermissionBackendException;
2423
import com.google.gerrit.server.permissions.ProjectPermission;
@@ -50,6 +49,7 @@
5049
import java.io.IOException;
5150
import java.io.InputStreamReader;
5251
import java.sql.Timestamp;
52+
import java.util.Arrays;
5353
import java.util.List;
5454
import java.util.Map.Entry;
5555

@@ -79,7 +79,7 @@ class QtCommandBuildApprove extends SshCommand {
7979
private MergedSender.Factory mergedSenderFactory;
8080

8181
@Inject
82-
private RevertedSender.Factory revertedSenderFactory;
82+
QtBuildFailedSender.Factory qtBuildFailedSenderFactory;
8383

8484
@Inject
8585
private BatchUpdate.Factory updateFactory;
@@ -264,14 +264,14 @@ private void updateChanges(List<Entry<ChangeData,RevCommit>> list,
264264
Change change = cd.change();
265265
if (passed) {
266266
sendMergeEvent(cd);
267+
sendMergedEmail(change.getId());
267268
logger.atInfo().log("qtcodereview: staging-approve change %s merged into %s",
268269
change, destBranchKey);
269270
} else {
271+
sendBuildFailedEmail(change.getId());
270272
logger.atInfo().log("qtcodereview: staging-approve change %s rejected for %s",
271273
change, destBranchKey);
272274
}
273-
274-
sendResultEmail(change.getId(), passed);
275275
}
276276
}
277277

@@ -309,19 +309,24 @@ private void readMessageParameter() throws UnloggedFailure {
309309
}
310310
}
311311

312-
private void sendResultEmail(Change.Id changeId, Boolean passed) {
312+
private void sendMergedEmail(Change.Id changeId) {
313313
try {
314-
if (passed) {
315-
MergedSender mcm = mergedSenderFactory.create(projectKey, changeId);
316-
mcm.setFrom(user.getAccountId());
317-
mcm.send();
318-
} else {
319-
RevertedSender rcm = revertedSenderFactory.create(projectKey, changeId);
320-
rcm.setFrom(user.getAccountId());
321-
rcm.send();
322-
}
314+
MergedSender mcm = mergedSenderFactory.create(projectKey, changeId);
315+
mcm.setFrom(user.getAccountId());
316+
mcm.send();
317+
} catch (Exception e) {
318+
logger.atWarning().log("qtcodereview: staging-approve Merged notification not sent for %s %s", changeId, e);
319+
}
320+
}
321+
322+
private void sendBuildFailedEmail(Change.Id changeId) {
323+
try {
324+
QtBuildFailedSender cm = qtBuildFailedSenderFactory.create(projectKey, changeId);
325+
cm.setFrom(user.getAccountId());
326+
cm.setChangeMessage(message, TimeUtil.nowTs());
327+
cm.send();
323328
} catch (Exception e) {
324-
logger.atWarning().log("qtcodereview: staging-approve Cannot email notification for %s %s", changeId, e);
329+
logger.atWarning().log("qtcodereview: staging-approve Build Failed not sent notification for %s %s", changeId, e);
325330
}
326331
}
327332

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtModule.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public class QtModule extends FactoryModule {
2323
@Override
2424
protected void configure() {
2525

26+
factory(QtBuildFailedSender.Factory.class);
2627
factory(QtChangeUpdateOp.Factory.class);
2728
DynamicSet.bind(binder(), ChangeMessageModifier.class).to(QtChangeMessageModifier.class);
2829

‎src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest {
4848
protected static final String R_PUSH = "refs/for/";
4949

5050
protected static final String CONTENT_DATA = "hereisjustsomecontentforthecommits";
51+
protected static final String BUILD_PASS_MESSAGE = "thebuildpassed";
52+
protected static final String BUILD_FAIL_MESSAGE = "thebuildfailed";
5153

5254
@Before
5355
public void ReduceLogging() throws Exception {
@@ -132,7 +134,7 @@ protected void QtApproveBuild(String branch, String buildId) throws Exception {
132134
commandStr += " --branch " + branch;
133135
commandStr += " --build-id "+ buildId;
134136
commandStr += " --result pass";
135-
commandStr += " --message thebuildpassed";
137+
commandStr += " --message " + BUILD_PASS_MESSAGE;
136138
String resultStr = adminSshSession.exec(commandStr);
137139
assertThat(adminSshSession.getError()).isNull();
138140
}
@@ -144,7 +146,7 @@ protected void QtFailBuild(String branch, String buildId) throws Exception {
144146
commandStr += " --branch " + branch;
145147
commandStr += " --build-id "+ buildId;
146148
commandStr += " --result fail";
147-
commandStr += " --message thebuildfailed";
149+
commandStr += " --message " + BUILD_FAIL_MESSAGE;
148150
String resultStr = adminSshSession.exec(commandStr);
149151
assertThat(adminSshSession.getError()).isNull();
150152
}

‎src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
//
2+
// Copyright (C) 2019 The Qt Company
3+
//
4+
5+
package com.googlesource.gerrit.plugins.qtcodereview;
6+
7+
import static com.google.common.truth.Truth.assertThat;
8+
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
9+
10+
import com.google.gerrit.acceptance.PushOneCommit;
11+
import com.google.gerrit.acceptance.RestResponse;
12+
import com.google.gerrit.acceptance.TestPlugin;
13+
import com.google.gerrit.acceptance.UseSsh;
14+
import com.google.gerrit.common.data.Permission;
15+
import com.google.gerrit.mail.Address;
16+
import com.google.gerrit.mail.EmailHeader;
17+
import com.google.gerrit.reviewdb.client.Change;
18+
import com.google.gerrit.server.project.ProjectConfig;
19+
import com.google.gerrit.server.project.testing.Util;
20+
import com.google.gerrit.testing.FakeEmailSender;
21+
import org.junit.Before;
22+
import org.junit.Test;
23+
24+
@TestPlugin(
25+
name = "gerrit-plugin-qt-workflow",
26+
sysModule = "com.googlesource.gerrit.plugins.qtcodereview.QtModule",
27+
sshModule = "com.googlesource.gerrit.plugins.qtcodereview.QtSshModule"
28+
)
29+
30+
@UseSsh
31+
public class QtEmailSendingIT extends QtCodeReviewIT {
32+
33+
@Before
34+
public void grantPermissions() throws Exception {
35+
grant(project, "refs/heads/master", Permission.QT_STAGE, false, REGISTERED_USERS);
36+
grant(project, "refs/staging/*", Permission.PUSH, false, adminGroupUuid());
37+
grant(project, "refs/builds/*", Permission.CREATE, false, adminGroupUuid());
38+
39+
ProjectConfig cfg = projectCache.get(project).getConfig();
40+
Util.allow(cfg, Permission.forLabel("Code-Review"), -2, +2, REGISTERED_USERS, "refs/*");
41+
}
42+
43+
@Test
44+
public void stagedByOwnerBuildPass() throws Exception {
45+
PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
46+
approve(c.getChangeId());
47+
QtStage(c);
48+
QtNewBuild("master", "test_build_01");
49+
sender.clear();
50+
QtApproveBuild("master", "test_build_01");
51+
52+
FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "merged").get(0);
53+
Address expectedTo = new Address(user.fullName, user.email);
54+
assertThat(m.rcpt()).containsExactly(expectedTo);
55+
assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
56+
.containsExactly(expectedTo);
57+
assertThat(m.body()).contains(c.getChangeId());
58+
}
59+
60+
@Test
61+
public void stagedByOwnerBuildFail() throws Exception {
62+
PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
63+
approve(c.getChangeId());
64+
QtStage(c);
65+
QtNewBuild("master", "test_build_02");
66+
sender.clear();
67+
QtFailBuild("master", "test_build_02");
68+
69+
FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "comment").get(0);
70+
Address expectedTo = new Address(user.fullName, user.email);
71+
assertThat(m.rcpt()).containsExactly(expectedTo);
72+
assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
73+
.containsExactly(expectedTo);
74+
assertThat(m.body()).contains(c.getChangeId());
75+
assertThat(m.body()).contains(BUILD_FAIL_MESSAGE);
76+
}
77+
78+
@Test
79+
public void stagedByOtherBuildPass() throws Exception {
80+
PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
81+
approve(c.getChangeId());
82+
QtStageByAdmin(c);
83+
QtNewBuild("master", "test_build_03");
84+
sender.clear();
85+
QtApproveBuild("master", "test_build_03");
86+
87+
FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "merged").get(0);
88+
Address expectedTo = new Address(user.fullName, user.email);
89+
assertThat(m.rcpt()).containsExactly(expectedTo);
90+
assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
91+
.containsExactly(expectedTo);
92+
assertThat(m.body()).contains(c.getChangeId());
93+
}
94+
95+
@Test
96+
public void stagedByOtherBuildFail() throws Exception {
97+
PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
98+
approve(c.getChangeId());
99+
QtStageByAdmin(c);
100+
QtNewBuild("master", "test_build_04");
101+
sender.clear();
102+
QtFailBuild("master", "test_build_04");
103+
104+
FakeEmailSender.Message m = sender.getMessages(c.getChangeId(), "comment").get(0);
105+
Address expectedTo = new Address(user.fullName, user.email);
106+
assertThat(m.rcpt()).containsExactly(expectedTo);
107+
assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
108+
.containsExactly(expectedTo);
109+
assertThat(m.body()).contains(c.getChangeId());
110+
assertThat(m.body()).contains(BUILD_FAIL_MESSAGE);
111+
}
112+
113+
private void QtStageByAdmin(PushOneCommit.Result c) throws Exception {
114+
RestResponse response = call_REST_API_Stage_By_Admin(c.getChangeId(), c.getCommit().getName());
115+
response.assertOK();
116+
assertThat(c.getChange().change().getStatus()).isEqualTo(Change.Status.STAGED);
117+
}
118+
119+
private RestResponse call_REST_API_Stage_By_Admin(String changeId, String revisionId) throws Exception {
120+
String url = "/changes/" + changeId + "/revisions/" + revisionId + "/gerrit-plugin-qt-workflow~stage";
121+
RestResponse response = adminRestSession.post(url);
122+
return response;
123+
}
124+
125+
}

0 commit comments

Comments
 (0)