Skip to content

Fixed article page bugs - #30527

Merged
Rohit0301 merged 8 commits into
mainfrom
fix-article-move-bugs
Jul 28, 2026
Merged

Fixed article page bugs#30527
Rohit0301 merged 8 commits into
mainfrom
fix-article-move-bugs

Conversation

@Rohit0301

@Rohit0301 Rohit0301 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes 30576

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

  • Bug fixes:
    • Fixed infinite fetching loop in KnowledgePagesHierarchy and added checks for pagination limits
    • Prevented duplicate nodes and preserved nested subtrees during article tree updates
  • Refactoring:
    • Replaced KnowledgeCenterLayout with KnowledgePagesHierarchy and KnowledgePagePureUtils
    • Updated recent viewed tracking to filter by entity ID rather than FQN

This will update automatically on new commits.

Greptile Summary

The PR revises article hierarchy loading and tree reconciliation.

  • Tracks per-node child offsets, loading, and exhaustion state.
  • Deduplicates fetched children and preserves or remaps loaded subtrees during updates and moves.
  • Adds hierarchy pagination, refresh, expansion, and deep-tree regression coverage.
  • Inlines the article page’s resizable layout and updates recently viewed entries by entity ID.

Confidence Score: 3/5

The PR is not yet safe to merge because stale request completions and mutable offset pagination can still leave the article hierarchy incomplete.

Force refresh clears pagination metadata without invalidating older child requests, allowing those requests to restore stale exhaustion, offsets, and tree state; positional pagination also remains vulnerable to sibling insertions or deletions shifting page boundaries.

Files Needing Attention: openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx; openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx Adds child pagination and refresh bookkeeping, but the previously reported stale-request and mutable-offset failures remain reachable.
openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts Deduplicates child merges and preserves or remaps nested subtrees during hierarchy updates.
openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterArticlesPage/ContextCenterArticlesPage.tsx Inlines the resizable knowledge-center layout and retains hierarchy refresh integration.
openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.tsx Changes recently viewed replacement semantics from FQN identity to stable entity ID.
openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.test.tsx Adds broad hierarchy regressions, but its force-refresh test covers only completed child requests rather than overlapping requests.

Sequence Diagram

sequenceDiagram
  participant UI as Hierarchy UI
  participant Child as Child request
  participant Refresh as Force refresh
  participant API as Hierarchy API
  UI->>Child: Load expanded node
  Child->>API: GET children at numeric offset
  UI->>Refresh: Refresh after create/edit/delete
  Refresh->>API: GET root hierarchy
  API-->>Refresh: Fresh hierarchy
  Refresh->>UI: Replace tree and clear pagination metadata
  API-->>Child: Older child response
  Child->>UI: Restore stale offset, exhaustion, and tree snapshot
Loading

Reviews (6): Last reviewed commit: "lint fix" | Re-trigger Greptile

@Rohit0301 Rohit0301 self-assigned this Jul 27, 2026
@Rohit0301 Rohit0301 added the safe to test Add this label to run secure Github workflows on PRs label Jul 27, 2026
@Rohit0301
Rohit0301 requested a review from a team as a code owner July 27, 2026 12:50
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

@github-actions github-actions Bot added the UI UI specific issues label Jul 27, 2026
@Rohit0301 Rohit0301 changed the title Fix article move bugs Fixed article page bugs Jul 27, 2026
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.73% (76890/116962) 49.6% (46193/93121) 50.83% (13914/27369)

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 57cfb026acad803fa92c46e99aba13f86dadffd3 in Playwright run 30350343282, attempt 1.

✅ 577 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 5 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) 49m 48s

⏱️ Max setup 3m 33s · max shard execution 16m 16s · max shard-job elapsed before upload 19m 50s · reporting 4s

🌐 200.11 requests/attempt · 2.80 app boots/UI scenario · 17.73% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 17.73% (convergence target: at most 15%).
  • Browser traffic was 200.11 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.8 per UI scenario (1710 boots / 611 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 118 0 0 3 0 0
✅ Shard chromium-02 119 0 0 0 0 0
✅ Shard chromium-03 98 0 0 0 0 0
✅ Shard chromium-04 104 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 23 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 27 0 0 2 0 0

📦 Download artifacts

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

@sonarqubecloud

Copy link
Copy Markdown

@Rohit0301
Rohit0301 enabled auto-merge July 28, 2026 11:57
@Rohit0301
Rohit0301 added this pull request to the merge queue Jul 28, 2026
Merged via the queue into main with commit 2cdecba Jul 28, 2026
77 of 80 checks passed
@Rohit0301
Rohit0301 deleted the fix-article-move-bugs branch July 28, 2026 15:51
@gitar-bot

gitar-bot Bot commented Jul 28, 2026

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

Fixes article hierarchy pagination limits and descendant preservation during moves, addressing the infinite loop and stale FQN findings. New unit and E2E tests have been added.

✅ 2 resolved
Bug: Moved subtree retains stale descendant FQNs after move

📄 openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx:489-503 📄 openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts:96-110
In handleMovePage, when a moved node already has loaded children, targetChildrenWithMovedSubtree re-attaches sourceNode.children verbatim under the node's new location. Those descendants keep their old fullyQualifiedNames (e.g. oldParent.source.grandchild) even though the moved node's FQN is now newSourceFQN. Since the tree uses fullyQualifiedName as the node key (convertToTreeData) and the expandedKeys are remapped to the new prefix (lines 473-479), the preserved subtree's keys no longer match: descendants won't expand and clicking one navigates to a non-existent old FQN. Remap the descendant FQNs by replacing the oldSourceFQN prefix with newSourceFQN before attaching them.

Bug: handleExpandAll can loop forever when childrenCount is unreachable

📄 openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx:184-198
The while (nodesPendingChildren.length > 0) loop in handleExpandAll only terminates when every node reaches children.length >= childrenCount. Unlike loadNodeChildren (which stops via nodesWithNoMoreChildrenRef when a page returns fewer than the limit), this loop has no page-exhaustion guard. If a node's stale childrenCount can never be satisfied by the unique children the server returns — e.g. concurrent deletes, permission-filtered results, or unionBy dedup collapsing overlapping paginated pages — collectUnloadedExpandableNodes keeps returning that node, the server eventually returns empty pages, and the loop spins indefinitely, firing endless API calls and hanging the browser tab. Add a termination guard: stop paging a node once a fetched page is shorter than KNOWLEDGE_CENTER_PAGINATION_LIMIT (or the response is empty), mirroring loadNodeChildren.

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 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Rohit0301 added a commit that referenced this pull request Jul 28, 2026
* Fixed Article page bugs

* fixed hierarchy issues

* addressed gitar comment

* addressed gitar comment

* lint fix

* fixed playwright test

* lint fix
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.

Fix hierarchy tree crash for nested pages

2 participants