Merge pull request #476 from janfaracik/show-pr-title

[JENKINS-60082] Show pull request title in name column

src/main/java/jenkins/branch/ItemColumn.java

@@ -100,31 +100,6 @@ public boolean isOrphaned(Object item) {
         return false;
     }
 
-    /**
-     * Gets the tool-tip title of a job.
-     *
-     * @param job the job.
-     * @return the tool-tip title unescaped for use in an attribute.
-     */
-    @SuppressWarnings("unused") // used via Jelly EL binding
-    public String getTitle(Object job) {
-        // Jelly will take care of quote and ampersand escaping for us
-        if (job instanceof Actionable) {
-            Actionable actionable = (Actionable) job;
-            ObjectMetadataAction action = actionable.getAction(ObjectMetadataAction.class);
-            if (action != null) {
-                String displayName = action.getObjectDisplayName();
-                if (StringUtils.isBlank(displayName) || displayName.equals(actionable.getDisplayName())) {
-                    // if the display name is the same, then the description is more useful
-                    String description = action.getObjectDescription();
-                    return description != null ? description : displayName;
-                }
-                return displayName;
-            }
-        }
-        return null;
-    }
-
     /**
      * Our extension.
      */

src/main/java/jenkins/branch/MultiBranchProject.java

@@ -2104,22 +2104,16 @@ private void observeNew(@NonNull SCMHead head, @NonNull SCMRevision revision, @N
                 throw new IllegalStateException(
                     "Name of created project " + project + " did not match expected " + encodedName);
             }
-            // HACK ALERT
-            // ==========
-            // We don't want to trigger a save, so we will do some trickery to ensure that observer.created(project)
-            // performs the save
-            BulkChange bc = new BulkChange(project);
-            try {
-                project.setDisplayName(getProjectDisplayName(project, rawName));
-            } catch (IOException e) {
-                // ignore even if it does happen we didn't want a save
-            } finally {
-                bc.abort();
-            }
             // decorate contract is to ensure it does not trigger a save
             _factory.decorate(project);
             // ok it is now up to the observer to ensure it does the actual save.
-            observer.created(project);
+            try (BulkChange bc = new BulkChange(project);) {
+                observer.created(project);
+                project.setDisplayName(getProjectDisplayName(project, rawName));
+                bc.commit();
+            } catch (IOException e) {
+                // Ignored
+            }
             doAutomaticBuilds(head, revision, rawName, project, revisionActions, null, null);
         }
 
@@ -2137,8 +2131,9 @@ private String getProjectDisplayName(@NonNull P project, @NonNull String rawName
                     .orElse(null);
             }
 
+            // Default to displaying the project's display name if a trait hasn't been provided
             if (naming == null) {
-                return rawName;
+                naming = MultiBranchProjectDisplayNamingStrategy.RAW_AND_OBJECT_DISPLAY_NAME;
             }
 
             final ObjectMetadataAction action = naming.needsObjectDisplayName()

src/main/java/jenkins/branch/MultiBranchProjectDisplayNamingStrategy.java

@@ -60,7 +60,14 @@ public String generateName(@NonNull final String rawName, final String displayNa
                 return rawName;
             }
 
-            return format("%s - %s", rawName, displayName);
+            // The raw name provided here in the context of pull requests is the pull request ID
+            // We tidy up the ID so that they display consistently between SCMs
+            String cleanedUpBranchName = rawName;
+            if (cleanedUpBranchName.startsWith("MR-") || cleanedUpBranchName.startsWith("PR-")) {
+                cleanedUpBranchName = "#" + cleanedUpBranchName.substring(3);
+            }
+
+            return format("%s (%s)", displayName, cleanedUpBranchName);
         }
     },
     ;

src/main/resources/jenkins/branch/ItemColumn/column.jelly

@@ -25,26 +25,28 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
          xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
-  <td style="${indenter.getCss(job)}">
+  <d:taglib uri="local">
+    <d:tag name="link">
+      <a href="${jobBaseUrl}${job.shortUrl}" class='model-link jenkins-table__link'>
+        <l:breakable value="${h.getRelativeDisplayNameFrom(job, itemGroup)}"/>
+      </a>
+    </d:tag>
+  </d:taglib>
+
+  <td style="${indenter.getCss(job)}" xmlns:local="local">
     <j:choose>
       <j:when test="${it.isOrphaned(job)}">
         <s>
