Rewrite rebuild actions using javascript java injection #580

panicking · 2025-02-03T01:58:03Z

First version of rebuild that includes all the job parameters. Some improvements should be done like have just one common.jelly but not clear how jelly include logic works.

Tested using gerrit-trigger

Some help is needed to clean up

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

src/main/resources/io/jenkins/plugins/pipelinegraphview/PipelineGraphViewAction/common.jelly

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

src/main/webapp/js/build.js

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

timja

Thanks! This is looking much better, few comments but this is very close.

timja · 2025-02-03T21:55:34Z

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

+     */
+    @RequirePOST
+    @JavaScriptMethod
+    public boolean doRebuildjob() throws IOException, ExecutionException {


Suggested change

public boolean doRebuildjob() throws IOException, ExecutionException {

public boolean doRebuild() throws IOException, ExecutionException {

timja · 2025-02-03T21:55:43Z

src/main/webapp/js/build.js

@@ -5,7 +5,13 @@ if (rebuildButton) {
    event.preventDefault();
    const buildUrl = `${rebuildButton.dataset.buildPath}?delay=0sec`
    if (rebuildButton.dataset.parameterized === 'true') {
-      window.location.href = buildUrl
+      var rebuildAction = window[`${rebuildButton.dataset.proxyName}`];
+      rebuildAction.doRebuildjob(function (success) {


Suggested change

rebuildAction.doRebuildjob(function (success) {

rebuildAction.doRebuild(function (success) {

timja · 2025-02-03T21:59:23Z

src/main/webapp/js/build.js

+        if (result) {
+          window.hoverNotification(rebuildButton.dataset.successMessage, rebuildButton);
+        }
+      });
    } else {
      fetch(buildUrl, {


do we need this anymore? Couldn't we just call the same code for rebuilding?

@timja I move this for a new cleanup, I need to remove even data is paremeterized I think.

Or, I can do it tomorrow. Basically I need to drop data-parameterized, the function and all the old code. Let's me think about

timja · 2025-02-03T22:00:00Z

src/main/webapp/js/build.js

+      rebuildAction.doRebuildjob(function (success) {
+        var result = success.responseJSON;
+        if (result) {
+          window.hoverNotification(rebuildButton.dataset.successMessage, rebuildButton);


as a future enhancement, would be good to provide a link to the scheduled build.

@timja you mean in the popup? I need even to think to show an error when you can't rebuild. Basically when you have a field that has a password you can't rebuild the job. I suggest to have two popup, one in green Build scheduled and one in red.

timja · 2025-02-03T22:00:15Z

.../plugins/pipelinegraphview/multipipelinegraphview/MultiPipelineGraphViewAction/jobMain.jelly

@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <?jelly escape-by-default='true'?>
-<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form">


change here is unneeded?

timja · 2025-02-03T22:01:26Z

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

+     * Handles the rebuild request and redirects to parameterized
+     * and non parameterized build when needed.


this doesn't seem to be true? It just adds a notification with the result?

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

timja

few minor cleanups then I think good to go.

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

.../plugins/pipelinegraphview/multipipelinegraphview/MultiPipelineGraphViewAction/jobMain.jelly

src/main/webapp/js/build.js

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java

Fix the rebuild that includes all the job parameters. Tested using gerrit-trigger Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

We don't need to distinguish between parameterized and non parameterized job We can drop some variables Suggested-by: Tim Jacomb <timjacomb1@gmail.com> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

timja

Thanks!

panicking requested a review from a team as a code owner February 3, 2025 01:58

panicking mentioned this pull request Feb 3, 2025

Rebuild button parameters not pass to the build #567

Closed

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show fixed Hide fixed

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show fixed Hide fixed

panicking force-pushed the fix-rebuild-trigger branch 2 times, most recently from 5071b79 to 811b5bd Compare February 3, 2025 02:15

KalleOlaviNiemitalo reviewed Feb 3, 2025

View reviewed changes

src/main/resources/io/jenkins/plugins/pipelinegraphview/PipelineGraphViewAction/common.jelly Outdated Show resolved Hide resolved

timja reviewed Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Outdated Show resolved Hide resolved

panicking force-pushed the fix-rebuild-trigger branch 3 times, most recently from 8f3d2ac to e9b47c3 Compare February 3, 2025 14:36

timja reviewed Feb 3, 2025

View reviewed changes

src/main/webapp/js/build.js Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show fixed Hide fixed

panicking force-pushed the fix-rebuild-trigger branch 3 times, most recently from f82a009 to 182417a Compare February 3, 2025 18:26

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show fixed Hide fixed

panicking force-pushed the fix-rebuild-trigger branch 3 times, most recently from f23b17b to 2049203 Compare February 3, 2025 20:48

timja reviewed Feb 3, 2025

View reviewed changes

panicking force-pushed the fix-rebuild-trigger branch from 2049203 to 88078b8 Compare February 3, 2025 22:19

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show resolved Hide resolved

panicking force-pushed the fix-rebuild-trigger branch 2 times, most recently from a6e3eb0 to d007dba Compare February 3, 2025 22:40

timja reviewed Feb 3, 2025

View reviewed changes

panicking force-pushed the fix-rebuild-trigger branch 2 times, most recently from 69860fa to 847abd3 Compare February 3, 2025 23:18

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/AbstractPipelineViewAction.java Fixed Show resolved Hide resolved

timja added the enhancement New feature or request label Feb 4, 2025

panicking force-pushed the fix-rebuild-trigger branch from 7055eab to 2e65734 Compare February 4, 2025 08:29

panicking added 2 commits February 4, 2025 09:44

Rewrite rebuild actions using javascript java injection

5486622

Fix the rebuild that includes all the job parameters. Tested using gerrit-trigger Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

Clean-up rebuild code to include even non parameterized job

d8e552b

We don't need to distinguish between parameterized and non parameterized job We can drop some variables Suggested-by: Tim Jacomb <timjacomb1@gmail.com> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>

panicking force-pushed the fix-rebuild-trigger branch from baaef34 to d8e552b Compare February 4, 2025 08:45

timja approved these changes Feb 4, 2025

View reviewed changes

timja merged commit 7857c96 into jenkinsci:main Feb 4, 2025
17 checks passed

panicking deleted the fix-rebuild-trigger branch February 5, 2025 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite rebuild actions using javascript java injection #580

Rewrite rebuild actions using javascript java injection #580

panicking commented Feb 3, 2025

timja left a comment •

edited

Loading

timja Feb 3, 2025

panicking Feb 3, 2025

timja Feb 3, 2025

panicking Feb 3, 2025

timja Feb 3, 2025

panicking Feb 3, 2025

panicking Feb 3, 2025

panicking Feb 3, 2025

timja Feb 3, 2025

panicking Feb 3, 2025

timja Feb 3, 2025

panicking Feb 3, 2025

timja Feb 3, 2025

panicking Feb 3, 2025

timja left a comment

timja left a comment

	public boolean doRebuildjob() throws IOException, ExecutionException {
	public boolean doRebuild() throws IOException, ExecutionException {

	rebuildAction.doRebuildjob(function (success) {
	rebuildAction.doRebuild(function (success) {

		* Handles the rebuild request and redirects to parameterized
		* and non parameterized build when needed.

Rewrite rebuild actions using javascript java injection #580

Rewrite rebuild actions using javascript java injection #580

Conversation

panicking commented Feb 3, 2025

timja left a comment • 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

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

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

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

timja left a comment •

edited

Loading