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
[JENKINS-66753] Ongoing build disappears from build history #5760
[JENKINS-66753] Ongoing build disappears from build history #5760
Conversation
// The value for `page-next-build` is modified inside of `entries.jelly` on render, so set the attribute | ||
// on element `#buildHistoryPage` after that component has been rendered to get | ||
// the correct value to use in `filter-build-history.js` | ||
document.getElementById("buildHistoryPage").setAttribute("page-next-build", "${it.nextBuildNumberToFetch ?: it.owner.nextBuildNumber}"); |
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.
Would it be possible to do this without an inline script block?
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.
I'm not too sure sadly. Is there a way of sending the Jelly variable to filter-build-history.js
without it?
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.
Add your value to a data-whatever
attribute on an element with ID and read it from there:
https://www.jenkins.io/doc/developer/security/xss-prevention/#the-good-way
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.
I could add an additional element (after <st:include page="entries.jelly" it="${page}" />
so that the value is correct) with page-next-build
set instead of the inline JS -
<st:include page="entries.jelly" it="${page}" />
<div id="properties" page-next-build="${it.nextBuildNumberToFetch ?: it.owner.nextBuildNumber}"></div>
If that's what you mean?
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.
Yes, exactly. Thanks!
(Just to be safe, CC @Wadeck )
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.
I've updated it to be like that :) No more inline JS 🎉
Could you clarify whether this change addresses a recent regression or a long-standing bug? The issue description indicates it's recent, but I've had to reload the page to see new builds in the past. |
it's a recent regression |
As Tim said above, it was a regression due to the redesign build history search bar story.
I did find an issue where there is a delay in loading the history when tabbing back to the build page, I have a PR in the works though which should improve that though. |
could you restore the PR template with a changelog entry? |
FTR this has a lot of votes on the changelog feedback https://www.jenkins.io/changelog/ |
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. Thanks! |
@janfaracik it appears that there is still a disappearance on the very first build. See JENKINS-66969 for the report that I just verified is visible with Jenkins 2.317. Only affects the first build as far as I can tell. Builds after the first build remain visible |
See JENKINS-66753
Proposed changelog entries
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).