From 456e43a8b63f6919b92a40b7071788fc928c244b Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Mon, 26 Sep 2016 16:40:03 +0200 Subject: [PATCH] Add assignee as CC/Reviewer When a user is added as assignee it should also be added as reviewer. Change-Id: I249a2f3f057999154dc29c155bf2b0608494bd0d --- .../acceptance/rest/change/AssigneeIT.java | 28 +++++++++++++++++ .../server/api/changes/ChangeApiImpl.java | 2 +- .../gerrit/server/change/PostReviewers.java | 4 +-- .../gerrit/server/change/PutAssignee.java | 30 +++++++++++++++++-- 4 files changed, 59 insertions(+), 5 deletions(-) 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 b6256ffce1..685cceb607 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 @@ -18,10 +18,12 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; import static java.util.concurrent.TimeUnit.SECONDS; +import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; 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.TestTimeUtil; @@ -80,6 +82,27 @@ public class AssigneeIT extends AbstractDaemonTest { assertThat(itr.next()._accountId).isEqualTo(admin.getId().get()); } + @Test + public void testAssigneeAddedAsReviewer() throws Exception { + ReviewerState state; + // Assignee is added as CC, if back-end is reviewDb (that does not support + // CC) CC is stored as REVIEWER + if (notesMigration.readChanges()) { + state = ReviewerState.CC; + } else { + state = ReviewerState.REVIEWER; + } + PushOneCommit.Result r = createChange(); + Iterable reviewers = getReviewers(r, state); + assertThat(reviewers).isNull(); + assertThat(setAssignee(r, user.email)._accountId) + .isEqualTo(user.getId().get()); + reviewers = getReviewers(r, state); + assertThat(reviewers).hasSize(1); + AccountInfo reviewer = Iterables.getFirst(reviewers, null); + assertThat(reviewer._accountId).isEqualTo(user.getId().get()); + } + @Test public void testSetAlreadyExistingAssignee() throws Exception { PushOneCommit.Result r = createChange(); @@ -112,6 +135,11 @@ public class AssigneeIT extends AbstractDaemonTest { return gApi.changes().id(r.getChange().getId().get()).getPastAssignees(); } + private Iterable getReviewers(PushOneCommit.Result r, + ReviewerState state) throws Exception { + return get(r.getChangeId()).reviewers.get(state); + } + private AccountInfo setAssignee(PushOneCommit.Result r, String identifieer) throws Exception { AssigneeInput input = new AssigneeInput(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index c3ebdc483d..f7cb8f4ced 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -425,7 +425,7 @@ class ChangeApiImpl implements ChangeApi { throws RestApiException { try { return putAssignee.apply(change, input).value(); - } catch (UpdateException e) { + } catch (UpdateException | IOException | OrmException e) { throw new RestApiException("Cannot set assignee", e); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 2518138a77..287ee7fdf7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -262,7 +262,7 @@ public class PostReviewers return addition; } - class Addition { + public class Addition { final AddReviewerResult result; final Op op; @@ -309,7 +309,7 @@ public class PostReviewers } } - class Op extends BatchUpdate.Op { + public class Op extends BatchUpdate.Op { final Map reviewers; final ReviewerState state; final NotifyHandling notify; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java index 5c46a83fa1..b04896015b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java @@ -15,19 +15,26 @@ package com.google.gerrit.server.change; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AssigneeInput; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.AccountInfo; 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.server.ReviewDb; import com.google.gerrit.server.account.AccountJson; +import com.google.gerrit.server.change.PostReviewers.Addition; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.UpdateException; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; +import java.io.IOException; + @Singleton public class PutAssignee implements RestModifyView { @@ -35,26 +42,45 @@ public class PutAssignee private final SetAssigneeOp.Factory assigneeFactory; private final BatchUpdate.Factory batchUpdateFactory; private final Provider db; + private final PostReviewers postReviewers; @Inject PutAssignee(SetAssigneeOp.Factory assigneeFactory, BatchUpdate.Factory batchUpdateFactory, - Provider db) { + Provider db, + PostReviewers postReviewers) { this.assigneeFactory = assigneeFactory; this.batchUpdateFactory = batchUpdateFactory; this.db = db; + this.postReviewers = postReviewers; } @Override public Response apply(ChangeResource rsrc, AssigneeInput input) - throws RestApiException, UpdateException { + throws RestApiException, UpdateException, OrmException, IOException { try (BatchUpdate bu = batchUpdateFactory.create(db.get(), rsrc.getChange().getProject(), rsrc.getControl().getUser(), TimeUtil.nowTs())) { SetAssigneeOp op = assigneeFactory.create(input); bu.addOp(rsrc.getId(), op); + + PostReviewers.Addition reviewersAddition = + addAssigneeAsCC(rsrc, input.assignee); + bu.addOp(rsrc.getId(), reviewersAddition.op); + bu.execute(); + reviewersAddition.gatherResults(); return Response.ok(AccountJson.toAccountInfo(op.getNewAssignee())); } } + + private Addition addAssigneeAsCC(ChangeResource rsrc, String assignee) + throws OrmException, RestApiException, IOException { + AddReviewerInput reviewerInput = new AddReviewerInput(); + reviewerInput.reviewer = assignee; + reviewerInput.state = ReviewerState.CC; + reviewerInput.confirmed = true; + reviewerInput.notify = NotifyHandling.NONE; + return postReviewers.prepareApplication(rsrc, reviewerInput); + } }