Fixes #38858 - Fix search cleared on bottom pagination #10901
Open
jeremylenz wants to merge 1 commit intotheforeman:3.17-stablefrom
Open
Fixes #38858 - Fix search cleared on bottom pagination #10901jeremylenz wants to merge 1 commit intotheforeman:3.17-stablefrom
jeremylenz wants to merge 1 commit intotheforeman:3.17-stablefrom
Conversation
When using the All Hosts page with replacementResponse pattern, the bottom pagination was clearing the search query because HostsIndex maintained a separate params state that wasn't synced with the API response values. This fix merges the API response's page and perPage values into the params object during render, ensuring the bottom pagination displays the correct page number while preserving the search query. This follows React best practices by calculating values during render instead of using useEffect. Also adds a test to verify the params merging behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Fixes #38858 - Fix search cleared on bottom pagination (revised) The previous fix had two issues: 1. Both per_page and perPage appeared in URL params 2. Search was still being cleared on bottom pagination Root cause: HostsIndex's params state doesn't sync with search changes made through TableIndexPage's SearchBar. The API response contains the current search query, page, and perPage, but they weren't being merged into the params passed to Table. Solution: Merge fresh values from API response into params: - page: current page number - per_page: current per-page value (using snake_case, not camelCase) - search: current search query This ensures: - Only snake_case per_page in URL (no duplicate perPage) - Search query always preserved when using bottom pagination - Page/perPage are always current values from API response 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Fixes #38858 - Prevent duplicate perPage/per_page in Table params Previous fix had two remaining issues: 1. Test was failing because mock API response lacked search field 2. Both perPage (camelCase) and per_page (snake_case) appeared in params Changes: - Added search field to mock API response in test - Destructured and excluded stale perPage from params before spreading - This ensures only per_page (snake_case) appears in Table params 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> (cherry picked from commit 45f7dfd)
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.
cherry-pick requested by community: https://community.theforeman.org/t/new-host-page-and-paging/45910/3?u=jeremylenz
(cherry picked from commit jeremylenz@45f7dfd)