TRT-2764: Sippy tests page: search bar does not complete search until page is refreshed#3721
TRT-2764: Sippy tests page: search bar does not complete search until page is refreshed#3721openshift-trt-agent[bot] wants to merge 4 commits into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@openshift-trt-agent[bot]: This pull request references TRT-2764 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-trt-agent[bot] The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @openshift-trt-agent[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test agentic-staging |
|
/ok-to-test |
Sippy Staging EnvironmentURL: https://repeat-uni-containing-develop.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~21:03 UTC). |
|
Scheduling required tests: |
|
/retest ci/prow/agentic-staging |
The requestSearch function in 11 table components mutated the filterModel object returned by useStableJSONQueryParam in place. This corrupted the hook's internal ref state: the intermediate re-render from setSearching(true) would detect the mutation via the defaultValue fallback, update the serialized cache, and then when the URL update arrived with the same content, the comparison found no change, so the useEffect never fired and fetchData was never called. Fix by creating a new object with spread syntax instead of mutating the existing filterModel. This ensures useStableJSONQueryParam sees a genuinely new reference on re-render, triggering the useEffect and completing the search. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6f7ff65 to
061b4cb
Compare
|
Rebased on latest main (picks up #3720 and #3723). Lint and Jest tests pass locally. Re: the All other CI checks pass: build, e2e, lint, unit, images, security. |
|
Scheduling required tests: |
|
/test agentic-staging |
Sippy Staging EnvironmentURL: https://seek-throughout-marble-between.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~14:14 UTC). |
|
Bug: If I enter a search term, and it executes, but then click the search magnifying glass icon immediately after (or even pressing enter twice), the page gets stuck in loading but doesn't make any API call (it doesn't need to because that search is already loaded) |
|
It would be nice when refreshing the page, if there is only one filter in the filterModel for the quick searches field (e.g. name), we re-populate the quick search bar. e.g. tests quick search searches name, but refresh the page and it remains in the filter model, but not the quick search bar. |
…n refresh Fix two issues with the quick search bar: 1. Pressing Enter or clicking the search icon when the same search term is already active no longer gets stuck in loading state. The requestSearch function in TestTable now checks if the filter would actually change before setting the searching state. 2. When refreshing the page, the quick search bar is now repopulated from the filterModel URL query parameter. GridToolbar accepts a new searchField prop that identifies which filter column corresponds to the search bar, and initializes the search text on mount. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@stbenjam Thanks for testing! Both issues are fixed in the latest push: Bug fix: double-search stuck loading The Enhancement: repopulate search bar on page refresh
Verified:
|
|
/test agentic-staging |
|
Scheduling required tests: |
Sippy Staging EnvironmentURL: https://dot-settlement-walker-islands.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~16:18 UTC). |
Two fixes in GridToolbar: 1. Remove onBlur handler from search TextField to prevent double-trigger when clicking the search icon (onBlur fires before onClick, causing two requestSearch calls that can race with state updates and leave isSearching stuck as true). 2. Only set initializedFromFilter ref when a search filter is actually found, so the search bar gets repopulated from URL params on page refresh even if the filterModel is initially empty on first render. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Fixed in 9ca19dd. The root cause was the The fix removes the Verified in browser: searched for "etcd", then pressed Enter twice and clicked the search icon. Page stays showing results, not stuck in loading: |
|
Fixed in 9ca19dd. The issue was in GridToolbar's The fix moves Verified: after searching for "etcd" and doing a full page reload, the search bar shows "etcd" and results are filtered: |
|
/test agentic-staging |
|
Scheduling required tests: |
Sippy Staging EnvironmentURL: https://heavily-appearance-composer-moral.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~17:09 UTC). |
|
Lol you should not populate the quick search field if the item is a NOT matcher And you should ONLY populate it if the filter model only has one active filter for name (or the relevant field) |
|
/retest ci/prow/agentic-staging |
|
/test agentic-staging |
Sippy Staging EnvironmentURL: https://feel-info-alto-trends.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~01:52 UTC). |
Only initialize the search bar from the URL filterModel when there is exactly one filter on the search field and it uses a positive 'contains' operator. This avoids misleadingly populating the search bar for NOT matchers or when multiple filters exist on the same column. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@stbenjam Fixed in db0562a. The search bar initialization in
If there are multiple filters on the field, or the single filter is a NOT matcher, the search bar stays empty. |
|
Scheduling required tests: |





Summary
Fixes the search bar on table pages (Tests, Jobs, etc.) not completing searches after pressing Enter. The URL would update with the search terms, but the table stayed in a loading/spinner state indefinitely. A manual page refresh was required to see results.
Root cause: The
requestSearchfunction in all 11 table components mutated thefilterModelobject in-place (const currentFilters = filterModel) before passing the same reference tosetFilterModel. This broke compatibility with theuseStableJSONQueryParamhook (introduced in PR #3711), which stabilizes object references to prevent duplicate API calls. Because the hook's internal ref was mutated in-place, it could not detect that the value had changed, so theuseEffectthat triggersfetchDatawould not fire.Fix: Create a new object in
requestSearchinstead of mutating the existingfilterModel, matching the immutable pattern already used by theaddFiltersfunction in these same components.Files changed (11 table components):
sippy-ng/src/tests/TestTable.jssippy-ng/src/tests/FeatureGates.jssippy-ng/src/jobs/JobTable.jssippy-ng/src/jobs/JobRunsTable.jssippy-ng/src/releases/PayloadStreamTestFailures.jssippy-ng/src/releases/PayloadStreamsTable.jssippy-ng/src/releases/PayloadTestFailures.jssippy-ng/src/releases/ReleasePayloadJobRuns.jssippy-ng/src/releases/ReleasePayloadPullRequests.jssippy-ng/src/releases/ReleasePayloadTable.jssippy-ng/src/repositories/RepositoriesTable.jsScreenshots
Before search (all tests loaded):
After searching "etcd" (results filtered immediately, no page refresh):
Test plan
make lintpassesmake testpasses (Go unit tests + Jest tests)Jira: https://issues.redhat.com/browse/TRT-2764
Generated with Claude Code