Skip to content

feat(reports): adding link to report content#40525

Open
SkinnyPigeon wants to merge 4 commits into
apache:masterfrom
SkinnyPigeon:feat-add-link-to-report-content
Open

feat(reports): adding link to report content#40525
SkinnyPigeon wants to merge 4 commits into
apache:masterfrom
SkinnyPigeon:feat-add-link-to-report-content

Conversation

@SkinnyPigeon
Copy link
Copy Markdown
Contributor

SUMMARY

One of my small annoyances when debugging broken reports on behalf of account managers, etc. at work is that every time I have to do it, I need to first open the report, check the name of the content, open the relevant Dashboard or Charts menu, and then manually search for the name of the chart of dashboard.

This adds a small button which links the content selected in the dropdown menu and opens it in a new tab, bypassing most of those steps. It would be a small but meaningful win to have this enabled

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

This shows the new button in action in both of the modals.

Screen.Recording.2026-05-29.at.16.41.39.mov

TESTING INSTRUCTIONS

  1. Build and deploy this branch with the examples installed
  2. Create a new report
  3. Select either a chart or dashboard from the dropdown
  4. Click the new button to open the content in a new tab

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot Bot added alert-reports Namespace | Anything related to the Alert & Reports feature change:frontend Requires changing the frontend labels May 29, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 29, 2026

Code Review Agent Run #625e5a

