Add support for CC state to AddReviewerInput
When this new state field is set to CC and NoteDb is in read-write mode, reviewer(s) will be added in the CC state. Change-Id: I17d7181d750e85026166d4cf3ef9eb7b8086dbc8
This commit is contained in:
@@ -4057,6 +4057,11 @@ set while adding the reviewer.
|
||||
|`reviewers` |optional|
|
||||
The newly added reviewers as a list of link:#reviewer-info[
|
||||
ReviewerInfo] entities.
|
||||
|`ccs` |optional|
|
||||
The newly CCed accounts as a list of link:#reviewer-info[
|
||||
ReviewerInfo] entities. This field will only appear if the requested
|
||||
`state` for the reviewer was `CC` *and* NoteDb is enabled on the
|
||||
server.
|
||||
|`error` |optional|
|
||||
Error message explaining why the reviewer could not be added. +
|
||||
If a group was specified in the input and an error is returned, it
|
||||
@@ -4989,6 +4994,9 @@ should be added as reviewer or the link:rest-api-groups.html#group-id[
|
||||
ID] of one group for which all members should be added as reviewers. +
|
||||
If an ID identifies both an account and a group, only the account is
|
||||
added as reviewer to the change.
|
||||
|`state` |optional|
|
||||
Add reviewer in this state. Possible reviewer states are `REVIEWER`
|
||||
and `CC`. If not given, defaults to `REVIEWER`.
|
||||
|`confirmed` |optional|
|
||||
Whether adding the reviewer is confirmed. +
|
||||
The Gerrit server may be configured to
|
||||
|
||||
@@ -15,6 +15,8 @@
|
||||
package com.google.gerrit.acceptance.rest.change;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.gerrit.extensions.client.ReviewerState.CC;
|
||||
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
|
||||
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
@@ -22,12 +24,18 @@ import com.google.gerrit.acceptance.RestResponse;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
|
||||
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
|
||||
import com.google.gerrit.extensions.client.ReviewerState;
|
||||
import com.google.gerrit.extensions.common.AccountInfo;
|
||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||
import com.google.gerrit.server.change.PostReviewers;
|
||||
import com.google.gerrit.server.mail.Address;
|
||||
import com.google.gerrit.testutil.FakeEmailSender.Message;
|
||||
import com.google.gson.stream.JsonReader;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
|
||||
public class ChangeReviewersIT extends AbstractDaemonTest {
|
||||
@@ -40,17 +48,14 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
|
||||
|
||||
int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
|
||||
int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
|
||||
List<TestAccount> users = new ArrayList<>(largeGroupSize);
|
||||
List<TestAccount> users =
|
||||
createAccounts(largeGroupSize, "addGroupAsReviewer");
|
||||
List<String> largeGroupUsernames = new ArrayList<>(mediumGroupSize);
|
||||
List<String> mediumGroupUsernames = new ArrayList<>(mediumGroupSize);
|
||||
for (int i = 0; i < largeGroupSize; i++) {
|
||||
users.add(accounts.create(name("u" + i), name("u" + i + "@example.com"),
|
||||
"Full Name " + i));
|
||||
largeGroupUsernames.add(users.get(i).username);
|
||||
if (i < mediumGroupSize) {
|
||||
mediumGroupUsernames.add(users.get(i).username);
|
||||
}
|
||||
for (TestAccount u : users) {
|
||||
largeGroupUsernames.add(u.username);
|
||||
}
|
||||
List<String> mediumGroupUsernames =
|
||||
largeGroupUsernames.subList(0, mediumGroupSize);
|
||||
gApi.groups().id(largeGroup).addMembers(
|
||||
largeGroupUsernames.toArray(new String[largeGroupSize]));
|
||||
gApi.groups().id(mediumGroup).addMembers(
|
||||
@@ -84,6 +89,160 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
|
||||
assertThat(result.confirm).isNull();
|
||||
assertThat(result.error).isNull();
|
||||
assertThat(result.reviewers).hasSize(mediumGroupSize);
|
||||
|
||||
// Verify that group members were added as reviewers.
|
||||
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
|
||||
assertReviewers(c, notesMigration.readChanges() ? REVIEWER : CC,
|
||||
users.subList(0, mediumGroupSize));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addCcAccount() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
String changeId = r.getChangeId();
|
||||
AddReviewerInput in = new AddReviewerInput();
|
||||
in.reviewer = user.email;
|
||||
in.state = CC;
|
||||
AddReviewerResult result = addReviewer(changeId, in);
|
||||
|
||||
assertThat(result.input).isEqualTo(user.email);
|
||||
assertThat(result.confirm).isNull();
|
||||
assertThat(result.error).isNull();
|
||||
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
|
||||
if (notesMigration.readChanges()) {
|
||||
assertThat(result.reviewers).isNull();
|
||||
assertThat(result.ccs).hasSize(1);
|
||||
AccountInfo ai = result.ccs.get(0);
|
||||
assertThat(ai._accountId).isEqualTo(user.id.get());
|
||||
assertReviewers(c, CC, user);
|
||||
} else {
|
||||
assertThat(result.ccs).isNull();
|
||||
assertThat(result.reviewers).hasSize(1);
|
||||
AccountInfo ai = result.reviewers.get(0);
|
||||
assertThat(ai._accountId).isEqualTo(user.id.get());
|
||||
assertReviewers(c, CC, user);
|
||||
}
|
||||
|
||||
// Verify email was sent to CCed account.
|
||||
List<Message> messages = sender.getMessages();
|
||||
assertThat(messages).hasSize(1);
|
||||
Message m = messages.get(0);
|
||||
assertThat(m.rcpt()).containsExactly(user.emailAddress);
|
||||
if (notesMigration.readChanges()) {
|
||||
assertThat(m.body())
|
||||
.contains(admin.fullName + " has uploaded a new change for review.");
|
||||
} else {
|
||||
assertThat(m.body()).contains("Hello " + user.fullName + ",\n");
|
||||
assertThat(m.body()).contains("I'd like you to do a code review.");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void addCcGroup() throws Exception {
|
||||
List<TestAccount> users = createAccounts(6, "addCcGroup");
|
||||
List<String> usernames = new ArrayList<>(6);
|
||||
for (TestAccount u : users) {
|
||||
usernames.add(u.username);
|
||||
}
|
||||
|
||||
List<TestAccount> firstUsers = users.subList(0, 3);
|
||||
List<String> firstUsernames = usernames.subList(0, 3);
|
||||
|
||||
PushOneCommit.Result r = createChange();
|
||||
String changeId = r.getChangeId();
|
||||
AddReviewerInput in = new AddReviewerInput();
|
||||
in.reviewer = createGroup("cc1");
|
||||
in.state = CC;
|
||||
gApi.groups().id(in.reviewer)
|
||||
.addMembers(firstUsernames.toArray(new String[firstUsernames.size()]));
|
||||
AddReviewerResult result = addReviewer(changeId, in);
|
||||
|
||||
assertThat(result.input).isEqualTo(in.reviewer);
|
||||
assertThat(result.confirm).isNull();
|
||||
assertThat(result.error).isNull();
|
||||
if (notesMigration.readChanges()) {
|
||||
assertThat(result.reviewers).isNull();
|
||||
} else {
|
||||
assertThat(result.ccs).isNull();
|
||||
}
|
||||
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
|
||||
assertReviewers(c, CC, firstUsers);
|
||||
|
||||
// Verify emails were sent to each of the group's accounts.
|
||||
List<Message> messages = sender.getMessages();
|
||||
assertThat(messages).hasSize(1);
|
||||
Message m = messages.get(0);
|
||||
List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
|
||||
for (TestAccount u : firstUsers) {
|
||||
expectedAddresses.add(u.emailAddress);
|
||||
}
|
||||
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
|
||||
|
||||
// CC a group that overlaps with some existing reviewers and CCed accounts.
|
||||
TestAccount reviewer = accounts.create(name("reviewer"),
|
||||
"addCcGroup-reviewer@example.com", "Reviewer");
|
||||
result = addReviewer(changeId, reviewer.username);
|
||||
assertThat(result.error).isNull();
|
||||
sender.clear();
|
||||
in.reviewer = createGroup("cc2");
|
||||
gApi.groups().id(in.reviewer)
|
||||
.addMembers(usernames.toArray(new String[usernames.size()]));
|
||||
gApi.groups().id(in.reviewer).addMembers(reviewer.username);
|
||||
result = addReviewer(changeId, in);
|
||||
assertThat(result.input).isEqualTo(in.reviewer);
|
||||
assertThat(result.confirm).isNull();
|
||||
assertThat(result.error).isNull();
|
||||
c = gApi.changes().id(r.getChangeId()).get();
|
||||
if (notesMigration.readChanges()) {
|
||||
assertThat(result.ccs).hasSize(3);
|
||||
assertThat(result.reviewers).isNull();
|
||||
assertReviewers(c, REVIEWER, reviewer);
|
||||
assertReviewers(c, CC, users);
|
||||
} else {
|
||||
assertThat(result.ccs).isNull();
|
||||
assertThat(result.reviewers).hasSize(3);
|
||||
List<TestAccount> expectedUsers = new ArrayList<>(users.size() + 2);
|
||||
expectedUsers.addAll(users);
|
||||
expectedUsers.add(reviewer);
|
||||
assertReviewers(c, CC, expectedUsers);
|
||||
}
|
||||
|
||||
messages = sender.getMessages();
|
||||
assertThat(messages).hasSize(1);
|
||||
m = messages.get(0);
|
||||
expectedAddresses = new ArrayList<>(4);
|
||||
for (int i = 0; i < 3; i++) {
|
||||
expectedAddresses.add(users.get(users.size() - i - 1).emailAddress);
|
||||
}
|
||||
if (notesMigration.readChanges()) {
|
||||
expectedAddresses.add(reviewer.emailAddress);
|
||||
}
|
||||
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void transitionCcToReviewer() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
String changeId = r.getChangeId();
|
||||
AddReviewerInput in = new AddReviewerInput();
|
||||
in.reviewer = user.email;
|
||||
in.state = CC;
|
||||
addReviewer(changeId, in);
|
||||
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
|
||||
assertReviewers(c, REVIEWER);
|
||||
assertReviewers(c, CC, user);
|
||||
|
||||
in.state = REVIEWER;
|
||||
addReviewer(changeId, in);
|
||||
c = gApi.changes().id(r.getChangeId()).get();
|
||||
if (notesMigration.readChanges()) {
|
||||
assertReviewers(c, REVIEWER, user);
|
||||
assertReviewers(c, CC);
|
||||
} else {
|
||||
// If NoteDb not enabled, should have had no effect.
|
||||
assertReviewers(c, REVIEWER);
|
||||
assertReviewers(c, CC, user);
|
||||
}
|
||||
}
|
||||
|
||||
private AddReviewerResult addReviewer(String changeId, String reviewer)
|
||||
@@ -107,4 +266,42 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
|
||||
jsonReader.setLenient(true);
|
||||
return newGson().fromJson(jsonReader, clazz);
|
||||
}
|
||||
|
||||
private static void assertReviewers(ChangeInfo c, ReviewerState reviewerState,
|
||||
TestAccount... accounts) throws Exception {
|
||||
List<TestAccount> accountList = new ArrayList<>(accounts.length);
|
||||
for (TestAccount a : accounts) {
|
||||
accountList.add(a);
|
||||
}
|
||||
assertReviewers(c, reviewerState, accountList);
|
||||
}
|
||||
|
||||
private static void assertReviewers(ChangeInfo c, ReviewerState reviewerState,
|
||||
Iterable<TestAccount> accounts) throws Exception {
|
||||
Collection<AccountInfo> actualAccounts = c.reviewers.get(reviewerState);
|
||||
if (actualAccounts == null) {
|
||||
assertThat(accounts.iterator().hasNext()).isFalse();
|
||||
return;
|
||||
}
|
||||
assertThat(actualAccounts).isNotNull();
|
||||
List<Integer> actualAccountIds = new ArrayList<>(actualAccounts.size());
|
||||
for (AccountInfo account : actualAccounts) {
|
||||
actualAccountIds.add(account._accountId);
|
||||
}
|
||||
List<Integer> expectedAccountIds = new ArrayList<>();
|
||||
for (TestAccount account : accounts) {
|
||||
expectedAccountIds.add(account.getId().get());
|
||||
}
|
||||
assertThat(actualAccountIds).containsExactlyElementsIn(expectedAccountIds);
|
||||
}
|
||||
|
||||
private List<TestAccount> createAccounts(int n, String emailPrefix)
|
||||
throws Exception {
|
||||
List<TestAccount> result = new ArrayList<>(n);
|
||||
for (int i = 0; i < n; i++) {
|
||||
result.add(accounts.create(name("u" + i),
|
||||
emailPrefix + "-" + i + "@example.com", "Full Name " + i));
|
||||
}
|
||||
return result;
|
||||
}
|
||||
}
|
||||
@@ -14,15 +14,22 @@
|
||||
|
||||
package com.google.gerrit.extensions.api.changes;
|
||||
|
||||
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
|
||||
|
||||
import com.google.gerrit.extensions.client.ReviewerState;
|
||||
import com.google.gerrit.extensions.restapi.DefaultInput;
|
||||
|
||||
public class AddReviewerInput {
|
||||
@DefaultInput
|
||||
public String reviewer;
|
||||
public Boolean confirmed;
|
||||
public ReviewerState state;
|
||||
|
||||
public boolean confirmed() {
|
||||
return (confirmed != null) ? confirmed : false;
|
||||
}
|
||||
}
|
||||
|
||||
public ReviewerState state() {
|
||||
return (state != null) ? state : REVIEWER;
|
||||
}
|
||||
}
|
||||
@@ -15,6 +15,7 @@
|
||||
package com.google.gerrit.extensions.api.changes;
|
||||
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.extensions.common.AccountInfo;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
@@ -48,6 +49,14 @@ public class AddReviewerResult {
|
||||
@Nullable
|
||||
public List<ReviewerInfo> reviewers;
|
||||
|
||||
/**
|
||||
* List of accounts CCed on the change. The size of this list may be
|
||||
* greater than one (e.g. when a group is CCed). Null if no accounts were CCed
|
||||
* or if reviewers is non-null.
|
||||
*/
|
||||
@Nullable
|
||||
public List<AccountInfo> ccs;
|
||||
|
||||
/**
|
||||
* Constructs a partially initialized result for the given reviewer.
|
||||
*
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
|
||||
package com.google.gerrit.server;
|
||||
|
||||
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
|
||||
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
|
||||
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
@@ -51,6 +52,7 @@ import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Date;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
@@ -122,7 +124,7 @@ public class ApprovalsUtil {
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all reviewers for a change.
|
||||
* Get all reviewers and CCed accounts for a change.
|
||||
*
|
||||
* @param allApprovals all approvals to consider; must all belong to the same
|
||||
* change.
|
||||
@@ -151,8 +153,16 @@ public class ApprovalsUtil {
|
||||
ChangeUpdate update, LabelTypes labelTypes, Change change,
|
||||
Iterable<Account.Id> wantReviewers) throws OrmException {
|
||||
PatchSet.Id psId = change.currentPatchSetId();
|
||||
Collection<Account.Id> existingReviewers;
|
||||
if (migration.readChanges()) {
|
||||
// If using NoteDB, we only want reviewers in the REVIEWER state.
|
||||
existingReviewers = notes.load().getReviewers().byState(REVIEWER);
|
||||
} else {
|
||||
// Prior to NoteDB, we gather all reviewers regardless of state.
|
||||
existingReviewers = getReviewers(db, notes).all();
|
||||
}
|
||||
return addReviewers(db, update, labelTypes, change, psId, false, null, null,
|
||||
wantReviewers, getReviewers(db, notes).all());
|
||||
wantReviewers, existingReviewers);
|
||||
}
|
||||
|
||||
private List<PatchSetApproval> addReviewers(ReviewDb db, ChangeUpdate update,
|
||||
@@ -191,6 +201,30 @@ public class ApprovalsUtil {
|
||||
return Collections.unmodifiableList(cells);
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds accounts to a change as reviewers in the CC state.
|
||||
*
|
||||
* @param notes change notes.
|
||||
* @param update change update.
|
||||
* @param wantCCs accounts to CC.
|
||||
* @return whether a change was made.
|
||||
* @throws OrmException
|
||||
*/
|
||||
public Collection<Account.Id> addCcs(ChangeNotes notes, ChangeUpdate update,
|
||||
Collection<Account.Id> wantCCs) throws OrmException {
|
||||
return addCcs(update, wantCCs, notes.load().getReviewers());
|
||||
}
|
||||
|
||||
private Collection<Account.Id> addCcs(ChangeUpdate update,
|
||||
Collection<Account.Id> wantCCs, ReviewerSet existingReviewers) {
|
||||
Set<Account.Id> need = new LinkedHashSet<>(wantCCs);
|
||||
need.removeAll(existingReviewers.all());
|
||||
for (Account.Id account : need) {
|
||||
update.putReviewer(account, CC);
|
||||
}
|
||||
return need;
|
||||
}
|
||||
|
||||
public void addApprovals(ReviewDb db, ChangeUpdate update,
|
||||
LabelTypes labelTypes, PatchSet ps, ChangeControl changeCtl,
|
||||
Map<String, Short> approvals) throws OrmException {
|
||||
|
||||
@@ -14,6 +14,8 @@
|
||||
|
||||
package com.google.gerrit.server.change;
|
||||
|
||||
import static com.google.gerrit.extensions.client.ReviewerState.CC;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.Lists;
|
||||
@@ -23,6 +25,7 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
|
||||
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
|
||||
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
|
||||
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
|
||||
import com.google.gerrit.extensions.client.ReviewerState;
|
||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||
@@ -49,6 +52,7 @@ import com.google.gerrit.server.git.UpdateException;
|
||||
import com.google.gerrit.server.group.GroupsCollection;
|
||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||
import com.google.gerrit.server.mail.AddReviewerSender;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.project.NoSuchProjectException;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
@@ -62,6 +66,8 @@ import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.text.MessageFormat;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -91,6 +97,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
private final AccountCache accountCache;
|
||||
private final ReviewerJson json;
|
||||
private final ReviewerAdded reviewerAdded;
|
||||
private final NotesMigration migration;
|
||||
|
||||
@Inject
|
||||
PostReviewers(AccountsCollection accounts,
|
||||
@@ -108,7 +115,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
@GerritServerConfig Config cfg,
|
||||
AccountCache accountCache,
|
||||
ReviewerJson json,
|
||||
ReviewerAdded reviewerAdded) {
|
||||
ReviewerAdded reviewerAdded,
|
||||
NotesMigration migration) {
|
||||
this.accounts = accounts;
|
||||
this.reviewerFactory = reviewerFactory;
|
||||
this.approvalsUtil = approvalsUtil;
|
||||
@@ -125,6 +133,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
this.accountCache = accountCache;
|
||||
this.json = json;
|
||||
this.reviewerAdded = reviewerAdded;
|
||||
this.migration = migration;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -136,7 +145,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
|
||||
try {
|
||||
Account.Id accountId = accounts.parse(input.reviewer).getAccountId();
|
||||
return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId));
|
||||
return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
|
||||
input.state());
|
||||
} catch (UnprocessableEntityException e) {
|
||||
try {
|
||||
return putGroup(rsrc, input);
|
||||
@@ -148,21 +158,24 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
}
|
||||
}
|
||||
|
||||
private AddReviewerResult putAccount(String reviewer, ReviewerResource rsrc)
|
||||
private AddReviewerResult putAccount(String reviewer, ReviewerResource rsrc,
|
||||
ReviewerState state)
|
||||
throws OrmException, UpdateException, RestApiException {
|
||||
Account member = rsrc.getReviewerUser().getAccount();
|
||||
ChangeControl control = rsrc.getReviewerControl();
|
||||
AddReviewerResult result = new AddReviewerResult(reviewer);
|
||||
if (isValidReviewer(member, control)) {
|
||||
addReviewers(rsrc.getChangeResource(), result,
|
||||
ImmutableMap.of(member.getId(), control));
|
||||
ImmutableMap.of(member.getId(), control), state);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private AddReviewerResult putGroup(ChangeResource rsrc, AddReviewerInput input)
|
||||
private AddReviewerResult putGroup(ChangeResource rsrc,
|
||||
AddReviewerInput input)
|
||||
throws UpdateException, RestApiException, OrmException, IOException {
|
||||
GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
|
||||
GroupDescription.Basic group =
|
||||
groupsCollection.parseInternal(input.reviewer);
|
||||
AddReviewerResult result = new AddReviewerResult(input.reviewer);
|
||||
if (!isLegalReviewerGroup(group.getGroupUUID())) {
|
||||
result.error = MessageFormat.format(
|
||||
@@ -211,7 +224,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
}
|
||||
}
|
||||
|
||||
addReviewers(rsrc, result, reviewers);
|
||||
addReviewers(rsrc, result, reviewers, input.state());
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -227,78 +240,107 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
|
||||
|
||||
private void addReviewers(
|
||||
ChangeResource rsrc, AddReviewerResult result, Map<Account.Id, ChangeControl> reviewers)
|
||||
ChangeResource rsrc, AddReviewerResult result,
|
||||
Map<Account.Id, ChangeControl> reviewers, ReviewerState state)
|
||||
throws OrmException, RestApiException, UpdateException {
|
||||
try (BatchUpdate bu = batchUpdateFactory.create(
|
||||
dbProvider.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
|
||||
Op op = new Op(rsrc, reviewers);
|
||||
Op op = new Op(rsrc, reviewers, state);
|
||||
Change.Id id = rsrc.getChange().getId();
|
||||
bu.addOp(id, op);
|
||||
bu.execute();
|
||||
|
||||
result.reviewers = Lists.newArrayListWithCapacity(op.added.size());
|
||||
for (PatchSetApproval psa : op.added) {
|
||||
// New reviewers have value 0, don't bother normalizing.
|
||||
result.reviewers.add(
|
||||
json.format(new ReviewerInfo(psa.getAccountId().get()),
|
||||
reviewers.get(psa.getAccountId()),
|
||||
ImmutableList.of(psa)));
|
||||
// Generate result details and fill AccountLoader. This occurs outside
|
||||
// the Op because the accounts are in a different table.
|
||||
if (migration.readChanges() && state == CC) {
|
||||
result.ccs = Lists.newArrayListWithCapacity(op.addedCCs.size());
|
||||
for (Account.Id accountId : op.addedCCs) {
|
||||
result.ccs.add(
|
||||
json.format(new ReviewerInfo(accountId.get()), reviewers.get(accountId)));
|
||||
}
|
||||
accountLoaderFactory.create(true).fill(result.ccs);
|
||||
} else {
|
||||
result.reviewers = Lists.newArrayListWithCapacity(op.addedReviewers.size());
|
||||
for (PatchSetApproval psa : op.addedReviewers) {
|
||||
// New reviewers have value 0, don't bother normalizing.
|
||||
result.reviewers.add(
|
||||
json.format(new ReviewerInfo(psa.getAccountId().get()),
|
||||
reviewers.get(psa.getAccountId()),
|
||||
ImmutableList.of(psa)));
|
||||
}
|
||||
accountLoaderFactory.create(true).fill(result.reviewers);
|
||||
}
|
||||
|
||||
// We don't do this inside Op, since the accounts are in a different
|
||||
// table.
|
||||
accountLoaderFactory.create(true).fill(result.reviewers);
|
||||
}
|
||||
}
|
||||
|
||||
private class Op extends BatchUpdate.Op {
|
||||
private final ChangeResource rsrc;
|
||||
private final Map<Account.Id, ChangeControl> reviewers;
|
||||
private final ReviewerState state;
|
||||
|
||||
private List<PatchSetApproval> added;
|
||||
private List<PatchSetApproval> addedReviewers;
|
||||
private Collection<Account.Id> addedCCs;
|
||||
private PatchSet patchSet;
|
||||
|
||||
Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers) {
|
||||
Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers,
|
||||
ReviewerState state) {
|
||||
this.rsrc = rsrc;
|
||||
this.reviewers = reviewers;
|
||||
this.state = state;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean updateChange(ChangeContext ctx)
|
||||
throws RestApiException, OrmException, IOException {
|
||||
added =
|
||||
approvalsUtil.addReviewers(
|
||||
ctx.getDb(),
|
||||
ctx.getNotes(),
|
||||
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
|
||||
rsrc.getControl().getLabelTypes(),
|
||||
rsrc.getChange(),
|
||||
reviewers.keySet());
|
||||
|
||||
if (added.isEmpty()) {
|
||||
return false;
|
||||
if (migration.readChanges() && state == CC) {
|
||||
addedCCs = approvalsUtil.addCcs(ctx.getNotes(),
|
||||
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
|
||||
reviewers.keySet());
|
||||
if (addedCCs.isEmpty()) {
|
||||
return false;
|
||||
}
|
||||
} else {
|
||||
addedReviewers =
|
||||
approvalsUtil.addReviewers(
|
||||
ctx.getDb(),
|
||||
ctx.getNotes(),
|
||||
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
|
||||
rsrc.getControl().getLabelTypes(),
|
||||
rsrc.getChange(),
|
||||
reviewers.keySet());
|
||||
if (addedReviewers.isEmpty()) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void postUpdate(Context ctx) throws Exception {
|
||||
emailReviewers(rsrc.getChange(), added);
|
||||
|
||||
if (!added.isEmpty()) {
|
||||
for (PatchSetApproval psa : added) {
|
||||
Account account = accountCache.get(psa.getAccountId()).getAccount();
|
||||
reviewerAdded.fire(rsrc.getChange(), patchSet, account,
|
||||
ctx.getAccount(),
|
||||
ctx.getWhen());
|
||||
if (addedReviewers != null || addedCCs != null) {
|
||||
if (addedReviewers == null) {
|
||||
addedReviewers = new ArrayList<>();
|
||||
}
|
||||
if (addedCCs == null) {
|
||||
addedCCs = new ArrayList<>();
|
||||
}
|
||||
emailReviewers(rsrc.getChange(), addedReviewers, addedCCs);
|
||||
if (!addedReviewers.isEmpty()) {
|
||||
for (PatchSetApproval psa : addedReviewers) {
|
||||
Account account = accountCache.get(psa.getAccountId()).getAccount();
|
||||
reviewerAdded.fire(rsrc.getChange(), patchSet, account,
|
||||
ctx.getAccount(), ctx.getWhen());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void emailReviewers(Change change, List<PatchSetApproval> added) {
|
||||
if (added.isEmpty()) {
|
||||
private void emailReviewers(Change change, List<PatchSetApproval> added,
|
||||
Collection<Account.Id> copied) {
|
||||
if (added.isEmpty() && copied.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -312,18 +354,27 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
|
||||
toMail.add(psa.getAccountId());
|
||||
}
|
||||
}
|
||||
if (!toMail.isEmpty()) {
|
||||
try {
|
||||
AddReviewerSender cm = addReviewerSenderFactory
|
||||
.create(change.getProject(), change.getId());
|
||||
cm.setFrom(userId);
|
||||
cm.addReviewers(toMail);
|
||||
cm.send();
|
||||
} catch (Exception err) {
|
||||
log.error("Cannot send email to new reviewers of change "
|
||||
+ change.getId(), err);
|
||||
List<Account.Id> toCopy = Lists.newArrayListWithCapacity(copied.size());
|
||||
for (Account.Id id : copied) {
|
||||
if (!id.equals(userId)) {
|
||||
toCopy.add(id);
|
||||
}
|
||||
}
|
||||
if (toMail.isEmpty() && toCopy.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
AddReviewerSender cm = addReviewerSenderFactory
|
||||
.create(change.getProject(), change.getId());
|
||||
cm.setFrom(userId);
|
||||
cm.addReviewers(toMail);
|
||||
cm.addExtraCC(toCopy);
|
||||
cm.send();
|
||||
} catch (Exception err) {
|
||||
log.error("Cannot send email to new reviewers of change "
|
||||
+ change.getId(), err);
|
||||
}
|
||||
}
|
||||
|
||||
public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) {
|
||||
|
||||
Reference in New Issue
Block a user