feat(charts): chart publishing UI

[ethan-liong]

SUMMARY

This MR introduces publishing for charts.

• adds the publish/draft icon when viewing a chart
• adds publish/draft status + filter in chart list
• adds publish to model

At a glance, it doesn't really do much besides provide a UI indicator as to whether a chart is tagged as published or not, but I figure that this might be the start of creating extra additional custom permissions on top of charts that an owner might deemed published and ready for more eyes. This is really just to get a foot in the door towards maybe creating "private" charts.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

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

[korbit-ai]

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

[github-actions]

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

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

[korbit-ai]

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Grammatical Error in UI Message ▹ view	✅ Fix detected
	Unused Import ▹ view	✅ Fix detected
	Typo in Migration Name ▹ view	🧠 Not in scope
	Raw SQL Execution in Migration ▹ view	✅ Fix detected
	Inefficient Two-Pass Table Operation ▹ view	✅ Fix detected

Files scanned

​

File Path	Reviewed
superset/migrations/versions/2025-03-25_21-03_bd200ee2fd0c_adding_publishing_filed_to_chart.py	✅
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts	✅
superset-frontend/src/explore/components/ExploreChartPublish/index.tsx	✅
superset-frontend/src/explore/components/ExploreChartHeader/index.jsx	✅
superset/models/slice.py	✅
superset-frontend/src/pages/ChartList/index.tsx	✅
superset/charts/api.py	✅
superset/charts/schemas.py	✅
superset/config.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

• You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.
• You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.
• Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.
• On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.
• Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

• Check out our docs on how you can make Korbit work best for you and your team.
• Customize Korbit for your organization through the Korbit Console.

Feedback and Support

• Tell us what you think of Korbit
• Schedule a call with our team
• Email us @ support@korbit.ai

[michael-s-molina]

Hi @ethan-liong. Thank you for the PR. This type of change generally needs a SIP where we can discuss what are the intended use cases and implications of introducing the concept of published/unpublished charts. It's also a good idea to think about the whole feature, given that if we merge the PR as is, we'll have a new status but no immediate applicability for it.

You can check #5602 for the SIP template.

[rusackas]

This seems like it's a great idea, but it does seem like it might need a SIP to answer any questions that come up. I'll seed the discussion with a few:
• Will there be a migration to convert all pre-existing charts to "Published" when this launches?
• If a chart is un-published, does it still appear in dashboards it was placed in? If not, how does it fail gracefully?
• Who has the RBAC permissions to publish/unpublish a chart? The owner(s) of the chart, and admins, I presume?
• Can an unpublished chart be added to a dashboard, either from the Save modal or from the drag-and-drop Dashboard builder?
• If an unpublished chart is in a dashboard, is it exported with the dashboard?

[mistercrunch]

I'm open to a SIP here, but don't think this is controversial, especially behind a feature flag. As long as it fits the implementation for dashboards, its in many ways is even more needed for Charts, given that there's an order of magnitude more charts than there are dashboards.

