-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
feat(dashboard): Add metadata bar to the header #27857
feat(dashboard): Add metadata bar to the header #27857
Conversation
Do we / should we hide this in print mode (standalone=true) or in an embedded context? I'm not sure what the behavior should be or what the options settings should be and generally think the more information the better, but I'm unclear if everyone would agree. |
+1 on the more the merrier. Knowing how fresh the dashboard is (the edit timestamp) is valuable even in an embedded context. The owners might not be something you want to see in an embedded context, but if anyone barks about that, I think that's also addressable — probably by means of a new config option (prop?) in the embedded component, just like you'd tweak other features (filter bar collapsing, etc) |
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.
/testenv up |
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.
Thank you for the improvement @justinpark.
superset/models/dashboard.py
Outdated
@property | ||
def created_by_name(self) -> str: | ||
if not self.created_by: | ||
return "" | ||
return str(self.created_by) | ||
|
||
@property | ||
def owners_by_name(self) -> list[str]: | ||
return [owner.get_full_name() for owner in self.owners] |
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.
This looks like more of frontend code than a new model property. If you check other instances of the Metadata component, this is being handled on the frontend as we may have different representations of the possible values.
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.
Done
@michael-s-molina Ephemeral environment spinning up at http://34.208.49.238:8080. Credentials are |
9b0cb6c
to
8dd320c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #27857 +/- ##
==========================================
+ Coverage 60.48% 60.80% +0.31%
==========================================
Files 1931 1937 +6
Lines 76236 76924 +688
Branches 8568 8606 +38
==========================================
+ Hits 46114 46774 +660
- Misses 28017 28048 +31
+ Partials 2105 2102 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Since we currently do not hide any items in the header in standalone mode, let's keep the metadata bar as it is for the time being. |
superset/models/dashboard.py
Outdated
@@ -242,6 +242,12 @@ def changed_by_name(self) -> str: | |||
return "" | |||
return str(self.changed_by) | |||
|
|||
@property |
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.
I think this is the same case as the owners and should be handled by the frontend.
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.
Sounds good
Ephemeral environment shutdown and build artifacts deleted. |
This is a great call out and we 100% need this. In the embedded context why would someone need to know about metadata thats really useful for a superset user? You are exposing details about superset and its users to an external application. It just causes confusion. In addition Tags like "Published" and the fav icon can be hidden in embedding. It should be an option and totally breaks embedding for us as we do not want to show this to end users in the embedded use case. Have raised issue #30188 for this. |
SUMMARY
As the dashboard currently lacks metadata information such as owners and last modified time, users have to navigate to edit mode and click "edit properties" to access this information. Unfortunately, there is no way to check the last modified time at all.
To address this issue, this commit integrates the metadata bar UI from the chart header into the dashboard header and updates the API properties accordingly. This enhancement will provide users with convenient access to the metadata information they need.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
after--dashboard-metadata.mov
TESTING INSTRUCTIONS
Go to a dashboard and then check the header
ADDITIONAL INFORMATION