Merge "Add a reviewer-deleted event"

This commit is contained in:
Hugo Arès
2016-05-03 12:31:22 +00:00
committed by Gerrit Code Review
13 changed files with 390 additions and 19 deletions

View File

@@ -247,6 +247,27 @@ reviewer:: link:json.html#account[account attribute]
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
created.
=== Reviewer Deleted
Sent when a reviewer (with a vote) is removed from a change.
type:: "reviewer-deleted"
change:: link:json.html#change[change attribute]
patchSet:: link:json.html#patchSet[patchSet attribute]
reviewer:: link:json.html#account[account attribute]
author:: link:json.html#account[account attribute]
approvals:: All link:json.html#approval[approval attributes] removed.
comment:: Review comment cover message.
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
created.
=== Topic Changed
Sent when the topic of a change has been changed.

View File

@@ -126,6 +126,14 @@ Called whenever a reviewer is added to a change.
reviewer-added --change <change id> --change-url <change url> --change-owner <change owner> --project <project name> --branch <branch> --reviewer <reviewer>
====
=== reviewer-deleted
Called whenever a reviewer (with a vote) is removed from a change.
====
reviewer-deleted --change <change id> --change-url <change url> --change-owner <change owner> --project <project name> --branch <branch> --reviewer <reviewer> [--<approval category id> <score> --<approval category id> <score> ...]
====
=== topic-changed
Called whenever a change's topic is changed from the Web UI or via the REST API.

View File

@@ -62,6 +62,12 @@ The `DeleteVote.vm` template will determine the contents of the email related
to removing votes on changes. It is a `ChangeEmail`: see `ChangeSubject.vm`
and `ChangeFooter.vm`.
=== DeleteReviewer.vm
The `DeleteReiewer.vm` template will determine the contents of the email related
to a user removing a reviewer (with a vote) from a change. It is a
`ChangeEmail`: see `ChangeSubject.vm` and `ChangeFooter.vm`.
=== Footer.vm
The `Footer.vm` template will determine the contents of the footer text

View File

@@ -105,6 +105,7 @@ public class SitePathInitializer {
extractMailExample("ChangeSubject.vm");
extractMailExample("Comment.vm");
extractMailExample("CommentFooter.vm");
extractMailExample("DeleteReviewer.vm");
extractMailExample("DeleteVote.vm");
extractMailExample("Footer.vm");
extractMailExample("Merged.vm");

View File

@@ -54,6 +54,7 @@ import com.google.gerrit.server.events.PatchSetCreatedEvent;
import com.google.gerrit.server.events.ProjectCreatedEvent;
import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.gerrit.server.events.ReviewerAddedEvent;
import com.google.gerrit.server.events.ReviewerDeletedEvent;
import com.google.gerrit.server.events.TopicChangedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.WorkQueue;
@@ -183,6 +184,9 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener,
/** Path of the reviewer added hook. */
private final Optional<Path> reviewerAddedHook;
/** Path of the reviewer deleted hook. */
private final Optional<Path> reviewerDeletedHook;
/** Path of the topic changed hook. */
private final Optional<Path> topicChangedHook;
@@ -269,6 +273,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener,
changeRestoredHook = hook(config, hooksPath, "change-restored");
refUpdatedHook = hook(config, hooksPath, "ref-updated");
reviewerAddedHook = hook(config, hooksPath, "reviewer-added");
reviewerDeletedHook = hook(config, hooksPath, "reviewer-deleted");
topicChangedHook = hook(config, hooksPath, "topic-changed");
claSignedHook = hook(config, hooksPath, "cla-signed");
refUpdateHook = hook(config, hooksPath, "ref-update");
@@ -699,6 +704,70 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener,
runHook(change.getProject(), reviewerAddedHook, args);
}
@Override
public void doReviewerDeletedHook(final Change change, Account account,
PatchSet patchSet, String comment, final Map<String, Short> approvals,
final Map<String, Short> oldApprovals, ReviewDb db) throws OrmException {
ReviewerDeletedEvent event = new ReviewerDeletedEvent(change);
event.change = changeAttributeSupplier(change);
event.patchSet = patchSetAttributeSupplier(change, patchSet);
event.reviewer = accountAttributeSupplier(account);
event.comment = comment;
event.approvals = Suppliers.memoize(
new Supplier<ApprovalAttribute[]>() {
@Override
public ApprovalAttribute[] get() {
LabelTypes labelTypes = projectCache.get(
change.getProject()).getLabelTypes();
if (!approvals.isEmpty()) {
ApprovalAttribute[] r = new ApprovalAttribute[approvals.size()];
int i = 0;
for (Map.Entry<String, Short> approval : approvals.entrySet()) {
r[i++] = getApprovalAttribute(labelTypes, approval,
oldApprovals);
}
return r;
}
return null;
}
});
dispatcher.get().postEvent(change, event, db);
if (!reviewerDeletedHook.isPresent()) {
return;
}
List<String> args = new ArrayList<>();
ChangeAttribute c = event.change.get();
AccountState owner = accountCache.get(change.getOwner());
addArg(args, "--change", c.id);
addArg(args, "--change-url", c.url);
addArg(args, "--change-owner", getDisplayName(owner.getAccount()));
addArg(args, "--project", c.project);
addArg(args, "--branch", c.branch);
addArg(args, "--reviewer", getDisplayName(account));
LabelTypes labelTypes = projectCache.get(
change.getProject()).getLabelTypes();
// append votes that were removed
for (Map.Entry<String, Short> approval : approvals.entrySet()) {
LabelType lt = labelTypes.byLabel(approval.getKey());
if (lt != null && approval.getValue() != null) {
addArg(args, "--" + lt.getName(), Short.toString(approval.getValue()));
if (oldApprovals != null && !oldApprovals.isEmpty()) {
Short oldValue = oldApprovals.get(approval.getKey());
if (oldValue != null) {
addArg(args, "--" + lt.getName() + "-oldValue",
Short.toString(oldValue));
}
}
}
}
runHook(change.getProject(), reviewerDeletedHook, args);
}
@Override
public void doTopicChangedHook(Change change, Account account,
String oldTopic, ReviewDb db)

