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
This commit is contained in:
@@ -61,8 +61,10 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
|
||||
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.CommitBuilder;
|
||||
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 @@ import java.util.Map;
|
||||
@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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
|
||||
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
|
||||
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
|
||||
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
|
||||
}
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@@ -52,6 +52,10 @@ public class InternalChangeQuery {
|
||||
return new ChangeStatusPredicate(status);
|
||||
}
|
||||
|
||||
private static Predicate<ChangeData> topic(String topic) {
|
||||
return new TopicPredicate(topic);
|
||||
}
|
||||
|
||||
private final QueryProcessor qp;
|
||||
|
||||
@Inject
|
||||
@@ -114,6 +118,11 @@ public class InternalChangeQuery {
|
||||
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();
|
||||
|
Reference in New Issue
Block a user