fix(native-filters): Reset column field for removed dataset

[agatapst]

SUMMARY

After removing dataset, which is used in some native filter, it was still possible to see field from this dataset. Dataset was removed, but field value was not removed from Filter Config Modal.
Described in #12457

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
First we set some native filter

Then we remove used dataset and come back to native filter config

After:

TEST PLAN

Set "DASHBOARD_NATIVE_FILTERS": True
Create new native filter with given dataset
Go to datasets and remove this dataset
Come back to native filter you have just created and check if you see 'Select' placeholder instead of field value from removed dataset

ADDITIONAL INFORMATION

• Has associated issue: Fixes [dashboard]Native filter FIELD loops when dataset in usewas removed #12457
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

@junlincc @rusackas @adam-stasiak

[codecov-io]

Codecov Report

Merging #12519 (0e2d4dc) into master (0de61df) will decrease coverage by 3.63%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #12519      +/-   ##
==========================================
- Coverage   66.85%   63.22%   -3.64%     
==========================================
  Files        1018      486     -532     
  Lines       49776    29982   -19794     
  Branches     4869        0    -4869     
==========================================
- Hits        33280    18956   -14324     
+ Misses      16373    11026    -5347     
+ Partials      123        0     -123     

Flag	Coverage Δ
cypress	?
javascript	?
python	63.22% <ø> (-0.80%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/db_engines/hive.py	0.00% <0.00%> (-85.72%)	⬇️
superset/sql_validators/postgres.py	50.00% <0.00%> (-50.00%)	⬇️
superset/db_engine_specs/hive.py	54.61% <0.00%> (-29.24%)	⬇️
superset/databases/commands/create.py	83.67% <0.00%> (-8.17%)	⬇️
superset/databases/commands/update.py	85.71% <0.00%> (-8.17%)	⬇️
superset/connectors/sqla/models.py	84.31% <0.00%> (-6.28%)	⬇️
superset/db_engine_specs/sqlite.py	90.62% <0.00%> (-6.25%)	⬇️
superset/databases/commands/test_connection.py	84.78% <0.00%> (-4.35%)	⬇️
superset/utils/celery.py	96.42% <0.00%> (-3.58%)	⬇️
superset/models/core.py	85.59% <0.00%> (-3.27%)	⬇️
... and 538 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0de61df...0e2d4dc. Read the comment docs.

[villebro]

LGTM

[adam-stasiak]

First issue:
For dashboard I have two native filters for 2 different datasets
I remove one dataset and go back to native filters
I can see that for filter with removed dataset -> datasource and field are cleaned - OK!
I can see in the second native filter (without touched dataset) - I can se cleaned field but should not! - NOK!

[adam-stasiak]

second issue:
After steps from issue#1 I set proper field for malformed filter
Change is applied
Refreshed page
Open filter configuration and scoping -> I can see cleaned FIELD for both -> but I think it is only on UI -> because left side of screenshot is showing available options :)

[agatapst]

Thanks a lot Adam, I think these issues are connected with each other. I am working on them now!

[adam-stasiak]

Retest for your fixes -> it is ok 🟢

[junlincc]

I remove one dataset and go back to native filters

@adam-stasiak you are really the QA KING. thanks for taking care of the project while i was gone... 🙏👑

[junlincc]

Thank you so much for the PR! @agatapst

for some reasons, I'm not seeing changes described above. but a spinning sign for the filter with deleted dataset.. not sure if I tested correctly? @adam-stasiak
https://user-images.githubusercontent.com/67837651/104804152-c7d3b100-577d-11eb-859e-e65880270444.mov

[adam-stasiak]

@junlincc I saw this spinner in situation od removed dataset - > I consulted this with Agata and good idea is to raise new issue for this as it needs design for not available filters. I left this on Friday and forgot to raise : | I will create an issue for this.

[agatapst]

@junlincc yes, actually @adam-stasiak asked me about that spinner and at first I thought about making separate issue - but if this PR is still not merged, let's improve it in this PR.
Below I am attaching screenshot when there is something wrong with filter (i.e. dataset was removed). There is a text instead of spinner. Any suggestions about the copy are welcomed. 🙂

[junlincc]

this error msg definitely need design 🙏 @mihir174

thanks for the change! @agatapst

[mihir174]

@agatapst @adam-stasiak @junlincc

This might be a bit of a heavy treatment, but here's the standard error component in the Superset design system -
We can have the body text = the cause of the error

[agatapst]

Thanks @mihir174 🙂
Could we make this message more generic, universal? We could show it for all errors connected with a given filter in the Filter Bar. Right now it is impossible to distinguish if the error comes exactly from removing dataset. It may be one of the reasons that loading filter is not possible - but for example the reason may be that some used in this filter field was removed from dataset. So it won't be the same as 'dataset removed'.
Maybe we can just tell the user to 'check configuration'? In the Config Modal the user is "taken" to this filter and can see what is going on.

cc @junlincc

Update: I have implemented the design. The copy is as above (but of course it can be changed).

[adam-stasiak]

Can we make the spacing bigger? It looks very tight. @agatapst @junlincc @mihir174

[adam-stasiak]

Issue: After I fixed filter (Change dataset to existing one) I can see error remains :(

fil.mov

[agatapst]

Thanks a lot Adam! @adam-stasiak
Could you check it once again? It seems ok now on my side:

I have also made the margin slightly bigger:

[junlincc]

@rusackas @mistercrunch please help merge this PR, LGTM with code reviews and few rounds of changes...

[junlincc]

thank you everyone for the PR, testing, design input, reviewing and merging. @agatapst @mihir174 @adam-stasiak @zhaoyongjie @villebro 🙏

@mistercrunch Max, just want to use this PR as an example to show how much work the team put into each single PR before merging.