Return group confirm hints in SuggestedReviewerInfo

These hints help a client or frontend prompt a user for confirmation
in advance if the user selects a suggested group that would need to
be explicitly confirmed to add as a reviewer.

Change-Id: Id1323c5a071ac954dfcbd250fa1a6c246258755a
This commit is contained in:
Logan Hanks 2016-07-20 15:42:52 -07:00
parent b06ac2e226
commit ab3c81edd8
5 changed files with 107 additions and 14 deletions

View File

@ -2209,13 +2209,15 @@ As result a list of link:#suggested-reviewer-info[SuggestedReviewerInfo] entries
"_account_id": 1000097,
"name": "Jane Roe",
"email": "jane.roe@example.com"
}
},
"count": 1
},
{
"group": {
"id": "4fd581c0657268f2bdcc26699fbf9ddb76e3a279",
"name": "Joiner"
}
},
"count": 5
}
]
----
@ -5260,6 +5262,25 @@ the link:rest-api-accounts.html#account-info[AccountInfo] entity, or
the `group` field that contains the
link:rest-api-changes.html#group-base-info[GroupBaseInfo] entity.
[options="header",cols="1,^1,5"]
|===========================
|Field Name ||Description
|`account` |optional|
An link:rest-api-accounts.html#account-info[AccountInfo] entity, if the
suggestion is an account.
|`group` |optional|
A link:rest-api-changes.html#group-base-info[GroupBaseInfo] entity, if the
suggestion is a group.
|`count` ||
The total number of accounts in the suggestion. This is `1` if `account` is
present. If `group` is present, the total number of accounts that are
members of the group is returned (this count includes members of nested
groups).
|`confirm` |optional|
True if `group` is present and `count` is above the threshold where the
`confirmed` flag must be passed to add the group as a reviewer.
|===========================
[[topic-input]]
=== TopicInput
The `TopicInput` entity contains information for setting a topic.

View File

@ -230,6 +230,45 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
assertThat(suggestedReviewerInfos).hasSize(1);
}
@Test
@GerritConfigs({
@GerritConfig(name = "addreviewer.maxAllowed", value="2"),
@GerritConfig(name = "addreviewer.maxWithoutConfirmation", value="1"),
})
public void suggestReviewersGroupSizeConsiderations() throws Exception {
AccountGroup largeGroup = group("large");
AccountGroup mediumGroup = group("medium");
// Both groups have Administrator as a member. Add two users to large
// group to push it past maxAllowed, and one to medium group to push it
// past maxWithoutConfirmation.
user("individual 0", "Test0 Last0", largeGroup, mediumGroup);
user("individual 1", "Test1 Last1", largeGroup);
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
SuggestedReviewerInfo reviewer;
// Individual account suggestions have count of 1 and no confirm.
reviewers = suggestReviewers(changeId, "test", 10);
assertThat(reviewers).hasSize(2);
reviewer = reviewers.get(0);
assertThat(reviewer.count).isEqualTo(1);
assertThat(reviewer.confirm).isNull();
// Large group should never be suggested.
reviewers = suggestReviewers(changeId, largeGroup.getName(), 10);
assertThat(reviewers).isEmpty();
// Medium group should be suggested with appropriate count and confirm.
reviewers = suggestReviewers(changeId, mediumGroup.getName(), 10);
assertThat(reviewers).hasSize(1);
reviewer = reviewers.get(0);
assertThat(reviewer.group.name).isEqualTo(mediumGroup.getName());
assertThat(reviewer.count).isEqualTo(2);
assertThat(reviewer.confirm).isTrue();
}
private List<SuggestedReviewerInfo> suggestReviewers(String changeId,
String query, int n) throws Exception {
return gApi.changes()

View File

@ -17,4 +17,6 @@ package com.google.gerrit.extensions.common;
public class SuggestedReviewerInfo {
public AccountInfo account;
public GroupBaseInfo group;
}
public int count;
public Boolean confirm;
}

