Merge "Better message if change submit fails due to dependency on outdated patch set"

java/com/google/gerrit/server/submit/CommitMergeStatus.java

@@ -16,11 +16,13 @@ package com.google.gerrit.server.submit;
 
 import static java.util.stream.Collectors.toSet;
 
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Provider;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Status codes set on {@link com.google.gerrit.server.git.CodeReviewCommit}s by {@link
@@ -96,9 +98,31 @@ public enum CommitMergeStatus {
               + " Is the change of this commit not visible or was it deleted?",
           commit, otherCommit);
     } else if (changes.size() == 1) {
+      ChangeData cd = changes.get(0);
+      if (cd.currentPatchSet().getRevision().get().equals(otherCommit)) {
+        return String.format(
+            "Commit %s depends on commit %s of change %d which cannot be merged.",
+            commit, otherCommit, cd.getId().get());
+      }
+      Optional<PatchSet> patchSet =
+          cd.patchSets()
+              .stream()
+              .filter(ps -> ps.getRevision().get().equals(otherCommit))
+              .findAny();
+      if (patchSet.isPresent()) {
+        return String.format(
+            "Commit %s depends on commit %s, which is outdated patch set %d of change %d."
+                + " The latest patch set is %d.",
+            commit,
+            otherCommit,
+            patchSet.get().getId().get(),
+            cd.getId().get(),
+            cd.currentPatchSet().getId().get());
+      }
+      // should not happen, fall-back to default message
       return String.format(
           "Commit %s depends on commit %s of change %d which cannot be merged.",
-          commit, otherCommit, changes.get(0).getId().get());
+          commit, otherCommit, cd.getId().get());
     } else {
       return String.format(
           "Commit %s depends on commit %s of changes %s which cannot be merged.",

javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java

@@ -30,6 +30,7 @@ import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import java.io.File;
 import java.io.InputStream;
@@ -525,6 +526,48 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertChangeMergedEvents();
   }
 
+  @Test
+  public void dependencyOnOutdatedPatchSetPreventsMerge() throws Exception {
+    // Create a change
+    PushOneCommit change = pushFactory.create(db, user.getIdent(), testRepo, "fix", "a.txt", "foo");
+    PushOneCommit.Result changeResult = change.to("refs/for/master");
+    PatchSet.Id patchSetId = changeResult.getPatchSetId();
+
+    // Create a successor change.
+    PushOneCommit change2 =
+        pushFactory.create(db, user.getIdent(), testRepo, "feature", "b.txt", "bar");
+    PushOneCommit.Result change2Result = change2.to("refs/for/master");
+
+    // Create new patch set for first change.
+    testRepo.reset(changeResult.getCommit().name());
+    amendChange(changeResult.getChangeId());
+
+    // Approve both changes
+    approve(changeResult.getChangeId());
+    approve(change2Result.getChangeId());
+
+    submitWithConflict(
+        change2Result.getChangeId(),
+        "Failed to submit 2 changes due to the following problems:\n"
+            + "Change "
+            + change2Result.getChange().getId()
+            + ": Depends on change that was not submitted."
+            + " Commit "
+            + change2Result.getCommit().name()
+            + " depends on commit "
+            + changeResult.getCommit().name()
+            + ", which is outdated patch set "
+            + patchSetId.get()
+            + " of change "
+            + changeResult.getChange().getId()
+            + ". The latest patch set is "
+            + changeResult.getPatchSetId().get()
+            + ".");
+
+    assertRefUpdatedEvents();
+    assertChangeMergedEvents();
+  }
+
   @Test
   public void dependencyOnDeletedChangePreventsMerge() throws Exception {
     // Create a change