Conversation
… ones
TableDetailsPageV1 already skips the entity fetch during the guided tour, but
both useEntityPermissions calls ran unconditionally - so the page asked the API
for permissions on a fully-qualified name with no entity behind it, then drove
the tour's tabs from whatever came back.
mockTablePermission, written for exactly this in mockTourData.constants.ts, was
dead code: nothing imported it. It also lacked ViewDataProfile, which is why
#profilerDetails never rendered and step 13 of the tour showed an empty panel.
Disable both permission calls for the tour through the hook's existing
{ enabled } option, and take the tour's grants from mockTablePermission with
ViewDataProfile added. No request is issued for the mock FQN and the permission
cache no longer gains an entry for a non-existent entity.
Tour.spec.ts now asserts #profilerDetails is visible at step 13 - it advanced
through that step without checking anything, which is why the empty panel went
unnoticed.
21 jest tests pass in TableDetailsPageV1.test.tsx.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
backend
safe to test
❌ 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 |
Code Review ✅ ApprovedFixes the guided tour's profiler step by disabling real permission fetches during the tour and using mock permissions instead. The OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } = useEntityPermissions(ResourceEntity.TABLE, tableFqn, { | ||
| enabled: !isTourDataset, | ||
| }); |
There was a problem hiding this comment.
During the tour, disabling useEntityPermissions makes every derived permission flag fall back to false, while only the aggregate tablePermissions object is replaced with mockTablePermission. As a result, the Queries tab shows a permission error because viewQueriesPermission remains false, and Lineage receives hasEditAccess={false} even though the tour mock grants EditAll. Derive these flags from the selected mock permissions or otherwise provide tour-specific derived values before disabling the queries.
|
| Count | Rule |
|---|---|
| 15 | react-hooks/exhaustive-deps |
| 1 | openmetadata-imports/no-cross-page-imports |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:114:1 |
openmetadata-imports/no-cross-page-imports |
Page features must not import another page feature. Move shared code to components, hooks, interfaces, or pure utilities. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:171:6 |
react-hooks/exhaustive-deps |
React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:220:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 't'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:445:6 |
react-hooks/exhaustive-deps |
React Hook useMemo has an unnecessary dependency: 'tableDetails.tags'. Either exclude it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:469:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'setDqLineageData'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:515:9 |
react-hooks/exhaustive-deps |
The 'onTableUpdate' function makes the dependencies of useCallback Hook (at line 671) change on every render. To fix this, wrap the definition of 'onTableUpdate |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:515:9 |
react-hooks/exhaustive-deps |
The 'onTableUpdate' function makes the dependencies of useCallback Hook (at line 687) change on every render. To fix this, wrap the definition of 'onTableUpdate |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:542:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'onTableUpdate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:556:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'onTableUpdate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:616:6 |
react-hooks/exhaustive-deps |
React Hook useMemo has missing dependencies: 'customizedPage?.tabs' and 'feedCount'. Either include them or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:643:5 |
react-hooks/exhaustive-deps |
React Hook useMemo has a missing dependency: 'tabs'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:643:6 |
react-hooks/exhaustive-deps |
React Hook useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:818:6 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:833:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'navigate'. Either include it or remove the dependency array. |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:905:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has missing dependencies: 'fetchActivityCount', 'fetchTaskCounts', and 'setTableDetails'. Either include them or remove the dependency arra |
| 🟡 | src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx:920:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has missing dependencies: 'getTestCaseFailureCount' and 'tableDetails'. Either include them or remove the dependency array. |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
Jest test CoverageUI tests summary
|
|
🔴 Playwright Results — workflow failedValidated commit ✅ 4471 passed · ❌ 1 failed · 🟡 9 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky ❌ Action needed: test(s) failed on every attempt against this PR’s validated commit — see Genuine Failures below. These are test failures, not CI budget or infrastructure issues. PerformanceBlocking targets: ✅ met · 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) 34m 50s ⏱️ Max setup 4m 54s · max shard execution 19m 17s · max shard-job elapsed before upload 22m 53s · reporting 19s 🌐 220.21 requests/attempt · 2.23 app boots/UI scenario · 40.10% common-shard skew Optimization targets still in progress:
Genuine Failures (failed on all attempts)❌
|
2 participants
Describe your changes:
Fixes #33548. Salvaged from the closed PR #33248 (tracking: #33528).
The guided tour renders a mock table that does not exist on the server.
TableDetailsPageV1alreadyknows that for the entity fetch:
but both
useEntityPermissionscalls ran unconditionally — so during the tour the page asked the APIfor permissions on a fully-qualified name with no entity behind it, and then drove the tour's tabs
from whatever came back.
Meanwhile
mockTablePermission, written for exactly this purpose inmockTourData.constants.ts, wasdead code — nothing imported it — and it lacked
ViewDataProfile.The visible symptom: step 13 of the tour, the profiler step, renders an empty panel. The tour's
own spec advanced through step 13 without asserting anything was there.
This disables both permission calls for the tour through the hook's existing
{ enabled }option andtakes the tour's grants from
mockTablePermission(withViewDataProfile: trueadded). No request isissued for the mock FQN, and the permission cache no longer gains an entry for a non-existent entity.
Type of change:
High-level design:
useEntityPermissionsalready accepts{ deleted, enabled }onmain, so this needs no hook change —enabled: !isTourDatasetat both call sites. The mock grants are selected in auseMemoso the tourvalue is stable and the fetched value flows through unchanged outside the tour.
isTourDatasetreplaces the repeated
isTourOpen || isTourPageexpression.Tests:
Use cases covered
Unit tests
TableDetailsPageV1.test.tsxasserts the hook is called with{ enabled: true }outside the tour.Backend integration tests
Not applicable — UI only.
Ingestion integration tests
Not applicable.
Playwright (UI) tests
playwright/e2e/Flow/Tour.spec.tsnow asserts#profilerDetailsis visible at step 13 — the specpreviously advanced through that step without checking anything, which is why the empty panel went
unnoticed.
Manual testing performed
yarn test src/pages/TableDetailsPageV1/TableDetailsPageV1.test.tsx— 21 passed.yarn lint:baseon all four changed files — 0 errors (16 pre-existingreact-hooks/exhaustive-depswarnings in this file, none at the changed lines).npx prettier --checkon all four — "All matched files use Prettier code style!".UI screen recording / screenshots:
Not attached — the change is a permission-source swap with no new UI. The behaviour it restores is
asserted by the
Tour.spec.tsstep-13 check above. Happy to record the tour if a reviewer wants it.Checklist:
Fixes <issue-number>: <short explanation>Fixes #33548.🤖 Generated with Claude Code
The PR should not merge until all tour permission consumers use grants consistent with the new mock-permission source.
Findings
Summary
This PR prevents real table-permission requests for the guided-tour dataset, supplies mock profiler permission, and strengthens the profiler tour assertion.
isTourDatasetcondition and disables both permission queries during the tour.mockTablePermissionfor child components and addsViewDataProfile.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Table details page] --> B{Tour dataset?} B -- No --> C[Fetch entity permissions] C --> D[Raw permissions and derived flags] B -- Yes --> E[Disable permission queries] E --> F[Raw permissions replaced with tour mock] E --> G[Derived flags remain all false] F --> H[Profiler uses mock ViewDataProfile] G --> I[Queries shows permission error] G --> J[Lineage receives no edit access]Reviews (1) · Last reviewed commit: "Fixes 33548: use the tour's mock permiss..."