Skip to content

Conversation

@chirag-madlani
Copy link
Collaborator

@chirag-madlani chirag-madlani commented Jan 26, 2026

fix(ui): plugin should be added based on navigationItems if provided

Describe your changes:

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

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • 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.

Summary by Gitar

  • Plugin sidebar filtering:
    • Modified mergePluginSidebarItems() to check navigationItems.isHidden before inserting plugin items
  • Empty navigation handling:
    • Added isEmpty() check for empty navigation arrays in getTreeDataForNavigationItems() and getHiddenKeysFromNavigationItems()
  • Null safety fix:
    • Added null-safe check in useCustomPages.ts for pageType comparison
  • Test coverage:
    • Added unit tests validating plugin filtering behavior with navigationItems

This will update automatically on new commits.


@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.8% (55629/84540) 44.85% (28712/64020) 47.69% (8739/18326)

@gitar-bot
Copy link

gitar-bot bot commented Jan 26, 2026

🔍 CI failure analysis for 0111498: Multiple jobs failed: Job (6,6) has 10 issues (90% flaky), Job (2,6) has catastrophic 200 failures with build errors. Both unrelated to PR's navigation changes.

Issue

Multiple playwright-ci-postgresql jobs failed in run 21367798581 with different failure patterns, indicating both test flakiness and severe infrastructure issues. All failures are unrelated to this PR's navigation sidebar changes.

Root Cause

Two distinct failure patterns in same run:

Job (6, 6) - Test Flakiness Pattern

  • 1 failed + 9 flaky tests (90% flaky rate)
  • Failed: User permissions check
  • Flaky: Glossary (4), Lineage (1), Roles (2), Tags (2)
  • Errors: Visibility assertion failures (timing issues)

Job (2, 6) - Catastrophic Infrastructure Failure ⚠️

  • 200 failed tests + 6 flaky
  • Build system errors: gyp ERR! make failed with exit code: 2
  • Missing dependencies: ModuleNotFoundError: No module named 'cachetools'
  • Widespread timeouts:
TimeoutError: page.waitForURL: Timeout 60000ms exceeded
TimeoutError: page.waitForResponse: Timeout 10000ms exceeded
TimeoutError: page.goto: Timeout 60000ms exceeded

Affected areas in job (2, 6):

  • Bulk Edit Entity (Glossary, Database Schema)
  • Column Bulk Operations (all features)
  • Certification Dropdown
  • Glossary features
  • 200+ tests across unrelated areas

Why 100% unrelated to this PR:

  1. PR changes only navigation utilities (useCustomPages.ts, CustomizeNavigation.ts)
  2. Failures in entity operations: Bulk editing, columns, certifications, glossary
  3. Build/dependency errors: Unrelated to UI navigation code
  4. No code path overlap: Sidebar filtering doesn't affect bulk operations or build system

Details

The disparity between jobs confirms infrastructure issues:

  • Job (6, 6): 10 issues (90% flaky) = test stability
  • Job (2, 6): 206 issues (97% failed) = environment catastrophe

If PR code caused failures, all matrix jobs would fail similarly. Instead:

  • Different failure magnitudes
  • Build errors only in job (2, 6)
  • Dependency errors only in job (2, 6)
  • Timeout patterns indicate application not responding

Evidence of infrastructure failure in job (2, 6):

  • ✅ Build system failure: make errors during setup
  • ✅ Missing Python dependencies despite no Python code changes
  • ✅ Widespread timeouts (60s, 10s) across all operations
  • ✅ 200 tests spanning unrelated features (bulk ops, columns, certs)
  • ✅ Application server unresponsive or extremely slow
  • ❌ Zero connection to navigation sidebar logic

Job (2, 6) likely causes:

  1. CI environment corruption: Build system broken
  2. Dependency installation failure: Missing Python modules
  3. Application server failure: Not starting or responding
  4. Resource exhaustion: Memory/CPU overload in this matrix job
  5. Matrix job isolation issue: Environment contaminated

Recommendation

DO NOT block this PR - Failures are infrastructure issues in specific CI matrix jobs.

Immediate actions:

  1. Retry all failed jobs - Clean environment should resolve

  2. Investigate job (2, 6) environment:

    • Why did make fail?
    • Why is cachetools missing?
    • Check application server logs
    • Verify matrix job isolation
  3. Compare with other matrix jobs: Check if (1,6), (3,6), (4,6), (5,6) passed

Long-term: Fix CI matrix job isolation and dependency management.

This PR contains valid navigation utility changes. The 200-test failure in job (2, 6) is a severe infrastructure problem with build errors and missing dependencies that have zero relationship to sidebar plugin filtering.

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Good improvements for null safety and plugin filtering. One minor code quality issue remains with short-circuit evaluation for side effects.

💡 Quality: Short-circuit evaluation used for side effects

📄 openmetadata-ui/src/main/resources/ui/src/utils/CustomizaNavigation/CustomizeNavigation.ts:215

The expression !navData?.isHidden && insertPluginItem(mergedItems, item) uses short-circuit evaluation to conditionally execute a side effect. While functionally correct, this pattern can be less readable than an explicit if statement.

Consider refactoring to:

sortedPluginItems.forEach((item) => {
  const navData = navigationItems?.find((i) => i.id === item.key);
  if (!navData?.isHidden) {
    insertPluginItem(mergedItems, item);
  }
});

This makes the conditional logic more explicit and easier to understand at a glance. The short-circuit pattern is better suited for assignment expressions rather than side-effect operations.

Rules 🎸 1 action taken

Gitar Rules

🎸 Flaky Test Retry: Retried run 21367798581 (2 jobs failed, attempt 2)

2 rules not applicable. Show all rules by commenting gitar display:verbose.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link

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 To release Will cherry-pick this PR into the release branch UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants