Bug 2058000 - Fix filter changes having no effect after loading more pushes - #9744
Open
camd wants to merge 1 commit into
Open
Bug 2058000 - Fix filter changes having no effect after loading more pushes#9744camd wants to merge 1 commit into
camd wants to merge 1 commit into
Conversation
✅ Deploy Preview for treeherder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…pushes When 'get next N' loads more pushes, the pushes store writes fromchange to the URL and dispatches a filtersUpdated event. App's handler rebuilt the FilterModel with the live window.location object, so the staleness check in handleUrlChanges (hasUrlFilterChanges) always compared the current URL against itself and never detected later filter changes. setFilterModel never fired again, the memoized job tree kept the old filter model, and tier (or any other filter) toggles were no-ops until a page reload. Fix: snapshot pathname/search/hash instead of storing the live object. The regression test must use BrowserRouter: under MemoryRouter the router location and window.location never refer to the same URL, so the bug cannot manifest (which is why existing filtering tests missed it).
camd
force-pushed
the
camd/bug-2058000-filter-after-get-next
branch
from
July 31, 2026 17:51
aeb88e7 to
fb123f3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9744 +/- ##
===========================================
+ Coverage 69.68% 83.26% +13.57%
===========================================
Files 629 630 +1
Lines 37413 37446 +33
Branches 3296 3351 +55
===========================================
+ Hits 26073 31181 +5108
+ Misses 11191 5884 -5307
- Partials 149 381 +232 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 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.
Problem
Bug 2058000: after clicking "get next N" to load more pushes, changing the shown task tiers (or any other filter — result status, classified state, etc.) had no effect. The tier button highlighted, the URL updated, but no additional jobs appeared until the page was reloaded.
Root cause
When "get next N" loads more pushes, the pushes store writes
fromchangeto the URL and dispatches afiltersUpdatedevent.App's handler for that event rebuilt the filter model asnew FilterModel(navigate, window.location)— storing the livewindow.locationobject asfilterModel.location.That live object poisons the staleness check that drives filter re-renders. On the next filter toggle,
handleUrlChangesaskshasUrlFilterChanges(filterModel.location.search, location.search)to decide whether to build a freshFilterModel— butfilterModel.locationiswindow.location, which by then already reflects the post-navigation URL. The check compares the URL against itself, never detects the change, andsetFilterModelnever fires. The memoized job tree (PushJobs/Platform/JobGroup) keeps the old filter-model identity and never re-filters. The tier button still highlights becauseSecondaryNavBarre-renders viauseLocationand reads the in-place-mutatedurlParams— matching the reported symptom exactly: button responds, jobs don't.Fix
handleFiltersUpdatednow snapshots{ pathname, search, hash }fromwindow.locationinstead of storing the live object.Relationship to #9721
This is a sibling of #9721 (push list resetting to latest 10 pushes) — both stem from the same URL-drift family, where raw
pushStatewrites leave React Router's location out of sync withwindow.location. But they are distinct defects and both fixes are needed:replaceLocation,fetchNextPushes) so raw URL writes become visible to the router, curing the phantom range-change reset.filtersUpdatedevent still fires after "get next N" and the handler still stored the livewindow.locationobject, so this bug reproduces on that branch too.The two changes touch different files and do not conflict; they can land in either order.
Testing
New regression test
tests/ui/job-view/FilterAfterLoadMorePushes_test.jsxwalks the reported STR: load 10 pushes → "get next 10" →fromchangewritten → toggle a filter → shown jobs must update. It fails on master at exactly the reported symptom (URL updates, jobs don't) and passes with the fix.The test deliberately uses
BrowserRouter: underMemoryRouterthe router location andwindow.locationnever refer to the same URL, so the bug cannot manifest — which is why the existing filtering tests never caught it.Full unit suite: 94 suites / 1048 tests pass. Lint clean.