feat(explore): export csv data pivoted for Pivot Table [ID-9] #17512

kgabryje · 2021-11-22T18:51:09Z

SUMMARY

Apply pivot table post processing to data downloaded as CSV in Pivot Table V2 chart. The CSV data will have pivoted format (see the difference on screenshots).

When on Pivot Table chart, the .CSV button is replaced by a dropdown containing 2 options - Original and Pivoted. Clicking the latter triggers download of pivoted data.
On all other charts, the CSV button works just like before.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Example pivot table:

Before:

After:

TESTING INSTRUCTIONS

Create a pivot table chart, click on download as csv icon. A dropdown with 2 options should open - Original and Pivoted.
Clicking Original should download a CSV file formatted like the one on the "Before" screenshot.
Clicking Pivoted should download a CSV file formatted like the one on the "After" screenshot

The CSV button on any other chart should work like before - begin download immediately instead of opening a dropdown menu.

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

CC @junlincc

codecov · 2021-11-22T19:11:30Z

Codecov Report

Merging #17512 (334210b) into master (e6212b1) will increase coverage by 0.00%.
The diff coverage is 94.28%.

@@           Coverage Diff            @@
##           master   #17512    +/-   ##
========================================
  Coverage   68.57%   68.57%            
========================================
  Files        1588     1596     +8     
  Lines       64947    65214   +267     
  Branches     6963     6949    -14     
========================================
+ Hits        44537    44721   +184     
- Misses      18520    18610    +90     
+ Partials     1890     1883     -7

Flag	Coverage Δ
hive	`?`
javascript	`57.42% <93.10%> (+0.35%)`	⬆️
mysql	`81.80% <100.00%> (?)`
postgres	`81.81% <100.00%> (-0.23%)`	⬇️
presto	`?`
python	`81.90% <100.00%> (-0.60%)`	⬇️
sqlite	`81.50% <100.00%> (-0.23%)`	⬇️

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

Impacted Files	Coverage Δ
...es/superset-ui-core/src/query/buildQueryContext.ts	`100.00% <ø> (ø)`
...packages/superset-ui-core/src/query/types/Query.ts	`100.00% <ø> (ø)`
superset/common/query_context_factory.py	`96.42% <ø> (ø)`
...c/explore/components/ExportToCSVDropdown/index.tsx	`90.00% <90.00%> (ø)`
...nd/src/explore/components/ExploreActionButtons.tsx	`74.00% <100.00%> (+3.54%)`	⬆️
superset/charts/data/api.py	`88.15% <100.00%> (+0.07%)`	⬆️
superset/charts/schemas.py	`99.33% <100.00%> (+<0.01%)`	⬆️
superset/common/query_context.py	`92.72% <100.00%> (+0.27%)`	⬆️
superset/db_engines/hive.py	`0.00% <0.00%> (-85.19%)`	⬇️
superset/db_engine_specs/hive.py	`70.27% <0.00%> (-16.99%)`	⬇️
... and 61 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 e6212b1...334210b. Read the comment docs.

kgabryje · 2021-12-01T20:46:29Z

/testenv up

github-actions · 2021-12-01T20:48:24Z

@kgabryje Ephemeral environment spinning up at http://52.39.232.35:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro

A few small comments, but this works and looks great! It would be nice if we could add a few tests:

Ensure that a query context with form_data validates successfully
Add tests for ExportToCSVDropdown that ensures it renders correctly with standard/pivoted options + the children.

superset-frontend/src/explore/components/ExploreActionButtons.tsx

villebro · 2021-12-02T09:28:12Z

superset/charts/data/api.py

@@ -221,6 +221,7 @@ def data(self) -> Response:
        if json_body is None:
            return self.response_400(message=_("Request is not JSON"))

+        form_data = json_body.pop("form_data", None)


Do we need to pop here? Can't we just do

Suggested change

form_data = json_body.pop("form_data", None)

form_data = json_body.get("form_data")

We don't need form_data in json_body. We pass it as a separate argument

kgabryje · 2021-12-02T10:45:33Z

A few small comments, but this works and looks great! It would be nice if we could add a few tests:

Ensure that a query context with form_data validates successfully

Add tests for ExportToCSVDropdown that ensures it renders correctly with standard/pivoted options + the children.

Added tests for ExportToCSVDropdown. I didn't add tests for query context validation, as we pop form_data from json_body before it's used to create context, so there's no change there.

kgabryje · 2021-12-02T16:44:36Z

A few small comments, but this works and looks great! It would be nice if we could add a few tests:

Ensure that a query context with form_data validates successfully

Add tests for ExportToCSVDropdown that ensures it renders correctly with standard/pivoted options + the children.

