jenkinsci / hipchat-plugin

HipChat notification plugin for Jenkins
https://plugins.jenkins.io/hipchat/
54 stars 85 forks source link

Update to use new display-url-api for user facing page links #82

Closed i386 closed 8 years ago

i386 commented 8 years ago

This allows Blue Ocean to specify what links get sent in HipChat messages.

@jenkinsci/code-reviewers

imod commented 8 years ago

👍

batmat commented 8 years ago

:+1:

i386 commented 8 years ago

getURL never returns a / prefix

i386 commented 8 years ago

@aldaris what do you think?

aldaris commented 8 years ago

If I may suggest one change:

diff --git a/src/test/java/jenkins/plugins/hipchat/utils/BuildUtilsTest.java b/src/test/java/jenkins/plugins/hipchat/utils/BuildUtilsTest.java
index 10c1988..c5c777c 100644
--- a/src/test/java/jenkins/plugins/hipchat/utils/BuildUtilsTest.java
+++ b/src/test/java/jenkins/plugins/hipchat/utils/BuildUtilsTest.java
@@ -32,7 +32,7 @@ import org.mockito.runners.MockitoJUnitRunner;
 public class BuildUtilsTest {

     @Rule
-    public JenkinsRuleWithServerPort rule = new JenkinsRuleWithServerPort();
+    public JenkinsRule rule = new JenkinsRule();
     @Mock
     private Jenkins jenkins;
     @Mock
@@ -133,12 +133,12 @@ public class BuildUtilsTest {
     public void collectedParametersContainCorrectUrl() throws Exception {
         setupMocks();

-        given(jenkins.getRootUrl()).willReturn("http://localhost:" + rule.getLocalPort() + "/jenkins/");
+        given(jenkins.getRootUrl()).willReturn(getRootUrl());
         given(run.getUrl()).willReturn("job/test%20project/1/");

         Map<String, String> collected = buildUtils.collectParametersFor(jenkins, run);

-        assertThat(collected).containsEntry("URL", "http://localhost:" + rule.getLocalPort() + "/jenkins/job/test%20project/1/");
+        assertThat(collected).containsEntry("URL", getRootUrl() + "job/test%20project/1/");
     }

     @Test
@@ -255,9 +255,7 @@ public class BuildUtilsTest {
         }
     }

-    final class JenkinsRuleWithServerPort extends JenkinsRule {
-        public int getLocalPort() {
-            return localPort;
-        }
+    private String getRootUrl() throws Exception {
+        return rule.getURL().toString();
     }
 }
aldaris commented 8 years ago

One final thing: could you squash the commits please? Thanks!

i386 commented 8 years ago

@aldaris patch applied and all commits squashed :)

i386 commented 8 years ago

Thanks mate!

rickthepick commented 7 years ago

Hi. One of our Jenkins users uses Blue Ocean - the others don't. I noticed that all the links in HipChat messages now link to Blue Ocean pages. How can I turn that off?

aldaris commented 7 years ago

I don't believe that's possible currently.

i386 commented 7 years ago

I'll release some updates today that will make it possible to disable on a per user basis