korbit-ai[bot] commented on code in PR #32336:
URL: https://github.com/apache/superset/pull/32336#discussion_r1976102049
##########
superset/reports/api.py:
##########
@@ -574,7 +575,7 @@ def slack_channels(self, **kwargs: Any) -> Response:
"""
try:
params = kwargs.get("rison", {})
- search_string = params.get("search_string")
+ search_string = get_recipients_list(params.get("search_string"))
Review Comment:
### Invalid Search String Processing <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The get_recipients_list function is being misused in the slack_channels
endpoint. This function is designed to parse recipient lists but is being used
to process a search string, which could lead to incorrect channel searching.
###### Why this matters
Using a recipient list parser for search string processing could split or
modify the search term unexpectedly, potentially causing the Slack channel
search to fail or return incorrect results.
###### Suggested change ∙ *Feature Preview*
Return to using the original search string directly:
```python
search_string = params.get("search_string")
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ec87b51-24cb-48ac-b55d-1557de7e5112?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:e2435be5-4116-4e0d-acf4-e2239b3a8496 -->
##########
superset/commands/report/execute.py:
##########
@@ -139,15 +139,21 @@ def update_report_schedule_slack_v2(self) -> None:
slack_recipients =
json.loads(recipient.recipient_config_json)
# we need to ensure that existing reports can also fetch
# ids from private channels
+ channels_list =
get_recipients_list(slack_recipients["target"])
+ channels = get_channels_with_search(
+ search_string=channels_list,
+ types=[
+ SlackChannelTypes.PRIVATE,
+ SlackChannelTypes.PUBLIC,
+ ],
+ exact_match=True,
+ )
+ if len(channels_list) != len(channels):
+ raise UpdateFailedError("Not all channels could be
found")
Review Comment:
### Slack Channel Validation Fails with Duplicate Inputs <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The channel validation in update_report_schedule_slack_v2 fails to handle
duplicates in the input list, which could cause false negatives.
###### Why this matters
If the input channels_list contains duplicate channel names, the length
comparison will incorrectly raise an error even if all channels are actually
found.
###### Suggested change ∙ *Feature Preview*
Update the validation to compare unique channel names against found channels:
```python
if len(set(channels_list)) != len(channels):
raise UpdateFailedError("Not all channels could be found")
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/250d1f8f-a9ec-4fd8-b7aa-1090ede64476?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:c31de323-3687-4543-9519-48682782138d -->
--
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]
