Fix: explicit pathname on Explore navigate() calls to stop random redirects - #31632
Open
shrabantipaul-collate wants to merge 12 commits into
Open
Fix: explicit pathname on Explore navigate() calls to stop random redirects#31632shrabantipaul-collate wants to merge 12 commits into
shrabantipaul-collate wants to merge 12 commits into
Conversation
… redirects
`handleSortValueChange`, `handleSortOrderChange`, `handleQuickFilterChange`,
`handleTreeSelect`, and `handleShowDeletedChange` in ExplorePageV1 all called
`navigate({ search: ... })` with no `pathname`. That's a relative navigation
resolved against the router's current route-match context rather than
`window.location.pathname` at click time, and that context can lag a
pushState fired moments earlier elsewhere in the app. When it does, one of
these handlers ends up merging its search params onto a completely different
page's pathname instead of Explore's, producing URLs like
`/context-center/dashboard?browsePath=...` and silently dropping the user
onto whatever page happened to be current a moment before.
Passing an explicit `pathname: location.pathname` (already available via
`useCustomLocation()` in this component) removes the reliance on relative
resolution entirely, matching the pattern `handleSearchIndexChange` already
used safely via `getExplorePath()`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shrabantipaul-collate
requested review from
ShaileshParmar11,
chirag-madlani and
karanh37
as code owners
August 17, 2026 12:01
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Contributor
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
shrabantipaul-collate
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Jest test CoverageUI tests summary
|
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 12:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
🔴 Playwright Results — workflow failedValidated commit ✅ 501 passed · ❌ 49 failed · 🟡 0 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ❌ unmet · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 56m 48s ⏱️ Max setup 3m 6s · max shard execution 23m 36s · max shard-job elapsed before upload 27m 34s · reporting 27s 🌐 216.21 requests/attempt · 2.72 app boots/UI scenario · 5.30% common-shard skew Optimization targets still in progress:
Genuine Failures (failed on all attempts)❌
|
…alls Address review feedback: passing location.pathname explicitly still reads router location state, which is the same class of value the original bug was caused by resolving incorrectly. Extract getExploreTabPath (the same ROUTES.EXPLORE_WITH_TAB-based construction getExplorePath already used safely) and use it in all 5 handlers instead, so none of them depend on the router's location/route-match context at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
| Count | Rule |
|---|---|
| 5 | react-hooks/exhaustive-deps |
| 2 | @typescript-eslint/no-non-null-assertion |
| 2 | openmetadata-imports/no-lower-layer-page-imports |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/pages/ExplorePage/ExplorePageV1.component.tsx:232:7 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'navigate' and 'tabsInfo'. Either include them or remove the dependency array. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.component.tsx:245:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.component.tsx:271:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.component.tsx:310:6 |
react-hooks/exhaustive-deps |
React Hook useMemo has a missing dependency: 'tabsInfo'. Either include it or remove the dependency array. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.component.tsx:600:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'performFetch'. Either include it or remove the dependency array. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.test.tsx:146:7 |
@typescript-eslint/no-non-null-assertion |
Forbidden non-null assertion. |
| 🟡 | src/pages/ExplorePage/ExplorePageV1.test.tsx:188:7 |
@typescript-eslint/no-non-null-assertion |
Forbidden non-null assertion. |
| 🟡 | src/utils/RouterUtils.ts:49:1 |
openmetadata-imports/no-lower-layer-page-imports |
Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here. |
| 🟡 | src/utils/RouterUtils.ts:50:1 |
openmetadata-imports/no-lower-layer-page-imports |
Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here. |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed…ependence - RouterUtils.test.ts: unit tests for getExploreTabPath, including a regression guard that its output does not depend on window.location. - ExplorePageV1.test.tsx: assert the pathname on every navigate() call (quick filter, tree select, show-deleted, sort value, sort order) now matches getExploreTabPath(tab). Adds a dedicated regression test that sets useCustomLocation to an unrelated stale pathname (/context-center/dashboard) and confirms the quick-filter navigation still targets Explore — the exact failure mode this fix addresses. Also adds first-time coverage for the two sort handlers, which had none. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
chirag-madlani
previously approved these changes
Aug 17, 2026
shrabantipaul-collate
had a problem deploying
to
test
August 17, 2026 14:08 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 17, 2026 14:08 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 17, 2026 14:08 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 08:13 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 08:13 — with
GitHub Actions
Error
GitHub Actions
Error
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 08:13 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 08:13 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 08:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
enabled auto-merge
August 18, 2026 09:02
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 09:27 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 09:27 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 09:27 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 09:27 — with
GitHub Actions
Failure
GitHub Actions
Failure
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
shrabantipaul-collate
temporarily deployed
to
test
August 18, 2026 09:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
On the bare /explore route (no :tab segment), useRequiredParams returns undefined for tab. getExploreTabPath(undefined) produces '/explore/' (trailing slash), which is a pathname change from '/explore' and fires SearchDropdown's close-on-navigate useEffect, collapsing the filter dropdown before checkCheckboxStatus can assert on it. When tab is present the existing getExploreTabPath(tab) logic still applies — it avoids the stale route-match-context race that the PR was originally fixing. When tab is absent there is no ambiguous context, so location.pathname is the correct, safe fallback. Fixes Playwright Entity spec failures: Certification Add Remove, Tag Add/Update/Remove, Glossary Term Add/Update/Remove, and the SearchSeparation reindex suite — all caused by the filter dropdown closing mid-assertion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
shrabantipaul-collate
had a problem deploying
to
test
August 18, 2026 10:49 — with
GitHub Actions
Error
GitHub Actions
Error
…pathname
The previous fallback (location.pathname when tab is absent) could still
be stale if React Router's location state reflected a different page at
the moment the handler fired — the same class of race the rest of the
fix was already guarding against.
ROUTES.EXPLORE is a module-level constant ('/explore'), completely
independent of all router state, so the pathname used for bare-route
navigations is never derived from anything that can lag.
Addresses Greptile review comment on PR #31632.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review ✅ ApprovedFixes random redirect bugs on the Explore page by using explicit absolute pathnames on navigate calls and adding regression test coverage. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
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.
Screen.Recording.2026-08-17.at.5.46.13.PM.mov
Describe your changes:
Users interacting with the Explore page — applying a filter, changing sort, clicking a browse-estate tree node, toggling "show deleted" — were intermittently and silently dropped onto a completely unrelated page (e.g. landing on
/glossaryor/context-center/dashboard?browsePath=...instead of staying on Explore).Root cause. Five handlers in
ExplorePageV1—handleSortValueChange,handleSortOrderChange,handleQuickFilterChange,handleTreeSelect,handleShowDeletedChange— callnavigate({ search: ... })with nopathname. In React Router v6 that's a relative navigation, resolved against the router's current route-match context rather thanwindow.location.pathnameat the literal moment of the call. That route-match context is React state updated through the router's own commit cycle, and can still reflect a previous route for a brief window after apushStatefires elsewhere in the app (e.g. a sidebar navigation). When one of these handlers fires inside that window, its search params get merged onto whatever page is currently resolved — not Explore — producing URLs like/glossary?quickFilter=...or/context-center/dashboard?browsePath=...and silently navigating the user away.Confirmed via
history.pushState/popstateinstrumentation in the browser console: captured stack traces showing the exactExploreTree→ExplorePageV1handler chain firing synchronously on a real user click, withnavigate()'s implicit relative-merge landing on an unrelated page's pathname.One handler in the same file,
handleSearchIndexChange, was already unaffected — it callsnavigate(getExplorePath({...})), andgetExplorePathbuilds a full absolute path fromROUTES.EXPLORE_WITH_TAB, so it's never subject to relative resolution.Changes
ExplorePageV1.component.tsx— pass an explicitpathname: location.pathname(already available viauseCustomLocation()) on all 5 vulnerablenavigate({ search: ... })calls, matching the safe patternhandleSearchIndexChangealready used. Addedlocation.pathnameto theuseCallbackdeps for the two memoized handlers (handleQuickFilterChange,handleTreeSelect) so they don't go stale if pathname changes independently ofparsedSearch.This removes the dependency on relative-navigation resolution entirely rather than trying to fix the timing race itself.
Type of change:
Checklist:
Tests
eslinton the changed file: 0 errors, same 5 pre-existingreact-hooks/exhaustive-depswarnings as before this change (confirmed viagit stashdiff — none introduced by this patch).history.pushState/popstateconsole traces reproducing the bug via a Domain quick-filter clear (landed on/glossary) and a browse-estate tree click (landed on/context-center/dashboard?browsePath=...), both while intentionally racing against another navigation.🤖 Generated with Claude Code
Greptile Summary
The PR completes the Explore navigation fix by constructing handler targets from canonical Explore routes rather than the potentially stale router pathname.
getExploreTabPathas a location-independent route helper.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant Handler as Explore handler participant Routes as Canonical Explore routes participant Router User->>Handler: Change filter, sort, tree, or deleted state Handler->>Routes: Build /explore/:tab or /explore Routes-->>Handler: Absolute Explore pathname Handler->>Router: "navigate({ pathname, search })" Router-->>User: Remain on Explore with updated queryReviews (12): Last reviewed commit: "fix(explore): use static ROUTES.EXPLORE ..." | Re-trigger Greptile