From f8501d6f1301789482a8d67d98c43e7a9adde9cb Mon Sep 17 00:00:00 2001 From: Tomas Ljunggren Date: Sun, 11 Mar 2012 17:57:56 +0100 Subject: Don't send mail for publishing comments for stage approved Task-number: QTQAINFRA-340 Change-Id: Ie034bdab9cf5f83b55cedf2c51f5b4bfa0aac0e9 --- .../httpd/rpc/patch/PatchDetailServiceImpl.java | 2 +- .../gerrit/server/mail/BuildApprovedSender.java | 38 ++++++++++++++++++++++ .../gerrit/server/patch/PublishComments.java | 11 +++++-- .../com/google/gerrit/server/mail/BuildApproved.vm | 2 +- .../server/mail/BuildApprovedSenderTest.java | 22 +++++++++++++ .../google/gerrit/sshd/commands/ReviewCommand.java | 2 +- .../gerrit/sshd/commands/StagingApprove.java | 9 ++--- 7 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 gerrit-server/src/test/java/com/google/gerrit/server/mail/BuildApprovedSenderTest.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index 300d1ae0d9..b3730cd216 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java @@ -129,7 +129,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements public void publishComments(final PatchSet.Id psid, final String msg, final Set tags, final AsyncCallback cb) { - Handler.wrap(publishCommentsFactory.create(psid, msg, tags)).to(cb); + Handler.wrap(publishCommentsFactory.create(psid, msg, tags, true)).to(cb); } /** diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/BuildApprovedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/BuildApprovedSender.java index f4ec23f166..b56b4ef49e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/BuildApprovedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/BuildApprovedSender.java @@ -27,6 +27,9 @@ import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.StringReader; import java.util.HashMap; import java.util.Map; @@ -37,6 +40,7 @@ public class BuildApprovedSender extends ReplyToChangeSender { } private final ApprovalTypes approvalTypes; + private String buildApprovedMessage; @Inject public BuildApprovedSender(EmailArguments ea, ApprovalTypes at, @Assisted Change c) { @@ -44,6 +48,10 @@ public class BuildApprovedSender extends ReplyToChangeSender { approvalTypes = at; } + public void setBuildApprovedMessage(final String m) { + buildApprovedMessage = m; + } + @Override protected void init() throws EmailException { super.init(); @@ -83,6 +91,36 @@ public class BuildApprovedSender extends ReplyToChangeSender { return ""; } + public String getBuildApprovedMessage() { + if (buildApprovedMessage == null) { + return ""; + } + StringBuilder txt = new StringBuilder(); + txt.append("Message:"); + txt.append('\n'); + BufferedReader r = new BufferedReader(new StringReader(buildApprovedMessage)); + String l; + try { + l = r.readLine(); + while (l != null) { + txt.append(" "); // Indent 2 spaces for each line in message + txt.append(l); + txt.append('\n'); + l = r.readLine(); + } + } catch (IOException e) { + // Ignore + } finally { + try { + r.close(); + } catch (IOException e) { + // Ignore + } + } + txt.append('\n'); + return txt.toString(); + } + private String format(final String type, final Map> list) { StringBuilder txt = new StringBuilder(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java index ce2e796d4d..686f0fb0c4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java @@ -67,7 +67,7 @@ public class PublishComments implements Callable { public interface Factory { PublishComments create(PatchSet.Id patchSetId, String messageText, - Set approvals); + Set approvals, boolean sendMail); } private final ReviewDb db; @@ -92,6 +92,7 @@ public class PublishComments implements Callable { private PatchSet patchSet; private ChangeMessage message; private List drafts; + private boolean sendMail = true; @Inject PublishComments(final ReviewDb db, final IdentifiedUser user, @@ -107,7 +108,8 @@ public class PublishComments implements Callable { @GerritServerConfig final Config config, @Assisted final PatchSet.Id patchSetId, @Assisted final String messageText, - @Assisted final Set approvals) { + @Assisted final Set approvals, + @Assisted final boolean sendMail) { this.db = db; this.user = user; this.types = approvalTypes; @@ -123,6 +125,7 @@ public class PublishComments implements Callable { this.patchSetId = patchSetId; this.messageText = messageText; this.approvals = approvals; + this.sendMail = sendMail; } @Override @@ -156,7 +159,9 @@ public class PublishComments implements Callable { } touchChange(); - email(); + if (sendMail) { + email(); + } fireHook(); return VoidResult.INSTANCE; } diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/BuildApproved.vm b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/BuildApproved.vm index 4d05422c9e..75fbdb2dee 100644 --- a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/BuildApproved.vm +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/BuildApproved.vm @@ -41,4 +41,4 @@ Change subject: $change.subject ...................................................................... -$email.changeDetail$email.approvals +$email.changeDetail$email.approvals$email.buildApprovedMessage diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/BuildApprovedSenderTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/BuildApprovedSenderTest.java new file mode 100644 index 0000000000..290a9f19ab --- /dev/null +++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/BuildApprovedSenderTest.java @@ -0,0 +1,22 @@ +package com.google.gerrit.server.mail; + +import static org.junit.Assert.*; + +import org.junit.Test; + +public class BuildApprovedSenderTest { + + @Test + public void testGetBuildApprovedMessage() { + + String message = "This is a\nmessage\nover several\nlines with indented\nrows"; + String expected = "Message:\n This is a\n message\n over several\n lines with indented\n rows\n\n"; + + + BuildApprovedSender sender = new BuildApprovedSender(null, null, null); + assertEquals("", sender.getBuildApprovedMessage()); + sender.setBuildApprovedMessage(message); + assertEquals(expected, sender.getBuildApprovedMessage()); + } + +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 7085a0466b..a50441d95b 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -292,7 +292,7 @@ public class ReviewCommand extends BaseCommand { } } - publishCommentsFactory.create(patchSetId, changeComment, aps).call(); + publishCommentsFactory.create(patchSetId, changeComment, aps, true).call(); try { if (abandonChange) { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java index 9cc6fe3bd8..c0830c27c7 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StagingApprove.java @@ -218,6 +218,9 @@ public class StagingApprove extends BaseCommand { // Validate change status and destination branch. validateChanges(); + // Use current message or read it from stdin. + prepareMessage(); + // If result is passed, check that the user has required access rights // to submit changes. if (passed) { @@ -237,9 +240,6 @@ public class StagingApprove extends BaseCommand { merger, hooks); } - // Use current message or read it from stdin. - prepareMessage(); - // Iterate through each open change and publish message. for (PatchSet patchSet : toApprove) { final PatchSet.Id patchSetId = patchSet.getId(); @@ -345,7 +345,7 @@ public class StagingApprove extends BaseCommand { IOException, InvalidChangeOperationException { if (message != null && message.length() > 0) { publishCommentsFactory.create(patchSetId, message, - new HashSet()).call(); + new HashSet(), false).call(); } } @@ -493,6 +493,7 @@ public class StagingApprove extends BaseCommand { final Change change = db.changes().get(changeId); final BuildApprovedSender sender = buildApprovedFactory.create(change); + sender.setBuildApprovedMessage(message); sender.setFrom(currentUser.getAccountId()); sender.setPatchSet(patchSet); sender.send(); -- cgit v1.2.3