Fix Plugin Manager 'Enabled' column sorting#26349
Fix Plugin Manager 'Enabled' column sorting#26349himanshu748 wants to merge 3 commits intojenkinsci:masterfrom
Conversation
The "Enabled" column in the Installed Plugins table uses `data="${state}"` where state is `true` or `null`.
Since the sortable.js sorts by the `data` attribute alphabetically, plugins with `false` or `null` were not
sorting effectively relative to `true`. This commit changes the `data` attribute for sorting
to `1` (true/active) and `0` (false/inactive) for deterministic boolean sorting.
There was a problem hiding this comment.
Pull request overview
This PR fixes JENKINS-60117 by improving the boolean sorting mechanism for the Plugin Manager's 'Enabled' column. The change replaces the previous sorting approach that used 'true' vs null values with a more deterministic '1' vs '0' numeric approach. However, the change also switches from using p.enabled to p.active for determining the sort value, which may introduce a semantic behavioral change.
Changes:
- Changed the data attribute for sorting from using
p.enabledwith 'true'/null values to usingp.activewith '1'/'0' values - This makes boolean sorting more deterministic by ensuring both states have explicit non-null string values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </j:if> | ||
| <j:set var="state" value="${p.enabled?'true':null}"/> | ||
| <td class="jenkins-table__cell--tight enable" data="${state}"> | ||
| <td class="jenkins-table__cell--tight enable" data="${p.active?'1':'0'}"> |
There was a problem hiding this comment.
This change modifies both the format (from 'true'/null to '1'/'0') and the semantic meaning (from p.enabled to p.active) of the sorting data. While the format change improves determinism, the semantic change may cause unexpected behavior.
The distinction between p.enabled and p.active:
p.enabled: Whether the plugin will be enabled after restart (based on configuration files)p.active: Whether the plugin is currently active in this session (runtime state)
The checkbox's checked attribute (line 192) uses p.enabled, which means it shows whether the plugin will be enabled after restart. However, the sorting now uses p.active, which reflects the current runtime state.
Scenario where this matters:
- User enables a disabled plugin → checkbox becomes checked, but sorting keeps it with disabled plugins until restart
- User disables an enabled plugin → checkbox becomes unchecked, but sorting keeps it with enabled plugins until restart
This creates a visual inconsistency where the sort order doesn't match the checkbox states. Consider whether the sorting should use p.enabled to match the checkbox, or if this behavioral change is intentional and should be documented.
| <td class="jenkins-table__cell--tight enable" data="${p.active?'1':'0'}"> | |
| <td class="jenkins-table__cell--tight enable" data="${p.enabled?'1':'0'}"> |
|
Could you restore the PR template, thanks |
The sorting data attribute should use p.enabled to match the checkbox's checked attribute, ensuring sort order reflects the configured state rather than the runtime state.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0c4755a to
8a9e67a
Compare
|
Fixed the 'Enabled' column sorting logic in the Plugin Manager (installed.jelly) to use '1' and '0' for numerical sorting consistency, as suggested. |
himanshu748
left a comment
There was a problem hiding this comment.
I have restored the PR template as requested.
Fixes https://github.com/jenkinsci/jenkins/issues/60117
Testing done
Verified that the 'Enabled' column in the Plugin Manager sorts properly by changing the sorting value from string "true"/"false" to numeric "1"/"0".
Tested locally.
Screenshots (UI changes only)
Before
(Sorting did not work correctly)
After
(Sorting works properly)
Proposed changelog entries
Proposed changelog category
/label bug, web-ui
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.Desired reviewers
@mention
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.lts-candidateto be considered.