Some clarifying questions (whether it's for the SIP or PR...)

• we should clarify whether published/draft affects access or RBAC (?) From my understanding it doesn't/shouldn't, meaning draft isn't "private" - other people can access your drafts if they desire, it simply curates the list view (where I believe you see your own drafts + what others have published (?))
• does publishing a dashboard implies that the charts its composed-of are (or should) be published (?) What if you add charts after publishing a dashboard? Is it confusing the see a published dashboard made out of "draft" charts (?)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.34%. Comparing base (76d897e) to head (07ff305).
Report is 1713 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #32969       +/-   ##
===========================================
+ Coverage   60.48%   83.34%   +22.86%     
===========================================
  Files        1931      550     -1381     
  Lines       76236    39537    -36699     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    32954    -13160     
+ Misses      28017     6583    -21434     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.36% <100.00%> (-0.80%)	⬇️
javascript	?
mysql	75.57% <100.00%> (?)
postgres	75.64% <100.00%> (?)
presto	52.88% <100.00%> (-0.93%)	⬇️
python	83.34% <100.00%> (+19.84%)	⬆️
sqlite	75.14% <100.00%> (?)
unit	61.41% <100.00%> (+3.78%)	⬆️

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.

[JZ6]

This seems like it's a great idea, but it does seem like it might need a SIP to answer any questions that come up. I'll seed the discussion with a few: •

We have had this feature in our deployment of superset for 2 years now, so I may be able to answer some of these questions based on how we have handled this:

Will there be a migration to convert all pre-existing charts to "Published" when this launches?

Ideally old charts should stay as draft, but maybe add a way for admins to bulk publish multiple/all charts at once?

If a chart is un-published, does it still appear in dashboards it was placed in? If not, how does it fail gracefully?

Perhaps a chart in a published dashboard can not be unpublished unless the dashboard is also in draft. We also have checks the other way, a dashboard can only be published if all charts within is published

Who has the RBAC permissions to publish/unpublish a chart? The owner(s) of the chart, and admins, I presume?

We use a seperate role with special publish perms, but yes only allowing owners and admins would make sense.

[mistercrunch]

Could be nice and simple if we kept it as simple as "published means the object is discoverable by others, namely in list views (namely the CRUD views and various dropdowns). Draft objects, while they don't show up in lists views, are accessible by others if they happen to have the link, or say if they happen to open a dashboard composed of draft chart".

It can be tricky to cascade things from say dashboard to charts, say when you publish a dashboard, do we need to ask the user whether they want to publish the charts it's made out of? Some of them? What about if/when they add a new chart to a published dashboard, should it auto-publish the new chart? I'd say we keep it simple and allow users to publish individual object without cascading

[ethan-l-geotab]

Thanks for the feedback.

So currently this PR is just in a state where it’s UI labels. For now, it’s just to kick off the thought process as to accountability and visibility of “charts in draft”. Right now it's just a sign of “hey, im done with this chart, I think it’s in a ready state for people to look at!”. The accountability is to double down on the certification, and to work in tandem with dashboards publication status. More in the answered questions

I was really just testing the waters I guess to gauge interest without fully committing to writing a SIP (It’s a little more overwhelming to me to propose a plan rather than just sorta do it and see the interest).

To answer a few of those questions:

• The migration currently sets it all to draft. I think it might actually be better to set it to publish to keep the current state where “everything is considered published”
• The ones who can publish a chart currently I believe should be the ones that can currently make changes to the chart with the “canOverwrite”.
• It is the intention that in the future, only published charts can end up on published dashboards. No dashboard should be published with unpublished charts. This is so that we have a baseline of "production ready” standards for all “production ready” dashboards where all the charts on there are “ready for production”. However, if we don't want cascading, thats fine too.
• Currently a draft/published status does not have access on RBAC. It’s just the UI stuff. However, if someone has a better suggestion on how to handle the privacy that would make sense in a Superset-y way, that’ll be cool.

Questions I can’t answer yet:

• What happens to a published chart when it’s unpublished and on a dashboard?
  I’m not too sure how to handle this:

1. Unpublish the dashboard but pop a warning for the user that they will unpublish the dashboards (should they even have permissions to unpublish dashboards with their chart on them?)
2. Show a warning perhaps that now there will be published dashboard with unpublished charts?
3. Block unpublishing. If changes need to be made to the chart, then perhaps a versioning system could be developed where a “new published” chart could take the place of the old one, but that’s a bit of a can of worms…
4. This isnt a problem if there is no cascading anyways. Big fan of keeping things simple....

With all that in mind and the current state of the PR, i was hoping that since it's feature flag, i could dodge the SIP, but i think for sure that the full vision of whats in mind, i think a SIP would be unavoidable. This was only intended to be the foundational work on which these features, permissions and controls would be added over time.

[michael-s-molina]

My 2 cents:

• The number of questions raised in this thread and the number of open questions are a clear indication of a SIP.
• I don't think we should use feature flags to think about new features as we code. Feature flags introduce conditions into the core codebase, increase complexity, and maintainability. If we don't know yet the extent of this feature, I would suggest creating a feature branch. We're doing this with Theming and Extensions for example.
• In my opinion, feature flags should be used when we already discussed the feature over a SIP and we use a feature flag to develop what was agreed. Not to think about the feature while modifying the core codebase.
• Introducing feature flags in PRs without any SIP was the reason for many poorly designed features that we had to re-implement is the past.

[villebro]

@ethan-liong as part of SIP-152, we're introducing the concept of Viewers to Charts and Dashboards. This means, that once rolled out, Draft/Published is redundant, as read only access will be provided by assigning subjects as Viewers. See my comment #32116 (comment) , more specifically these bullets:

• We replace all owner references with the new Subject model that can be a User, Group or Role. This covers Dashboards, Charts, Datasets and Alerts & Reports.
• We introduce the Viewer property to Dashboard and Chart entities (for Alerts & Reports having a Viewer is not very useful). During the migration we map all Dashboard RBAC roles as Viewers and remove the DASHBOARD_RBAC feature flag.
• We map Roles that have implicit access to Dashboards and Charts to their respective Viewer property, and remove the implicit dashboard/chart access control logic. However, for Dashboards that are in Draft state we leave the Viewer property empty. Finally we remove the "published" property from the Dashboard model, and remove the Draft/Published pill (good riddance!).

[JZ6]

@ethan-liong as part of SIP-152, we're introducing the concept of Viewers to Charts and Dashboards. This means, that once rolled out, Draft/Published is redundant, as read only access will be provided by assigning subjects as Viewers. See my comment #32116 (comment) , more specifically these bullets:

• We replace all owner references with the new Subject model that can be a User, Group or Role. This covers Dashboards, Charts, Datasets and Alerts & Reports.
• We introduce the Viewer property to Dashboard and Chart entities (for Alerts & Reports having a Viewer is not very useful). During the migration we map all Dashboard RBAC roles as Viewers and remove the DASHBOARD_RBAC feature flag.
• We map Roles that have implicit access to Dashboards and Charts to their respective Viewer property, and remove the implicit dashboard/chart access control logic. However, for Dashboards that are in Draft state we leave the Viewer property empty. Finally we remove the "published" property from the Dashboard model, and remove the Draft/Published pill (good riddance!).

awesome! do you know which version of superset this is expected to be released in? thanks!

[mistercrunch]

This means, that once rolled out, Draft/Published is redundant

I think it's probably still relevant, it's really about the owner of the chart/dashboard saying "ok I want for this dashboard I created to be discoverable by others". That user may not be an Admin so they don't really have the rights to grant access to others...