-
Notifications
You must be signed in to change notification settings - Fork 296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: patch slack ID reference issue #5314
Conversation
organization = make_organization() | ||
slack_team_identity = make_slack_team_identity() | ||
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() | ||
slack_channel = make_slack_channel(slack_team_identity) | ||
organization.slack_team_identity = slack_team_identity | ||
organization.save() | ||
|
||
user = make_user(organization=organization) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make_organization_and_user_with_slack_identities
does the same thing as these 6 lines
@@ -303,14 +303,14 @@ def post_slack_rate_limit_message(integration_id): | |||
return | |||
|
|||
default_route = integration.channel_filters.get(is_default=True) | |||
if (slack_channel_id := default_route.slack_channel_id_or_org_default_id) is not None: | |||
if (slack_channel := default_route.slack_channel_or_org_default) is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since I've been mucking around a lot lately in engine/apps/slack/scenarios/distribute_alerts.py
, I felt an obligation to add full test coverage for IncomingAlertStep.process_signal
.. voila
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Which issue(s) this PR closes
Fixes https://github.com/grafana/irm/issues/469
Checklist
pr:no public docs
PR label added if not required)release:
). These labels dictate how your PR willshow up in the autogenerated release notes.