Submit whole topic: disable submit button if not ready

The current implementation just disables the submit button if the current
user could not submit all changes in the topic. Additionally we want to
block the submit button if the changes do not pass the submit rule checks
individually. When the submission is disabled, show a tooltip explaining
why there is a problem.

This change is also still not what we want to have eventually:
We're having a cache invalidation problem, when other changes in
the same topic change and affect the submission of the whole topic here.

Change-Id: Ic242e738efa9b53cbf0dfd167e64b070c84dac29

gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java

@@ -94,6 +94,10 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       "Submit patch set ${patchSet} into ${branch}";
   private static final String DEFAULT_TOPIC_TOOLTIP =
       "Submit all ${topicSize} changes of the same topic";
+  private static final String BLOCKED_TOPIC_TOOLTIP =
+      "Other changes in this topic are not ready";
+  private static final String BLOCKED_HIDDEN_TOPIC_TOOLTIP =
+      "Other hidden changes in this topic are not ready";
 
   public enum Status {
     SUBMITTED, MERGED
@@ -233,22 +237,32 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     }
   }
 
-  private boolean areChangesSubmittable(List<ChangeData> changes,
+  /**
+   * @param changes list of changes to be submitted at once
+   * @param identifiedUser the user who is checking to submit
+   * @return a reason why any of the changes is not submittable or null
+   */
+  private String problemsForSubmittingChanges(List<ChangeData> changes,
       IdentifiedUser identifiedUser) {
     for (ChangeData c : changes) {
       try {
         ChangeControl changeControl = c.changeControl().forUser(
             identifiedUser);
-        if (!changeControl.canSubmit()) {
-          return false;
+        if (!changeControl.isVisible(dbProvider.get())) {
+          return BLOCKED_HIDDEN_TOPIC_TOOLTIP;
         }
+        if (!changeControl.canSubmit()) {
+          return BLOCKED_TOPIC_TOOLTIP;
+        }
+        checkSubmitRule(c, c.currentPatchSet(), false);
       } catch (OrmException e) {
-        log.error("Failed to get a ChangeControl for Change.Id " +
-            String.valueOf(c.getId()), e);
-        return false;
+        log.error("Error checking if change is submittable", e);
+        throw new OrmRuntimeException(e);
+      } catch (ResourceConflictException e) {
+        return BLOCKED_TOPIC_TOOLTIP;
       }
     }
-    return true;
+    return null;
   }
 
   @Override
@@ -274,17 +288,22 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       }
       Map<String, String> params = ImmutableMap.of(
           "topicSize", String.valueOf(changesByTopic.size()));
-      // TODO(sbeller):
-      // If the button is visible but disabled the problem of submitting
-      // is at another change in the same topic. Tell the user via
-      // tooltip. Caution: Check access control for those changes.
-      return new UiAction.Description()
-        .setLabel(submitTopicLabel)
-        .setTitle(Strings.emptyToNull(
-            submitTopicTooltip.replace(params)))
-        .setVisible(true)
-        .setEnabled(areChangesSubmittable(
-            changesByTopic, resource.getUser()));
+      String topicProblems = problemsForSubmittingChanges(changesByTopic,
+          resource.getUser());
+      if (topicProblems != null) {
+        return new UiAction.Description()
+          .setLabel(submitTopicLabel)
+          .setTitle(topicProblems)
+          .setVisible(true)
+          .setEnabled(false);
+      } else {
+        return new UiAction.Description()
+          .setLabel(submitTopicLabel)
+          .setTitle(Strings.emptyToNull(
+              submitTopicTooltip.replace(params)))
+          .setVisible(true)
+          .setEnabled(true);
+      }
     } else {
       RevId revId = resource.getPatchSet().getRevision();
       Map<String, String> params = ImmutableMap.of(
@@ -336,13 +355,16 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   private Change submitThisChange(RevisionResource rsrc, IdentifiedUser caller,
       boolean force) throws ResourceConflictException, OrmException,
       IOException {
-    List<SubmitRecord> submitRecords = checkSubmitRule(rsrc, force);
+    ReviewDb db = dbProvider.get();
+    ChangeData cd = changeDataFactory.create(db, rsrc.getControl());
+    List<SubmitRecord> submitRecords = checkSubmitRule(cd,
+        rsrc.getPatchSet(), force);
+
     final Timestamp timestamp = TimeUtil.nowTs();
     Change change = rsrc.getChange();
     ChangeUpdate update = updateFactory.create(rsrc.getControl(), timestamp);
     update.submit(submitRecords);
 
-    ReviewDb db = dbProvider.get();
     db.changes().beginTransaction(change.getId());
     try {
       BatchMetaDataUpdate batch = approve(rsrc, update, caller, timestamp);
@@ -364,13 +386,18 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       boolean force, String topic) throws ResourceConflictException, OrmException,
       IOException {
     Preconditions.checkNotNull(topic);
-    List<SubmitRecord> submitRecords = checkSubmitRule(rsrc, force);
     final Timestamp timestamp = TimeUtil.nowTs();
+
+    ReviewDb db = dbProvider.get();
+    ChangeData cd = changeDataFactory.create(db, rsrc.getControl());
+
+    List<SubmitRecord> submitRecords = checkSubmitRule(cd,
+        rsrc.getPatchSet(), force);
+
     Change change = rsrc.getChange();
     ChangeUpdate update = updateFactory.create(rsrc.getControl(), timestamp);
     update.submit(submitRecords);
 
-    ReviewDb db = dbProvider.get();
     db.changes().beginTransaction(change.getId());
 
     List<ChangeData> changesByTopic = queryProvider.get().byTopicOpen(topic);
@@ -498,12 +525,11 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     }
   }
 
-  private List<SubmitRecord> checkSubmitRule(RevisionResource rsrc,
-      boolean force) throws ResourceConflictException, OrmException {
-    ChangeData cd =
-        changeDataFactory.create(dbProvider.get(), rsrc.getControl());
+  private List<SubmitRecord> checkSubmitRule(ChangeData cd,
+      PatchSet patchSet, boolean force)
+          throws ResourceConflictException, OrmException {
     List<SubmitRecord> results = new SubmitRuleEvaluator(cd)
-        .setPatchSet(rsrc.getPatchSet())
+        .setPatchSet(patchSet)
         .canSubmit();
     Optional<SubmitRecord> ok = findOkRecord(results);
     if (ok.isPresent()) {
@@ -514,8 +540,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     } else if (results.isEmpty()) {
       throw new IllegalStateException(String.format(
           "ChangeControl.canSubmit returned empty list for %s in %s",
-          rsrc.getPatchSet().getId(),
-          rsrc.getChange().getProject().get()));
+          patchSet.getId(),
+          cd.change().getProject().get()));
     }
 
     for (SubmitRecord record : results) {
@@ -562,8 +588,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
                 throw new IllegalStateException(String.format(
                     "Unsupported SubmitRecord.Label %s for %s in %s",
                     lbl.toString(),
-                    rsrc.getPatchSet().getId(),
-                    rsrc.getChange().getProject().get()));
+                    patchSet.getId(),
+                    cd.change().getProject().get()));
             }
           }
           throw new ResourceConflictException(msg.toString());
@@ -572,8 +598,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
           throw new IllegalStateException(String.format(
               "Unsupported SubmitRecord %s for %s in %s",
               record,
-              rsrc.getPatchSet().getId(),
-              rsrc.getChange().getProject().get()));
+              patchSet.getId().getId(),
+              cd.change().getProject().get()));
       }
     }
     throw new IllegalStateException();