From 0ef8d9bf36e0b3bb097a31dbac50f4469f505104 Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Fri, 16 Aug 2024 14:18:57 +0100 Subject: [PATCH 1/9] Show pull request title --- .../jenkins/branch/ItemColumn/column.jelly | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/main/resources/jenkins/branch/ItemColumn/column.jelly b/src/main/resources/jenkins/branch/ItemColumn/column.jelly index 1b2139b8..64feff6b 100644 --- a/src/main/resources/jenkins/branch/ItemColumn/column.jelly +++ b/src/main/resources/jenkins/branch/ItemColumn/column.jelly @@ -25,26 +25,36 @@ THE SOFTWARE. - + + + + + + + + + ${it.getTitle(job)} + + + + + + + + - - - + - - - + - - - + From 13cedc4a043ac4a8f991d7d463b4b09ce11098e8 Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Fri, 16 Aug 2024 14:21:20 +0100 Subject: [PATCH 2/9] Update column.jelly --- src/main/resources/jenkins/branch/ItemColumn/column.jelly | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/jenkins/branch/ItemColumn/column.jelly b/src/main/resources/jenkins/branch/ItemColumn/column.jelly index 64feff6b..deb0fd89 100644 --- a/src/main/resources/jenkins/branch/ItemColumn/column.jelly +++ b/src/main/resources/jenkins/branch/ItemColumn/column.jelly @@ -27,14 +27,14 @@ THE SOFTWARE. xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt"> - + ${it.getTitle(job)} - + From a37b5d34358469a0a48e19c9c9d07d9190a5676f Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Fri, 16 Aug 2024 15:07:30 +0100 Subject: [PATCH 3/9] Update ItemColumn.java --- src/main/java/jenkins/branch/ItemColumn.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/jenkins/branch/ItemColumn.java b/src/main/java/jenkins/branch/ItemColumn.java index 24114efc..be226b2a 100644 --- a/src/main/java/jenkins/branch/ItemColumn.java +++ b/src/main/java/jenkins/branch/ItemColumn.java @@ -101,10 +101,10 @@ public boolean isOrphaned(Object item) { } /** - * Gets the tool-tip title of a job. + * Gets the title of a job. * * @param job the job. - * @return the tool-tip title unescaped for use in an attribute. + * @return the unescaped title for displaying in a table column. */ @SuppressWarnings("unused") // used via Jelly EL binding public String getTitle(Object job) { From c3620a4ffbb6a0af81eaf1d33c869053eab6c185 Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Sat, 17 Aug 2024 10:59:41 +0100 Subject: [PATCH 4/9] Don't wrap PR IDs --- src/main/resources/jenkins/branch/ItemColumn/column.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/jenkins/branch/ItemColumn/column.jelly b/src/main/resources/jenkins/branch/ItemColumn/column.jelly index deb0fd89..6135bc15 100644 --- a/src/main/resources/jenkins/branch/ItemColumn/column.jelly +++ b/src/main/resources/jenkins/branch/ItemColumn/column.jelly @@ -34,7 +34,7 @@ THE SOFTWARE. ${it.getTitle(job)} - + From 9de361d4413e05d7b8720e3f6b9e7285b6d528a2 Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Wed, 28 Aug 2024 10:55:41 +0100 Subject: [PATCH 5/9] Update MultiBranchProject.java --- src/main/java/jenkins/branch/MultiBranchProject.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/branch/MultiBranchProject.java b/src/main/java/jenkins/branch/MultiBranchProject.java index c01c9086..a8f2b4ec 100644 --- a/src/main/java/jenkins/branch/MultiBranchProject.java +++ b/src/main/java/jenkins/branch/MultiBranchProject.java @@ -2137,8 +2137,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.OBJECT_DISPLAY_NAME; } final ObjectMetadataAction action = naming.needsObjectDisplayName() From 315ce8db1bd1b55db81d606a30811b3d1f076024 Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Wed, 28 Aug 2024 19:06:51 +0100 Subject: [PATCH 6/9] Another take on displaying titles --- src/main/java/jenkins/branch/ItemColumn.java | 25 ------------------- .../jenkins/branch/MultiBranchProject.java | 2 +- ...ltiBranchProjectDisplayNamingStrategy.java | 9 ++++++- .../jenkins/branch/ItemColumn/column.jelly | 10 +------- 4 files changed, 10 insertions(+), 36 deletions(-) diff --git a/src/main/java/jenkins/branch/ItemColumn.java b/src/main/java/jenkins/branch/ItemColumn.java index be226b2a..4e419733 100644 --- a/src/main/java/jenkins/branch/ItemColumn.java +++ b/src/main/java/jenkins/branch/ItemColumn.java @@ -100,31 +100,6 @@ public boolean isOrphaned(Object item) { return false; } - /** - * Gets the title of a job. - * - * @param job the job. - * @return the unescaped title for displaying in a table column. - */ - @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. */ diff --git a/src/main/java/jenkins/branch/MultiBranchProject.java b/src/main/java/jenkins/branch/MultiBranchProject.java index a8f2b4ec..feb433b1 100644 --- a/src/main/java/jenkins/branch/MultiBranchProject.java +++ b/src/main/java/jenkins/branch/MultiBranchProject.java @@ -2139,7 +2139,7 @@ private String getProjectDisplayName(@NonNull P project, @NonNull String rawName // Default to displaying the project's display name if a trait hasn't been provided if (naming == null) { - naming = MultiBranchProjectDisplayNamingStrategy.OBJECT_DISPLAY_NAME; + naming = MultiBranchProjectDisplayNamingStrategy.RAW_AND_OBJECT_DISPLAY_NAME; } final ObjectMetadataAction action = naming.needsObjectDisplayName() diff --git a/src/main/java/jenkins/branch/MultiBranchProjectDisplayNamingStrategy.java b/src/main/java/jenkins/branch/MultiBranchProjectDisplayNamingStrategy.java index da62023d..a08c25e7 100644 --- a/src/main/java/jenkins/branch/MultiBranchProjectDisplayNamingStrategy.java +++ b/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); } }, ; diff --git a/src/main/resources/jenkins/branch/ItemColumn/column.jelly b/src/main/resources/jenkins/branch/ItemColumn/column.jelly index 6135bc15..59e29f65 100644 --- a/src/main/resources/jenkins/branch/ItemColumn/column.jelly +++ b/src/main/resources/jenkins/branch/ItemColumn/column.jelly @@ -28,15 +28,7 @@ THE SOFTWARE. - - - - - - ${it.getTitle(job)} - - - + From 02109d5b8213f2508eb27da09886786ef157c692 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sun, 13 Oct 2024 11:59:59 +0100 Subject: [PATCH 7/9] Some progress --- .../jenkins/branch/MultiBranchProject.java | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/main/java/jenkins/branch/MultiBranchProject.java b/src/main/java/jenkins/branch/MultiBranchProject.java index feb433b1..d7dac7a5 100644 --- a/src/main/java/jenkins/branch/MultiBranchProject.java +++ b/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); } From 8718f94a74fb97953012825ce13c6f995fbf31ed Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sun, 13 Oct 2024 15:34:49 +0100 Subject: [PATCH 8/9] Fix tests --- src/test/java/integration/CategorizationTest.java | 14 +++++++------- src/test/java/integration/MigrationTest.java | 14 ++++---------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/test/java/integration/CategorizationTest.java b/src/test/java/integration/CategorizationTest.java index c9b81834..eb4a8f88 100644 --- a/src/test/java/integration/CategorizationTest.java +++ b/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)")) )) ) )); diff --git a/src/test/java/integration/MigrationTest.java b/src/test/java/integration/MigrationTest.java index c29d20aa..7c9ca887 100644 --- a/src/test/java/integration/MigrationTest.java +++ b/src/test/java/integration/MigrationTest.java @@ -235,20 +235,14 @@ public void evaluate() throws Throwable { @Test @LocalData public void nameMangling_folder_reload() throws Exception { - r.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - OrganizationFolder foo = r.j.jenkins.getItemByFullName("foo", OrganizationFolder.class); + r.then((j) -> { + OrganizationFolder foo = j.jenkins.getItemByFullName("foo", OrganizationFolder.class); foo.doReload(); assertDataMigrated(foo); - } }); - r.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - TopLevelItem foo = r.j.jenkins.getItem("foo"); + r.then((j) -> { + TopLevelItem foo = j.jenkins.getItem("foo"); assertDataMigrated(foo); - } }); } From c6198ed21ab2887ff8e542ea74cc73120a3d91fd Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Sun, 13 Oct 2024 16:02:10 +0100 Subject: [PATCH 9/9] Discard changes to src/test/java/integration/MigrationTest.java --- src/test/java/integration/MigrationTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/java/integration/MigrationTest.java b/src/test/java/integration/MigrationTest.java index 7c9ca887..c29d20aa 100644 --- a/src/test/java/integration/MigrationTest.java +++ b/src/test/java/integration/MigrationTest.java @@ -235,14 +235,20 @@ public void evaluate() throws Throwable { @Test @LocalData public void nameMangling_folder_reload() throws Exception { - r.then((j) -> { - OrganizationFolder foo = j.jenkins.getItemByFullName("foo", OrganizationFolder.class); + r.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + OrganizationFolder foo = r.j.jenkins.getItemByFullName("foo", OrganizationFolder.class); foo.doReload(); assertDataMigrated(foo); + } }); - r.then((j) -> { - TopLevelItem foo = j.jenkins.getItem("foo"); + r.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + TopLevelItem foo = r.j.jenkins.getItem("foo"); assertDataMigrated(foo); + } }); }