JENKINS-44208 Web url builder extension #1063

vivek · 2017-05-12T23:25:16Z

Description

Renders 'Open Blue Ocean' as Task action menu on all Jenkins pages
See BlueOceanUrlFactory and BlueOceanUrlAction
To control individual Jenkins ModelObject url mapping implement BlueOceanUrlMapper
See implementations BlueOceanUrlActionFactoryImpl and BlueOceanUrlActionImpl

Basically, idea is for plugins interested in customizing blueocean url mapping for given ModelObject, they can implement BlueOceanUrlMapper. If there is need to customize Open Blue Ocean menu item/action, then implement BlueOceanUrlAction and BlueOceanUrlActionFactory.

Submitter checklist

Link to JIRA ticket in description, if appropriate.
Change is code complete and matches issue description
Appropriate unit or acceptance tests or explanation to why this change has no tests
Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.
Ran Acceptance Test Harness against PR changes.

Reviewer checklist

Run the changes and verified the change matches the issue description
Reviewed the code
Verified that the appropriate tests have been written or valid explanation given

given model object

This is to provide fine grained control for other plugins to generate url for given model object.

vivek · 2017-05-15T23:58:08Z

@brody @i386 Ended up using 48x48, it looked better:

i386 · 2017-05-15T23:59:08Z

Nice 👍

vivek · 2017-05-16T00:20:20Z

@alvarolobato PTAL. I think I have everything you need.

Implement BlueOceanUrlMapper to control custom mapping of jenkins ModelObject to url
Implement BlueOceanUrlActionFactory and BlueOceanUrlAction to customize the view of action menu to open blueocean. You might need to implement it for the ModelObject not handled by default implementation.

alvarolobato

Thanks @vivek . Looks good, just some minor comments.

alvarolobato · 2017-05-16T13:58:26Z

blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/BlueOceanUrlMapperImpl.java

