Skip to content

Fixed context center bugs - #31118

Open
Rohit0301 wants to merge 12 commits into
mainfrom
context-center-bug-fixes
Open

Fixed context center bugs#31118
Rohit0301 wants to merge 12 commits into
mainfrom
context-center-bug-fixes

Conversation

@Rohit0301

@Rohit0301 Rohit0301 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Screen.Recording.2026-08-07.at.8.00.20.PM.mov
Screen.Recording.2026-08-07.at.8.00.39.PM.mov
Screenshot 2026-08-07 at 8 00 10 PM Screenshot 2026-08-07 at 8 00 53 PM Screenshot 2026-08-07 at 8 17 10 PM

Fixes #

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

  • Context Center Dashboard & Navigation:
    • Added click handlers and navigation across dashboard cards for documents, folders, articles, and memories (ContextCenterDashboardPage.tsx, DashboardFoldersCard.tsx, ContextKnowledgePillarCard.tsx).
  • Error Handling & UI States:
    • Handled file upload errors with error toasts and retry handling (UploadDocumentModal.component.tsx).
  • Backend Services:
    • Preserved updater user during context file extraction updates instead of defaulting to admin (ContextFileExtractionService.java).

This will update automatically on new commits.

@Rohit0301 Rohit0301 self-assigned this Aug 6, 2026
@Rohit0301
Rohit0301 requested review from a team as code owners August 6, 2026 13:58
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 skip-pr-checks label.

@github-actions github-actions Bot added the UI UI specific issues label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Comment thread openmetadata-ui/src/main/resources/ui/playwright.config.ts Outdated
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 041c6772c3ec7ea7c1cc92d2ae243004f44f11a5 in Playwright run 31207070814, attempt 1.

✅ 717 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking 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) 57m 26s

⏱️ Max setup 3m 3s · max shard execution 18m 24s · max shard-job elapsed before upload 24m 38s · reporting 4s

