Change label and tooltip of submit button if submitting atomic topics
This is just changing the ui. There is no change in functionality yet.
Traditionally if you could not submit the change, the submit button
was just not there (visible=false). This behavior is still there if
atomic topics are enabled and the submission is prevented by the
current change. That doesn't break with the history.
If however another change in the same topic blocks submission the
submit button is shown, but disabled. It would be a nice follow up
change to give the reason to the user what exactly blocks submission,
but that blocker may not be accessible for the current user.
Designing a precise and helpful error message, which doesn't leak
any secrets is done in a follow up commit.
Change-Id: Ib32a4511a0308da32a8c5d1c7b1a6f124a03576e
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index be7d05c..0bb29ef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -61,8 +61,10 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.OrmRuntimeException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -72,6 +74,8 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.sql.Timestamp;
@@ -82,8 +86,12 @@
@Singleton
public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
UiAction<RevisionResource> {
+ private static final Logger log = LoggerFactory.getLogger(Submit.class);
+
private static final String DEFAULT_TOOLTIP =
"Submit patch set ${patchSet} into ${branch}";
+ private static final String DEFAULT_TOPIC_TOOLTIP =
+ "Submit all ${topicSize} changes of the same topic";
public enum Status {
SUBMITTED, MERGED
@@ -114,6 +122,10 @@
private final ChangesCollection changes;
private final String label;
private final ParameterizedString titlePattern;
+ private final String submitTopicLabel;
+ private final ParameterizedString submitTopicTooltip;
+ private final boolean submitWholeTopic;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
Submit(@GerritPersonIdent PersonIdent serverIdent,
@@ -129,7 +141,8 @@
ChangesCollection changes,
ChangeIndexer indexer,
LabelNormalizer labelNormalizer,
- @GerritServerConfig Config cfg) {
+ @GerritServerConfig Config cfg,
+ Provider<InternalChangeQuery> queryProvider) {
this.serverIdent = serverIdent;
this.dbProvider = dbProvider;
this.repoManager = repoManager;
@@ -149,6 +162,14 @@
this.titlePattern = new ParameterizedString(MoreObjects.firstNonNull(
cfg.getString("change", null, "submitTooltip"),
DEFAULT_TOOLTIP));
+ submitWholeTopic = cfg.getBoolean("change", null, "submitWholeTopic" , false);
+ this.submitTopicLabel = MoreObjects.firstNonNull(
+ Strings.emptyToNull(cfg.getString("change", null, "submitTopicLabel")),
+ "Submit whole topic");
+ this.submitTopicTooltip = new ParameterizedString(MoreObjects.firstNonNull(
+ cfg.getString("change", null, "submitTopicTooltip"),
+ DEFAULT_TOPIC_TOOLTIP));
+ this.queryProvider = queryProvider;
}
@Override
@@ -209,22 +230,69 @@
}
}
+ private boolean areChangesSubmittable(List<ChangeData> changes,
+ IdentifiedUser identifiedUser) {
+ for (ChangeData c : changes) {
+ try {
+ ChangeControl changeControl = c.changeControl().forUser(
+ identifiedUser);
+ if (!changeControl.canSubmit()) {
+ return false;
+ }
+ } catch (OrmException e) {
+ log.error("Failed to get a ChangeControl for Change.Id " +
+ String.valueOf(c.getId()), e);
+ return false;
+ }
+ }
+ return true;
+ }
+
@Override
public UiAction.Description getDescription(RevisionResource resource) {
PatchSet.Id current = resource.getChange().currentPatchSetId();
- RevId revId = resource.getPatchSet().getRevision();
- Map<String, String> params = ImmutableMap.of(
- "patchSet", String.valueOf(resource.getPatchSet().getPatchSetId()),
- "branch", resource.getChange().getDest().getShortName(),
- "commit", ObjectId.fromString(revId.get()).abbreviate(7).name());
-
- return new UiAction.Description()
- .setLabel(label)
- .setTitle(Strings.emptyToNull(titlePattern.replace(params)))
- .setVisible(!resource.getPatchSet().isDraft()
- && resource.getChange().getStatus().isOpen()
- && resource.getPatchSet().getId().equals(current)
- && resource.getControl().canSubmit());
+ String topic = resource.getChange().getTopic();
+ boolean visible = !resource.getPatchSet().isDraft()
+ && resource.getChange().getStatus().isOpen()
+ && resource.getPatchSet().getId().equals(current)
+ && resource.getControl().canSubmit();
+ if (!visible) {
+ return new UiAction.Description()
+ .setLabel("")
+ .setTitle("")
+ .setVisible(false);
+ }
+ if (submitWholeTopic && !topic.isEmpty()) {
+ List<ChangeData> changesByTopic = null;
+ try {
+ changesByTopic = queryProvider.get().byTopic(topic);
+ } catch (OrmException e) {
+ throw new OrmRuntimeException(e);
+ }
+ 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()));
+ } else {
+ RevId revId = resource.getPatchSet().getRevision();
+ Map<String, String> params = ImmutableMap.of(
+ "patchSet", String.valueOf(resource.getPatchSet().getPatchSetId()),
+ "branch", resource.getChange().getDest().getShortName(),
+ "commit", ObjectId.fromString(revId.get()).abbreviate(7).name());
+ return new UiAction.Description()
+ .setLabel(label)
+ .setTitle(Strings.emptyToNull(titlePattern.replace(params)))
+ .setVisible(true);
+ }
}
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 97fcffb..c50c3a2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -52,6 +52,10 @@
return new ChangeStatusPredicate(status);
}
+ private static Predicate<ChangeData> topic(String topic) {
+ return new TopicPredicate(topic);
+ }
+
private final QueryProcessor qp;
@Inject
@@ -114,6 +118,11 @@
return query(and(project(project), open()));
}
+ public List<ChangeData> byTopic(String topic)
+ throws OrmException {
+ return query(topic(topic));
+ }
+
private List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
try {
return qp.queryChanges(p).changes();
