From 7759a6fca6b019a275074c9afcedaff3516c5ae4 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Mon, 27 Jun 2011 10:25:38 -0600 Subject: [PATCH] Add a Restored.vm template and use it. The restore action has been erroneously using the Abandoned.vm template. Create a template and sender for the restore command. Refactor some code to make this additonal sender share much in common with the AbandonedSender. Change-Id: Id1c61ead7cf71c3476759f0a968f1bf013b77b47 --- Documentation/config-mail.txt | 8 ++++ .../httpd/rpc/changedetail/AbandonChange.java | 10 ++-- .../httpd/rpc/changedetail/RestoreChange.java | 12 ++--- .../com/google/gerrit/server/ChangeUtil.java | 37 +++++++-------- .../server/config/GerritRequestModule.java | 2 + .../gerrit/server/mail/AbandonedSender.java | 3 +- .../server/mail/ReplyToChangeSender.java | 4 ++ .../gerrit/server/mail/RestoredSender.java | 46 +++++++++++++++++++ .../com/google/gerrit/server/mail/Restored.vm | 44 ++++++++++++++++++ .../gerrit/sshd/commands/ReviewCommand.java | 6 ++- 10 files changed, 139 insertions(+), 33 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java create mode 100644 gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm diff --git a/Documentation/config-mail.txt b/Documentation/config-mail.txt index 168bbfe169..8aa7d08225 100644 --- a/Documentation/config-mail.txt +++ b/Documentation/config-mail.txt @@ -83,6 +83,14 @@ The `ReplacePatchSet.vm` template will determine the contents of the email related to a user submitting a new patchset for a change. It is a `ChangeEmail`: see `ChangeSubject.vm` and `ChangeFooter.vm`. +Restored.vm +~~~~~~~~~~~ + +The `Restored.vm` template will determine the contents of the email related +to a change being restored. It is a `ChangeEmail`: see `ChangeSubject.vm` and +`ChangeFooter.vm`. + + Mail Variables and Methods -------------------------- diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java index 8eb1bbe672..b4f595c416 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java @@ -43,7 +43,7 @@ class AbandonChange extends Handler { private final ChangeControl.Factory changeControlFactory; private final ReviewDb db; private final IdentifiedUser currentUser; - private final AbandonedSender.Factory abandonedSenderFactory; + private final AbandonedSender.Factory senderFactory; private final ChangeDetailFactory.Factory changeDetailFactory; private final PatchSet.Id patchSetId; @@ -55,14 +55,14 @@ class AbandonChange extends Handler { @Inject AbandonChange(final ChangeControl.Factory changeControlFactory, final ReviewDb db, final IdentifiedUser currentUser, - final AbandonedSender.Factory abandonedSenderFactory, + final AbandonedSender.Factory senderFactory, final ChangeDetailFactory.Factory changeDetailFactory, @Assisted final PatchSet.Id patchSetId, @Assisted @Nullable final String message, final ChangeHookRunner hooks) { this.changeControlFactory = changeControlFactory; this.db = db; this.currentUser = currentUser; - this.abandonedSenderFactory = abandonedSenderFactory; + this.senderFactory = senderFactory; this.changeDetailFactory = changeDetailFactory; this.patchSetId = patchSetId; @@ -81,8 +81,8 @@ class AbandonChange extends Handler { throw new NoSuchChangeException(changeId); } - ChangeUtil.abandon(patchSetId, currentUser, message, db, - abandonedSenderFactory, hooks); + ChangeUtil.abandon(patchSetId, currentUser, message, db, senderFactory, + hooks); return changeDetailFactory.create(changeId).call(); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java index fa8785a134..7bd826eb4a 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java @@ -21,8 +21,8 @@ import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.*; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; @@ -41,7 +41,7 @@ class RestoreChange extends Handler { private final ChangeControl.Factory changeControlFactory; private final ReviewDb db; private final IdentifiedUser currentUser; - private final AbandonedSender.Factory abandonedSenderFactory; + private final RestoredSender.Factory senderFactory; private final ChangeDetailFactory.Factory changeDetailFactory; private final PatchSet.Id patchSetId; @@ -53,14 +53,14 @@ class RestoreChange extends Handler { @Inject RestoreChange(final ChangeControl.Factory changeControlFactory, final ReviewDb db, final IdentifiedUser currentUser, - final AbandonedSender.Factory abandonedSenderFactory, + final RestoredSender.Factory senderFactory, final ChangeDetailFactory.Factory changeDetailFactory, @Assisted final PatchSet.Id patchSetId, @Assisted @Nullable final String message, final ChangeHookRunner hooks) { this.changeControlFactory = changeControlFactory; this.db = db; this.currentUser = currentUser; - this.abandonedSenderFactory = abandonedSenderFactory; + this.senderFactory = senderFactory; this.changeDetailFactory = changeDetailFactory; this.patchSetId = patchSetId; @@ -79,8 +79,8 @@ class RestoreChange extends Handler { throw new NoSuchChangeException(changeId); } - ChangeUtil.restore(patchSetId, currentUser, message, db, - abandonedSenderFactory, hooks); + ChangeUtil.restore(patchSetId, currentUser, message, db, senderFactory, + hooks); return changeDetailFactory.create(changeId).call(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index a3dcfea430..57ec5f60ae 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -38,6 +38,8 @@ import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.ReplyToChangeSender; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.mail.RevertedSender; import com.google.gwtorm.client.AtomicUpdate; import com.google.gwtorm.client.OrmConcurrencyException; @@ -211,7 +213,7 @@ public class ChangeUtil { public static void abandon(final PatchSet.Id patchSetId, final IdentifiedUser user, final String message, final ReviewDb db, - final AbandonedSender.Factory abandonedSenderFactory, + final AbandonedSender.Factory senderFactory, final ChangeHookRunner hooks) throws NoSuchChangeException, InvalidChangeOperationException, EmailException, OrmException { final Change.Id changeId = patchSetId.getParentKey(); @@ -246,15 +248,9 @@ public class ChangeUtil { } }); - updatedChange(db, updatedChange, cmsg, + updatedChange(db, user, updatedChange, cmsg, senderFactory, "Change is no longer open or patchset is not latest"); - // Email the reviewers - final AbandonedSender cm = abandonedSenderFactory.create(updatedChange); - cm.setFrom(user.getAccountId()); - cm.setChangeMessage(cmsg); - cm.send(); - hooks.doChangeAbandonedHook(updatedChange, user.getAccount(), message); } @@ -362,7 +358,7 @@ public class ChangeUtil { public static void restore(final PatchSet.Id patchSetId, final IdentifiedUser user, final String message, final ReviewDb db, - final AbandonedSender.Factory abandonedSenderFactory, + final RestoredSender.Factory senderFactory, final ChangeHookRunner hooks) throws NoSuchChangeException, InvalidChangeOperationException, EmailException, OrmException { final Change.Id changeId = patchSetId.getParentKey(); @@ -397,25 +393,20 @@ public class ChangeUtil { } }); - updatedChange(db, updatedChange, cmsg, + updatedChange(db, user, updatedChange, cmsg, senderFactory, "Change is not abandoned or patchset is not latest"); - // Email the reviewers - final AbandonedSender cm = abandonedSenderFactory.create(updatedChange); - cm.setFrom(user.getAccountId()); - cm.setChangeMessage(cmsg); - cm.send(); - hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message); } - private static void updatedChange(final ReviewDb db, final Change change, - final ChangeMessage cmsg, final String err) throws NoSuchChangeException, - InvalidChangeOperationException, OrmException { + private static void updatedChange(final ReviewDb db, final IdentifiedUser user, + final Change change, final ChangeMessage cmsg, + ReplyToChangeSender.Factory senderFactory, final String err) + throws NoSuchChangeException, InvalidChangeOperationException, + EmailException, OrmException { if (change == null) { throw new InvalidChangeOperationException(err); } - db.changeMessages().insert(Collections.singleton(cmsg)); final List approvals = @@ -424,6 +415,12 @@ public class ChangeUtil { a.cache(change); } db.patchSetApprovals().update(approvals); + + // Email the reviewers + final ReplyToChangeSender cm = senderFactory.create(change); + cm.setFrom(user.getAccountId()); + cm.setChangeMessage(cmsg); + cm.send(); } public static String sortKey(long lastUpdated, int id){ diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index 4c386e0c6e..72dcd31dbb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -35,6 +35,7 @@ import com.google.gerrit.server.mail.MergeFailSender; import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.mail.RegisterNewEmailSender; import com.google.gerrit.server.mail.ReplacePatchSetSender; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.patch.AddReviewer; import com.google.gerrit.server.patch.PublishComments; @@ -79,6 +80,7 @@ public class GerritRequestModule extends FactoryModule { factory(ReplacePatchSetSender.Factory.class); factory(AbandonedSender.Factory.class); factory(RemoveReviewer.Factory.class); + factory(RestoredSender.Factory.class); factory(RevertedSender.Factory.class); factory(CommentSender.Factory.class); factory(MergedSender.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java index e67984978a..bb28a842bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java @@ -20,7 +20,8 @@ import com.google.inject.assistedinject.Assisted; /** Send notice about a change being abandoned by its owner. */ public class AbandonedSender extends ReplyToChangeSender { - public static interface Factory { + public static interface Factory extends + ReplyToChangeSender.Factory { AbandonedSender create(Change change); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java index 4c3ed76465..401a0b816c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java @@ -18,6 +18,10 @@ import com.google.gerrit.reviewdb.Change; /** Alert a user to a reply to a change, usually commentary made during review. */ public abstract class ReplyToChangeSender extends ChangeEmail { + public static interface Factory { + public T create(Change change); + } + protected ReplyToChangeSender(EmailArguments ea, Change c, String mc) { super(ea, c, mc); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java new file mode 100644 index 0000000000..fee73d6520 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java @@ -0,0 +1,46 @@ +// Copyright (C) 2011 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; + +import com.google.gerrit.reviewdb.Change; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +/** Send notice about a change being restored by its owner. */ +public class RestoredSender extends ReplyToChangeSender { + public static interface Factory extends + ReplyToChangeSender.Factory { + RestoredSender create(Change change); + } + + @Inject + public RestoredSender(EmailArguments ea, @Assisted Change c) { + super(ea, c, "restore"); + } + + @Override + protected void init() throws EmailException { + super.init(); + + ccAllApprovals(); + bccStarredBy(); + bccWatchesNotifyAllComments(); + } + + @Override + protected void formatChange() throws EmailException { + appendText(velocifyFile("Restored.vm")); + } +} diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm new file mode 100644 index 0000000000..c0e6b914f1 --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm @@ -0,0 +1,44 @@ +## Copyright (C) 2011 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. +## +## +## Template Type: +## ------------- +## This is a velocity mail template, see: http://velocity.apache.org and the +## gerrit-docs:config-mail.txt for more info on modifying gerrit mail templates. +## +## Template File Names and extensions: +## ---------------------------------- +## Gerrit will use templates ending in ".vm" but will ignore templates ending +## in ".vm.example". If a .vm template does not exist, the default internal +## gerrit template which is the same as the .vm.example will be used. If you +## want to override the default template, copy the .vm.exmaple file to a .vm +## file and edit it appropriately. +## +## This Template: +## -------------- +## The Restored.vm template will determine the contents of the email related +## to a change being restored. It is a ChangeEmail: see ChangeSubject.vm and +## ChangeFooter.vm. +## +$fromName has restored this change. + +Change subject: $change.subject +...................................................................... + + +#if ($coverLetter) +$coverLetter + +#end 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 467488a639..f6ba0a20d0 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 @@ -32,6 +32,7 @@ import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PublishComments; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; @@ -126,6 +127,9 @@ public class ReviewCommand extends BaseCommand { @Inject private PublishComments.Factory publishCommentsFactory; + @Inject + private RestoredSender.Factory restoredSenderFactory; + @Inject private ChangeHookRunner hooks; @@ -237,7 +241,7 @@ public class ReviewCommand extends BaseCommand { if (restoreChange) { if (changeControl.canRestore()) { ChangeUtil.restore(patchSetId, currentUser, changeComment, db, - abandonedSenderFactory, hooks); + restoredSenderFactory, hooks); } else { throw error("Not permitted to restore change"); }