-
Notifications
You must be signed in to change notification settings - Fork 16.6k
fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests #37529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Code Review Agent Run #51c985Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…nent - Create Menu core component for Ant Design dropdown menu interactions - Use hover-first approach with keyboard/dispatchEvent fallbacks - Scope popup selector by filtering for expected item text (avoid wrong popup) - Remove unused Menu methods (YAGNI): getItem, getSubmenu, selectItem, waitFor* - Remove magic waitForTimeout(100) - rely on popup visibility wait - Fix waitForChartsToLoad to wait for all indicators (count=0), not just first - Keep waitForLoad and waitForChartsToLoad as separate methods - Use existing Toast component for success toast assertions - Add toast assertions to both Export YAML and Export as Example tests - Add afterEach cleanup of downloaded files - Export Toast from core components index Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
31db274 to
0f7ef7b
Compare
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #b73c4bActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…click
Two fixes for Dashboard Export test reliability:
1. waitForChartsToLoad: Loading indicators ([aria-label="Loading"]) persist
in the DOM as hidden elements after charts load. Changed from DOM count
check to browser-context getComputedStyle visibility evaluation, which
returns immediately when charts are already loaded (no timeout penalty).
Also increased default timeout from PAGE_LOAD (10s) to API_RESPONSE (15s)
since chart data loading involves API calls.
2. Menu submenu click: Ant Design renders submenu popups in portals that can
be positioned outside the viewport or behind chart content (z-index).
Changed from Playwright click() to dispatchEvent('click') to bypass
viewport and pointer interception checks.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ac2d5de to
11a900f
Compare
Code Review Agent Run #05856eActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…dpoint The Export as Example handler was wrapping the API endpoint with ensureAppRoot() before passing it to SupersetClient.get(). Since SupersetClient.getUrl() already prepends the app root internally, this caused a double prefix (e.g., /app/prefix/app/prefix/api/v1/...) when Superset is deployed under a subdirectory (APPLICATION_ROOT). This made the Export as Example download silently fail in app_prefix deployments — the API call returned a 404, so no download event fired and the Playwright test timed out waiting for the download. The Export YAML handler was not affected because it passes the raw endpoint path without ensureAppRoot(). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review Agent Run #93051cActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
The World Health dashboard has multiple charts that can take 10-15s to load on cold CI runs. Combined with menu interaction and download time, the total exceeds the default 30s test timeout on first attempt. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical bug in the Dashboard Export as Example feature that caused 404 errors on Superset deployments using APPLICATION_ROOT (subdirectory deployments), and re-enables previously skipped E2E tests.
Changes:
- Fixed double-prefix bug in Export as Example by removing incorrect
ensureAppRoot()wrapper before passing endpoint toSupersetClient.get() - Created reusable
Menucore component with robust fallback strategy for Ant Design dropdown submenu interactions (hover → keyboard → dispatchEvent) - Improved chart loading detection in DashboardPage using browser-context visibility evaluation instead of DOM element count
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx | Removed ensureAppRoot() wrapper causing double-prefix bug in Export as Example endpoint |
| superset-frontend/playwright/tests/experimental/dashboard/export.spec.ts | Consolidated and re-enabled 2 E2E tests (Export YAML and Export as Example) with proper cleanup |
| superset-frontend/playwright/pages/DashboardPage.ts | Added waitForChartsToLoad() method and refactored download methods to use new Menu component |
| superset-frontend/playwright/components/core/Menu.ts | New reusable Menu component with three-tier fallback strategy for reliable submenu interaction |
| superset-frontend/playwright/components/core/index.ts | Exported Menu and Toast components |
Code Review Agent Run #4a53fdActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…ror handling Adds tests to verify SupersetClient.get is called with the correct endpoint path (no double app-root prefix) and that error/success toasts are shown appropriately. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
8025b0e to
09e01a7
Compare
Code Review Agent Run #e9121eActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
- Remove invalid `{ timeout }` arg from dispatchEvent (not a valid option)
- Replace local SUBMENU_OPEN constant with global TIMEOUT.FORM_LOAD
- Extract magic number 150ms to named ANIMATION_DELAY constant
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review Agent Run #f07ffaActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
SUMMARY
Bug fix:
Export as Examplewas broken in subdirectory deployments (app prefix) becauseensureAppRoot()double-prefixed the API endpoint —SupersetClient.get()already adds the app root internally, so the request went to/app/prefix/app/prefix/api/v1/...and returned 404.Test infrastructure: Re-enables the skipped Dashboard Export Playwright E2E tests by creating a reusable
Menucomponent that reliably handles Ant Design dropdown submenu interactions.Changes
DownloadMenuItems/index.tsx— RemoveensureAppRoot()wrapper onExport as Exampleendpoint to fix double app-root prefixDownloadMenuItems.test.tsx— Add unit tests asserting the correct endpoint path (prevents regression) and error toast on failureMenu.ts(Playwright) — New reusable component with hover → keyboard → dispatchEvent fallback for opening Ant Design submenus; usesdispatchEvent('click')for item selection to bypass viewport/pointer interception from chart content; uses globalTIMEOUT.FORM_LOADconstant and namedANIMATION_DELAYfor maintainabilityDashboardPage.ts(Playwright) — AddwaitForChartsToLoad()using browser-contextgetComputedStylevisibility checks; addselectDownloadOption()using the Menu componentexport.spec.ts(Playwright) — Remove skip, add 60s timeout for cold-cache CI runs, add download cleanup and toast assertionsBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — Export as Example silently failed (404) in app-prefix deployments; now works correctly.
TESTING INSTRUCTIONS
npx jest --no-coverage src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsxPLAYWRIGHT_ADMIN_PASSWORD=admin INCLUDE_EXPERIMENTAL=true npx playwright test --grep "Dashboard Export"APPLICATION_ROOT=/prefix/, open a dashboard, click Download → Export as Example — should download a zip (previously 404'd)ADDITIONAL INFORMATION