Conversation
|
The preview environment for this pull request is ready at 4775.prenv.trento.suse.com. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the session-storage failure handling and restore browser history coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds user-scoped session persistence for Hosts filters and page size.
Changes:
- Adds view-settings, search-parameter, and persistence-hook utilities.
- Integrates persistence with Hosts, authentication lifecycle, tables, and pagination.
- Adds unit, component, and Cypress coverage.
Final findings:
- Moderate (2 votes): Clear the existing view-settings entry when
sessionStorage.setItemfails, and add coverage. - Moderate (1 vote): Restore dedicated browser back/forward history coverage.
File summaries
| File | Summary |
|---|---|
test/e2e/cypress/e2e/hosts_overview.cy.js |
Updates Hosts persistence E2E coverage. |
assets/js/pages/Layout/Layout.jsx |
Clears settings on logout. |
assets/js/pages/HostsList/HostsList.test.jsx |
Tests Hosts list behavior. |
assets/js/pages/HostsList/HostsList.jsx |
Enables persistent Hosts search parameters. |
assets/js/pages/Guard/Guard.jsx |
Initializes settings per user. |
assets/js/lib/viewSettings/index.test.js |
Tests view-settings storage. |
assets/js/lib/viewSettings/index.js |
Stores user-scoped view settings. |
assets/js/lib/searchParams/index.test.js |
Tests search-parameter helpers. |
assets/js/lib/searchParams/index.js |
Provides search-parameter update helpers. |
assets/js/hooks/usePersistentSearchParams.test.jsx |
Tests persistent search parameters. |
assets/js/hooks/usePersistentSearchParams.js |
Restores and persists URL parameters. |
assets/js/common/Table/Table.test.jsx |
Tests URL-bound table behavior. |
assets/js/common/Table/Table.jsx |
Binds page size to URL parameters. |
assets/js/common/Table/index.js |
Exports the table component. |
assets/js/common/Pagination/Pagination.jsx |
Makes page-size selection controlled. |
Review details
Suppressed comments (1)
test/e2e/cypress/e2e/hosts_overview.cy.js:228
- This replacement no longer exercises browser back/forward behavior, and the deleted cases also covered returning to the previous page and avoiding duplicate history entries. Since the new hook performs mount-time
setSearchParams(..., { replace: true }), keep a dedicated back/forward assertion (or add an equivalent hook test) so regressions in history handling are still detected.
hostsOverviewPage.visit();
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
arbulu89
marked this pull request as ready for review
September 15, 2026 07:32
arbulu89
force-pushed
the
persistent-frontend-filters
branch
from
September 15, 2026 14:29
a92e2a3 to
f95d808
Compare
arbulu89
force-pushed
the
persistent-frontend-filters
branch
from
September 16, 2026 13:01
f95d808 to
f813de2
Compare
arbulu89
force-pushed
the
persistent-frontend-filters
branch
from
September 16, 2026 13:57
f813de2 to
67b4da2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add persistent filters usage in the frontend. This way, once the user has selected a filter in a page, moves somewhere else, and moves back, the same filters are applied.
The code creates a
viewSettingslibrary to handle the session storage and a hook that "upgrades"searchParamsto get persistent filters if they exist combined with the query string params.Caveats:
Hosts overview, to keep the PR smaller.How was this tested?
UT and e2e
Documentation changes
I guess we will need to document the feature