-          <a href="${jobBaseUrl}${job.shortUrl}" class='model-link inside jenkins-table_link' title="${it.getTitle(job)}">
-            <l:breakable value="${h.getRelativeDisplayNameFrom(job, itemGroup)}"/>
-          </a>
+          <local:link />
         </s>
       </j:when>
       <j:when test="${it.isPrimary(job)}">
         <strong>
-          <a href="${jobBaseUrl}${job.shortUrl}" class='model-link inside jenkins-table_link' title="${it.getTitle(job)}">
-            <l:breakable value="${h.getRelativeDisplayNameFrom(job, itemGroup)}"/>
-          </a>
+          <local:link />
         </strong>
       </j:when>
       <j:otherwise>
-        <a href="${jobBaseUrl}${job.shortUrl}" class='model-link inside jenkins-table_link' title="${it.getTitle(job)}">
-          <l:breakable value="${h.getRelativeDisplayNameFrom(job, itemGroup)}"/>
-        </a>
+        <local:link />
       </j:otherwise>
     </j:choose>
   </td>

src/test/java/integration/CategorizationTest.java

@@ -136,7 +136,7 @@ public void given_multibranch_when_changeRequestsWanted_then_onlyUncategorizedAn
             assertThat(prj.getItems(),
                     containsInAnyOrder(
                             hasProperty("displayName", is("master")),
-                            hasProperty("displayName", is("CR-" + crNum))
+                            hasProperty("displayName", is("Change request #1 (CR-1)"))
                     )
             );
             assertThat(prj.getViews(),
@@ -204,7 +204,7 @@ public void given_multibranch_when_noBranchesWanted_then_uncategorizedViewPresen
             assertThat(prj.getItems(),
                     containsInAnyOrder(
                             hasProperty("displayName", is("master-1.0")),
-                            hasProperty("displayName", is("CR-" + crNum))
+                            hasProperty("displayName", is("Change request #1 (CR-1)"))
                     )
             );
             assertThat(prj.getViews(),
@@ -222,7 +222,7 @@ public void given_multibranch_when_noBranchesWanted_then_uncategorizedViewPresen
                             allOf(
                                     instanceOf(MultiBranchProjectViewHolder.ViewImpl.class),
                                     hasProperty("viewName", is(ChangeRequestSCMHeadCategory.DEFAULT.getName())),
-                                    hasProperty("items", contains(hasProperty("displayName", is("CR-" + crNum))))
+                                    hasProperty("items", contains(hasProperty("displayName", is("Change request #1 (CR-1)"))))
                             )
                     ));
         }
@@ -253,8 +253,8 @@ public void given_multibranch_when_wantsEverything_then_hasEverything()
                             hasProperty("displayName", is("feature")),
                             hasProperty("displayName", is("master-1.0")),
                             hasProperty("displayName", is("master-1.1")),
-                            hasProperty("displayName", is("CR-" + crNum1)),
-                            hasProperty("displayName", is("CR-" + crNum2))
+                            hasProperty("displayName", is("Change request #1 (CR-1)")),
+                            hasProperty("displayName", is("Change request #2 (CR-2)"))
                     )
             );
             assertThat(prj.getViews(),
@@ -279,8 +279,8 @@ public void given_multibranch_when_wantsEverything_then_hasEverything()
                                     instanceOf(MultiBranchProjectViewHolder.ViewImpl.class),
                                     hasProperty("viewName", is(ChangeRequestSCMHeadCategory.DEFAULT.getName())),
                                     hasProperty("items", containsInAnyOrder(
-                                            hasProperty("displayName", is("CR-" + crNum2)),
-                                            hasProperty("displayName", is("CR-" + crNum1))
+                                            hasProperty("displayName", is("Change request #1 (CR-1)")),
+                                            hasProperty("displayName", is("Change request #2 (CR-2)"))
                                     ))
                             )
                     ));