sadpandajoe commented on code in PR #45508:
URL: https://github.com/apache/superset/pull/45508#discussion_r4368562620
##########
superset/commands/report/update.py:
##########
@@ -131,7 +131,7 @@ def _normalize_recipients(recipients: Any) ->
list[tuple[str, str]]:
config = json.loads(config)
except json.JSONDecodeError:
pass
- if isinstance(config, dict):
+ if recipient_type == ReportRecipientType.EMAIL and isinstance(config,
dict):
Review Comment:
Limiting the empty `ccTarget`/`bccTarget` stripping to Email means a
Slack/SlackV2/Webhook recipient stored as `{"target": "C123"}` (for example
created through the API) now compares as changed when the modal re-saves it
with `ccTarget: ""` and `bccTarget: ""` (`AlertReportModal.tsx` still sends
them for every non-Teams method). A non-admin editor who only renames a
schedule that runs as another user then gets
`ReportScheduleRunAsContentForbiddenError`;
`test_empty_cc_and_bcc_change_non_email_recipients` now asserts that rejection.
Is that intended, or should the empty-value normalization stay for non-Teams
types and only exclude Teams?
--
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]
