Improve error message for submit rules in Submit action
Previously, we would display "This change depends on other changes
which are not ready" when encountering a problem with the submit rules
in the Submit action.
This commit uses the exception's message instead. Since
ResourceConflictException is a REST API exception, displaying it's
message to a user is allowed.
Also improve the other error messages to include the change ID, when
doing so is a reasonable action (the change is not hidden).
Change-Id: I3b050c65844f3cdc21285b8477fec0a8bf123db5
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 54ecd18..ef08316 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -95,14 +95,10 @@
"Submit all ${topicSize} changes of the same topic "
+ "(${submitSize} changes including ancestors and other "
+ "changes related by topic)";
- private static final String BLOCKED_SUBMIT_TOOLTIP =
- "This change depends on other changes which are not ready";
private static final String BLOCKED_HIDDEN_SUBMIT_TOOLTIP =
"This change depends on other hidden changes which are not ready";
- private static final String BLOCKED_WORK_IN_PROGRESS = "This change is marked work in progress";
private static final String CLICK_FAILURE_TOOLTIP = "Clicking the button would fail";
private static final String CHANGE_UNMERGEABLE = "Problems with integrating this change";
- private static final String CHANGES_NOT_MERGEABLE = "Problems with change(s): ";
public static class Output {
transient Change change;
@@ -261,12 +257,16 @@
return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
}
if (!can.contains(ChangePermission.SUBMIT)) {
- return BLOCKED_SUBMIT_TOOLTIP;
+ return "You don't have permission to submit change " + c.getId();
}
if (c.change().isWorkInProgress()) {
- return BLOCKED_WORK_IN_PROGRESS;
+ return "Change " + c.getId() + " is marked work in progress";
}
- MergeOp.checkSubmitRule(c, false);
+ try {
+ MergeOp.checkSubmitRule(c, false);
+ } catch (ResourceConflictException e) {
+ return "Change " + c.getId() + " is not ready: " + e.getMessage();
+ }
}
Collection<ChangeData> unmergeable = unmergeableChanges(cs);
@@ -278,11 +278,10 @@
return CHANGE_UNMERGEABLE;
}
}
- return CHANGES_NOT_MERGEABLE
+
+ return "Problems with change(s): "
+ unmergeable.stream().map(c -> c.getId().toString()).collect(joining(", "));
}
- } catch (ResourceConflictException e) {
- return BLOCKED_SUBMIT_TOOLTIP;
} catch (PermissionBackendException | OrmException | IOException e) {
log.error("Error checking if change is submittable", e);
throw new OrmRuntimeException("Could not determine problems for the change", e);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index f89f2a1..171babd 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -105,7 +105,9 @@
public void revisionActionsTwoChangesInTopic() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
approve(changeId);
- String changeId2 = createChangeWithTopic().getChangeId();
+ PushOneCommit.Result change2 = createChangeWithTopic();
+ int legacyId2 = change2.getChange().getId().get();
+ String changeId2 = change2.getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) {
@@ -113,7 +115,7 @@
assertThat(info.enabled).isNull();
assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST");
- assertThat(info.title).isEqualTo("This change depends on other changes which are not ready");
+ assertThat(info.title).matches("Change " + legacyId2 + " is not ready: needs Code-Review");
} else {
noSubmitWholeTopicAssertions(actions, 1);
