opendev/gerrit
Code Issues Proposed changes

PostReview: Ignore empty labels on closed changes

Browse Source 
A user may have cast a vote on a change (Code-Review+2), had the
change submitted, and then lose their permission to that label in this
context.  Don't try to squash away labels on a closed change.  Instead
just bail early if there were no labels requested in the input and the
change is already closed.

Change-Id: I0be02ec5dc625250bed9a44d29d0dfc24fe4745a
This commit is contained in:
Shawn Pearce
2016-03-28 22:40:07 -07:00
parent 04e7766158
commit 273ca99db0
1 changed files with 10 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -531,6 +531,13 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
Map<String, Short> inLabels = MoreObjects.firstNonNull(in.labels, Map<String, Short> inLabels = MoreObjects.firstNonNull(in.labels,
Collections.<String, Short> emptyMap()); Collections.<String, Short> emptyMap());
// If no labels were modified and change is closed, abort early.
// This avoids trying to record a modified label caused by a user
// losing access to a label after the change was submitted.
if (inLabels.isEmpty() && ctx.getChange().getStatus().isClosed()) {
return false;
}
List<PatchSetApproval> del = Lists.newArrayList(); List<PatchSetApproval> del = Lists.newArrayList();
List<PatchSetApproval> ups = Lists.newArrayList(); List<PatchSetApproval> ups = Lists.newArrayList();
Map<String, PatchSetApproval> current = scanLabels(ctx, del); Map<String, PatchSetApproval> current = scanLabels(ctx, del);
@@ -604,10 +611,9 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
} }
} }
if (!del.isEmpty() || !ups.isEmpty()) { if ((!del.isEmpty() || !ups.isEmpty())
if (ctx.getChange().getStatus().isClosed()) { && ctx.getChange().getStatus().isClosed()) {
throw new ResourceConflictException("change is closed"); throw new ResourceConflictException("change is closed");
}
} }
forceCallerAsReviewer(ctx, current, ups, del); forceCallerAsReviewer(ctx, current, ups, del);
ctx.getDb().patchSetApprovals().delete(del); ctx.getDb().patchSetApprovals().delete(del);