opendev/gerrit
Code Issues Proposed changes

Fix inconsistent behaviour when adding reviewers

Browse Source 
It was possible to add reviewers to a change when they had no visibility
to said change and/or were not active users. This is inconsistent with
what happens when adding groups. When adding groups, the non-active
users and the users with no visibility to the change would not be added.
Make the behavior of adding single reviewers consistent with adding
groups.

Change-Id: I606d43e6ca5ada88f26d3b14dd6f16c2306f74af
This commit is contained in:
Simon Lei
2014-11-27 10:11:55 -05:00
parent 9896d4c6bf
commit 4248ea8c86
1 changed files with 17 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -151,11 +151,12 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
private PostResult putAccount(ReviewerResource rsrc) throws OrmException,
EmailException, IOException {
Account.Id id = rsrc.getUser().getAccountId();
ChangeControl control = rsrc.getControl().forUser(
identifiedUserFactory.create(id));
Account member = rsrc.getUser().getAccount();
ChangeControl control = rsrc.getControl();
PostResult result = new PostResult();
addReviewers(rsrc, result, ImmutableMap.of(id, control));
if (isValidReviewer(member, control)) {
addReviewers(rsrc, result, ImmutableMap.of(member.getId(), control));
}
return result;
}
@@ -206,13 +207,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
}
for (Account member : members) {
if (member.isActive()) {
IdentifiedUser user = identifiedUserFactory.create(member.getId());
// Does not account for draft status as a user might want to let a
// reviewer see a draft.
if (control.forUser(user).isRefVisible()) {
reviewers.put(user.getAccountId(), control);
}
if (isValidReviewer(member, control)) {
reviewers.put(member.getId(), control);
}
}
@@ -220,6 +216,16 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
return result;
}
private boolean isValidReviewer(Account member, ChangeControl control) {
if (member.isActive()) {
IdentifiedUser user = identifiedUserFactory.create(member.getId());
// Does not account for draft status as a user might want to let a
// reviewer see a draft.
return control.forUser(user).isRefVisible();
}
return false;
}
private void addReviewers(ChangeResource rsrc, PostResult result,
Map<Account.Id, ChangeControl> reviewers)
throws OrmException, EmailException, IOException {