+    public String getUrl(@Nonnull ModelObject modelObject) {
+        BlueOrganization organization = getOrganization(modelObject);
+        if(organization == null){ //no organization, best we can do is to land user on landing page
+            return getLandingPagePath();


Maybe better just return null and let the default behaviour return the landing page or another mapper handle it?

In any case this is ordinal -9999 so it would always be the last one.

@alvarolobato getLandingPagePath() is computed using BlueOceanUIProvider, so should be fine, anyways ordinal value is going to let other plugins override it.

You are right, I miss read the method.

alvarolobato · 2017-05-16T14:00:16Z

blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/BlueOceanUrlActionImpl.java

+ * @author Vivek Pandey
+ */
+@Extension(ordinal = -9999)
+public class BlueOceanUrlActionImpl implements BlueOceanUrlAction {


Same as above

alvarolobato · 2017-05-16T14:00:26Z

...cean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/BlueOceanUrlActionFactoryImpl.java

+ * @author Vivek Pandey
+ */
+@Extension(ordinal = -9999)
+public class BlueOceanUrlActionFactoryImpl extends BlueOceanUrlActionFactory {


I think this should be moved outside blueocean-pipeline-api-impl there's nothing specific to pipeline here anymore, and should be possible to use it without pipeline plugins

Agreed, it used to depend on this plugin but with this PR, we can pretty much move it inside blueocean-rest-impl.

alvarolobato · 2017-05-16T15:29:37Z

blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/BlueOceanUrlActionImpl.java

+
+import javax.annotation.Nonnull;
+
+import static io.jenkins.blueocean.BlueOceanUrlMapperImpl.getLandingPagePath;


This probably should need to be moved here, as we may not have the dependency with pipeline plugins

alvarolobato · 2017-05-16T15:34:58Z

🐝

- Removed try.js as it's not more needed

vivek · 2017-05-16T22:39:02Z

@alvarolobato PTAL. I think I have answered your concerns.

There are ATH tests failing as #open-blueocean-in-context element is no more there. I will fix those tests after I figure how to test these action menu. Thats coming up next.

- BlueOceanUrlAction is a RootAction so that `Open Blue Ocean` gets added reliably on Jenkins home page. - Make sure, BlueOceanUrlAction is added only once to left menu

…ension

vivek · 2017-05-18T00:46:16Z

@imeredith PTAL at ATH changes.

vivek · 2017-05-18T02:55:50Z

@imeredith tests are failing with same error:

Timed out while waiting for element <body.page-head-removed> to be present for 20000 milliseconds.  - expected "found" but got: "not found"
    at Timeout._onTimeout (/Users/vivek/ws/blueocean/acceptance-tests/src/main/js/custom_commands/removePageHead.js:50:18)

Perhaps these changes expose this timing bug? @imeredith It will be great if you could try out this branch.

imeredith · 2017-05-19T09:29:50Z

@vivek .useXpath() sets all selectors in the future to be xpath. A css one is failing so im guessing your use of xpath caused this (noticed in changes). You can do .useCss() to fix that

imeredith · 2017-05-19T09:45:29Z

Also the stuff in addOpenBlueOceanLinkToFooter is just flaky in general. If you cant get a test to pass it, try restarting the ATH server. its a real PITA.

vivek · 2017-05-19T14:30:49Z

@imeredith Ah, right. I reset selector to css. Lets see how this run goes. addOpenBlueOceanLinkToFooter is deleted because there is no need for this trick as that button on top is removed.

Provides methods to open and check visibility of 'open blue ocean' link

vivek · 2017-05-19T17:43:53Z

@imeredith I changed openBlueOcean.js from command to page object. This way there is one place to open blueocean page and check it's visibility. All looks good now. PTAL before I merge:)

michaelneale · 2017-05-22T03:08:08Z

acceptance-tests/runner/.blueocean-ath-jenkins-url

probably should be ignored

It is in my latest stuff

michaelneale · 2017-05-22T03:09:46Z

🐝 from me but need @imeredith to give this the nod - as it is a bit of a cross cutting change, but looks great!

imeredith · 2017-05-22T06:52:32Z

Why a pageobject and not a command? For a start its not a page, but it exists on most of classic pages.

vivek · 2017-05-22T16:27:50Z

@imeredith nightwatch page objects can be anything that appears on a page and tests want access to it, so it's fine for it to be page object without any side effect.

Other reasons listed below:

It exposes two methods, open() and visible(), if we go with command approach there will be two commands then. This makes it modular.
Fixes flakiness. From what I experienced, command objects trying to useXpath() doesn't work well even if it gets reset with useCss(). OTOH, page object element's locateStrategy works quite reliably.

…ension

imeredith · 2017-05-22T22:53:48Z

🐝

vivek added 3 commits May 12, 2017 10:03

JENKINS-44208# Generate url for organization

0080cbd

Fix for NPE, also handles when no OrganizationFactory is present for

9e85309

given model object

Extensions point to contribute blueocean url for jenkins model objects

36d7dfd

vivek added DO NOT MERGE work-in-progress labels May 12, 2017

BlueOceanUrlAction is not a root action

f71aef6

vivek mentioned this pull request May 13, 2017

JENKINS-44208# Generate url for organization #1061

Closed

8 tasks

vivek added 2 commits May 15, 2017 16:30

Added BlueOceanUrlMapper extension point

04d513a

This is to provide fine grained control for other plugins to generate url for given model object.

Added blueocean icon

f2472c8

vivek added 3 commits May 15, 2017 17:12

Doc fix

1f9c354

Added test for customer mapper

644f223

Renamed BlueOceanUrlFactory to BlueOceanUrlActionFactory

cc6d0d2

vivek changed the title ~~Web url builder extension~~ JENKINS-44208 Web url builder extension May 16, 2017

alvarolobato reviewed May 16, 2017

View reviewed changes

Try blueocean implementation classes moved to blueocean-rest-impl

e435602

- Removed try.js as it's not more needed

vivek added needs-review and removed DO NOT MERGE work-in-progress labels May 16, 2017

vivek added 3 commits May 17, 2017 16:31

Bug fixes

885ad6f

- BlueOceanUrlAction is a RootAction so that `Open Blue Ocean` gets added reliably on Jenkins home page. - Make sure, BlueOceanUrlAction is added only once to left menu

Merge remote-tracking branch 'origin/master' into web-url-builder-ext…

94dd165

…ension

ATH fixes related to 'Open Blue Ocean' changes

e8d21a9

set css selector back after doing Xpath

304ffaf

vivek added 2 commits May 19, 2017 09:55

Fix bug related to open blueocean button not appearing in some cases

98c6ba5

openBlueOcean is now a page object

74aecac

Provides methods to open and check visibility of 'open blue ocean' link

Removed commented out code

b513bb9

michaelneale reviewed May 22, 2017

View reviewed changes

michaelneale mentioned this pull request May 22, 2017

ATH: Java + logins #1072

Merged

8 tasks

Merge remote-tracking branch 'origin/master' into web-url-builder-ext…

16a7c2b

…ension

vivek merged commit 7faf481 into master May 22, 2017

vivek deleted the web-url-builder-extension branch May 22, 2017 23:07

cliffmeyers mentioned this pull request May 23, 2017

bug/JENKINS 44303 js lint or js test failures don't fail the build #1059

Merged

8 tasks


		import javax.annotation.Nonnull;

		import static io.jenkins.blueocean.BlueOceanUrlMapperImpl.getLandingPagePath;

JENKINS-44208 Web url builder extension #1063

JENKINS-44208 Web url builder extension #1063

Uh oh!

Conversation

vivek commented May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Submitter checklist

Reviewer checklist

Uh oh!

vivek commented May 15, 2017

Uh oh!

i386 commented May 15, 2017

Uh oh!

vivek commented May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alvarolobato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvarolobato commented May 16, 2017

Uh oh!

vivek commented May 16, 2017

Uh oh!

vivek commented May 18, 2017

Uh oh!

vivek commented May 18, 2017

Uh oh!

imeredith commented May 19, 2017

Uh oh!

imeredith commented May 19, 2017

Uh oh!

vivek commented May 19, 2017

Uh oh!

vivek commented May 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelneale commented May 22, 2017

Uh oh!

imeredith commented May 22, 2017

Uh oh!

vivek commented May 22, 2017

Uh oh!

imeredith commented May 22, 2017

Uh oh!

Uh oh!

vivek commented May 12, 2017 •

edited

Loading

vivek commented May 16, 2017 •

edited

Loading