From 62d9e3637d1226f6c8f8d3b69d00d9c04be79b64 Mon Sep 17 00:00:00 2001 From: Maxime Guerreiro Date: Fri, 18 May 2018 12:45:28 +0000 Subject: [PATCH] Deduplicate conditions on current change in Submit Before this commit, the change was part of the change set on which the permissions and submit records were checked. As such, these checks were done twice, using potentially invalid cached submit records (stale). Instead, we want to ignore the change and only consider the parent (and the topic) changes. Change-Id: I28c302db7f8f153903493d999eb587527681d895 --- .../google/gerrit/server/restapi/change/Submit.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java index ef083165ef..51a4090a78 100644 --- a/java/com/google/gerrit/server/restapi/change/Submit.java +++ b/java/com/google/gerrit/server/restapi/change/Submit.java @@ -236,6 +236,11 @@ public class Submit } /** + * Returns a message describing what prevents the current change from being submitted - or null. + * This method only considers parent changes, and changes in the same topic. The caller is + * responsible for making sure the current change to be submitted can indeed be submitted + * (permissions, submit rules, is not a WIP...) + * * @param cd the change the user is currently looking at * @param cs set of changes to be submitted at once * @param user the user who is checking to submit @@ -247,6 +252,11 @@ public class Submit return BLOCKED_HIDDEN_SUBMIT_TOOLTIP; } for (ChangeData c : cs.changes()) { + if (cd.getId().equals(c.getId())) { + // We ignore the change about to be submitted, as these checks are already done in the + // #apply and #getDescription methods. + continue; + } Set can = permissionBackend .user(user) @@ -293,6 +303,7 @@ public class Submit public UiAction.Description getDescription(RevisionResource resource) { Change change = resource.getChange(); if (!change.getStatus().isOpen() + || change.isWorkInProgress() || !resource.isCurrent() || !resource.permissions().testOrFalse(ChangePermission.SUBMIT)) { return null; // submit not visible