opendev/gerrit
Code Issues Proposed changes

Submit: Point at problematic other commits in tooltip

Browse Source 
The information given should also be available via the "Submitted Together"
tab, but having spelled out only the problematic changes may be beneficial,
too. (In case you're missing the green check marks in the "Submitted
Together" tab.)

Change-Id: Ic10c2e737da5d4894d2517f12b7a6ce0f3ef1891
Signed-off-by: Stefan Beller <sbeller@google.com>
This commit is contained in:
Stefan Beller
2016-03-01 17:05:01 -08:00
parent 6b6bdbdce0
commit 7016d7a874
3 changed files with 31 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
View File

@@ -103,7 +103,7 @@ public class ActionsIT extends AbstractDaemonTest {
assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST");
assertThat(info.title).isEqualTo(
"See the \"Submitted Together\" tab for problems");
"See the \"Submitted Together\" tab for problems, specially see: 2");
} else {
noSubmitWholeTopicAssertions(actions, 1);
}

6
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
View File

@@ -293,7 +293,11 @@ public class SubmitResolvingMergeCommitIT extends AbstractDaemonTest {
boolean expected) throws MissingObjectException,
IncorrectObjectTypeException, IOException, OrmException {
ChangeSet cs = mergeSuperSet.completeChangeSet(db, change.change());
assertThat(submit.isPatchSetMergeable(cs)).isEqualTo(expected);
if (expected) {
assertThat(submit.unmergeableChanges(cs)).isEmpty();
} else {
assertThat(submit.unmergeableChanges(cs)).isNotEmpty();
}
}
private void assertMergeable(ChangeData change, boolean expected)

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

@@ -14,11 +14,14 @@
package com.google.gerrit.server.change;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.ParameterizedString;
@@ -67,6 +70,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -92,7 +96,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private static final String CLICK_FAILURE_TOOLTIP =
"Clicking the button would fail";
private static final String CHANGES_NOT_MERGEABLE =
"See the \"Submitted Together\" tab for problems";
"See the \"Submitted Together\" tab for problems, specially see: ";
public static class Output {
transient Change change;
@@ -241,11 +245,18 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
MergeOp.checkSubmitRule(c);
}
Boolean csIsMergeable = isPatchSetMergeable(cs);
if (csIsMergeable == null) {
Collection<ChangeData> unmergeable = unmergeableChanges(cs);
if (unmergeable == null) {
return CLICK_FAILURE_TOOLTIP;
} else if (!csIsMergeable) {
return CHANGES_NOT_MERGEABLE;
} else if (!unmergeable.isEmpty()) {
return CHANGES_NOT_MERGEABLE + Joiner.on(", ").join(
Iterables.transform(unmergeable,
new Function<ChangeData, String>() {
@Override
public String apply(ChangeData cd) {
return String.valueOf(cd.getId().get());
}
}));
}
} catch (ResourceConflictException e) {
return BLOCKED_SUBMIT_TOOLTIP;
@@ -384,10 +395,11 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
return change != null ? change.getStatus().name().toLowerCase() : "deleted";
}
public Boolean isPatchSetMergeable(ChangeSet cs) throws OrmException, IOException {
Map<ChangeData, Boolean> mergeabilityMap = new HashMap<>();
public Collection<ChangeData> unmergeableChanges(ChangeSet cs)
throws OrmException, IOException {
Set<ChangeData> mergeabilityMap = new HashSet<>();
for (ChangeData change : cs.changes()) {
mergeabilityMap.put(change, false);
mergeabilityMap.add(change);
}
Multimap<Branch.NameKey, ChangeData> cbb = cs.changesByBranch();
@@ -418,17 +430,19 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
// Skip whole check, cannot determine if mergeable
return null;
}
mergeabilityMap.put(change, mergeable);
if (mergeable) {
mergeabilityMap.remove(change);
}
if (isLastInChain && isMergeCommit && mergeable) {
for (ChangeData c : targetBranch) {
mergeabilityMap.put(c, true);
mergeabilityMap.remove(c);
}
break;
}
}
}
return !mergeabilityMap.values().contains(Boolean.FALSE);
return mergeabilityMap;
}
private HashMap<Change.Id, RevCommit> findCommits(