You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sfirke (via GitHub)" <gi...@apache.org> on 2023/10/05 19:20:53 UTC
[PR] chore: replace "dashboard" -> "report" in chart email report modal [superset]
sfirke opened a new pull request, #25540:
URL: https://github.com/apache/superset/pull/25540
### SUMMARY
When setting up a new email report for a chart, the popup incorrect says "dashboard" and "screenshot."
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
In Superset 3.0.0, click the `...` on a chart, then Manage email report, then Set up an email report. You'll see:
a) This shouldn't say screenshot because the user can pick a text or .CSV format choice
b) This should say chart instead of dashboard
### TESTING INSTRUCTIONS
N/A, just changing a line of text.
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-2015529390
Ugh I am bad at rebasing! 馃槚
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-1749525979
Also I see that the string I modified `A screenshot of the dashboard will be sent to your email at` appears in ten translation files when I search the code base. Should I make the corresponding modification in each of those 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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-1749524288
I'm confused, what does this failed check say I should do?
> Error: 337:14 error Replace `鈴幝仿仿仿仿仿仿仿仿仿仿仿穥t('The路report路will路be路sent路to路your路email路at')}鈴幝仿仿仿仿仿仿仿仿仿穈 with `{t('The路report路will路be路sent路to路your路email路at')}` prettier/prettier
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-2015543152
reopening at the awful state so I can force push and kind of fix this
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke merged PR #25540:
URL: https://github.com/apache/superset/pull/25540
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-2015612633
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/25540?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 69.09%. Comparing base [(`0c40bea`)](https://app.codecov.io/gh/apache/superset/commit/0c40bea0643ec2b0cc725ec646c9fa97319d9565?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e4dcb53`)](https://app.codecov.io/gh/apache/superset/pull/25540?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 838 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #25540 +/- ##
==========================================
+ Coverage 68.51% 69.09% +0.57%
==========================================
Files 1915 1919 +4
Lines 75100 74969 -131
Branches 8314 8349 +35
==========================================
+ Hits 51455 51800 +345
+ Misses 21503 21118 -385
+ Partials 2142 2051 -91
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/25540/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage 螖 | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/25540/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.40% <酶> (+1.08%)` | :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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/25540?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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
Re: [PR] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-2015740612
> I suppose my only little question is this: does a user reliably know that a Report is a screenshot of a dashboard?
Ah, I sent this PR because I was setting up a Report for a single chart and didn't like that it said "dashboard." I see now that when I set up a report for a dashboard, it uses this same modal, and in that case "dashboard" and "screenshot" are correct.
Maybe that's where you're coming from? A report is not always a screenshot of a dashboard, it can be a screenshot or .csv of a single chart. Thus this fix which makes it agnostic as to which is being configured.
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke closed pull request #25540: chore: replace "dashboard" -> "report" in chart email report modal
URL: https://github.com/apache/superset/pull/25540
--
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] chore: replace "dashboard" -> "report" in chart email report modal [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25540:
URL: https://github.com/apache/superset/pull/25540#issuecomment-2015672677
@rusackas I rebased this one and it's passing CI. It makes the translations slightly out of date, I don't know how to deal with that...
--
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