From 49ab6f597e17ff2be9d51aefb37406bfb36217c4 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 7 Dec 2016 10:40:32 +0100 Subject: [PATCH] Send email notification to user that is assigned to a change Users want to get informed when they are assigned to a change. So far there was no email notification on assigning a change to a user. Change-Id: I7953b552f9814c283392b02520f5129e97cff9ff Signed-off-by: Edwin Kempin --- Documentation/config-mail.txt | 6 ++ .../acceptance/rest/change/AssigneeIT.java | 7 +- .../gerrit/pgm/init/SitePathInitializer.java | 2 + .../gerrit/server/change/SetAssigneeOp.java | 25 ++++++- .../server/config/GerritGlobalModule.java | 2 + .../server/mail/send/MailSoyTofuProvider.java | 2 + .../server/mail/send/SetAssigneeSender.java | 73 +++++++++++++++++++ .../google/gerrit/server/mail/SetAssignee.soy | 71 ++++++++++++++++++ .../gerrit/server/mail/SetAssigneeHtml.soy | 49 +++++++++++++ 9 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SetAssigneeSender.java create mode 100644 gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssignee.soy create mode 100644 gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy diff --git a/Documentation/config-mail.txt b/Documentation/config-mail.txt index 63a4344aaa..b8a2e5fb85 100644 --- a/Documentation/config-mail.txt +++ b/Documentation/config-mail.txt @@ -120,6 +120,12 @@ The Reverted templates will determine the contents of the email related to a change being reverted. It is a `ChangeEmail`: see `ChangeSubject.soy` and ChangeFooter. +=== SetAssignee.soy and SetAssigneeHtml.soy + +The SetAssignee templates will determine the contents of the email related to a +user being assigned to a change. It is a `ChangeEmail`: see `ChangeSubject.soy` +and ChangeFooter. + == Mail Variables and Methods diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AssigneeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AssigneeIT.java index 685cceb607..822e48b660 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AssigneeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AssigneeIT.java @@ -25,6 +25,7 @@ import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.api.changes.AssigneeInput; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.testutil.FakeEmailSender.Message; import com.google.gerrit.testutil.TestTimeUtil; import org.junit.AfterClass; @@ -59,6 +60,10 @@ public class AssigneeIT extends AbstractDaemonTest { assertThat(setAssignee(r, user.email)._accountId) .isEqualTo(user.getId().get()); assertThat(getAssignee(r)._accountId).isEqualTo(user.getId().get()); + + assertThat(sender.getMessages()).hasSize(1); + Message m = sender.getMessages().get(0); + assertThat(m.rcpt()).containsExactly(user.emailAddress); } @Test @@ -66,7 +71,7 @@ public class AssigneeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); setAssignee(r, user.email); assertThat(setAssignee(r, user.email)._accountId) - .isEqualTo(user.getId().get()); + .isEqualTo(user.getId().get()); } @Test diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java index 7d6dc32fae..7487e2b68c 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java @@ -127,6 +127,8 @@ public class SitePathInitializer { extractMailExample("RestoredHtml.soy"); extractMailExample("Reverted.soy"); extractMailExample("RevertedHtml.soy"); + extractMailExample("SetAssignee.soy"); + extractMailExample("SetAssigneeHtml.soy"); if (!ui.isBatch()) { System.err.println(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java index 44275ab861..47323c2b86 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java @@ -32,16 +32,24 @@ import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.extensions.events.AssigneeChanged; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate.Context; +import com.google.gerrit.server.mail.send.SetAssigneeSender; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.validators.AssigneeValidationListener; import com.google.gerrit.server.validators.ValidationException; import com.google.gwtorm.server.OrmException; +import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.Optional; public class SetAssigneeOp extends BatchUpdate.Op { + private static final Logger log = + LoggerFactory.getLogger(SetAssigneeOp.class); + public interface Factory { SetAssigneeOp create(String assignee); } @@ -53,6 +61,8 @@ public class SetAssigneeOp extends BatchUpdate.Op { private final String assignee; private final String anonymousCowardName; private final AssigneeChanged assigneeChanged; + private final SetAssigneeSender.Factory setAssigneeSenderFactory; + private final Provider user; private Change change; private Account newAssignee; @@ -63,8 +73,10 @@ public class SetAssigneeOp extends BatchUpdate.Op { ChangeMessagesUtil cmUtil, AccountInfoCacheFactory.Factory accountInfosFactory, DynamicSet validationListeners, - AssigneeChanged assigneeChanged, @AnonymousCowardName String anonymousCowardName, + AssigneeChanged assigneeChanged, + SetAssigneeSender.Factory setAssigneeSenderFactory, + Provider user, @Assisted String assignee) { this.accounts = accounts; this.cmUtil = cmUtil; @@ -73,6 +85,8 @@ public class SetAssigneeOp extends BatchUpdate.Op { this.assigneeChanged = assigneeChanged; this.anonymousCowardName = anonymousCowardName; this.assignee = checkNotNull(assignee); + this.setAssigneeSenderFactory = setAssigneeSenderFactory; + this.user = user; } @Override @@ -138,6 +152,15 @@ public class SetAssigneeOp extends BatchUpdate.Op { @Override public void postUpdate(Context ctx) throws OrmException { + try { + SetAssigneeSender cm = setAssigneeSenderFactory + .create(change.getProject(), change.getId(), newAssignee.getId()); + cm.setFrom(user.get().getAccountId()); + cm.send(); + } catch (Exception err) { + log.error("Cannot send email to new assignee of change " + change.getId(), + err); + } assigneeChanged.fire(change, ctx.getAccount(), oldAssignee, ctx.getWhen()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index acae738607..551c421494 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -143,6 +143,7 @@ import com.google.gerrit.server.mail.send.MailTemplates; import com.google.gerrit.server.mail.send.MergedSender; import com.google.gerrit.server.mail.send.RegisterNewEmailSender; import com.google.gerrit.server.mail.send.ReplacePatchSetSender; +import com.google.gerrit.server.mail.send.SetAssigneeSender; import com.google.gerrit.server.mail.send.VelocityRuntimeProvider; import com.google.gerrit.server.mime.FileTypeRegistry; import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry; @@ -259,6 +260,7 @@ public class GerritGlobalModule extends FactoryModule { factory(ProjectState.Factory.class); factory(RegisterNewEmailSender.Factory.class); factory(ReplacePatchSetSender.Factory.class); + factory(SetAssigneeSender.Factory.class); bind(PermissionCollection.Factory.class); bind(AccountVisibility.class) .toProvider(AccountVisibilityProvider.class) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java index 9f95defa14..72bafdee08 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java @@ -66,6 +66,8 @@ public class MailSoyTofuProvider implements Provider { "RestoredHtml.soy", "Reverted.soy", "RevertedHtml.soy", + "SetAssignee.soy", + "SetAssigneeHtml.soy", }; private final SitePaths site; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SetAssigneeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SetAssigneeSender.java new file mode 100644 index 0000000000..41ad9b9bdb --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SetAssigneeSender.java @@ -0,0 +1,73 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.mail.send; + +import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.mail.RecipientType; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +public class SetAssigneeSender extends ChangeEmail { + public interface Factory { + SetAssigneeSender create(Project.NameKey project, Change.Id id, + Account.Id assignee); + } + + private final Account.Id assignee; + + @Inject + public SetAssigneeSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id, + @Assisted Account.Id assignee) + throws OrmException { + super(ea, "setassignee", newChangeData(ea, project, id)); + this.assignee = assignee; + } + + @Override + protected void init() throws EmailException { + super.init(); + + add(RecipientType.TO, assignee); + } + + @Override + protected void formatChange() throws EmailException { + appendText(textTemplate("SetAssignee")); + if (useHtml()) { + appendHtml(soyHtmlTemplate("SetAssigneeHtml")); + } + } + + public String getAssigneeName() { + return getNameFor(assignee); + } + + @Override + protected void setupSoyContext() { + super.setupSoyContext(); + soyContextEmailData.put("assigneeName", getAssigneeName()); + } + + @Override + protected boolean supportsHtml() { + return true; + } +} diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssignee.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssignee.soy new file mode 100644 index 0000000000..ca4f26700c --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssignee.soy @@ -0,0 +1,71 @@ +/** + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +{namespace com.google.gerrit.server.mail.template} + +/** + * The .SetAssignee template will determine the contents of the email related + * to a user being assigned to a change. + * @param change + * @param email + * @param fromName + * @param patchSet + * @param projectName + */ +{template .SetAssignee autoescape="strict" kind="text"} + Hello{sp} + {$email.assigneeName}, + + {\n} + {\n} + + {$fromName} has assigned a change to you. + + {sp}Please visit + + {\n} + {\n} + + {sp}{sp}{sp}{sp}{$email.changeUrl} + + {\n} + {\n} + + to view the change. + + {\n} + {\n} + + Change subject: {$change.subject}{\n} + ......................................................................{\n} + + {\n} + + {$email.changeDetail}{\n} + + {if $email.sshHost} + {\n} + {sp}{sp}git pull ssh:{print '//'}{$email.sshHost}/{$projectName} + {sp}{$patchSet.refName} + {\n} + {/if} + + {if $email.includeDiff} + {\n} + {$email.unifiedDiff} + {\n} + {/if} +{/template} diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy new file mode 100644 index 0000000000..bbf16d60df --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy @@ -0,0 +1,49 @@ +/** + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +{namespace com.google.gerrit.server.mail.template} + +/** + * @param email + * @param fromName + * @param patchSet + * @param projectName + */ +{template .SetAssigneeHtml autoescape="strict" kind="html"} +

+ {$fromName} has assigned a change to{sp} + {$email.assigneeName}.{sp} +

+ + {if $email.changeUrl} +

+ {call .ViewChangeButton data="all" /} +

+ {/if} + + {call .Pre}{param content: $email.changeDetail /}{/call} + + {if $email.sshHost} + {call .Pre}{param content kind="html"} + git pull ssh:{print '//'}{$email.sshHost}/{$projectName} + {sp}{$patchSet.refName} + {/param}{/call} + {/if} + + {if $email.includeDiff} + {call .Pre}{param content: $email.unifiedDiff /}{/call} + {/if} +{/template}