You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "shailendrasingh98 (via GitHub)" <gi...@apache.org> on 2023/02/23 10:36:00 UTC
[GitHub] [superset] shailendrasingh98 opened a new pull request, #23170: feat(alert/reports): Pdf as attachment for alert and report
shailendrasingh98 opened a new pull request, #23170:
URL: https://github.com/apache/superset/pull/23170
feat(alert/reports): Pdf as attachment for alert and report
### SUMMARY
The old alert system has an option to send chart / dashboard report as PNG in the email/slack. This is adding the pdf file as an attachment to the alert/report system.
### BEFORE SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
<img width="1213" alt="Screenshot 2023-02-23 at 3 48 37 PM" src="https://user-images.githubusercontent.com/28503344/220881333-9eb38a85-7357-4fe6-bfc8-3be8e93a349d.png">
### AFTER SCREENSHOTS OR ANIMATED GIF
<img width="1213" alt="Screenshot 2023-02-23 at 3 46 59 PM" src="https://user-images.githubusercontent.com/28503344/220881546-04a5162e-f74c-497b-a1ce-8281b7ccedaa.png">
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/issues/13177
- [ ] 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] mike-fischer1 commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "mike-fischer1 (via GitHub)" <gi...@apache.org>.
mike-fischer1 commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1534982137
is anyone going to review the pr and/or make changes?
--
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
Re: [PR] feat(alert/reports): Pdf as attachment for alert and report [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1749260468
We have a new PR for this in progress that will use vector-based pdfs instead of rasterized ones. I'll close this PR for now.
--
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] kasiazjc commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "kasiazjc (via GitHub)" <gi...@apache.org>.
kasiazjc commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1477507910
Hi @shailendrasingh98! Thanks so much for working on this feature. I have some small design suggestions - this modal currently seems really cluttered in general, so I think it would be great if you could improve the spacings and alignment of the radi buttons and check box to make it even.
Here is the figma file in which you can check the spacings (the modal looks slightly different, but the only thing relevant is the section that you've been working on) - https://www.figma.com/file/2C46hz9VgPMKnU8CORU3kY/Project-Template-(Copy)?node-id=4%3A4427&t=qBgrNpkNiMg79e02-1
Would you be able to make the changes? Thank you!
--
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
Re: [PR] feat(alert/reports): Pdf as attachment for alert and report [superset]
Posted by "galdropit (via GitHub)" <gi...@apache.org>.
galdropit commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-2102192807
> We have a new PR for this in progress that will use vector-based pdfs instead of rasterized ones. I'll close this PR for now.
Hi @eschutho, do you have an ETA on when PDF reports might be available? Thanks!
--
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 #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1558427968
@shailendrasingh98 I think there are a growing number of people that are eager to get their hands on what you've built. Is there any way we can help this along? It sounds like a rebase would be the first step, then hopefully we can help with tests and whatnot as well.
--
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
Re: [PR] feat(alert/reports): Pdf as attachment for alert and report [superset]
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1747866162
@shailendrasingh98 will you be able to continue this work?
--
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] shailendrasingh98 commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "shailendrasingh98 (via GitHub)" <gi...@apache.org>.
shailendrasingh98 commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1445795446
@srinisubramanian @betodealmeida @villebro I have raised this PR to have PDF report as in attachment for dashboard in alert and report section. Please review changes. i see there there is a lot of request for such feature.
I can also extend it to make downloadable from dashboard/charts
--
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] shailendrasingh98 commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "shailendrasingh98 (via GitHub)" <gi...@apache.org>.
shailendrasingh98 commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1498429355
@kasiazjc Thank you so much for feedback. I certainly can look into and make changes as needed. Currently i am just struggling to get integration tests run.
--
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] mike-fischer1 commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "mike-fischer1 (via GitHub)" <gi...@apache.org>.
mike-fischer1 commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1729799755
any updates?
--
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 #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #23170:
URL: https://github.com/apache/superset/pull/23170#discussion_r1118476589
##########
superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx:
##########
@@ -1451,6 +1449,23 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
</StyledCheckbox>
</div>
)}
+ {dashboardFormatOptionEnabled && (
+ <>
+ <div className="inline-container">
+ <StyledRadioGroup
+ onChange={onFormatChange}
+ value={reportFormat}
+ >
+ <StyledRadio value="PNG">
+ {TRANSLATIONS.SEND_AS_PNG_TEXT}
+ </StyledRadio>
+ <StyledRadio value="PDF">
+ {TRANSLATIONS.SEND_AS_PDF_TEXT}
+ </StyledRadio>
+ </StyledRadioGroup>
+ </div>
+ </>
+ )}
Review Comment:
This is duplicating the chart specific formatting logic on 1418-1439 (=increasing code maintenance debt). I think we should consolidate these two.
##########
superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx:
##########
@@ -490,6 +491,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
contentType === 'chart' &&
(isFeatureEnabled(FeatureFlag.ALERTS_ATTACH_REPORTS) || isReport);
+ const dashboardFormatOptionEnabled =
+ contentType === 'dashboard' &&
+ (isFeatureEnabled(FeatureFlag.ALERTS_ATTACH_REPORTS) || isReport);
+
Review Comment:
If we really want to introduce a new variable `dashboardFormatOptionEnabled` we really should rename the old one on line 490 to `chartFormatOptionEnabled`.
##########
superset/reports/notifications/base.py:
##########
@@ -28,7 +28,8 @@
class NotificationContent:
name: str
header_data: HeaderDataType # this is optional to account for error states
- csv: Optional[bytes] = None # bytes for csv file
+ data: Optional[bytes] = None # bytes for data attachment
+ data_format: Optional[str] = None # data attachment format (csv, xlsx, pdf, etc)
Review Comment:
I think we should use an `Enum` or `Literal` as the type here to make it detectable by the linter if a `data_format` is valid.
--
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] shailendrasingh98 commented on a diff in pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "shailendrasingh98 (via GitHub)" <gi...@apache.org>.
shailendrasingh98 commented on code in PR #23170:
URL: https://github.com/apache/superset/pull/23170#discussion_r1119079080
##########
superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx:
##########
@@ -490,6 +491,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
contentType === 'chart' &&
(isFeatureEnabled(FeatureFlag.ALERTS_ATTACH_REPORTS) || isReport);
+ const dashboardFormatOptionEnabled =
+ contentType === 'dashboard' &&
+ (isFeatureEnabled(FeatureFlag.ALERTS_ATTACH_REPORTS) || isReport);
+
Review Comment:
Thank you! updated code.
--
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 pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1507960805
@shailendrasingh98 can you rebase the PR to resolve the conflicts? After that I can re-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] codecov[bot] commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1446946605
# [Codecov](https://codecov.io/gh/apache/superset/pull/23170?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 [#23170](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd5ff8c) into [master](https://codecov.io/gh/apache/superset/commit/f4ffed24babc8923cc812bc7bf3cb07ffd7e367a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4ffed2) will **decrease** coverage by `1.73%`.
> The diff coverage is `49.88%`.
> :exclamation: Current head bd5ff8c differs from pull request most recent head dfe55e6. Consider uploading reports for the commit dfe55e6 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23170 +/- ##
==========================================
- Coverage 67.54% 65.82% -1.73%
==========================================
Files 1881 1906 +25
Lines 72392 73392 +1000
Branches 7882 7959 +77
==========================================
- Hits 48898 48307 -591
- Misses 21472 23045 +1573
- Partials 2022 2040 +18
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.73% <29.24%> (-0.15%)` | :arrow_down: |
| presto | `?` | |
| unit | `?` | |
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://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../packages/superset-ui-core/src/chart/types/Base.ts](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY2hhcnQvdHlwZXMvQmFzZS50cw==) | `100.00% <ø> (ø)` | |
| [...ackages/superset-ui-core/src/utils/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdXRpbHMvZmVhdHVyZUZsYWdzLnRz) | `100.00% <ø> (ø)` | |
| [...gins/legacy-plugin-chart-world-map/src/WorldMap.js](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvV29ybGRNYXAuanM=) | `0.00% <0.00%> (ø)` | |
| [...plugins/legacy-plugin-chart-world-map/src/index.js](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvaW5kZXguanM=) | `66.66% <ø> (ø)` | |
| [...egacy-plugin-chart-world-map/src/transformProps.js](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvdHJhbnNmb3JtUHJvcHMuanM=) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/BigNumber/BigNumberViz.tsx](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclZpei50c3g=) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQm94UGxvdC9FY2hhcnRzQm94UGxvdC50c3g=) | `0.00% <0.00%> (ø)` | |
| [.../plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvRnVubmVsL0VjaGFydHNGdW5uZWwudHN4) | `0.00% <0.00%> (ø)` | |
| [...ns/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR2F1Z2UvRWNoYXJ0c0dhdWdlLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...ns/plugin-chart-echarts/src/Graph/EchartsGraph.tsx](https://codecov.io/gh/apache/superset/pull/23170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR3JhcGgvRWNoYXJ0c0dyYXBoLnRzeA==) | `0.00% <0.00%> (ø)` | |
| ... and [263 more](https://codecov.io/gh/apache/superset/pull/23170?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] shailendrasingh98 commented on a diff in pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "shailendrasingh98 (via GitHub)" <gi...@apache.org>.
shailendrasingh98 commented on code in PR #23170:
URL: https://github.com/apache/superset/pull/23170#discussion_r1119080409
##########
superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx:
##########
@@ -1451,6 +1449,23 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
</StyledCheckbox>
</div>
)}
+ {dashboardFormatOptionEnabled && (
+ <>
+ <div className="inline-container">
+ <StyledRadioGroup
+ onChange={onFormatChange}
+ value={reportFormat}
+ >
+ <StyledRadio value="PNG">
+ {TRANSLATIONS.SEND_AS_PNG_TEXT}
+ </StyledRadio>
+ <StyledRadio value="PDF">
+ {TRANSLATIONS.SEND_AS_PDF_TEXT}
+ </StyledRadio>
+ </StyledRadioGroup>
+ </div>
+ </>
+ )}
Review Comment:
Refactored code as suggested. Thanks!
--
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] mike-fischer1 commented on pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "mike-fischer1 (via GitHub)" <gi...@apache.org>.
mike-fischer1 commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1549167659
is anyone going to review the pr and/or make changes?
--
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 #23170: feat(alert/reports): Pdf as attachment for alert and report
Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1522947042
Hi @shailendrasingh98! Any news?
--
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
Re: [PR] feat(alert/reports): Pdf as attachment for alert and report [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho closed pull request #23170: feat(alert/reports): Pdf as attachment for alert and report
URL: https://github.com/apache/superset/pull/23170
--
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
Re: [PR] feat(alert/reports): Pdf as attachment for alert and report [superset]
Posted by "jcoelho-pt (via GitHub)" <gi...@apache.org>.
jcoelho-pt commented on PR #23170:
URL: https://github.com/apache/superset/pull/23170#issuecomment-1787504264
@eschutho the PR you mentioned is the [#25696](https://github.com/apache/superset/pull/25696 )?
--
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