🌐 184.17 requests/attempt · 2.67 app boots/UI scenario · 13.13% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.67 per UI scenario (2047 boots / 767 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
🟡 Shard chromium-01 197 0 1 0 0 0
✅ Shard chromium-02 138 0 0 0 0 0
✅ Shard chromium-03 115 0 0 0 0 0
✅ Shard chromium-04 116 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0
🟡 1 flaky test(s) (passed on retry)
  • Features/ContextCenterMemories.spec.tsArrowDown + Enter keyboard navigation selects the linked table result (shard chromium-01, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@Rohit0301 Rohit0301 added the safe to test Add this label to run secure Github workflows on PRs label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.22% (78212/118099) 50.22% (47213/94009) 51.45% (14242/27678)

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 57 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 57 warning(s) across 10 changed file(s).

Count Rule
11 sonarjs/no-duplicate-string
11 @typescript-eslint/no-non-null-assertion
9 sonarjs/cyclomatic-complexity
6 react-hooks/exhaustive-deps
5 sonarjs/expression-complexity
4 i18next/no-literal-string
3 sonarjs/cognitive-complexity
2 react/no-array-index-key
2 sonarjs/no-nested-functions
1 sonarjs/no-nested-conditional
All findings
Location Rule Message
🟡 src/components/ContextCenter/ArchiveView/ArchiveView.component.tsx:155:36 react/no-array-index-key Do not use Array index in keys
🟡 src/components/ContextCenter/ContextKnowledgePillarCard/ContextKnowledgePillarCard.component.tsx:81:28 react/no-array-index-key Do not use Array index in keys
🟡 src/components/ContextCenter/ContextSimplePillarCard/ContextSimplePillarCard.component.tsx:65:13 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:251:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 36 which is greater than 10 authorized.","cost":26,"secondaryLocations":[{"line":251,"column":3,"endLine":251,"endColum
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:257:54 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 9 times.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:325:16 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":325,"column":15,"endLine":325,"endColum
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:364:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'form' and 't'. Either include them or remove the dependency array.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:418:57 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:418:57 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 18 which is greater than 10 authorized.","cost":8,"secondaryLocations":[{"line":418,"column":56,"endLine":418,"endColum
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:428:13 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:429:15 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:693:22 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/components/ContextCenter/CreateMemoryModal/CreateMemoryModal.component.tsx:851:47 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/ContextCenter/DashboardFoldersCard/DashboardFoldersCard.test.tsx:24:15 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:55:54 i18next/no-literal-string disallow literal string: x
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:70:64 i18next/no-literal-string disallow literal string: close
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:113:73 i18next/no-literal-string disallow literal string: <button data-testid={delete-${name}} onClick={onDelete}> del
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:118:71 i18next/no-literal-string disallow literal string: <button data-testid={retry-${name}} onClick={onRetry}> retry
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:226:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:236:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:248:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:248:59 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:281:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:296:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:309:17 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:315:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:325:7 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:337:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:342:38 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:358:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/ContextCenter/UploadDocumentModal/UploadDocumentModal.test.tsx:380:7 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.
🟡 src/components/DataAssets/DataAssetSelectList/DataAssetPickerShell.tsx:43:18 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed.
🟡 src/components/DataAssets/DataAssetSelectList/DataAssetPickerShell.tsx:43:18 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":43,"column":17,"endLine":43,"endColumn"
🟡 src/components/DataAssets/DataAssetSelectList/DataAssetPickerShell.tsx:95:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 15 which is greater than 10 authorized.","cost":5,"secondaryLocations":[{"line":95,"column":3,"endLine":95,"endColumn":
🟡 src/components/DataAssets/DataAssetSelectList/DataAssetPickerShell.tsx:225:17 jsx-a11y/no-autofocus The autoFocus prop should not be used, as it can reduce usability and accessibility for users.
🟡 src/components/DataAssets/DataAssetSelectList/DataAssetPickerShell.tsx:232:56 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:109:5 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":109,"column":4,"endLine":109,"endColumn
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:301:30 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:342:8 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchKnowledgePages'. Either include it or remove the dependency array.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:359:9 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:369:8 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'fetchKnowledgePages' and 'pageOffset'. Either include them or remove the dependency array.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:429:7 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'addQuickLinkKnowledgePage'. Either include it or remove the dependency array.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:438:8 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'onPageChange' and 't'. Either include them or remove the dependency array. If 'onPageChange' changes too often,
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:552:23 jsx-a11y/control-has-associated-label A control must be associated with a text label.
🟡 src/components/KnowledgeCenter/KnowledgePageListComponent/KnowledgePageListComponent.tsx:552:23 jsx-a11y/anchor-has-content Anchors must have content and the content must be accessible by a screen reader.
🟡 src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:79:43 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":79,"column":42,"endLine":79,"endColumn"
🟡 src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:112:5 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:119:5 sonarjs/expression-complexity Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡 src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:184:6 react-hooks/exhaustive-deps React Hook useCallback has an unnecessary dependency: 't'. Either exclude it or remove the dependency array.
🟡 src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:511:24 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.

… and 7 more. Run make ui-checkstyle-changed locally for the full list.


Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 5 resolved / 5 findings

Fixes context center bugs by adding navigation and click handlers across dashboard cards, updating file upload error handling, and preserving updater users during file extractions. Resolved issues regarding interactive items, Playwright configuration, folder deep-linking, optional chaining, and dead props.

✅ 5 resolved
Bug: RecentItem always interactive, ignores missing onClick

📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ContextKnowledgePillarCard/ContextKnowledgePillarCard.component.tsx:39-53 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ContextKnowledgePillarCard/ContextKnowledgePillarCard.test.tsx:70-84
In ContextKnowledgePillarCard.component.tsx, RecentItem unconditionally renders role="button", tabIndex={0}, and wires onClick/onKeyDown that call item.onClick(). It never gates interactivity on whether item.onClick exists. This contradicts the new test 'renders a recent item without onClick as non-interactive' (which asserts queryByRole('button', {name:'Item 1'}) is absent), so that test will fail; and if any caller ever omits onClick, clicking/pressing Enter throws 'item.onClick is not a function' (handleKeyDown uses item.onClick() with no guard, and handleClick's item?.onClick() only guards item, not onClick). Gate the interactive props on the presence of item.onClick.

Bug: Playwright baseURL port changed 8585->3000 for whole suite

📄 openmetadata-ui/src/main/resources/ui/playwright.config.ts:34-37 📄 openmetadata-ui/src/main/resources/ui/playwright.config.ts:28-31
playwright.config.ts changes defaultBaseURL from localhost:8585 to localhost:3000 for both h2 and default modes. This affects the entire Playwright suite, not just the Context Center specs in this PR. If CI runs against the app served on 8585 (backend) rather than the Vite dev server on 3000, every e2e test will target a non-serving/incorrect port and fail. The h2-mode doc comment (lines 28-31) still documents https://localhost:8585, so the change is also undocumented/inconsistent. Confirm the CI serving port and scope this change, or update the comment and verify all lanes serve on 3000.

Edge Case: Folder deep-link param not cleared after selection

📄 openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterDocumentsPage/ContextCenterDocumentsPage.tsx:343-349
The new effect in ContextCenterDocumentsPage reads the 'folder' query param and sets selectedFolderId, but unlike the sibling 'document' effect it never removes the param from the URL afterward. Because the effect guards on !selectedFolderId, once the user navigates to a different folder within the page the stale ?folder= remains in the URL; a subsequent reload would still work, but the URL no longer reflects the actual selection. Consider clearing the param after applying it, mirroring the document handler.

Quality: Inconsistent optional-chaining on item.onClick

📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ContextKnowledgePillarCard/ContextKnowledgePillarCard.component.tsx:39-49
In RecentItem, handleClick calls item?.onClick() while handleKeyDown calls item.onClick(). The optional chaining on item (never null here) is misleading and the two handlers guard differently. Make both consistent and guard the actual function (item.onClick?.()).

Quality: onOpenFolder prop is now dead code

📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/DashboardFoldersCard/DashboardFoldersCard.interface.ts:20 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/DashboardFoldersCard/DashboardFoldersCard.component.tsx:41-46 📄 openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterDashboardPage/ContextCenterDashboardPage.tsx:639
After this change the component no longer destructures or uses onOpenFolder (folder rows now toggle expansion via handleToggleExpand, files use onOpenFile). The prop remains required in DashboardFoldersCard.interface.ts and is still passed by ContextCenterDashboardPage.tsx and every test, so it is dead code that forces callers to supply an unused required callback. Remove onOpenFolder from the interface and its call sites, or wire it back in if folder navigation is still intended.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant