Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: don't show metadata for embedded dashboards #30875

Merged

Conversation

sadpandajoe
Copy link
Member

@sadpandajoe sadpandajoe commented Nov 8, 2024

SUMMARY

As part of #27857 we started to show metadata even on embedded dashboards. This could have a potential of displaying data that embedded don't need to see. This PR will remove it if we are in embedded view.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

@@ -496,6 +496,7 @@ class Header extends PureComponent {
const refreshWarning =
dashboardInfo.common?.conf
?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE;
const isEmbedded = !dashboardInfo?.userId;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non blocking] curious whether there's a more explicit way to check whether we're in an embedded context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, but this comes up often... we need to invent this kind of state/check for myriad reasons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not ideal, but this is consistent with other places we assess if it's an embedded user or not.

@sadpandajoe
Copy link
Member Author

/testenv up

Copy link
Contributor

github-actions bot commented Nov 8, 2024

@sadpandajoe Ephemeral environment spinning up at http://34.210.58.93:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

logic:
Edit mode is ON, embedded is TRUE: MetadataBar should not render.
Edit mode is ON, embedded is FALSE: MetadataBar should not render.
Edit mode is OFF, embedded is TRUE: MetadataBar should not render.
Edit mode is OFF, embedded is FALSE: MetadataBar should render.
@sadpandajoe sadpandajoe force-pushed the joe/hide-dashboard-meta-on-embedded branch from 56e34ee to 3f653bd Compare November 8, 2024 17:43
@sadpandajoe sadpandajoe marked this pull request as ready for review November 8, 2024 18:19
@dosubot dosubot bot added the embedded label Nov 8, 2024
Copy link
Contributor

@Vitor-Avila Vitor-Avila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sadpandajoe sadpandajoe merged commit ac3a10d into apache:master Nov 12, 2024
61 checks passed
@sadpandajoe sadpandajoe deleted the joe/hide-dashboard-meta-on-embedded branch November 12, 2024 17:54
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

sadpandajoe added a commit that referenced this pull request Nov 13, 2024
@sadpandajoe sadpandajoe added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Nov 13, 2024
asher-lab added a commit to asher-lab/superset that referenced this pull request Nov 22, 2024
* fix: Exception handling for SQL Lab views (apache#30897)

* fix: don't show metadata for embedded dashboards (apache#30875)

* feat: add logging durations for screenshot async service (apache#30884)

* refactor(Avatar): Migrate Avatar to Ant Design 5 (apache#30740)

* refactor(input): Migrate Input component to Ant Design 5 (apache#30730)

* fix(empty dashboards): Allow downloading a screenshot of an empty dashboard (apache#30767)

Co-authored-by: Geido <60598000+geido@users.noreply.github.com>

* chore(ci): add tai and michael to helm owners (apache#30925)

* fix(helm): use submodule on helm release action (apache#30924)

* docs: add Free2Move to INTHEWILD.md (apache#30930)

* fix: blocks UI elements on right side (apache#30886)

Co-authored-by: Evan Rusackas <evan@preset.io>

* chore(deps): Migrate from `crate[sqlalchemy]` to `sqlalchemy-cratedb` (apache#29243)

* chore: update change log, UPDATING.md and bug-report.yml for 4.1 release (apache#30915)

* chore(docs): Update list of supported databases to include CrateDB (apache#30946)

* feat(trino,presto): add missing time grains (apache#30926)

* fix(Dashboard): Exclude edit param in async screenshot (apache#30962)

* fix(Card): Use correct class names for Ant Design 5 Card component (apache#30964)

* chore(Accessibility): Fix accessibility for 'Show x entries' dropdown in tables (apache#30818)

* build(deps): bump cross-spawn from 7.0.3 to 7.0.6 in /superset-frontend/cypress-base (apache#30969)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(release validation): scripts now support RSA and EDDSA keys. (apache#30967)

* build(deps): bump cross-spawn from 7.0.3 to 7.0.6 in /docs (apache#30970)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: add performance information to tooltip (apache#30948)

* fix(helm-chart): Fix broken PodDisruptionBudget due to introduction of extraLabels. (apache#30966)

* chore(helm): bumping app version to 4.1.1 in helm chart (apache#30918)

* chore: add unit tests for `is_mutating()` (apache#31021)

* chore(🦾): bump python numexpr 2.10.0 -> 2.10.1 (apache#31006)

Co-authored-by: GitHub Action <action@github.com>

* chore(🦾): bump python cffi 1.16.0 -> 1.17.1 (apache#31002)

Co-authored-by: GitHub Action <action@github.com>

* chore(🦾): bump python flask-babel subpackage(s) (apache#31000)

Co-authored-by: GitHub Action <action@github.com>

* fix(Dashboard): Retain colors when color scheme not set (apache#30646)

* fix(explore): verified props is not updated (apache#31008)

* chore: publish wheels (apache#30981)

* docs: Embedded sdk (apache#30972)

* fix(imports): import query_context for imports with charts (apache#30887)

* docs: Update doc about CSV upload (apache#30867)

Co-authored-by: Evan Rusackas <evan@preset.io>

* chore(🦾): bump python cattrs 23.2.3 -> 24.1.2 (apache#30998)

Co-authored-by: GitHub Action <action@github.com>

* fix(dataset): use sqlglot for DML check (apache#31024)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>
Co-authored-by: Maxime Beauchemin <maximebeauchemin@gmail.com>
Co-authored-by: Mehmet Salih Yavuz <salih.yavuz@proton.me>
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Co-authored-by: Paolo Terzi <PaoloTerzi@users.noreply.github.com>
Co-authored-by: Sukuna <90980311+samarsrivastav@users.noreply.github.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Andreas Motl <andreas.motl@elmyra.de>
Co-authored-by: Andreas Motl <andreas.motl@crate.io>
Co-authored-by: Levis Mbote <111055098+LevisNgigi@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
Co-authored-by: Christoph Keller <github@christophkeller.cc>
Co-authored-by: lodu <48859312+lodu@users.noreply.github.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: JUST.in DO IT <justin.park@airbnb.com>
Co-authored-by: David Hotham <david.hotham@microsoft.com>
Co-authored-by: Giampaolo Capelli <giampaolo.capelli@gmail.com>
Co-authored-by: Linden <zairrow@gmail.com>
Co-authored-by: Seiya <20365512+seiyab@users.noreply.github.com>
Co-authored-by: Asher Manangan <amanangan@powercosts.com>
@mistercrunch mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Nov 27, 2024
betodealmeida pushed a commit that referenced this pull request Jan 14, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels embedded size/M v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch 🍒 4.1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Superset 4.1.0RC2 - Dashboard embedding - now includes editor and last modified timestamp
4 participants