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(csv-export): pivot v2 with verbose names #18633

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

villebro
Copy link
Member

@villebro villebro commented Feb 9, 2022

SUMMARY

When exporting a pivoted CSV for the Pivot v2 chart with columns or metrics that have a verbose name, the export fails. This adds the option of passing the verbose_map from the datasource to the get_column_name and get_metric_name util functions + adds some missing tests. Note that remapping the column names is usually not needed, hence mapping is optional.

TESTING INSTRUCTIONS

  1. Create a Pivot v2 using the FCC 2018 Survey example dataset
  2. Set the metric as the saved metric "COUNT(*)"
  3. Set the columns as "Job location preference"
  4. Set the rows as "Willing to relocate"
  5. Export a Pivoted CSV and observe a 500

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

@villebro villebro force-pushed the villebro/fix-pivoted-csv branch from 15a20d0 to e53598b Compare February 9, 2022 08:23
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #18633 (9b3bfe5) into master (28e729b) will increase coverage by 0.00%.
The diff coverage is 86.76%.

❗ Current head 9b3bfe5 differs from pull request most recent head 51cec08. Consider uploading reports for the commit 51cec08 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #18633   +/-   ##
=======================================
  Coverage   66.31%   66.32%           
=======================================
  Files        1595     1595           
  Lines       62599    62603    +4     
  Branches     6297     6297           
=======================================
+ Hits        41513    41519    +6     
+ Misses      19440    19438    -2     
  Partials     1646     1646           
Flag Coverage Δ
hive 52.14% <67.85%> (-0.01%) ⬇️
mysql 81.27% <78.57%> (+<0.01%) ⬆️
postgres 81.32% <78.57%> (+<0.01%) ⬆️
presto 51.98% <67.85%> (-0.01%) ⬇️
python 81.75% <78.57%> (+<0.01%) ⬆️
sqlite 81.02% <78.57%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...t-frontend/src/explore/reducers/getInitialState.ts 0.00% <ø> (ø)
superset-frontend/src/utils/localStorageHelpers.ts 90.00% <ø> (ø)
superset/viz.py 58.13% <33.33%> (ø)
superset/views/core.py 77.59% <50.00%> (ø)
superset/charts/post_processing.py 67.17% <57.14%> (-1.05%) ⬇️
...nd/src/explore/components/DataTablesPane/index.tsx 74.44% <86.66%> (ø)
.../src/explore/components/DataTableControl/index.tsx 97.36% <93.75%> (ø)
...set-frontend/src/explore/actions/exploreActions.ts 47.36% <100.00%> (ø)
...et-frontend/src/explore/reducers/exploreReducer.js 33.33% <100.00%> (ø)
superset-frontend/src/utils/common.js 88.67% <100.00%> (ø)
... and 11 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 28e729b...51cec08. Read the comment docs.

Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for the fix!

@villebro villebro merged commit fdbcbb5 into apache:master Feb 9, 2022
@villebro villebro deleted the villebro/fix-pivoted-csv branch February 9, 2022 12:15
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.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 preset-io size/L 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants