fix: Missing applied filters indicator

[john-bodley]

SUMMARY

This PR fixes a bug which is exemplifies all that is wrong with Superset. Spaghetti code (exacerbated by the fact that the native and legacy filters use different form-data encodings) and half baked features—legacy charts vs. ECharts.

Specifically this issue—where the applied filter indicator is missing—seems to only surface for NVD3TimeSeriesViz charts with time comparisons using the non-native temporal filters. The non-native filters seem to encode which filters are applied using the applied_time_extras form-data field which is defined in the merge_extra_filters method via an extraction of the extra_filters field—which is then disposed of.

The problem lies when this method is called multiple times. This the case when Advanced Analytics are used which requires running extra queries and re-evaluating the query object thus re-invoking the merge_extra_filters method, however this time with the extra_filters field missing from the form-data. The result is the applied_time_extras is now empty which translates to the filter indicator not reflecting that the temporal field has been defined. The solution is merely to preserve (as opposed to overwrite) the applied_time_extras field.

Note this issue only impacted the applied filter indicator, i.e., the filters were correctly applied to the underlying chart.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

TESTING INSTRUCTIONS

Added unit tests and tested locally.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

[codecov]

Codecov Report

Merging #22137 (ba07e4f) into master (2f0d5f1) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #22137      +/-   ##
==========================================
- Coverage   66.99%   66.98%   -0.01%     
==========================================
  Files        1832     1832              
  Lines       69922    69921       -1     
  Branches     7571     7571              
==========================================
- Hits        46841    46840       -1     
  Misses      21122    21122              
  Partials     1959     1959              

Flag	Coverage Δ
hive	52.60% <50.00%> (-0.01%)	⬇️
mysql	78.15% <100.00%> (-0.01%)	⬇️
postgres	78.21% <100.00%> (-0.01%)	⬇️
presto	52.49% <50.00%> (-0.01%)	⬇️
python	81.38% <100.00%> (-0.01%)	⬇️
sqlite	76.67% <100.00%> (-0.01%)	⬇️
unit	50.85% <50.00%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
superset/utils/core.py	90.88% <100.00%> (-0.01%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ktmud]

I'm also having trouble understanding the spaghetti code, but the explanation makes sense to me.