Skip to content
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-73669] don't change unrelated checkboxes in rowSelectionCont… #9648

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Aug 22, 2024

when a row in table contains more than 1 checkbox the rowSelectionController shouldn't change all checkboxes but only the ones that match a given class.

See JENKINS-73669.

Testing done

Tested in context of jenkinsci/agent-maintenance-plugin#213

Proposed changelog entries

  • don't change unrelated checkboxes in rowSelectionController

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

…roller

when a row in table contains more than 1 checkbox the
rowSelectionController shouldn't change all checkboxes but only the one
in the same column
@NotMyFault NotMyFault added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Aug 24, 2024
@NotMyFault NotMyFault requested a review from a team August 24, 2024 09:57
@NotMyFault NotMyFault added the needs-security-review Awaiting review by a security team member label Aug 24, 2024
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this isn’t used elsewhere? Another option would be to by default do all but allow narrowing it?

Approved anyway

@mawinter69
Copy link
Contributor Author

I assume this isn’t used elsewhere? Another option would be to by default do all but allow narrowing it?

I intend to use it, see link above

Copy link
Contributor

@SNanda8895 SNanda8895 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no security problem here.

@Kevin-CB Kevin-CB added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Aug 28, 2024
@timja
Copy link
Member

timja commented Aug 28, 2024

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 28, 2024
@timja timja merged commit fc73c26 into jenkinsci:master Aug 30, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants