From 9933eb35df25f99ffef24721a0e3c0890444b3c0 Mon Sep 17 00:00:00 2001
From: David Ostrovsky <david@ostrovsky.org>
Date: Tue, 20 May 2014 16:08:20 +0200
Subject: [PATCH] Allow to customize Submit button label and tooltip

Ibe3547942 renamed "Submit" button to "Merge Change" with justification
why to prefer "Merge Change" label over "Submit".

I5d7e1d21a reverted that change and restored it back to "Submit" with
another justification for why "Merge Change" can be misleading.

This change externalize the label name and tooltip for Submit button
to enable site administrator to customize it.

Bug: issue 2667
Change-Id: Iec78b31051f6b0022af1f337f5d492b78c7df096
---
 Documentation/config-gerrit.txt               | 15 ++++++++
 .../common/data/ParameterizedStringTest.java  | 38 +++++++++++++++++--
 .../google/gerrit/client/change/Actions.java  |  8 +++-
 .../gerrit/client/change/Actions.ui.xml       |  4 +-
 .../google/gerrit/server/change/Submit.java   | 33 +++++++++++++---
 5 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 2f551f2125..881f19d917 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -790,6 +790,21 @@ deleted or published.
 +
 Default is true.
 
+[[change.submitLabel]]change.submitLabel::
++
+Label name for the submit button.
++
+Default is "Submit".
+
+[[change.submitTooltip]]change.submitTooltip::
++
+Tooltip for the submit button.  Variables available for replacement
+include `${patchSet}` for the current patch set number (1, 2, 3),
+`${branch}` for the branch name ("master") and `${commit}` for the
+abbreviated commit SHA-1 (`c9c0edb`).
++
+Default is "Submit patch set ${patchSet} into ${branch}".
+
 [[changeMerge]]
 === Section changeMerge
 
