Skip to content

Fixes 28904: Document page refactoring#28874

Merged
Rohit0301 merged 6 commits into
mainfrom
document-page-refactoring
Jun 10, 2026
Merged

Fixes 28904: Document page refactoring#28874
Rohit0301 merged 6 commits into
mainfrom
document-page-refactoring

Conversation

@Rohit0301

@Rohit0301 Rohit0301 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes 28904

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

  • Data Schema Migration:
    • Replaced the local DocFile interface with the standardized ContextFile schema throughout the document components.
    • Simplified file state management by removing helper mapping functions like contextFileToDocumentItem and contextFileToUploadedDocumentItem.
  • Component Cleanup:
    • Removed obsolete components, including ArticleCard, UploadedDocumentCard, ArticleListSection, UploadedDocumentsSection, and DeleteModal (along with their associated tests).
  • Refactored Document View:
    • Optimized FileRow and DocumentPreviewPanel rendering using useMemo for formatted values like fileSize and relative dates.
    • Fixed UI container overflow issues in DocumentsView by adjusting CSS layouts for better scrolling behavior.
  • Utility Enhancements:
    • Centralized file download logic by updating handleAssetDownload to use ContextFile directly.

This will update automatically on new commits.

@Rohit0301 Rohit0301 self-assigned this Jun 9, 2026
@Rohit0301 Rohit0301 requested a review from a team as a code owner June 9, 2026 13:46
@Rohit0301 Rohit0301 added the safe to test Add this label to run secure Github workflows on PRs label Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 62%
62.71% (67055/106912) 43.98% (36994/84103) 46.29% (11375/24573)

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (13 flaky)

✅ 4271 passed · ❌ 0 failed · 🟡 13 flaky · ⏭️ 88 skipped

Shard Passed Failed Flaky Skipped
✅ Shard 1 301 0 0 4
🟡 Shard 2 802 0 4 9
🟡 Shard 3 805 0 3 8
🟡 Shard 4 841 0 2 12
🟡 Shard 5 719 0 2 47
🟡 Shard 6 803 0 2 8
🟡 13 flaky test(s) (passed on retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › EditAll User: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with only VIEW cannot PATCH results (shard 2, 1 retry)
  • Features/Glossary/GlossaryWorkflow.spec.ts › should start term as Draft when glossary has reviewers (shard 2, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 2 retries)
  • Features/Table.spec.ts › Table pagination with sorting should works (shard 3, 1 retry)
  • Features/Tasks/DomainFiltering.spec.ts › selecting All Domains removes domain filter from feed API call (shard 3, 1 retry)
  • Pages/CustomProperties.spec.ts › Should verify property name is visible for apiCollection in right panel (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Domain Rbac (shard 4, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should perform CRUD and Removal operations for dashboard (shard 5, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/PlatformLineage.spec.ts › Verify domain platform view (shard 6, 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

@Rohit0301 Rohit0301 force-pushed the document-page-refactoring branch from 1287030 to def4dd1 Compare June 10, 2026 05:42
@sonarqubecloud

Copy link
Copy Markdown

@Rohit0301 Rohit0301 merged commit 29ea73a into main Jun 10, 2026
56 of 62 checks passed
@Rohit0301 Rohit0301 deleted the document-page-refactoring branch June 10, 2026 11:40
@gitar-bot

gitar-bot Bot commented Jun 10, 2026

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

Refactors the Documents page by standardizing on the ContextFile schema and removing obsolete UI components. Bulk move logic was updated to address non-existent folder ID and name references, and no issues were found.

✅ 1 resolved
Bug: Bulk move updates non-existent folderId/folderName fields

📄 openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterDocumentsPage/ContextCenterDocumentsPage.tsx:337-349
After migrating allDocuments from the old DocFile shape to the generated ContextFile, handleBulkMove still writes the legacy fields folderId and folderName onto each moved document. ContextFile has no such top-level properties — the parent folder lives under folder (an EntityReference, accessed as folder?.id). The folder filter in this same file uses allDocuments.filter((d) => d.folder?.id === selectedFolderId), and DocumentFolderView groups files via file.folder?.id === folder.id. Because folder.id is never updated, after a successful bulk move the affected files still display under their original folder and do not appear under the destination folder until a full refetch. Note handleFileMoved (single-file move) already does this correctly by updating folder: { ...d.folder, id: targetFolderId, type: ... }. The two move paths are inconsistent, and the bulk path is the buggy one. (This may also fail tsc excess-property checks since the returned object literal is contextually typed as ContextFile.)

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document Page code refactoring and cleanup

2 participants