View File

@ -145,17 +145,23 @@ public class ReviewersUtil {
for (AccountInfo a : suggestedAccounts) {
SuggestedReviewerInfo info = new SuggestedReviewerInfo();
info.account = a;
info.count = 1;
reviewer.add(info);
}
for (GroupReference g : suggestAccountGroup(suggestReviewers, projectControl)) {
if (suggestGroupAsReviewer(suggestReviewers, projectControl.getProject(),
g, visibilityControl)) {
GroupAsReviewer result = suggestGroupAsReviewer(
suggestReviewers, projectControl.getProject(), g, visibilityControl);
if (result.allowed || result.allowedWithConfirmation) {
GroupBaseInfo info = new GroupBaseInfo();
info.id = Url.encode(g.getUUID().get());
info.name = g.getName();
SuggestedReviewerInfo suggestedReviewerInfo = new SuggestedReviewerInfo();
suggestedReviewerInfo.group = info;
suggestedReviewerInfo.count = result.size;
if (result.allowedWithConfirmation) {
suggestedReviewerInfo.confirm = true;
}
reviewer.add(suggestedReviewerInfo);
}
}
@ -270,13 +276,22 @@ public class ReviewersUtil {
suggestReviewers.getLimit()));
}
private boolean suggestGroupAsReviewer(SuggestReviewers suggestReviewers,
private class GroupAsReviewer {
boolean allowed;
boolean allowedWithConfirmation;
int size;
}
private GroupAsReviewer suggestGroupAsReviewer(SuggestReviewers suggestReviewers,
Project project, GroupReference group,
VisibilityControl visibilityControl) throws OrmException, IOException {
GroupAsReviewer result = new GroupAsReviewer();
int maxAllowed = suggestReviewers.getMaxAllowed();
int maxAllowedWithoutConfirmation =
suggestReviewers.getMaxAllowedWithoutConfirmation();
if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
return false;
return result;
}
try {
@ -285,25 +300,33 @@ public class ReviewersUtil {
.listAccounts(group.getUUID(), project.getNameKey());
if (members.isEmpty()) {
return false;
return result;
}
if (maxAllowed > 0 && members.size() > maxAllowed) {
return false;
result.size = members.size();
if (maxAllowed > 0 && result.size > maxAllowed) {
return result;
}
boolean needsConfirmation = result.size > maxAllowedWithoutConfirmation;
// require that at least one member in the group can see the change
for (Account account : members) {
if (visibilityControl.isVisibleTo(account.getId())) {
return true;
if (needsConfirmation) {
result.allowedWithConfirmation = true;
} else {
result.allowed = true;
}
return result;
}
}
} catch (NoSuchGroupException e) {
return false;
return result;
} catch (NoSuchProjectException e) {
return false;
return result;
}
return false;
return result;
}
}

View File

@ -35,6 +35,7 @@ public class SuggestReviewers {
private final boolean suggestAccounts;
private final int suggestFrom;
private final int maxAllowed;
private final int maxAllowedWithoutConfirmation;
protected int limit;
protected String query;
protected final int maxSuggestedReviewers;
@ -73,6 +74,10 @@ public class SuggestReviewers {
return maxAllowed;
}
public int getMaxAllowedWithoutConfirmation() {
return maxAllowedWithoutConfirmation;
}
@Inject
public SuggestReviewers(AccountVisibility av,
IdentifiedUser.GenericFactory identifiedUserFactory,
@ -96,5 +101,8 @@ public class SuggestReviewers {
this.suggestFrom = cfg.getInt("suggest", null, "from", 0);
this.maxAllowed = cfg.getInt("addreviewer", "maxAllowed",
PostReviewers.DEFAULT_MAX_REVIEWERS);
this.maxAllowedWithoutConfirmation = cfg.getInt(
"addreviewer", "maxWithoutConfirmation",
PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
}
}