You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/02/05 02:10:26 UTC
[GitHub] [incubator-superset] suddjian opened a new pull request #9088:
[dashboard] Fix metadata state
suddjian opened a new pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088
### CATEGORY
Choose one
- [x] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
`json_metadata` fields weren't being properly populated from the bootstrap data. Now all fields are present. Not sure why we were doing extra work to exclude those fields from the state object.
I also swapped out the plain old `<textarea>` for a fancy schmancy `<AceEditor>`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
<img width="872" alt="Screen Shot 2020-02-04 at 6 09 39 PM" src="https://user-images.githubusercontent.com/1858430/73804711-879c3e00-4779-11ea-968d-4ec9cbbc39e7.png">
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat edited a comment on issue
#9088: [dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on issue #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#issuecomment-582671214
@suddjian Thanks for this fix. I also found a few dashboard metadata was missing and almost filed an issue.
Here is a list of metadata:
https://github.com/apache/incubator-superset/blob/315a11dfe265c6a20ed014610eb7351966292c38/superset/views/dashboard/api.py#L44
But there are another 2 fields not in the above list: **`color scheme`** and **`label_colors`**:
<img width="520" alt="Screen Shot 2020-02-05 at 3 49 58 PM" src="https://user-images.githubusercontent.com/27990562/73894137-5e8ab480-4830-11ea-99fa-7f3398dfa8ad.png">
<img width="592" alt="Screen Shot 2020-02-05 at 3 57 56 PM" src="https://user-images.githubusercontent.com/27990562/73894107-49ae2100-4830-11ea-909d-5747b5b7588e.png">
Not sure these 2 extra fields will be picked up in this PR.
And if possible, could you add these 2 fields into `DashboardJSONMetadataSchema` (it's out of scope of this PR)? Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9088: [dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#discussion_r375578403
##########
File path: superset/assets/src/dashboard/reducers/getInitialState.js
##########
@@ -280,10 +280,7 @@ export default function(bootstrapData) {
dashboardInfo: {
id: dashboard.id,
slug: dashboard.slug,
- metadata: {
- timed_refresh_immune_slices:
- dashboard.metadata.timed_refresh_immune_slices,
- },
+ metadata: dashboard.metadata,
Review comment:
@graceguo-supercat The fix here is to include all the data contained in the `json_metadata` blob, so any other fields you're concerned about will be included as long as they are in that object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9088: [dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#discussion_r375426049
##########
File path: superset/assets/src/dashboard/components/PropertiesModal.jsx
##########
@@ -49,19 +50,25 @@ const defaultProps = {
class PropertiesModal extends React.PureComponent {
constructor(props) {
super(props);
+ this.defaultMetadataValue = JSON.stringify(
Review comment:
This is always valid JSON, it's coming from a json object supplied by the backend.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda merged pull request #9088:
[dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
craig-rueda merged pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on issue #9088:
[dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#issuecomment-582671214
@suddjian Thanks for this fix. I also found a few dashboard metadata was missing and almost filed an issue.
Here is a list of metadata:
https://github.com/apache/incubator-superset/blob/315a11dfe265c6a20ed014610eb7351966292c38/superset/views/dashboard/api.py#L44
But there are another 2 fields not in the above list:
I can change the default **`color scheme`** and **`label_colors`**:
<img width="520" alt="Screen Shot 2020-02-05 at 3 49 58 PM" src="https://user-images.githubusercontent.com/27990562/73894137-5e8ab480-4830-11ea-99fa-7f3398dfa8ad.png">
<img width="592" alt="Screen Shot 2020-02-05 at 3 57 56 PM" src="https://user-images.githubusercontent.com/27990562/73894107-49ae2100-4830-11ea-909d-5747b5b7588e.png">
Not sure these 2 extra fields will be picked up in this PR. thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9088: [dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#discussion_r375426049
##########
File path: superset/assets/src/dashboard/components/PropertiesModal.jsx
##########
@@ -49,19 +50,25 @@ const defaultProps = {
class PropertiesModal extends React.PureComponent {
constructor(props) {
super(props);
+ this.defaultMetadataValue = JSON.stringify(
Review comment:
This is always valid JSON, it's coming from a json object supplied by the backend in the bootstrap data.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request
#9088: [dashboard] Fix metadata state
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9088: [dashboard] Fix metadata state
URL: https://github.com/apache/incubator-superset/pull/9088#discussion_r375407843
##########
File path: superset/assets/src/dashboard/components/PropertiesModal.jsx
##########
@@ -49,19 +50,25 @@ const defaultProps = {
class PropertiesModal extends React.PureComponent {
constructor(props) {
super(props);
+ this.defaultMetadataValue = JSON.stringify(
Review comment:
this will throw if the metadata is invalid, is that what we want here? Maybe we should wrap it in a try/catch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org