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
This commit is contained in:
Martin Fick
2011-06-27 10:25:38 -06:00
parent c3a2f83605
commit 7759a6fca6
10 changed files with 139 additions and 33 deletions

View File

@@ -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 related to a user submitting a new patchset for a change. It is a
`ChangeEmail`: see `ChangeSubject.vm` and `ChangeFooter.vm`. `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 Mail Variables and Methods
-------------------------- --------------------------

View File

@@ -43,7 +43,7 @@ class AbandonChange extends Handler<ChangeDetail> {
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db; private final ReviewDb db;
private final IdentifiedUser currentUser; private final IdentifiedUser currentUser;
private final AbandonedSender.Factory abandonedSenderFactory; private final AbandonedSender.Factory senderFactory;
private final ChangeDetailFactory.Factory changeDetailFactory; private final ChangeDetailFactory.Factory changeDetailFactory;
private final PatchSet.Id patchSetId; private final PatchSet.Id patchSetId;
@@ -55,14 +55,14 @@ class AbandonChange extends Handler<ChangeDetail> {
@Inject @Inject
AbandonChange(final ChangeControl.Factory changeControlFactory, AbandonChange(final ChangeControl.Factory changeControlFactory,
final ReviewDb db, final IdentifiedUser currentUser, final ReviewDb db, final IdentifiedUser currentUser,
final AbandonedSender.Factory abandonedSenderFactory, final AbandonedSender.Factory senderFactory,
final ChangeDetailFactory.Factory changeDetailFactory, final ChangeDetailFactory.Factory changeDetailFactory,
@Assisted final PatchSet.Id patchSetId, @Assisted final PatchSet.Id patchSetId,
@Assisted @Nullable final String message, final ChangeHookRunner hooks) { @Assisted @Nullable final String message, final ChangeHookRunner hooks) {
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.db = db; this.db = db;
this.currentUser = currentUser; this.currentUser = currentUser;
this.abandonedSenderFactory = abandonedSenderFactory; this.senderFactory = senderFactory;
this.changeDetailFactory = changeDetailFactory; this.changeDetailFactory = changeDetailFactory;
this.patchSetId = patchSetId; this.patchSetId = patchSetId;
@@ -81,8 +81,8 @@ class AbandonChange extends Handler<ChangeDetail> {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
} }
ChangeUtil.abandon(patchSetId, currentUser, message, db, ChangeUtil.abandon(patchSetId, currentUser, message, db, senderFactory,
abandonedSenderFactory, hooks); hooks);
return changeDetailFactory.create(changeId).call(); return changeDetailFactory.create(changeId).call();
} }

View File

@@ -21,8 +21,8 @@ import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.*; import com.google.gerrit.reviewdb.*;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser; 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.EmailException;
import com.google.gerrit.server.mail.RestoredSender;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -41,7 +41,7 @@ class RestoreChange extends Handler<ChangeDetail> {
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db; private final ReviewDb db;
private final IdentifiedUser currentUser; private final IdentifiedUser currentUser;
private final AbandonedSender.Factory abandonedSenderFactory; private final RestoredSender.Factory senderFactory;
private final ChangeDetailFactory.Factory changeDetailFactory; private final ChangeDetailFactory.Factory changeDetailFactory;
private final PatchSet.Id patchSetId; private final PatchSet.Id patchSetId;
@@ -53,14 +53,14 @@ class RestoreChange extends Handler<ChangeDetail> {
@Inject @Inject
RestoreChange(final ChangeControl.Factory changeControlFactory, RestoreChange(final ChangeControl.Factory changeControlFactory,
final ReviewDb db, final IdentifiedUser currentUser, final ReviewDb db, final IdentifiedUser currentUser,
final AbandonedSender.Factory abandonedSenderFactory, final RestoredSender.Factory senderFactory,
final ChangeDetailFactory.Factory changeDetailFactory, final ChangeDetailFactory.Factory changeDetailFactory,
@Assisted final PatchSet.Id patchSetId, @Assisted final PatchSet.Id patchSetId,
@Assisted @Nullable final String message, final ChangeHookRunner hooks) { @Assisted @Nullable final String message, final ChangeHookRunner hooks) {
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.db = db; this.db = db;
this.currentUser = currentUser; this.currentUser = currentUser;
this.abandonedSenderFactory = abandonedSenderFactory; this.senderFactory = senderFactory;
this.changeDetailFactory = changeDetailFactory; this.changeDetailFactory = changeDetailFactory;
this.patchSetId = patchSetId; this.patchSetId = patchSetId;
@@ -79,8 +79,8 @@ class RestoreChange extends Handler<ChangeDetail> {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
} }
ChangeUtil.restore(patchSetId, currentUser, message, db, ChangeUtil.restore(patchSetId, currentUser, message, db, senderFactory,
abandonedSenderFactory, hooks); hooks);
return changeDetailFactory.create(changeId).call(); return changeDetailFactory.create(changeId).call();
} }

View File

@@ -38,6 +38,8 @@ import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.AbandonedSender;
import com.google.gerrit.server.mail.EmailException; 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.gerrit.server.mail.RevertedSender;
import com.google.gwtorm.client.AtomicUpdate; import com.google.gwtorm.client.AtomicUpdate;
import com.google.gwtorm.client.OrmConcurrencyException; import com.google.gwtorm.client.OrmConcurrencyException;
@@ -211,7 +213,7 @@ public class ChangeUtil {
public static void abandon(final PatchSet.Id patchSetId, public static void abandon(final PatchSet.Id patchSetId,
final IdentifiedUser user, final String message, final ReviewDb db, final IdentifiedUser user, final String message, final ReviewDb db,
final AbandonedSender.Factory abandonedSenderFactory, final AbandonedSender.Factory senderFactory,
final ChangeHookRunner hooks) throws NoSuchChangeException, final ChangeHookRunner hooks) throws NoSuchChangeException,
InvalidChangeOperationException, EmailException, OrmException { InvalidChangeOperationException, EmailException, OrmException {
final Change.Id changeId = patchSetId.getParentKey(); 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"); "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); hooks.doChangeAbandonedHook(updatedChange, user.getAccount(), message);
} }
@@ -362,7 +358,7 @@ public class ChangeUtil {
public static void restore(final PatchSet.Id patchSetId, public static void restore(final PatchSet.Id patchSetId,
final IdentifiedUser user, final String message, final ReviewDb db, final IdentifiedUser user, final String message, final ReviewDb db,
final AbandonedSender.Factory abandonedSenderFactory, final RestoredSender.Factory senderFactory,
final ChangeHookRunner hooks) throws NoSuchChangeException, final ChangeHookRunner hooks) throws NoSuchChangeException,
InvalidChangeOperationException, EmailException, OrmException { InvalidChangeOperationException, EmailException, OrmException {
final Change.Id changeId = patchSetId.getParentKey(); 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"); "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); hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message);
} }
private static void updatedChange(final ReviewDb db, final Change change, private static void updatedChange(final ReviewDb db, final IdentifiedUser user,
final ChangeMessage cmsg, final String err) throws NoSuchChangeException, final Change change, final ChangeMessage cmsg,
InvalidChangeOperationException, OrmException { ReplyToChangeSender.Factory senderFactory, final String err)
throws NoSuchChangeException, InvalidChangeOperationException,
EmailException, OrmException {
if (change == null) { if (change == null) {
throw new InvalidChangeOperationException(err); throw new InvalidChangeOperationException(err);
} }
db.changeMessages().insert(Collections.singleton(cmsg)); db.changeMessages().insert(Collections.singleton(cmsg));
final List<PatchSetApproval> approvals = final List<PatchSetApproval> approvals =
@@ -424,6 +415,12 @@ public class ChangeUtil {
a.cache(change); a.cache(change);
} }
db.patchSetApprovals().update(approvals); 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){ public static String sortKey(long lastUpdated, int id){

View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.mail.MergeFailSender;
import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.mail.MergedSender;
import com.google.gerrit.server.mail.RegisterNewEmailSender; import com.google.gerrit.server.mail.RegisterNewEmailSender;
import com.google.gerrit.server.mail.ReplacePatchSetSender; 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.mail.RevertedSender;
import com.google.gerrit.server.patch.AddReviewer; import com.google.gerrit.server.patch.AddReviewer;
import com.google.gerrit.server.patch.PublishComments; import com.google.gerrit.server.patch.PublishComments;
@@ -79,6 +80,7 @@ public class GerritRequestModule extends FactoryModule {
factory(ReplacePatchSetSender.Factory.class); factory(ReplacePatchSetSender.Factory.class);
factory(AbandonedSender.Factory.class); factory(AbandonedSender.Factory.class);
factory(RemoveReviewer.Factory.class); factory(RemoveReviewer.Factory.class);
factory(RestoredSender.Factory.class);
factory(RevertedSender.Factory.class); factory(RevertedSender.Factory.class);
factory(CommentSender.Factory.class); factory(CommentSender.Factory.class);
factory(MergedSender.Factory.class); factory(MergedSender.Factory.class);

View File

@@ -20,7 +20,8 @@ import com.google.inject.assistedinject.Assisted;
/** Send notice about a change being abandoned by its owner. */ /** Send notice about a change being abandoned by its owner. */
public class AbandonedSender extends ReplyToChangeSender { public class AbandonedSender extends ReplyToChangeSender {
public static interface Factory { public static interface Factory extends
ReplyToChangeSender.Factory<AbandonedSender> {
AbandonedSender create(Change change); AbandonedSender create(Change change);
} }

View File

@@ -18,6 +18,10 @@ import com.google.gerrit.reviewdb.Change;
/** Alert a user to a reply to a change, usually commentary made during review. */ /** Alert a user to a reply to a change, usually commentary made during review. */
public abstract class ReplyToChangeSender extends ChangeEmail { public abstract class ReplyToChangeSender extends ChangeEmail {
public static interface Factory<T extends ReplyToChangeSender> {
public T create(Change change);
}
protected ReplyToChangeSender(EmailArguments ea, Change c, String mc) { protected ReplyToChangeSender(EmailArguments ea, Change c, String mc) {
super(ea, c, mc); super(ea, c, mc);
} }

View File

@@ -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> {
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"));
}
}

View File

@@ -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

View File

@@ -32,6 +32,7 @@ import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.AbandonedSender;
import com.google.gerrit.server.mail.EmailException; 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.patch.PublishComments;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -126,6 +127,9 @@ public class ReviewCommand extends BaseCommand {
@Inject @Inject
private PublishComments.Factory publishCommentsFactory; private PublishComments.Factory publishCommentsFactory;
@Inject
private RestoredSender.Factory restoredSenderFactory;
@Inject @Inject
private ChangeHookRunner hooks; private ChangeHookRunner hooks;
@@ -237,7 +241,7 @@ public class ReviewCommand extends BaseCommand {
if (restoreChange) { if (restoreChange) {
if (changeControl.canRestore()) { if (changeControl.canRestore()) {
ChangeUtil.restore(patchSetId, currentUser, changeComment, db, ChangeUtil.restore(patchSetId, currentUser, changeComment, db,
abandonedSenderFactory, hooks); restoredSenderFactory, hooks);
} else { } else {
throw error("Not permitted to restore change"); throw error("Not permitted to restore change");
} }