Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(native-filters): Fix encoding of non-temporal default values #23779

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

john-bodley
Copy link
Member

SUMMARY

Whilst working on #23269 I misinterpreted the encoding of the dashboard level default values for the non-temporal filters, i.e., in the following dashboards.json_metadata,

{
  "default_filters": "{\"1\": {\"baz\": [\"foo\"]}, \"2\": {\"bar\": \"foo\"}",
  ...
}

only filters which allow multiple selections (baz) are encoded as a list whereas non-multiple selections are encoded as a scalar (bar), whereas I had initially thought they were all encoded as a list.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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

codecov bot commented Apr 21, 2023

Codecov Report

Merging #23779 (028b31f) into master (b92ace4) will not change coverage.
The diff coverage is 0.00%.

❗ Current head 028b31f differs from pull request most recent head 3a36fb7. Consider uploading reports for the commit 3a36fb7 to get more accurate results

@@           Coverage Diff           @@
##           master   #23779   +/-   ##
=======================================
  Coverage   67.87%   67.87%           
=======================================
  Files        1925     1925           
  Lines       74389    74389           
  Branches     8108     8108           
=======================================
  Hits        50494    50494           
  Misses      21818    21818           
  Partials     2077     2077           
Flag Coverage Δ
hive 52.92% <0.00%> (ø)
mysql 78.74% <0.00%> (ø)
postgres 78.82% <0.00%> (ø)
presto 52.84% <0.00%> (ø)
python 82.65% <0.00%> (ø)
sqlite 77.31% <0.00%> (ø)
unit 52.72% <0.00%> (ø)

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

Impacted Files Coverage Δ
...uperset/utils/dashboard_filter_scopes_converter.py 15.78% <0.00%> (ø)

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

@john-bodley john-bodley marked this pull request as ready for review April 21, 2023 22:50
Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@john-bodley john-bodley merged commit 4b3e507 into master Apr 21, 2023
john-bodley added a commit to airbnb/superset-fork that referenced this pull request Apr 21, 2023
@john-bodley john-bodley deleted the john-bodley--fix-native-filter-migration branch April 21, 2023 23:57
Always-prog added a commit to TechAuditBI/superset that referenced this pull request Apr 24, 2023
john-bodley added a commit to airbnb/superset-fork that referenced this pull request Apr 26, 2023
sebastianliebscher pushed a commit to sebastianliebscher/superset that referenced this pull request Apr 28, 2023
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants