-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix cloud URL when Manage Jenkins UI is deferred #26107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
timja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR / issue, cc @janfaracik, we'll need to dig into and see why its returning null and causing issues now.
No problem. Long time Jenkins user, happy to contribute. I traced the regression to the lazy loading introduced here: https://github.com/janfaracik/jenkins/blob/366b31bf16ec6c48e9e03e6c55cac39767b5b9e4/core/src/main/resources/lib/layout/settings-subpage.jelly#L120 |
|
Please also see #26002 (comment). Can you test if that is sufficient? |
@mawinter69 Yes, setting |
Yes please |
timja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
Congratulations on getting your very first Jenkins core pull request merged 🎉🥳 |
Fixes #26106
In the new experimental deferred render,
getNearestAncestorUrlreturnsnulland sends the user to an invalid URL. Userequest.getContextPathto build the cloud URL in that case.Testing done
Let me know if any UTE is desired/required here. Unsure what goes for these experimental features. I did test that the URL builds correctly with this change.
Proposed changelog entries
Proposed changelog category
/label bug
/remove-label regression-fix
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.