View File

@@ -155,7 +155,23 @@ public interface ChangeHooks {
PatchSet patchSet, ReviewDb db) throws OrmException;
/**
* Fire the Topic Changed Hook.
* Fire the Reviewer Deleted Hook
*
* @param change The change itself.
* @param account The reviewer that was removed.
* @param patchSet The patchset that the reviewer was removed from.
* @param comment The comment given.
* @param approvals Map of label IDs to scores.
* @param oldApprovals Map of label IDs to old approval scores
* @param db The review database.
* @throws OrmException
*/
void doReviewerDeletedHook(Change change, Account account, PatchSet patchSet,
String comment, Map<String, Short> approvals,
Map<String, Short> oldApprovals, ReviewDb db) throws OrmException;
/**
* Fire the Topic Changed Hook
*
* @param change The change itself.
* @param account The gerrit user who changed the topic.

View File

@@ -90,6 +90,12 @@ public final class DisabledChangeHooks implements ChangeHooks, EventDispatcher {
ReviewDb db) {
}
@Override
public void doReviewerDeletedHook(Change change, Account account,
PatchSet patchSet, String comment, Map<String, Short> approvals,
Map<String, Short> oldApprovals, ReviewDb db) {
}
@Override
public void doTopicChangedHook(Change change, Account account, String oldTopic,
ReviewDb db) {

View File

@@ -17,55 +17,84 @@ package com.google.gerrit.server.change;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.DeleteReviewer.Input;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.Context;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.mail.DeleteReviewerSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@Singleton
public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
private static final Logger log = LoggerFactory
.getLogger(DeleteReviewer.class);
public static class Input {
}
private final Provider<ReviewDb> dbProvider;
private final ApprovalsUtil approvalsUtil;
private final PatchSetUtil psUtil;
private final ChangeMessagesUtil cmUtil;
private final BatchUpdate.Factory batchUpdateFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final ChangeHooks hooks;
private final Provider<IdentifiedUser> user;
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
@Inject
DeleteReviewer(Provider<ReviewDb> dbProvider,
ApprovalsUtil approvalsUtil,
PatchSetUtil psUtil,
ChangeMessagesUtil cmUtil,
BatchUpdate.Factory batchUpdateFactory,
IdentifiedUser.GenericFactory userFactory) {
IdentifiedUser.GenericFactory userFactory,
ChangeHooks hooks,
Provider<IdentifiedUser> user,
DeleteReviewerSender.Factory deleteReviewerSenderFactory) {
this.dbProvider = dbProvider;
this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil;
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.userFactory = userFactory;
this.hooks = hooks;
this.user = user;
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
}
@Override
@@ -74,7 +103,7 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
try (BatchUpdate bu = batchUpdateFactory.create(dbProvider.get(),
rsrc.getChangeResource().getProject(),
rsrc.getChangeResource().getUser(), TimeUtil.nowTs())) {
Op op = new Op(rsrc.getReviewerUser().getAccountId());
Op op = new Op(rsrc.getReviewerUser().getAccount());
bu.addOp(rsrc.getChange().getId(), op);
bu.execute();
}
@@ -83,29 +112,44 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
}
private class Op extends BatchUpdate.Op {
private final Account.Id reviewerId;
private final Account reviewer;
ChangeMessage changeMessage;
Change currChange;
PatchSet currPs;
List<PatchSetApproval> del = new ArrayList<>();
Map<String, Short> newApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
Op(Account.Id reviewerId) {
this.reviewerId = reviewerId;
Op(Account reviewerAccount) {
this.reviewer = reviewerAccount;
}
@Override
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException {
PatchSet.Id currPs = ctx.getChange().currentPatchSetId();
Account.Id reviewerId = reviewer.getId();
currChange = ctx.getChange();
currPs = psUtil.current(dbProvider.get(), ctx.getNotes());
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
StringBuilder msg = new StringBuilder();
List<PatchSetApproval> del = Lists.newArrayList();
for (PatchSetApproval a : approvals(ctx, reviewerId)) {
if (ctx.getControl().canRemoveReviewer(a)) {
del.add(a);
if (a.getPatchSetId().equals(currPs)
&& a.getValue() != 0) {
if (a.getPatchSetId().equals(currPs.getId()) && a.getValue() != 0) {
oldApprovals.put(a.getLabel(), a.getValue());
if (msg.length() == 0) {
msg.append("Removed the following votes:\n\n");
msg.append("Removed reviewer ").append(reviewer.getFullName())
.append(" with the following votes:\n\n");
}
msg.append("* ")
.append(a.getLabel()).append(formatLabelValue(a.getValue()))
.append(" by ").append(userFactory.create(a.getAccountId()).getNameEmail())
msg.append("* ").append(a.getLabel())
.append(formatLabelValue(a.getValue())).append(" by ")
.append(userFactory.create(a.getAccountId()).getNameEmail())
.append("\n");
}
} else {
@@ -116,21 +160,37 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
throw new ResourceNotFoundException();
}
ctx.getDb().patchSetApprovals().delete(del);
ChangeUpdate update = ctx.getUpdate(currPs);
ChangeUpdate update = ctx.getUpdate(currPs.getId());
update.removeReviewer(reviewerId);
if (msg.length() > 0) {
ChangeMessage changeMessage =
new ChangeMessage(new ChangeMessage.Key(ctx.getChange().getId(),
changeMessage = new ChangeMessage(
new ChangeMessage.Key(currChange.getId(),
ChangeUtil.messageUUID(ctx.getDb())),
ctx.getUser().getAccountId(),
ctx.getWhen(), currPs);
ctx.getUser().getAccountId(), ctx.getWhen(), currPs.getId());
changeMessage.setMessage(msg.toString());
cmUtil.addChangeMessage(ctx.getDb(), update, changeMessage);
}
return true;
}
@Override
public void postUpdate(Context ctx) {
if (changeMessage == null) {
return;
}
emailReviewers(ctx.getProject(), currChange, del, changeMessage);
try {
hooks.doReviewerDeletedHook(currChange, reviewer, currPs,
changeMessage.getMessage(), newApprovals, oldApprovals,
dbProvider.get());
} catch (OrmException e) {
log.warn("ChangeHook.doCommentAddedHook delivery failed", e);
}
}
private Iterable<PatchSetApproval> approvals(ChangeContext ctx,
final Account.Id accountId) throws OrmException {
return Iterables.filter(
@@ -151,4 +211,29 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
}
}
}
private void emailReviewers(Project.NameKey projectName, Change change,
List<PatchSetApproval> dels, ChangeMessage changeMessage) {
// The user knows they removed themselves, don't bother emailing them.
List<Account.Id> toMail = Lists.newArrayListWithCapacity(dels.size());
Account.Id userId = user.get().getAccountId();
for (PatchSetApproval psa : dels) {
if (!psa.getAccountId().equals(userId)) {
toMail.add(psa.getAccountId());
}
}
if (!toMail.isEmpty()) {
try {
DeleteReviewerSender cm =
deleteReviewerSenderFactory.create(projectName, change.getId());
cm.setFrom(userId);
cm.addReviewers(toMail);
cm.setChangeMessage(changeMessage);
cm.send();
} catch (Exception err) {
log.error("Cannot email update for change " + change.getId(), err);
}
}
}
}

View File

@@ -109,6 +109,7 @@ import com.google.gerrit.server.index.change.ReindexAfterUpdate;
import com.google.gerrit.server.mail.AddKeySender;
import com.google.gerrit.server.mail.AddReviewerSender;
import com.google.gerrit.server.mail.CreateChangeSender;
import com.google.gerrit.server.mail.DeleteReviewerSender;
import com.google.gerrit.server.mail.EmailModule;
import com.google.gerrit.server.mail.FromAddressGenerator;
import com.google.gerrit.server.mail.FromAddressGeneratorProvider;
@@ -205,6 +206,7 @@ public class GerritGlobalModule extends FactoryModule {
factory(AccountInfoCacheFactory.Factory.class);
factory(AddReviewerSender.Factory.class);
factory(DeleteReviewerSender.Factory.class);
factory(AddKeySender.Factory.class);
factory(BatchUpdate.Factory.class);
factory(CapabilityControl.Factory.class);

View File

@@ -0,0 +1,31 @@
// 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.events;
import com.google.common.base.Supplier;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ApprovalAttribute;
public class ReviewerDeletedEvent extends PatchSetEvent {
static final String TYPE = "reviewer-deleted";
public Supplier<AccountAttribute> reviewer;
public Supplier<ApprovalAttribute[]> approvals;
public String comment;
public ReviewerDeletedEvent(Change change) {
super(TYPE, change);
}
}

View File

@@ -0,0 +1,81 @@
// 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;
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.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
/** Let users know that a reviewer and possibly her review have
* been removed. */
public class DeleteReviewerSender extends ReplyToChangeSender {
private final Set<Account.Id> reviewers = new HashSet<>();
public static interface Factory extends
ReplyToChangeSender.Factory<DeleteReviewerSender> {
@Override
DeleteReviewerSender create(Project.NameKey project, Change.Id change);
}
@Inject
public DeleteReviewerSender(EmailArguments ea,
@Assisted Project.NameKey project,
@Assisted Change.Id id)
throws OrmException {
super(ea, "deleteReviewer", newChangeData(ea, project, id));
}
public void addReviewers(Collection<Account.Id> cc) {
reviewers.addAll(cc);
}
@Override
protected void init() throws EmailException {
super.init();
ccAllApprovals();
bccStarredBy();
ccExistingReviewers();
includeWatchers(NotifyType.ALL_COMMENTS);
add(RecipientType.TO, reviewers);
}
@Override
protected void formatChange() throws EmailException {
appendText(velocifyFile("DeleteReviewer.vm"));
}
public List<String> getReviewerNames() {
if (reviewers.isEmpty()) {
return null;
}
List<String> names = new ArrayList<>();
for (Account.Id id : reviewers) {
names.add(getNameFor(id));
}
return names;
}
}

View File

@@ -21,6 +21,7 @@ public class EmailModule extends FactoryModule {
protected void configure() {
factory(AbandonedSender.Factory.class);
factory(CommentSender.Factory.class);
factory(DeleteReviewerSender.Factory.class);
factory(DeleteVoteSender.Factory.class);
factory(RestoredSender.Factory.class);
factory(RevertedSender.Factory.class);

View File

@@ -0,0 +1,44 @@
## 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.
##
##
## 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.example file to a .vm
## file and edit it appropriately.
##
## This Template:
## --------------
## The DeleteReviewer.vm template will determine the contents of the email
## related to removal of a reviewer (and the reviewer's votes) from reviews.
## It is a ChangeEmail: see ChangeSubject.vm and ChangeFooter.vm.
##
$fromName has removed $email.joinStrings($email.reviewerNames, ', ') from this change.
Change subject: $change.subject
......................................................................
#if ($email.coverLetter)
$email.coverLetter
#end