Instead of deleting patch-set-approval set vote to zero
Problem: some reviewers get removed from change after it gets submitted.
On submitting a change we used to delete those patch-set-approvals where
the reviewer didn't have permission to vote in that label or the label
didn't exist any more.
On the other side, when a reviewer is added to a change this is recorded
as patch-set-approval on some label, with zero vote. If the chosen label
happens to be one where the reviewer doesn't have permission to vote,
this patch-set-approval will be deleted on submit and the reviewer will
be removed from the change.
Even if the added reviewer did have permission to vote in the chosen label,
the label could get deleted before the change is submitted. Again, the
reviewer will be removed from the change.
Instead of deleting patch-set-approvals set the vote to zero so that we
don't accidentally remove reviewers.
NOTE: this issue doesn't occur when using NoteDb. In NoteDb we keep
track of reviewers explicitly and this is why for NoteDb we still want
to delete these approvals.
Change-Id: I4d4f8d7b55c12a0484257192e7d0ed8d2f71ebcc
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
index bdf9f052..b32d668 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
@@ -361,8 +361,8 @@
PatchSet.Id psId = update.getPatchSetId();
ctx.getDb().patchSetApprovals().upsert(
convertPatchSet(normalized.getNormalized(), psId));
- ctx.getDb().patchSetApprovals().delete(
- convertPatchSet(normalized.deleted(), psId));
+ ctx.getDb().patchSetApprovals().update(
+ zero(convertPatchSet(normalized.deleted(), psId)));
for (PatchSetApproval psa : normalized.updated()) {
update.putApprovalFor(psa.getAccountId(), psa.getLabel(), psa.getValue());
}
@@ -400,6 +400,19 @@
return Iterables.transform(approvals, convertPatchSet(psId));
}
+ private static Iterable<PatchSetApproval> zero(
+ Iterable<PatchSetApproval> approvals) {
+ return Iterables.transform(approvals,
+ new Function<PatchSetApproval, PatchSetApproval>() {
+ @Override
+ public PatchSetApproval apply(PatchSetApproval in) {
+ PatchSetApproval copy = new PatchSetApproval(in.getPatchSetId(), in);
+ copy.setValue((short) 0);
+ return copy;
+ }
+ });
+ }
+
private String getByAccountName() {
checkNotNull(submitter,
"getByAccountName called before submitter populated");
