Fix orphaned messages in Slack #2023
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98b9b64 to
153072d
Compare
vstpme
commented
May 31, 2023
| @@ -80,7 +80,8 @@ def get_alert_group(self): | |||
| self.alert_group.slack_message = self | |||
| self.alert_group.save(update_fields=["slack_message"]) | |||
| return self.alert_group | |||
| return self.alert.group | |||
There was a problem hiding this comment.
There's no alert field on SlackMessage
vstpme
commented
May 31, 2023
| @@ -217,32 +218,3 @@ def send_slack_notification(self, user, alert_group, notification_policy): | |||
| pass | |||
| else: | |||
| raise e | |||
|
|
|||
| @classmethod | |||
| def get_alert_group_from_slack_message_payload(cls, slack_team_identity, payload): | |||
There was a problem hiding this comment.
vstpme
commented
May 31, 2023
| REQUIRED_PERMISSIONS = [] | ||
| ACTION_VERBOSE = "" |
There was a problem hiding this comment.
ACTION_VERBOSE is not needed anymore, it was used to send messages to AG thread with information about interaction attempt. Now it's a simple warning popup in case of authorization failure.
iskhakov
approved these changes
Jun 1, 2023
| "alert_group_pk": alert_group_pk, | ||
| "message_ts": message_ts, | ||
| "alert_group_pk": alert_group.pk, | ||
| "message_ts": payload.get("message_ts") or payload["container"]["message_ts"], |
| self._get_alert_group_from_action(payload) # Try to get alert_group_pk from PRESSED button | ||
| or self._get_alert_group_from_message(payload) # Try to use alert_group_pk from ANY button in message | ||
| or self._get_alert_group_from_slack_message_in_db(slack_team_identity, payload) # Fetch message from DB | ||
| ) |
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
joeyorlando
reviewed
Jun 5, 2023
Comment on lines
+421
to
+433
| def _alert_group_action_value(self, **kwargs): | ||
| """ | ||
| Store organization and alert group IDs in Slack button or select menu values. | ||
| alert_group_pk is used in apps.slack.scenarios.step_mixins.AlertGroupActionsMixin to get the right alert group | ||
| when handling AG actions in Slack. | ||
| """ | ||
|
|
||
| data = { | ||
| "organization_id": self.alert_group.channel.organization_id, | ||
| "alert_group_pk": self.alert_group.pk, | ||
| **kwargs, | ||
| } | ||
| return json.dumps(data) # Slack block elements allow to pass value as string only (max 2000 chars) |
| @@ -80,7 +80,8 @@ def get_alert_group(self): | |||
| self.alert_group.slack_message = self | |||
| self.alert_group.save(update_fields=["slack_message"]) | |||
| return self.alert_group | |||
| return self.alert.group | |||
| else: | |||
There was a problem hiding this comment.
what does _alert_group (a few lines above) refer to? I see an alert_group reference defined, but no _alert_group
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does
Reworks Slack handlers for buttons and select menus for AG Slack messages.
Current implementation
SlackMessageinstance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo issue for more info.Changes in this PR
SlackMessagemodel to figure out the alert group being interacted on, instead embedalert_group_pkto every button and use it when receiving interaction requests from Slack.SlackMessageinstance will be created on interaction). This is possible because some buttons already havealert_group_pkembedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't havealert_group_pkembedded).AlertGroupSlackRendererchecking that correct data is embedded into buttons.incidenttoAlert Group.Which issue(s) this PR fixes
Related to https://github.com/grafana/oncall-private/issues/1841
Checklist
pr:no public docsPR label added if not required)CHANGELOG.mdupdated (orpr:no changelogPR label added if not required)