opendev/gerrit
Code Issues Proposed changes

List reviewers with dummy approvals on closed changes

Browse Source 
Teams that allow submit bypassing review in some form or other may
want to add reviewers to a change after it has been submitted. This
was allowed, but the result was not being returned by ChangeJson in
the closed change case.

Change-Id: Ibfd3728993d9eacfe0e0002275d9a956d6e79024
This commit is contained in:
Dave Borowitz
2014-05-13 16:19:19 -07:00
parent e6ffd895ac
commit a19b319af1
2 changed files with 26 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -129,6 +129,27 @@ public class ChangeIT extends AbstractDaemonTest {
assertEquals(ImmutableSet.of(user.id), getReviewers(cApi.get()));
}
@Test
public void addReviewerToClosedChange() throws GitAPIException,
IOException, RestApiException {
PushOneCommit.Result r = createChange();
ChangeApi cApi = gApi.changes().id("p~master~" + r.getChangeId());
cApi.revision(r.getCommit().name())
.review(ReviewInput.approve());
cApi.revision(r.getCommit().name())
.submit();
assertEquals(ImmutableSet.of(admin.getId()), getReviewers(cApi.get()));
AddReviewerInput in = new AddReviewerInput();
in.reviewer = user.email;
gApi.changes()
.id("p~master~" + r.getChangeId())
.addReviewer(in);
assertEquals(ImmutableSet.of(admin.getId(), user.id),
getReviewers(cApi.get()));
}
@Test
public void createEmptyChange() throws RestApiException {
ChangeInfo in = new ChangeInfo();

9
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -565,19 +565,20 @@ public class ChangeJson {
allUsers.add(psa.getAccountId());
}
// We can only approximately reconstruct what the submit rule evaluator
// would have done. These should really come from a stored submit record.
Set<String> labelNames = Sets.newHashSet();
Multimap<Account.Id, PatchSetApproval> current = HashMultimap.create();
for (PatchSetApproval a : cd.currentApprovals()) {
LabelType type = labelTypes.byLabel(a.getLabelId());
if (type != null && a.getValue() != 0) {
if (type != null) {
labelNames.add(type.getName());
// Not worth the effort to distinguish between votable/non-votable for 0
// values on closed changes, since they can't vote anyway.
current.put(a.getAccountId(), a);
}
}
// We can only approximately reconstruct what the submit rule evaluator
// would have done. These should really come from a stored submit record.
//
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels = new TreeMap<>(labelTypes.nameComparator());
for (String name : labelNames) {