You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "vitoldi (via GitHub)" <gi...@apache.org> on 2023/05/10 13:54:21 UTC

[GitHub] [superset] vitoldi opened a new pull request, #24005: feat: dashboard page xlsx export

vitoldi opened a new pull request, #24005:
URL: https://github.com/apache/superset/pull/24005

   ### SUMMARY
   The Superset 2.1.0 release has new feature: excel export. But the feature is only available on the Chart page. Our customers have no access to the Chart page, so the Dashboard page is a primary place where we wanted Excel Export option.
   
   This PR adds the ability to export to Excel on the Dashboard page.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Before:
   ![before](https://github.com/apache/superset/assets/82170648/ee42d03e-50f4-4a46-b61e-18bb9dac36ed)
   
   After:
   ![after](https://github.com/apache/superset/assets/82170648/97a629e6-cfd2-40fb-8fe3-74bf3e2ae22d)
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] rusackas commented on pull request #24005: feat: dashboard page xlsx export

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1553736973

   @vitoldi I think we can merge this, but now there's a conflict. Would you mind doing a quick rebase?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] sebastianliebscher commented on pull request #24005: feat: dashboard page xlsx export

Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1731363017

   This feature is released with version [3.0.0](https://github.com/apache/superset/blob/master/CHANGELOG.md#300-thu-aug-24-133627-2023--0600).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] EugeneTorap commented on pull request #24005: feat: dashboard page xlsx export

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1542431238

   Hello @vitoldi!
   Can you use `Excel` instead of `.XLSX` in `Export to .XLSX` ?
   Can you add #23711 issue in description and write: `Resolves #23711` in order to automatically close the issue.
   cc: @villebro @dpgaspar


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] vitoldi commented on pull request #24005: feat: dashboard page xlsx export

Posted by "vitoldi (via GitHub)" <gi...@apache.org>.
vitoldi commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1554168075

   @rusackas, I have resolved the conflict.
   
   ![Uploading 1 (1).png…]()
   
   ![2](https://github.com/apache/superset/assets/82170648/539dba80-14a1-4e83-8d95-c5d13dc3d084)
   
   ![1](https://github.com/apache/superset/assets/82170648/229ea3d7-d39b-4e98-9662-2db8dca2eab1)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] EugeneTorap commented on pull request #24005: feat: dashboard page xlsx export

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1543546412

   Hi @vitoldi, can you merge `master` branch -> your branch in order to fix CI tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] rusackas commented on a diff in pull request #24005: feat: dashboard page xlsx export

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #24005:
URL: https://github.com/apache/superset/pull/24005#discussion_r1194435890


##########
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx:
##########
@@ -324,8 +326,24 @@ class Chart extends React.Component {
     }
   };
 
+  exportFullCSV() {
+    this.exportCSV(true);
+  }
+
   exportCSV(isFullCSV = false) {
-    this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, {
+    this.exportTable('csv', isFullCSV);
+  }
+
+  exportXLSX() {
+    this.exportTable('xlsx', false);

Review Comment:
   I think this is totally fine for now, but just wondering if folks will also want to do a "Full Excel" export like we do with CSV (when that feature flag is enabled). I'm totally fine skipping it for now, since it might cause performance issues, but it just made me wonder ¯\\\_(ツ)_/¯ 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] EugeneTorap commented on pull request #24005: feat: dashboard page xlsx export

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1543757426

   @vitoldi, pls, fix the prettier error
   ```
   prettier.................................................................Failed
   - hook id: prettier
   - files were modified by this hook
   
   superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] vitoldi commented on pull request #24005: feat: dashboard page xlsx export

Posted by "vitoldi (via GitHub)" <gi...@apache.org>.
vitoldi commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1542478011

    > Hello @vitoldi! Can you use `Excel` instead of `.XLSX` in `Export to .XLSX` ? Can you add #23711 issue to the description and write: `Resolves #23711` in order to automatically close the issue. cc: @villebro @dpgaspar
   
   Hello @EugeneTorap, it's done.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] rusackas commented on pull request #24005: feat: dashboard page xlsx export

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1548717706

   > @villebro @dpgaspar @michael-s-molina Can you review this PR?
   
   Officially requested these reviewers.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] EugeneTorap commented on a diff in pull request #24005: feat: dashboard page xlsx export

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on code in PR #24005:
URL: https://github.com/apache/superset/pull/24005#discussion_r1191200470


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -170,6 +173,16 @@ test('Should "export to CSV"', async () => {
   expect(props.exportCSV).toBeCalledWith(371);
 });
 
+test('Should "export to XLSX"', async () => {
+  const props = createProps();
+  renderWrapper(props);
+  expect(props.exportXLSX).toBeCalledTimes(0);
+  userEvent.hover(screen.getByText('Download'));
+  userEvent.click(await screen.findByText('Export to .XLSX'));

Review Comment:
   @vitoldi pls, fix the unit test. Use `Excel` instead of `.XLSX`
   ```
   Error: Unable to find an element with the text: Export to .XLSX.
   This could be because the text is broken up by multiple elements.
   In this case, you can provide a function for your text matcher to make your matcher more flexible.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] EugeneTorap commented on pull request #24005: feat: dashboard page xlsx export

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1544293789

   @villebro @dpgaspar @michael-s-molina Can you review this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] vitoldi commented on a diff in pull request #24005: feat: dashboard page xlsx export

Posted by "vitoldi (via GitHub)" <gi...@apache.org>.
vitoldi commented on code in PR #24005:
URL: https://github.com/apache/superset/pull/24005#discussion_r1191240730


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -170,6 +173,16 @@ test('Should "export to CSV"', async () => {
   expect(props.exportCSV).toBeCalledWith(371);
 });
 
+test('Should "export to XLSX"', async () => {
+  const props = createProps();
+  renderWrapper(props);
+  expect(props.exportXLSX).toBeCalledTimes(0);
+  userEvent.hover(screen.getByText('Download'));
+  userEvent.click(await screen.findByText('Export to .XLSX'));

Review Comment:
   ![1](https://github.com/apache/superset/assets/82170648/83cf2e57-2a06-4e5f-a012-5754472e7f18)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] vitoldi commented on pull request #24005: feat: dashboard page xlsx export

Posted by "vitoldi (via GitHub)" <gi...@apache.org>.
vitoldi commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1543851026

   @EugeneTorap, prettier error fixed.
   
   ![1](https://github.com/apache/superset/assets/82170648/68bc3ac3-a371-4f5c-9393-42163201de7e)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] commented on pull request #24005: feat: dashboard page xlsx export

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24005:
URL: https://github.com/apache/superset/pull/24005#issuecomment-1544293222

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#24005](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1171e4a) into [master](https://app.codecov.io/gh/apache/superset/commit/0788b5fdcc69ecafc7dfed0ffd760d05ceb7a34b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0788b5f) will **increase** coverage by `11.15%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 1171e4a differs from pull request most recent head 9456f59. Consider uploading reports for the commit 9456f59 to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #24005       +/-   ##
   ===========================================
   + Coverage   56.98%   68.14%   +11.15%     
   ===========================================
     Files        1941     1941               
     Lines       75304    75312        +8     
     Branches     8166     8167        +1     
   ===========================================
   + Hits        42912    51320     +8408     
   + Misses      30303    21903     -8400     
     Partials     2089     2089               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | javascript | `54.56% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...end/src/dashboard/components/SliceHeader/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyL2luZGV4LnRzeA==) | `90.74% <100.00%> (+0.17%)` | :arrow_up: |
   | [...dashboard/components/SliceHeaderControls/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMvaW5kZXgudHN4) | `72.82% <100.00%> (+0.60%)` | :arrow_up: |
   | [.../src/dashboard/components/gridComponents/Chart.jsx](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `57.52% <100.00%> (+1.55%)` | :arrow_up: |
   | [superset-frontend/src/logger/LogUtils.ts](https://app.codecov.io/gh/apache/superset/pull/24005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2xvZ2dlci9Mb2dVdGlscy50cw==) | `97.22% <100.00%> (+0.07%)` | :arrow_up: |
   
   ... and [310 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/24005/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] rusackas commented on a diff in pull request #24005: feat: dashboard page xlsx export

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #24005:
URL: https://github.com/apache/superset/pull/24005#discussion_r1194437226


##########
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx:
##########
@@ -324,8 +326,24 @@ class Chart extends React.Component {
     }
   };
 
+  exportFullCSV() {
+    this.exportCSV(true);
+  }
+
   exportCSV(isFullCSV = false) {
-    this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, {
+    this.exportTable('csv', isFullCSV);
+  }
+
+  exportXLSX() {
+    this.exportTable('xlsx', false);

Review Comment:
   While I'm on the topic, I wonder if the logger should also discern between regular and "full" CSV exports, in case anyone is trying to discern performance impact. Maybe an _addidional_ log for full CSV exports? 
   
   Again, not the point of this PR, but mentioning the bycatch...



##########
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx:
##########
@@ -324,8 +326,24 @@ class Chart extends React.Component {
     }
   };
 
+  exportFullCSV() {
+    this.exportCSV(true);
+  }
+
   exportCSV(isFullCSV = false) {
-    this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, {
+    this.exportTable('csv', isFullCSV);
+  }
+
+  exportXLSX() {
+    this.exportTable('xlsx', false);

Review Comment:
   While I'm on the topic, I wonder if the logger should also discern between regular and "full" CSV exports, in case anyone is trying to discern performance impact. Maybe an _additional_ log for full CSV exports? 
   
   Again, not the point of this PR, but mentioning the bycatch...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] dpgaspar merged pull request #24005: feat: dashboard page xlsx export

Posted by "dpgaspar (via GitHub)" <gi...@apache.org>.
dpgaspar merged PR #24005:
URL: https://github.com/apache/superset/pull/24005


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org