serenajiang opened a new pull request #17753:
URL: https://github.com/apache/superset/pull/17753


   <!---
   Please write the PR title following the conventions at 
https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   
   The bug: When a user saved a dashboard that contained a chart with non-ascii 
characters, the json_metadata became malformed, causing the dashboard to fail 
to load. The root cause is in #17570, which added `positions` into the 
json_metadata blob. Something went wrong in the json parsing which causes 
json_metadata to be truncated if the positions json contains emojis or other 
non-standard characters.
   
   The root cause PR is pretty large, so we had trouble figuring out where this 
goes wrong. I suspect the fix forward is some combination of
   * Fix json parsing to handle non-ascii characters correctly
   * Validate that json_metadata is well-formed before writing it
   * Don't write the `positions` json, since it's already contained by a 
different column
   
   Opening this revert PR as a starting point. This PR reverts #17570 and the 
accompanying #17707.
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   to reproduce bug:
   * make dashboard
   * add chart with emoji in title
   * save dashboard
   * see error after save
   * refresh and see error on load
   
   to verify this fix:
   * make dashboard
   * add chart with emoji in title
   * save dashboard
   * verify dashboard saved
   * refresh and view dashboard
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   
   @geido @etr2460 @graceguo-supercat @rusackas 
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to