JENKINS-40979 add trigger cause messages #996

i386 · 2017-04-23T02:37:25Z

Description

Adds trigger cause to Run detail
If there are no commits the cause is displayed in Activity and Branch message column

See JENKINS-40979.

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

@vivek there are some Java API changes to help make this happen.

…riggered * Adds trigger cause to Run detail * If there are no commits the cause is displayed in Activity and Branch message column

i386 · 2017-04-23T02:39:49Z

...n-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/FilteringTreePruner.java

@@ -25,7 +28,7 @@ public TreePruner accept(Object node, Property prop) {

        // for merge properties, the current restrictions on the property names should
        // still apply to the child TreePruner
-        if (prop.merge)
+        if (prop.merge && child != null)


@vivek Use of merge in the 'Cause' @Exported(name="cause", merge = true) caused this NPE. This check fixes it but perhaps this should go upstream?

Yeah, please open a PR in stapler with this fix.

Fixed jenkinsci/stapler#112

i386 · 2017-04-23T02:40:23Z

@michaelneale this is going to make it 1,000,000 times easier to find the correct runs of the ATH.

i386 · 2017-04-23T04:43:04Z

blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/Export.java

@@ -158,6 +160,9 @@ public Object getValue(Property property, Object model, ExportConfig config) thr
                    printError(model.getClass(), e);
                    return SKIP;
                }