Actionable Suggestions - 0
Additional Suggestions - 5
  • superset-frontend/src/features/alerts/AlertReportModal.tsx - 1
    • Inconsistent button sizing · Line 2411-2411
      The chart link button (line 2411) uses `iconSize="s"` without specifying a button size, creating potential inconsistency with other icon buttons. Compare with line 493 which uses `iconSize="m"`. Adding `size="sm"` would ensure consistent compact button rendering.
  • superset-frontend/src/features/alerts/AlertReportModal.test.tsx - 4
    • Unnecessary @ts-ignore suppression · Line 697-697
      The `// @ts-ignore` on line 697 is unnecessary — `window.open` is typed via `"lib": ["dom", ...]` in tsconfig.json. Replace with `jest.spyOn(window, 'open').mockImplementation(jest.fn())` or cast to `jest.Mock` to maintain type safety without suppressing errors.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
       @@ -694,7 +694,7 @@ test('open chart button opens explore with slice_id', async () => {
          expect(openChartButton).toBeInTheDocument();
       
          const origOpen = window.open;
      -  // @ts-ignore
          window.open = jest.fn();
          await userEvent.click(openChartButton);
          expect(window.open).toHaveBeenCalledWith('/explore/?slice_id=1', '_blank', 'noopener');
    • Unnecessary @ts-ignore suppression · Line 702-702
      The `// @ts-ignore` on line 702 suppresses a type error for `window.open = origOpen` — this is unnecessary since `origOpen` was captured from the typed `window.open` and tsconfig includes DOM types.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
       @@ -699,7 +699,6 @@ test('open chart button opens explore with slice_id', async () => {
          await userEvent.click(openChartButton);
          expect(window.open).toHaveBeenCalledWith('/explore/?slice_id=1', '_blank', 'noopener');
          // restore
      -  // @ts-ignore
          window.open = origOpen;
        });
    • Unnecessary @ts-ignore suppression · Line 722-722
      The `// @ts-ignore` on line 722 is unnecessary — `window.open` is typed via the DOM lib. Consider using `jest.spyOn(window, 'open').mockImplementation(jest.fn())` for a fully typed mock.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
       @@ -719,7 +719,6 @@ test('open dashboard button opens dashboard url', async () => {
          expect(openDashButton).toBeInTheDocument();
       
          const origOpen = window.open;
      -  // @ts-ignore
          window.open = jest.fn();
          await userEvent.click(openDashButton);
          expect(window.open).toHaveBeenCalledWith('/superset/dashboard/1', '_blank', 'noopener');
    • Unnecessary @ts-ignore suppression · Line 727-727
      The `// @ts-ignore` on line 727 is unnecessary — `origOpen` has the same type as `window.open` (both typed via DOM lib), so the assignment is type-safe.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
       @@ -724,7 +724,6 @@ test('open dashboard button opens dashboard url', async () => {
          await userEvent.click(openDashButton);
          expect(window.open).toHaveBeenCalledWith('/superset/dashboard/1', '_blank', 'noopener');
          // restore
      -  // @ts-ignore
          window.open = origOpen;
        });
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • superset-frontend/src/features/alerts/AlertReportModal.tsx - 1
Review Details
  • Files reviewed - 2 · Commit Range: 33f0386..33f0386
    • superset-frontend/src/features/alerts/AlertReportModal.test.tsx
    • superset-frontend/src/features/alerts/AlertReportModal.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@github-actions
Copy link
Copy Markdown
Contributor

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

Please read our New Contributor Welcome & Expectations guide.

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

Comment thread superset-frontend/src/features/alerts/AlertReportModal.test.tsx Outdated
Comment thread superset-frontend/src/features/alerts/AlertReportModal.test.tsx Outdated
Comment thread superset-frontend/src/features/alerts/AlertReportModal.tsx
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.08%. Comparing base (b0da0cf) to head (9c6b99a).
⚠️ Report is 52 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #40525   +/-   ##
=======================================
  Coverage   64.07%   64.08%           
=======================================
  Files        2592     2592           
  Lines      139578   139586    +8     
  Branches    32431    32433    +2     
=======================================
+ Hits        89441    89449    +8     
  Misses      48599    48599           
  Partials     1538     1538           
Flag Coverage Δ
javascript 67.34% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SkinnyPigeon SkinnyPigeon marked this pull request as draft May 29, 2026 15:40
@SkinnyPigeon SkinnyPigeon marked this pull request as ready for review May 29, 2026 18:58
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 29, 2026

Code Review Agent Run #7407c8

Actionable Suggestions - 0
Additional Suggestions - 2
  • superset-frontend/src/features/alerts/AlertReportModal.tsx - 2
    • Redundant ensureAppRoot call · Line 1428-1432
      The `ensureAppRoot()` call on line 1430 is redundant — `navigateTo()` at line 1431 already applies `ensureAppRoot()` internally (verified in `navigationUtils.ts:26`). Passing the raw path to `navigateTo()` eliminates duplication while preserving correct subdirectory-prefixed URLs.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
       @@ -1426,8 +1426,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
          };
       
          const openDashboardInNewTab = (dashboardId?: number | string | null) => {
            if (!dashboardId) return;
      -    const url = ensureAppRoot(`/superset/dashboard/${dashboardId}`);
      -    navigateTo(url, { newWindow: true });
      +    navigateTo(`/superset/dashboard/${dashboardId}`, { newWindow: true });
          };
    • Redundant ensureAppRoot call · Line 1440-1444
      Same semantic duplication as `openDashboardInNewTab`: the `ensureAppRoot()` call on line 1442 is redundant because `navigateTo()` on line 1443 already handles app root prefixing internally.
      Code suggestion
      --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
      +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
       @@ -1438,8 +1438,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
          };
       
          const openChartInNewTab = (chartId?: number | string | null) => {
            if (!chartId) return;
      -    const url = ensureAppRoot(`/explore/?slice_id=${chartId}`);
      -    navigateTo(url, { newWindow: true });
      +    navigateTo(`/explore/?slice_id=${chartId}`, { newWindow: true });
          };
Review Details
  • Files reviewed - 2 · Commit Range: 33f0386..b5b4391
    • superset-frontend/src/features/alerts/AlertReportModal.test.tsx
    • superset-frontend/src/features/alerts/AlertReportModal.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 1, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit ee9a083
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a1d303b7ab563000856c8f7
😎 Deploy Preview https://deploy-preview-40525--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alert-reports Namespace | Anything related to the Alert & Reports feature change:frontend Requires changing the frontend size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant