[GitHub] [incubator-superset] john-bodley opened a new pull request #8523: [fix] Re-cleanup legacy filters

[GitBox <gi...@apache.org>]

john-bodley opened a new pull request #8523: [fix] Re-cleanup legacy filters
URL: https://github.com/apache/incubator-superset/pull/8523
 
 
   ### CATEGORY
   
   Choose one
   
   - [x] Bug Fix
   - [ ] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   
   Whilst spelunking through our production data in preparation to migrate slices from the Druid native JSON-based API to SQLAlchemy I discovered that a non-trivial amount of slices (~ 10%) still contained the legacy filter fields in the form-data; `filters`, `having`, `having_filters`, and `where` in addition to `adhoc_filters` (which takes precedence). Though these fields are not problematic it adds cruft to the form-data and makes analyses unnecessarily more complex.    
   
   These fields should have been removed in [this](https://github.com/apache/incubator-superset/blob/9fc37ea9f1ff032dea923012613d4e7189ada178/superset/migrations/versions/bddc498dd179_adhoc_filters.py) migration (the `convert_legacy_filters_into_adhoc` method deletes the old fields) and after trying several flows I wasn't able to determine how these fields were persisting (note loading/saving a slice with legacy filters will replace them with ad-hoc). 
   
   This PR performs the following: 
   
   1. Re-applies the previous migration logic. The hypothesis is that maybe when the previous migration was run the code was in a state where these fields persisted. 
   2. Updates the example data to remove the obsolete fields and updates the filters to be ad-hoc.  
   
   ### TEST PLAN
   
   Ran the DB upgrade on a dump of our production data and verified that the legacy filters were no longer present in the form-data.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [x] Requires DB Migration.
   - [x] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ### REVIEWERS
   
   to: @betodealmeida  @graceguo-supercat @michellethomas @mistercrunch @villebro 

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