Merge "Improve error message for submit rules in Submit action"

This commit is contained in:
Patrick Hiesel
2018-05-23 15:57:30 +00:00
committed by Gerrit Code Review
2 changed files with 13 additions and 12 deletions

View File

@@ -95,14 +95,10 @@ public class Submit
"Submit all ${topicSize} changes of the same topic " "Submit all ${topicSize} changes of the same topic "
+ "(${submitSize} changes including ancestors and other " + "(${submitSize} changes including ancestors and other "
+ "changes related by topic)"; + "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 = private static final String BLOCKED_HIDDEN_SUBMIT_TOOLTIP =
"This change depends on other hidden changes which are not ready"; "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 CLICK_FAILURE_TOOLTIP = "Clicking the button would fail";
private static final String CHANGE_UNMERGEABLE = "Problems with integrating this change"; 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 { public static class Output {
transient Change change; transient Change change;
@@ -261,12 +257,16 @@ public class Submit
return BLOCKED_HIDDEN_SUBMIT_TOOLTIP; return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
} }
if (!can.contains(ChangePermission.SUBMIT)) { if (!can.contains(ChangePermission.SUBMIT)) {
return BLOCKED_SUBMIT_TOOLTIP; return "You don't have permission to submit change " + c.getId();
} }
if (c.change().isWorkInProgress()) { if (c.change().isWorkInProgress()) {
return BLOCKED_WORK_IN_PROGRESS; return "Change " + c.getId() + " is marked work in progress";
}
try {
MergeOp.checkSubmitRule(c, false);
} catch (ResourceConflictException e) {
return "Change " + c.getId() + " is not ready: " + e.getMessage();
} }
MergeOp.checkSubmitRule(c, false);
} }
Collection<ChangeData> unmergeable = unmergeableChanges(cs); Collection<ChangeData> unmergeable = unmergeableChanges(cs);
@@ -278,11 +278,10 @@ public class Submit
return CHANGE_UNMERGEABLE; return CHANGE_UNMERGEABLE;
} }
} }
return CHANGES_NOT_MERGEABLE
return "Problems with change(s): "
+ unmergeable.stream().map(c -> c.getId().toString()).collect(joining(", ")); + unmergeable.stream().map(c -> c.getId().toString()).collect(joining(", "));
} }
} catch (ResourceConflictException e) {
return BLOCKED_SUBMIT_TOOLTIP;
} catch (PermissionBackendException | OrmException | IOException e) { } catch (PermissionBackendException | OrmException | IOException e) {
log.error("Error checking if change is submittable", e); log.error("Error checking if change is submittable", e);
throw new OrmRuntimeException("Could not determine problems for the change", e); throw new OrmRuntimeException("Could not determine problems for the change", e);

View File

@@ -105,7 +105,9 @@ public class ActionsIT extends AbstractDaemonTest {
public void revisionActionsTwoChangesInTopic() throws Exception { public void revisionActionsTwoChangesInTopic() throws Exception {
String changeId = createChangeWithTopic().getChangeId(); String changeId = createChangeWithTopic().getChangeId();
approve(changeId); 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); Map<String, ActionInfo> actions = getActions(changeId);
commonActionsAssertions(actions); commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) { if (isSubmitWholeTopicEnabled()) {
@@ -113,7 +115,7 @@ public class ActionsIT extends AbstractDaemonTest {
assertThat(info.enabled).isNull(); assertThat(info.enabled).isNull();
assertThat(info.label).isEqualTo("Submit whole topic"); assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST"); 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 { } else {
noSubmitWholeTopicAssertions(actions, 1); noSubmitWholeTopicAssertions(actions, 1);