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/06/05 09:09:01 UTC
[GitHub] [superset] vitoldi opened a new pull request, #24287: added dashboard page full xlsx export
vitoldi opened a new pull request, #24287:
URL: https://github.com/apache/superset/pull/24287
### SUMMARY
This is a continuation of the MR https://github.com/apache/superset/pull/24005 problem.
This PR adds the ability of full Excel export on the Dashboard page.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: **ALLOW_FULL_CSV_EXPORT**
- [x] 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] vitoldi commented on pull request #24287: feat: add dashboard page full xlsx export
Posted by "vitoldi (via GitHub)" <gi...@apache.org>.
vitoldi commented on PR #24287:
URL: https://github.com/apache/superset/pull/24287#issuecomment-1592925243
> LGTM, thanks for the great work here!
Thanks for the review
--
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] villebro merged pull request #24287: feat: add dashboard page full xlsx export
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #24287:
URL: https://github.com/apache/superset/pull/24287
--
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] villebro commented on a diff in pull request #24287: feat: add dashboard page full xlsx export
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #24287:
URL: https://github.com/apache/superset/pull/24287#discussion_r1230872784
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -226,6 +227,43 @@ test('Should not show export full CSV if report is not table', async () => {
expect(screen.queryByText('Export to full .CSV')).not.toBeInTheDocument();
});
+test('Export full Excel is under featureflag', async () => {
+ // @ts-ignore
+ global.featureFlags = {
+ [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: false,
+ };
+ const props = createProps('table');
+ renderWrapper(props);
+ userEvent.hover(screen.getByText('Download'));
+ expect(await screen.findByText('Export to Excel')).toBeInTheDocument();
+ expect(screen.queryByText('Export to full Excel')).not.toBeInTheDocument();
+});
+
+test('Should "export full Excel"', async () => {
+ // @ts-ignore
+ global.featureFlags = {
+ [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true,
Review Comment:
General note (not complaining to the author of this fine PR!): I wish we would not have used `CSV` all over the codebase. In the future, let's be more generic, like `ALLOW_FULL_FILE_EXPORT` or similar.
--
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 #24287: feat: add dashboard page full xlsx export
Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #24287:
URL: https://github.com/apache/superset/pull/24287#issuecomment-1592851676
Hey @villebro @dpgaspar! Can you review the 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] codecov[bot] commented on pull request #24287: feat: add dashboard page full xlsx export
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24287:
URL: https://github.com/apache/superset/pull/24287#issuecomment-1576521398
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#24287](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6a9d2ac) into [master](https://app.codecov.io/gh/apache/superset/commit/18d2257a47b0039fc3414298f887e74778acd814?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (18d2257) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 6a9d2ac differs from pull request most recent head ecd37f1. Consider uploading reports for the commit ecd37f1 to get more accurate results
```diff
@@ Coverage Diff @@
## master #24287 +/- ##
=======================================
Coverage 57.24% 57.25%
=======================================
Files 1957 1957
Lines 75624 75628 +4
Branches 8223 8223
=======================================
+ Hits 43294 43298 +4
Misses 30222 30222
Partials 2108 2108
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `54.73% <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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...end/src/dashboard/components/SliceHeader/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyL2luZGV4LnRzeA==) | `90.74% <ø> (ø)` | |
| [...dashboard/components/SliceHeaderControls/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMvaW5kZXgudHN4) | `70.40% <100.00%> (+0.61%)` | :arrow_up: |
| [.../src/dashboard/components/gridComponents/Chart.jsx](https://app.codecov.io/gh/apache/superset/pull/24287?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `58.26% <100.00%> (+0.73%)` | :arrow_up: |
: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=apache)
--
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