fix: preserve search filters across pagination pages#3492
Merged
crivetimihai merged 4 commits intomainfrom Mar 10, 2026
Merged
Conversation
Member
|
Thanks @suciu-daniel — clean fix for #3128. The approach of reading search input values during pagination and passing them as query params to the server is correct. Tests updated appropriately. LGTM. |
VladMRusu1
previously approved these changes
Mar 10, 2026
Fixes #3128 The admin UI search filters (for tools, resources, prompts, gateways) were only filtering visible DOM rows on the current page, not across all records. This caused confusion when users searched and then navigated to page 2, where the search filter was lost. Changes: - Updated pagination_controls.html to dynamically read current search input values (q and tags) when loading pages - Removed obsolete client-side DOM filtering calls from clearSearch() function in admin.js that only filtered visible rows - Server-side filtering already works correctly via the 'q' parameter; this fix ensures pagination preserves those filters The fix ensures that when users: 1. Enter a search query in the search box 2. Navigate to page 2, 3, etc. using pagination controls The search filter is preserved and applied server-side across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
Updated the test to verify that clearSearch clears input values and triggers server-side reload, rather than calling the removed client-side DOM filtering functions. This aligns with the fix for #3128 where we removed client-side filtering in favor of proper server-side filtering across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
…ation When the user clears the search input and paginates before the debounced reload fires (~250ms window), stale q/tags values from data-extra-params would persist in the pagination request. Fix by calling url.searchParams.delete() when the input exists but is empty, ensuring the live DOM state is always authoritative over server-rendered extras. Add 9 tests covering the dynamic input-reading logic in pagination_controls.html: override, clear, trim, and tableName guard. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
442071c to
4d4aae3
Compare
crivetimihai
previously approved these changes
Mar 10, 2026
Member
crivetimihai
left a comment
There was a problem hiding this comment.
Review: Approved ✅
Approach
Sound fix. The pagination loadPage() method now reads the live DOM search input value instead of relying solely on data-extra-params (which becomes stale if the user modifies the search after the last server render). Removing client-side DOM filtering in clearSearch() is correct — it was redundant with the server-side reload.
Changes made during review
Rebased onto main — removed 3 merge commits for a clean 3-commit history.
Bug fix — the original code only set q/tags when the input had a value, but didn't clear them when empty. This means if a user clears the search input and paginates before the 250ms debounced reload fires, the stale q from data-extra-params would persist. Fixed by calling url.searchParams.delete('q') / delete('tags') when the input exists but is empty.
Tests — added 9 tests covering: input value forwarding, stale extraParams override, empty input clearing, whitespace trimming, non-search param preservation, and tableName guard. Total: 82 tests passing (was 73).
Security & Performance
- No issues. Search values flow through
URLSearchParams(auto-encoded) and are validated server-side. - Reading two DOM inputs by ID in
loadPage()is negligible overhead.
Consistency
- Follows the existing
pagination_controls.htmlpattern (extraParams loop, then overrides). - Input IDs match
PANEL_SEARCH_CONFIGinadmin.js.
…dPage Alpine.js $el resolves to the clicked button element (not the component root) when loadPage is called from a @click handler via goToPage. This caused this.$el.dataset.extraParams to always return undefined, silently breaking server-side query param forwarding (q, tags, team_id, etc.) during pagination. Fix by storing the root element reference in init() where $el correctly points to the x-data element, then using this._rootEl in loadPage. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
crivetimihai
approved these changes
Mar 10, 2026
Member
crivetimihai
left a comment
There was a problem hiding this comment.
Updated Review: Approved ✅
Additional fix added: pre-existing $el bug
During Playwright E2E testing, I discovered that this.$el inside Alpine.js loadPage() resolves to the clicked button element, not the component root. This caused this.$el.dataset.extraParams to always return undefined, silently breaking all server-side query param forwarding (q, tags, team_id, etc.) during pagination.
Root cause: Alpine.js scopes $el to the element where the directive is evaluated. When @click="goToPage(2)" triggers on a button, $el stays as that button through the goToPage → loadPage call chain.
Fix: Store the root element in init() where $el is correctly scoped (this._rootEl = $el), then use this._rootEl in loadPage().
Full change summary (4 commits)
- Original PR (2 commits, contributor): Dynamic search input reading + remove client-side DOM filtering
- Empty input edge case:
url.searchParams.delete('q'/'tags')when input cleared, preventing stale extraParams leaking. +9 tests. $el→_rootElfix: Pre-existing bug wherethis.$elresolved to clicked button, breakingdata-extra-paramsreading inloadPage().
Validation
- 870/870 JS tests pass (0 regressions)
- Playwright E2E: confirmed search filter preserved across pagination pages after fix
- No security or performance concerns
- Consistent with existing codebase patterns
MohanLaksh
pushed a commit
that referenced
this pull request
Mar 12, 2026
* fix: preserve search filters across pagination pages Fixes #3128 The admin UI search filters (for tools, resources, prompts, gateways) were only filtering visible DOM rows on the current page, not across all records. This caused confusion when users searched and then navigated to page 2, where the search filter was lost. Changes: - Updated pagination_controls.html to dynamically read current search input values (q and tags) when loading pages - Removed obsolete client-side DOM filtering calls from clearSearch() function in admin.js that only filtered visible rows - Server-side filtering already works correctly via the 'q' parameter; this fix ensures pagination preserves those filters The fix ensures that when users: 1. Enter a search query in the search box 2. Navigate to page 2, 3, etc. using pagination controls The search filter is preserved and applied server-side across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * test: update clearSearch test to reflect server-side filtering Updated the test to verify that clearSearch clears input values and triggers server-side reload, rather than calling the removed client-side DOM filtering functions. This aligns with the fix for #3128 where we removed client-side filtering in favor of proper server-side filtering across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * fix(ui): clear stale search params when input is emptied during pagination When the user clears the search input and paginates before the debounced reload fires (~250ms window), stale q/tags values from data-extra-params would persist in the pagination request. Fix by calling url.searchParams.delete() when the input exists but is empty, ensuring the live DOM state is always authoritative over server-rendered extras. Add 9 tests covering the dynamic input-reading logic in pagination_controls.html: override, clear, trim, and tableName guard. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(ui): use stored root element ref instead of $el in pagination loadPage Alpine.js $el resolves to the clicked button element (not the component root) when loadPage is called from a @click handler via goToPage. This caused this.$el.dataset.extraParams to always return undefined, silently breaking server-side query param forwarding (q, tags, team_id, etc.) during pagination. Fix by storing the root element reference in init() where $el correctly points to the x-data element, then using this._rootEl in loadPage. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Yosiefeyob
pushed a commit
that referenced
this pull request
Mar 13, 2026
* fix: preserve search filters across pagination pages Fixes #3128 The admin UI search filters (for tools, resources, prompts, gateways) were only filtering visible DOM rows on the current page, not across all records. This caused confusion when users searched and then navigated to page 2, where the search filter was lost. Changes: - Updated pagination_controls.html to dynamically read current search input values (q and tags) when loading pages - Removed obsolete client-side DOM filtering calls from clearSearch() function in admin.js that only filtered visible rows - Server-side filtering already works correctly via the 'q' parameter; this fix ensures pagination preserves those filters The fix ensures that when users: 1. Enter a search query in the search box 2. Navigate to page 2, 3, etc. using pagination controls The search filter is preserved and applied server-side across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * test: update clearSearch test to reflect server-side filtering Updated the test to verify that clearSearch clears input values and triggers server-side reload, rather than calling the removed client-side DOM filtering functions. This aligns with the fix for #3128 where we removed client-side filtering in favor of proper server-side filtering across all pages. Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * fix(ui): clear stale search params when input is emptied during pagination When the user clears the search input and paginates before the debounced reload fires (~250ms window), stale q/tags values from data-extra-params would persist in the pagination request. Fix by calling url.searchParams.delete() when the input exists but is empty, ensuring the live DOM state is always authoritative over server-rendered extras. Add 9 tests covering the dynamic input-reading logic in pagination_controls.html: override, clear, trim, and tableName guard. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(ui): use stored root element ref instead of $el in pagination loadPage Alpine.js $el resolves to the clicked button element (not the component root) when loadPage is called from a @click handler via goToPage. This caused this.$el.dataset.extraParams to always return undefined, silently breaking server-side query param forwarding (q, tags, team_id, etc.) during pagination. Fix by storing the root element reference in init() where $el correctly points to the x-data element, then using this._rootEl in loadPage. Refs: #3128 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>
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
3 participants
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
Fixes #3128
The admin UI search filters (for tools, resources, prompts, gateways) were only filtering visible DOM rows on the current page, not across all records. This caused confusion when users searched and then navigated to page 2, where the search filter was lost.
Changes
Testing
The fix ensures that when users:
Impact
Related Issues
Closes #3128