Skip to content

reporting: health: persist category/subject/granularity selection in the URL - #10490

Open
issmirnov wants to merge 1 commit into
opnsense:masterfrom
issmirnov:health-url-state
Open

reporting: health: persist category/subject/granularity selection in the URL#10490
issmirnov wants to merge 1 commit into
opnsense:masterfrom
issmirnov:health-url-state

Conversation

@issmirnov

Copy link
Copy Markdown

Important notices

Before you submit a pull request, we ask you kindly to acknowledge the following:

If AI was used, please disclose:

  • Model used: Claude Sonnet 5, via Claude Code
  • Extent of AI involvement: I described the bug and asked Claude Code to investigate, implement, and verify a fix. It located the relevant controller/view/JS, implemented the change, and verified it by provisioning a throwaway OPNsense 26.1.6 VM and driving the actual UI in a browser rather than relying on code review alone. That live test caught two real bugs on the first attempt (a granularity restore that silently used the wrong detail level, and a bootstrap-select button that didn't visually refresh after a restore), which were then fixed and re-verified the same way. I reviewed the diff and the test evidence before submitting this PR.

Describe the problem

Reloading the Reporting > Health page always resets the Category, Subject, and Granularity selectors back to their defaults, discarding whatever the user was looking at and making it impossible to bookmark or share a specific view.


Describe the proposed solution

Read the three selections from the URL query string on load, falling back to today's defaults when a value is absent or no longer valid (e.g. a removed interface), and keep the query string in sync via history.replaceState() as the user changes any of them.

Two implementation details worth flagging for review:

  • Restoring a saved detail level needs to seed HealthGraph.currentDetailLevel directly, since setting the select's value alone doesn't fire its change handler.
  • Restoring a saved category/subject needs an explicit selectpicker('refresh') call, since bootstrap-select only redraws its button label on that, not on a re-triggered changed.bs.select event. This was invisible before because the only value ever selected here programmatically was whatever the widget already displayed by default.

Verified on a throwaway OPNsense 26.1.6 VM: set a non-default selection, confirmed the URL updates, then hard-refreshed and confirmed both the visual dropdown state and the actual data fetch (network request) use the restored values, including a graceful fallback when a saved subject is no longer valid.

Screenshots/recording to follow in a comment.


Related issue

Ref: #10489

…the URL

Reloading the Reporting > Health page always resets the Category,
Subject, and Granularity selectors back to their defaults, discarding
whatever the user was looking at and making it impossible to bookmark
or share a specific view.

Read the three selections from the URL query string on load, falling
back to today's defaults when a value is absent or no longer valid
(e.g. a removed interface), and keep the query string in sync via
history.replaceState() as the user changes any of them.

Restoring a saved detail level needs to seed HealthGraph's
currentDetailLevel directly, since setting the select's value alone
doesn't fire its change handler. Restoring a saved category/subject
needs an explicit selectpicker('refresh') call, since bootstrap-select
only redraws its button label on that, not on a re-triggered
changed.bs.select event; this was invisible before because the only
value ever selected here was whatever the widget already displayed.
@issmirnov

Copy link
Copy Markdown
Author

Demo:
opnsense-health-url-persist-demo

@swhite2

swhite2 commented Jul 8, 2026

Copy link
Copy Markdown
Member

The more common pattern for persistence in core is to use localStorage. This doesn't cover the link-sharing use case, but then again that particular use case has rarely/never popped up over the years.

I'm not sure we should go for URL params, but if we do, better to sync it with localStorage persistence

Comment on lines +145 to +150
if (savedGranularity !== null && ['0', '1', '2', '3'].includes(savedGranularity)) {
$('#detail-select').val(savedGranularity);
// .val() alone doesn't fire the change handler, so seed the graph's
// state directly or the first fetch below would use the old default
healthGraph.currentDetailLevel = savedGranularity;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This block of logic is the only thing that executes on page load in tandem with the code below:

                // trigger event for first category, restoring a saved selection when still valid
                const categoryToSelect = (savedCategory && rrdOptions.data.hasOwnProperty(savedCategory))
                    ? savedCategory
                    : Object.keys(rrdOptions.data)[0];
                $('#health-category-select').val(categoryToSelect);
                $('#health-category-select').selectpicker('refresh');
                $('#health-category-select').trigger('changed.bs.select');

Therefore, and since touching the healthGraph internals bypassing the API is a big no-no, why not combine the two blocks and call

$('#detail-select').val(savedGranularity).trigger('change');

so the proper update() is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants