Skip to content

Revamped workflow details header page - #31158

Merged
anuj-kumary merged 8 commits into
mainfrom
workflow-header
Aug 7, 2026
Merged

Revamped workflow details header page#31158
anuj-kumary merged 8 commits into
mainfrom
workflow-header

Conversation

@anuj-kumary

@anuj-kumary anuj-kumary commented Aug 7, 2026

Copy link
Copy Markdown
Member

Describe your changes:

Screenshot 2026-08-07 at 11 29 12 AM

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

  • UI enhancements:
    • Revamped WorkflowHeader to use HeaderShell component with gradient variant and enhanced breadcrumb navigation
    • Added support for AI mode layout variants and improved WorkflowBuilder header styling
  • Testing:
    • Rewrote CsvJobsTray E2E spec to use real backend API calls instead of request mocking

This will update automatically on new commits.

anuj-kumary and others added 2 commits August 7, 2026 12:01
…kend calls

Remove all page.route() mocks and replace with real export jobs created via
the Explore UI. Tests run serially to prevent job-list contamination, use
performAdminLogin for API polling, and rely on the tray's natural auto-open
behaviour instead of injected mock state transitions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@anuj-kumary anuj-kumary self-assigned this Aug 7, 2026
@anuj-kumary
anuj-kumary requested a review from a team as a code owner August 7, 2026 06:45
@anuj-kumary anuj-kumary added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch skip-pr-checks Bypass PR metadata validation check labels 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.21% (78151/118020) 50.22% (47200/93984) 51.43% (14218/27645)

Comment thread openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/CsvJobsTray.spec.ts Outdated
anuj-kumary and others added 6 commits August 7, 2026 13:01
…ross-worker contamination

Add data-testid="csv-jobs-tray-item-{jobId}" to each tray item so Playwright
tests can target the exact job they created rather than using .first(). This
prevents parallel workers sharing the admin user's job list from causing false
positives or assertion failures.

Also fix the first test's RUNNING-job leak by waiting for the job to reach
COMPLETED before exiting, so afterEach clear-completed can dismiss it cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…revent cross-worker contamination"

This reverts commit 7d3c546.
@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), 7 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), 7 warning(s) across 3 changed file(s).

Count Rule
2 sonarjs/no-duplicate-string
2 sonarjs/cyclomatic-complexity
2 react-hooks/exhaustive-deps
1 sonarjs/no-nested-conditional
All findings
Location Rule Message
🟡 src/components/WorkflowDefinitions/WorkflowBuilder/WorkflowHeader.test.tsx:206:31 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/WorkflowDefinitions/WorkflowBuilder/WorkflowHeader.test.tsx:247:28 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/WorkflowDefinitions/WorkflowBuilder/WorkflowHeader.tsx:47:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":47,"column":3,"endLine":47,"endColumn":
🟡 src/pages/WorkflowDefinitions/WorkflowBuilder/WorkflowBuilder.tsx:76:4 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 21 which is greater than 10 authorized.","cost":11,"secondaryLocations":[{"line":76,"column":3,"endLine":76,"endColumn"
🟡 src/pages/WorkflowDefinitions/WorkflowBuilder/WorkflowBuilder.tsx:333:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'edges' and 'nodes'. Either include them or remove the dependency array.
🟡 src/pages/WorkflowDefinitions/WorkflowBuilder/WorkflowBuilder.tsx:397:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 't' and 'workflowLogic'. Either include them or remove the dependency array.
🟡 src/pages/WorkflowDefinitions/WorkflowBuilder/WorkflowBuilder.tsx:511:23 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.

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

make ui-checkstyle-changed

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit f2263d68c835c88eb4d1adb81e3ec0ab1868e5f7 in Playwright run 31159066853, attempt 1.

✅ 577 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) 54m 1s

⏱️ Max setup 3m 7s · max shard execution 18m 5s · max shard-job elapsed before upload 21m 27s · reporting 4s

🌐 199.09 requests/attempt · 2.80 app boots/UI scenario · 3.96% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.8 per UI scenario (1682 boots / 600 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 156 0 0 0 0 0
🟡 Shard chromium-02 132 0 1 0 0 0
✅ Shard chromium-03 138 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)
  • Pages/Entity.spec.tsDomain Propagation (shard chromium-02, 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

@anuj-kumary
anuj-kumary added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit 9b8cd6c Aug 7, 2026
85 of 86 checks passed
@anuj-kumary
anuj-kumary deleted the workflow-header branch August 7, 2026 12:20
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 2.0 branch.

github-actions Bot pushed a commit that referenced this pull request Aug 7, 2026
* Revamped workflow details header page

* test(ui): rewrite CsvJobsTray spec without API mocking — use real backend calls

Remove all page.route() mocks and replace with real export jobs created via
the Explore UI. Tests run serially to prevent job-list contamination, use
performAdminLogin for API polling, and rely on the tray's natural auto-open
behaviour instead of injected mock state transitions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Revert "test(ui): rewrite CsvJobsTray spec without API mocking — use real backend calls"

This reverts commit c906954.

* test(ui): scope CsvJobsTray assertions to specific jobId to prevent cross-worker contamination

Add data-testid="csv-jobs-tray-item-{jobId}" to each tray item so Playwright
tests can target the exact job they created rather than using .first(). This
prevents parallel workers sharing the admin user's job list from causing false
positives or assertion failures.

Also fix the first test's RUNNING-job leak by waiting for the job to reach
COMPLETED before exiting, so afterEach clear-completed can dismiss it cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Revert "test(ui): scope CsvJobsTray assertions to specific jobId to prevent cross-worker contamination"

This reverts commit 7d3c546.

* fix lint checks

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit 9b8cd6c)
@gitar-bot

gitar-bot Bot commented Aug 7, 2026

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

Revamps the workflow details header page with the HeaderShell component and enhanced breadcrumbs, and updates the CsvJobsTray test spec to use real backend calls, addressing the Playwright timeout issue. No issues found.

✅ 2 resolved
Quality: isVisible({ timeout }) does not wait in Playwright

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/CsvJobsTray.spec.ts:88-92
In the afterEach cleanup, await clearButton.isVisible({ timeout: 2_000 }) passes a timeout option, but Playwright (1.57) Locator.isVisible() ignores timeout and returns immediately based on the current DOM state — it does not wait 2s for the tray to render. If the tray hasn't rendered yet at cleanup time the clear branch is skipped, weakening the best-effort cross-test cleanup that this serial suite relies on. Use clearButton.waitFor({ state: 'visible', timeout: 2_000 }).then(() => true).catch(() => false) (or an expect(...).toBeVisible({ timeout }) inside the try) if waiting is actually intended.

Quality: Glossary home crumb may mislead for non-glossary workflows

📄 openmetadata-ui/src/main/resources/ui/src/pages/WorkflowDefinitions/WorkflowBuilder/WorkflowBuilder.tsx:414-428
The new breadcrumb prepends getGlossaryHomeCrumb(t), which links to ROUTES.GLOSSARY (the Glossary home). The workflow builder can host governance workflows that are not glossary-specific, so anchoring every workflow's breadcrumb to the Glossary home could mislead navigation. If this header is only ever used for glossary approval workflows this is fine; otherwise consider a workflow/governance root crumb instead.

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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 1.13 branch.
Please cherry-pick the changes manually.
You can find more details here.

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 skip-pr-checks Bypass PR metadata validation check 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.

2 participants