diff --git a/gerrit-common/src/test/java/com/google/gerrit/common/data/ParameterizedStringTest.java b/gerrit-common/src/test/java/com/google/gerrit/common/data/ParameterizedStringTest.java
index 615f49f2e8..b350a2725f 100644
--- a/gerrit-common/src/test/java/com/google/gerrit/common/data/ParameterizedStringTest.java
+++ b/gerrit-common/src/test/java/com/google/gerrit/common/data/ParameterizedStringTest.java
@@ -14,15 +14,17 @@
 
 package com.google.gerrit.common.data;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+
 import org.junit.Test;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
 public class ParameterizedStringTest {
   @Test
   public void testEmptyString() {
@@ -394,4 +396,32 @@ public class ParameterizedStringTest {
     assertEquals("foo@example.com", p.bind(a)[0]);
     assertEquals("foo@example.com", p.replace(a));
   }
+
+  @Test
+  public void testReplaceSubmitTooltipWithVariables() {
+    ParameterizedString p = new ParameterizedString(
+        "Submit patch set ${patchSet} into ${branch}");
+    assertEquals(2, p.getParameterNames().size());
+    assertTrue(p.getParameterNames().contains("patchSet"));
+
+    Map<String, String> params = ImmutableMap.of(
+        "patchSet", "42",
+        "branch", "foo");
+    assertNotNull(p.bind(params));
+    assertEquals(2, p.bind(params).length);
+    assertEquals("42", p.bind(params)[0]);
+    assertEquals("foo", p.bind(params)[1]);
+    assertEquals("Submit patch set 42 into foo", p.replace(params));
+  }
+
+  @Test
+  public void testReplaceSubmitTooltipWithoutVariables() {
+    ParameterizedString p = new ParameterizedString(
+            "Submit patch set 40 into master");
+    Map<String, String> params = ImmutableMap.of(
+        "patchSet", "42",
+        "branch", "foo");
+    assertEquals(0, p.bind(params).length);
+    assertEquals("Submit patch set 40 into master", p.replace(params));
+  }
 }
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
index 92444d0748..906efcad25 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
@@ -30,6 +30,7 @@ import com.google.gwt.uibinder.client.UiHandler;
 import com.google.gwt.user.client.ui.Button;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 
 import java.util.TreeSet;
 
@@ -113,7 +114,12 @@ class Actions extends Composite {
     if (hasUser) {
       canSubmit = actions.containsKey("submit");
       if (canSubmit) {
-        submit.setTitle(actions.get("submit").title());
+        ActionInfo action = actions.get("submit");
+        submit.setTitle(action.title());
+        submit.setHTML(new SafeHtmlBuilder()
+            .openDiv()
+            .append(action.label())
+            .closeDiv());
       }
       a2b(actions, "/", deleteRevision);
       a2b(actions, "cherrypick", cherrypick);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.ui.xml
index 72e6a61185..cb2b37f36c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.ui.xml
@@ -100,8 +100,6 @@ limitations under the License.
       <div><ui:msg>Restore</ui:msg></div>
     </g:Button>
 
-    <g:Button ui:field='submit' styleName='{style.submit}' visible='false'>
-      <div><ui:msg>Submit</ui:msg></div>
-    </g:Button>
+    <g:Button ui:field='submit' styleName='{style.submit}' visible='false'/>
   </g:FlowPanel>
 </ui:UiBinder>
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 8ed242ca7b..be2788a11c 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
@@ -16,15 +16,18 @@ package com.google.gerrit.server.change;
 
 import static com.google.gerrit.common.data.SubmitRecord.Status.OK;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Table;
+import com.google.gerrit.common.data.ParameterizedString;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -38,6 +41,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
+import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeUtil;
@@ -46,6 +50,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ProjectUtil;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.LabelNormalizer;
 import com.google.gerrit.server.git.MergeQueue;
@@ -62,6 +67,8 @@ import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 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 java.io.IOException;
@@ -73,6 +80,9 @@ import java.util.Map;
 @Singleton
 public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     UiAction<RevisionResource> {
+  private static final String DEFAULT_TOOLTIP =
+      "Submit patch set ${patchSet} into ${branch}";
+
   public enum Status {
     SUBMITTED, MERGED
   }
@@ -98,6 +108,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   private final LabelNormalizer labelNormalizer;
   private final AccountsCollection accounts;
   private final ChangesCollection changes;
+  private final String label;
+  private final ParameterizedString titlePattern;
 
   @Inject
   Submit(@GerritPersonIdent PersonIdent serverIdent,
@@ -110,7 +122,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       AccountsCollection accounts,
       ChangesCollection changes,
       ChangeIndexer indexer,
-      LabelNormalizer labelNormalizer) {
+      LabelNormalizer labelNormalizer,
+      @GerritServerConfig Config cfg) {
     this.serverIdent = serverIdent;
     this.dbProvider = dbProvider;
     this.repoManager = repoManager;
@@ -122,6 +135,12 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     this.changes = changes;
     this.indexer = indexer;
     this.labelNormalizer = labelNormalizer;
+    this.label = Objects.firstNonNull(
+        Strings.emptyToNull(cfg.getString("change", null, "submitLabel")),
+        "Submit");
+    this.titlePattern = new ParameterizedString(Objects.firstNonNull(
+        cfg.getString("change", null, "submitTooltip"),
+        DEFAULT_TOOLTIP));
   }
 
   @Override
@@ -185,11 +204,15 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   @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()
-      .setTitle(String.format(
-          "Submit patch set %d into %s",
-          resource.getPatchSet().getPatchSetId(),
-          resource.getChange().getDest().getShortName()))
+      .setLabel(label)
+      .setTitle(Strings.emptyToNull(titlePattern.replace(params)))
       .setVisible(!resource.getPatchSet().isDraft()
           && resource.getChange().getStatus().isOpen()
           && resource.getPatchSet().getId().equals(current)