+            } else if (model instanceof Item || model instanceof Run) {


@vivek I have skipped serialisation of Item and Run objects in case they are returned by a Cause object. These are evil for performance reasons anyway. I have checked implementations in core and I can't see any that do this but I've added it to be on the safe side (who knows what plugins will do).

Its a hack but should be fine until we find something else pulling in something else during serialization thats not item or run. We can revisit if its trouble later on.

i386 · 2017-04-23T12:51:10Z

@cliffmeyers this is the PR with the hanging tests

vivek

@i386 We could have used causeAction directly instead of wrapping in a backend abstraction, no? I guess there are many actions and we could end up duplicating wrapping them in our backend. Other than that, there are couple of minor comments and looks good.

vivek · 2017-04-24T16:52:56Z

blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java

    @Exported(name = CAUSE_OF_BLOCKAGE)
    public abstract String getCauseOfBlockage();

+    @ExportedBean
+    public static abstract class Cause {


Its a proper abstraction, perhaps should be pulled out in separate file? Also 'Blue' prefix to be consistent?

vivek · 2017-04-24T16:55:24Z

blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java

+        public abstract Object getCause();
+
+        @Exported(name = "_class")
+        public abstract String get_Class();


Why do we need this in abstraction? Stapler does it automatically, or implementations can override that behavior, no?

vivek · 2017-04-24T16:59:42Z

...n-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/FilteringTreePruner.java

@@ -25,7 +28,7 @@ public TreePruner accept(Object node, Property prop) {

        // for merge properties, the current restrictions on the property names should
        // still apply to the child TreePruner
-        if (prop.merge)
+        if (prop.merge && child != null)


Yeah, please open a PR in stapler with this fix.

vivek · 2017-04-24T17:03:23Z

blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/Export.java

@@ -158,6 +160,9 @@ public Object getValue(Property property, Object model, ExportConfig config) thr
                    printError(model.getClass(), e);
                    return SKIP;
                }
+            } else if (model instanceof Item || model instanceof Run) {


Its a hack but should be fine until we find something else pulling in something else during serialization thats not item or run. We can revisit if its trouble later on.

i386 · 2017-04-24T21:00:18Z

We could have used causeAction directly instead of wrapping in a backend abstraction, no?

@vivek only if you are happy with Cause leaking out into blueocean-rest's public API - which I assume we do not?

vivek · 2017-04-24T21:05:26Z

@i386 I meant using action mechanism, causeAction is served via actions element. Well, its now available only via tree query, which we don't have it yet. I guess, we can deal with Cause this way and handle other future actions via action mechanism.

i386 · 2017-04-24T21:08:28Z

@vivek we would need to pass the cause through to the client everywhere in the system... (everywhere we use BlueRun) would be easier when we get @kzantow decorated extensions that automatically fetch action's for us.

vivek · 2017-04-24T21:09:45Z

@i386 Sounds good 👍

i386 · 2017-04-24T21:29:42Z

@vivek fixed the comments about naming here.

i386 · 2017-04-24T21:30:08Z

@cliffmeyers @tfennelly in order to merge I need to get this change to jsbuilder merged and a release made tfennelly/jenkins-js-builder#2

vivek

LGTM 🐝

…eature/JENKINS-40979

i386 · 2017-04-25T00:57:00Z

Requires jsbuilder PR to be merged, released then bumped across all modules.

i386 · 2017-04-27T01:25:47Z

ATH https://ci.blueocean.io/job/ATH-Jenkinsfile/job/master/2273/

michaelneale · 2017-04-27T01:43:36Z

🐝 ATH is happy

JENKINS-40979 Developer would like to see why the Pipeline has been t…

8469fe3

…riggered * Adds trigger cause to Run detail * If there are no commits the cause is displayed in Activity and Branch message column

i386 requested review from vivek and cliffmeyers April 23, 2017 02:37

.

3487e0f

i386 commented Apr 23, 2017

View reviewed changes

James Dumay added 5 commits April 23, 2017 13:06

.

b6c82d6

Add test for activity

ea5be6e

Tests for branch tab

5ebe9be

Inline run causes

9ee53a0

Skip item or run objects from serilization

6586b86

i386 added the needs-review label Apr 23, 2017

i386 commented Apr 23, 2017

View reviewed changes

import eslint

07f3fe4

vivek reviewed Apr 24, 2017

View reviewed changes

i386 mentioned this pull request Apr 24, 2017

JENKINS-40979 NPE where child can be null jenkinsci/stapler#112

Merged

James Dumay added 4 commits April 25, 2017 07:21

Upgrade jsbuilder @jenkins-cd/js-builder@0.0.51-jdumay-1

6d4c96e

s/Cause/BlueCause/

02a0c1d

s/Cause/BlueCause/

9cd8c9a

Remove _clazz

91d70ef

vivek approved these changes Apr 24, 2017

View reviewed changes

Merge branch 'master' of github.com:jenkinsci/blueocean-plugin into f…

dcba389

…eature/JENKINS-40979

James Dumay added 6 commits April 26, 2017 19:06

upgrade jsbuilder

8d0d0d3

merge

08b4e59

.

c00a124

jsbuilder

7800962

fix shrink

e7b2b4d

.

133061e

i386 merged commit 6d49f4e into master Apr 27, 2017

i386 deleted the feature/JENKINS-40979 branch April 27, 2017 01:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JENKINS-40979 add trigger cause messages #996

JENKINS-40979 add trigger cause messages #996

i386 commented Apr 23, 2017 •

edited

Loading

i386 Apr 23, 2017 •

edited

Loading

vivek Apr 24, 2017

i386 Apr 24, 2017

vivek Apr 24, 2017

i386 commented Apr 23, 2017

i386 Apr 23, 2017 •

edited

Loading

vivek Apr 24, 2017

i386 commented Apr 23, 2017

vivek left a comment

vivek Apr 24, 2017

vivek Apr 24, 2017

vivek Apr 24, 2017

vivek Apr 24, 2017

i386 commented Apr 24, 2017 •

edited

Loading

vivek commented Apr 24, 2017

i386 commented Apr 24, 2017

vivek commented Apr 24, 2017

i386 commented Apr 24, 2017

i386 commented Apr 24, 2017

vivek left a comment

i386 commented Apr 25, 2017

i386 commented Apr 27, 2017

michaelneale commented Apr 27, 2017

JENKINS-40979 add trigger cause messages #996

JENKINS-40979 add trigger cause messages #996

Conversation

i386 commented Apr 23, 2017 • edited Loading

Description

Submitter checklist

Reviewer checklist

i386 Apr 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i386 commented Apr 23, 2017

i386 Apr 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i386 commented Apr 23, 2017

vivek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i386 commented Apr 24, 2017 • edited Loading

vivek commented Apr 24, 2017

i386 commented Apr 24, 2017

vivek commented Apr 24, 2017

i386 commented Apr 24, 2017

i386 commented Apr 24, 2017

vivek left a comment

Choose a reason for hiding this comment

i386 commented Apr 25, 2017

i386 commented Apr 27, 2017

michaelneale commented Apr 27, 2017

i386 commented Apr 23, 2017 •

edited

Loading

i386 Apr 23, 2017 •

edited

Loading

i386 Apr 23, 2017 •

edited

Loading

i386 commented Apr 24, 2017 •

edited

Loading