Added tests for ExportToCSVDropdown. I didn't add tests for query context validation, as we pop form_data from json_body before it's used to create context, so there's no change there.

After some discussion, I implemented Ville's suggestions. We already had schema validations tests in place, so I just added form_data to query context generator.

villebro

A few last comments

villebro · 2021-12-02T17:00:55Z

superset-frontend/packages/superset-ui-core/src/query/types/Query.ts

@@ -158,6 +163,7 @@ export interface QueryContext {
  /** Response format */
  result_format: string;
  queries: QueryObject[];
+  form_data: QueryFormData;


Should this be optional, as it is in the python code?

form_data?: QueryFormData;

villebro · 2021-12-02T17:07:41Z

superset/charts/schemas.py

@@ -1157,6 +1157,8 @@ class ChartDataQueryContextSchema(Schema):
    result_type = EnumField(ChartDataResultType, by_value=True)
    result_format = EnumField(ChartDataResultFormat, by_value=True)

+    form_data = fields.Raw()


I think we can add allow_none=True and required=False here, as we probably want to support explicitly passing a null value

tests/common/query_context_generator.py

villebro

LGTM, thanks for this great improvement!

github-actions · 2021-12-03T11:42:54Z

Ephemeral environment shutdown and build artifacts deleted.

kgabryje requested review from eschutho and villebro November 22, 2021 18:51

pull-request-size bot added the size/S label Nov 22, 2021

kgabryje added 3 commits December 1, 2021 20:20

feat(explore): export csv data pivoted for Pivot Table

b824546

Implement dropdown with download csv options

730bf6e

Change label to "Original"

3c413d2

kgabryje force-pushed the feat/export-csv-pivoted branch from 1515eb5 to 3c413d2 Compare December 1, 2021 19:21

pull-request-size bot added size/L and removed size/S labels Dec 1, 2021

kgabryje requested review from michael-s-molina and geido December 1, 2021 19:26

kgabryje changed the title ~~[WIP] feat(explore): export csv data pivoted for Pivot Table [ID-9]~~ feat(explore): export csv data pivoted for Pivot Table [ID-9] Dec 1, 2021

villebro reviewed Dec 2, 2021

View reviewed changes

Add tests

b99058a

Add form data to query context

9668fd7

villebro reviewed Dec 3, 2021

View reviewed changes

Add form data to query context generator

e924b14

kgabryje force-pushed the feat/export-csv-pivoted branch from 1353297 to e924b14 Compare December 3, 2021 10:27

Explicitly make form_data optional

334210b

villebro approved these changes Dec 3, 2021

View reviewed changes

kgabryje merged commit 07e8837 into apache:master Dec 3, 2021

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(explore): export csv data pivoted for Pivot Table [ID-9] #17512

feat(explore): export csv data pivoted for Pivot Table [ID-9] #17512

kgabryje commented Nov 22, 2021 •

edited

Loading

codecov bot commented Nov 22, 2021 •

edited

Loading

kgabryje commented Dec 1, 2021

github-actions bot commented Dec 1, 2021

villebro left a comment

villebro Dec 2, 2021

kgabryje Dec 2, 2021

kgabryje commented Dec 2, 2021

kgabryje commented Dec 2, 2021 •

edited

Loading

villebro left a comment

villebro Dec 2, 2021

kgabryje Dec 3, 2021

villebro Dec 2, 2021

kgabryje Dec 3, 2021

villebro left a comment

github-actions bot commented Dec 3, 2021

	form_data = json_body.pop("form_data", None)
	form_data = json_body.get("form_data")

feat(explore): export csv data pivoted for Pivot Table [ID-9] #17512

feat(explore): export csv data pivoted for Pivot Table [ID-9] #17512

Conversation

kgabryje commented Nov 22, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Nov 22, 2021 • edited Loading

Codecov Report

kgabryje commented Dec 1, 2021

github-actions bot commented Dec 1, 2021

villebro left a comment

Choose a reason for hiding this comment

villebro Dec 2, 2021

Choose a reason for hiding this comment

kgabryje Dec 2, 2021

Choose a reason for hiding this comment

kgabryje commented Dec 2, 2021

kgabryje commented Dec 2, 2021 • edited Loading

villebro left a comment

Choose a reason for hiding this comment

villebro Dec 2, 2021

Choose a reason for hiding this comment

kgabryje Dec 3, 2021

Choose a reason for hiding this comment

villebro Dec 2, 2021

Choose a reason for hiding this comment

kgabryje Dec 3, 2021

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 3, 2021

kgabryje commented Nov 22, 2021 •

edited

Loading

codecov bot commented Nov 22, 2021 •

edited

Loading

kgabryje commented Dec 2, 2021 •

edited

Loading