bxsx commented on code in PR #42494:
URL: https://github.com/apache/superset/pull/42494#discussion_r3662001545
##########
superset/reports/notifications/slack_mixin.py:
##########
@@ -31,18 +31,30 @@ def _message_template(
content: NotificationContent,
table: str = "",
) -> str:
- return __(
- """*%(name)s*
+ if content.include_cta:
+ return __(
+ """*%(name)s*
%(description)s
<%(url)s|Explore in Superset>
+%(table)s
+""",
Review Comment:
That asymmetry is pre-existing behavior, deliberately preserved: the Slack
error template has never contained a CTA link (unlike the email error template
— see _error_template in both files on master). The contract of this toggle is
"unchecked ⇒ no notification from this schedule contains the link"; the checked
state preserves the status quo of every template. Adding a CTA to Slack error
notifications would be a behavior change unrelated to this feature — happy to
explore that in a follow-up if there's appetite.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
