From 14c1b37c87ada3458bbd07024dabc0480f9da3a7 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 4 Nov 2024 15:49:22 -0500 Subject: [PATCH] chore: add more slack related tests (#5227) Follow up test PR to https://github.com/grafana/oncall/pull/5199 and https://github.com/grafana/oncall/pull/5224 --- engine/apps/slack/scenarios/slack_channel.py | 4 +- engine/apps/slack/tasks.py | 41 +++- .../__init__.py | 0 .../test_alert_group_actions.py | 0 .../test_distribute_alerts.py | 0 .../test_manage_responders.py | 0 .../test_paging.py | 0 .../test_resolution_note.py | 0 .../test_shift_swap_requests.py | 0 .../scenario_steps/test_slack_channel.py | 217 ++++++++++++++++++ .../test_slack_channel_integration.py | 0 .../test_slack_usergroup_steps.py | 0 engine/apps/slack/tests/test_reset_slack.py | 80 +++++-- 13 files changed, 318 insertions(+), 24 deletions(-) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/__init__.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_alert_group_actions.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_distribute_alerts.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_manage_responders.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_paging.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_resolution_note.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_shift_swap_requests.py (100%) create mode 100644 engine/apps/slack/tests/scenario_steps/test_slack_channel.py rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_slack_channel_integration.py (100%) rename engine/apps/slack/tests/{test_scenario_steps => scenario_steps}/test_slack_usergroup_steps.py (100%) diff --git a/engine/apps/slack/scenarios/slack_channel.py b/engine/apps/slack/scenarios/slack_channel.py index 51d94c22f5..e624ed0d7e 100644 --- a/engine/apps/slack/scenarios/slack_channel.py +++ b/engine/apps/slack/scenarios/slack_channel.py @@ -81,7 +81,7 @@ def process_scenario( clean_slack_channel_leftovers.apply_async((slack_team_identity.id, slack_id)) -class SlackChannelUnArchivedEventStep(scenario_step.ScenarioStep): +class SlackChannelUnarchivedEventStep(scenario_step.ScenarioStep): def process_scenario( self, slack_user_identity: "SlackUserIdentity", @@ -126,6 +126,6 @@ def process_scenario( { "payload_type": PayloadType.EVENT_CALLBACK, "event_type": EventType.CHANNEL_UNARCHIVED, - "step": SlackChannelUnArchivedEventStep, + "step": SlackChannelUnarchivedEventStep, }, ] diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index e4822ffb92..84e5ca7653 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -1,6 +1,6 @@ import logging import random -from typing import Optional +import typing from celery import uuid as celery_uuid from celery.exceptions import Retry @@ -433,7 +433,7 @@ def populate_slack_channels(): def start_populate_slack_channels_for_team( - slack_team_identity_id: int, delay: int, cursor: Optional[str] = None + slack_team_identity_id: int, delay: int, cursor: typing.Optional[str] = None ) -> None: # save active task id in cache to make only one populate task active per team task_id = celery_uuid() @@ -445,7 +445,7 @@ def start_populate_slack_channels_for_team( @shared_dedicated_queue_retry_task( autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None ) -def populate_slack_channels_for_team(slack_team_identity_id: int, cursor: Optional[str] = None) -> None: +def populate_slack_channels_for_team(slack_team_identity_id: int, cursor: typing.Optional[str] = None) -> None: """ Make paginated request to get slack channels. On ratelimit - update info for got channels, save collected channels ids in cache and restart the task with the last successful pagination cursor to avoid any data loss during delay @@ -539,7 +539,7 @@ def populate_slack_channels_for_team(slack_team_identity_id: int, cursor: Option @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=0) -def clean_slack_integration_leftovers(organization_id, *args, **kwargs): +def clean_slack_integration_leftovers(organization_id: int, *args, **kwargs) -> None: """ This task removes binding to slack (e.g ChannelFilter's slack channel) for a given organization. It is used when user changes slack integration. @@ -549,11 +549,11 @@ def clean_slack_integration_leftovers(organization_id, *args, **kwargs): logger.info(f"Cleaning up for organization {organization_id}") ChannelFilter.objects.filter(alert_receive_channel__organization_id=organization_id).update(slack_channel=None) - OnCallSchedule.objects.filter(organization_id=organization_id).update(channel=None, user_group=None) + OnCallSchedule.objects.filter(organization_id=organization_id).update(slack_channel=None, user_group=None) @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=10) -def clean_slack_channel_leftovers(slack_team_identity_id, slack_channel_id): +def clean_slack_channel_leftovers(slack_team_identity_id: int, slack_channel_id: str) -> None: """ This task removes binding to slack channel after a channel is archived in Slack. @@ -561,7 +561,11 @@ def clean_slack_channel_leftovers(slack_team_identity_id, slack_channel_id): to that channel via `on_delete=models.SET_NULL`. """ from apps.alerts.models import ChannelFilter + from apps.schedules.models import OnCallSchedule from apps.slack.models import SlackTeamIdentity + from apps.user_management.models import Organization + + orgs_to_clean_default_slack_channel: typing.List[Organization] = [] try: sti = SlackTeamIdentity.objects.get(id=slack_team_identity_id) @@ -572,6 +576,25 @@ def clean_slack_channel_leftovers(slack_team_identity_id, slack_channel_id): return for org in sti.organizations.all(): - ChannelFilter.objects.filter(alert_receive_channel__organization=org, slack_channel_id=slack_channel_id).update( - slack_channel_id=None - ) + org_id = org.id + + if org.default_slack_channel_slack_id == slack_channel_id: + logger.info( + f"Set default_slack_channel to None for org_id={org_id} slack_channel_id={slack_channel_id} since slack_channel is arcived or deleted" + ) + org.default_slack_channel = None + orgs_to_clean_default_slack_channel.append(org) + + # The channel no longer exists, so update any integration routes (ie. ChannelFilter) or schedules + # that reference it + ChannelFilter.objects.filter( + alert_receive_channel__organization=org, + slack_channel__slack_id=slack_channel_id, + ).update(slack_channel=None) + + OnCallSchedule.objects.filter( + organization_id=org_id, + slack_channel__slack_id=slack_channel_id, + ).update(slack_channel=None) + + Organization.objects.bulk_update(orgs_to_clean_default_slack_channel, ["default_slack_channel"], batch_size=5000) diff --git a/engine/apps/slack/tests/test_scenario_steps/__init__.py b/engine/apps/slack/tests/scenario_steps/__init__.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/__init__.py rename to engine/apps/slack/tests/scenario_steps/__init__.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_alert_group_actions.py b/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_alert_group_actions.py rename to engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py rename to engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_manage_responders.py b/engine/apps/slack/tests/scenario_steps/test_manage_responders.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_manage_responders.py rename to engine/apps/slack/tests/scenario_steps/test_manage_responders.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_paging.py b/engine/apps/slack/tests/scenario_steps/test_paging.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_paging.py rename to engine/apps/slack/tests/scenario_steps/test_paging.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py rename to engine/apps/slack/tests/scenario_steps/test_resolution_note.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py b/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py rename to engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py diff --git a/engine/apps/slack/tests/scenario_steps/test_slack_channel.py b/engine/apps/slack/tests/scenario_steps/test_slack_channel.py new file mode 100644 index 0000000000..a0205e7e1a --- /dev/null +++ b/engine/apps/slack/tests/scenario_steps/test_slack_channel.py @@ -0,0 +1,217 @@ +from unittest.mock import patch + +import pytest +from django.utils import timezone + +from apps.slack.models import SlackChannel +from apps.slack.scenarios import slack_channel as slack_channel_scenarios + + +@pytest.mark.django_db +class TestSlackChannelCreatedOrRenamedEventStep: + def test_process_scenario_channel_created( + self, + make_organization_and_user_with_slack_identities, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel_id = "C12345678" + channel_name = "new-channel" + payload = { + "event": { + "channel": { + "id": slack_channel_id, + "name": channel_name, + } + } + } + + # Ensure the SlackChannel does not exist + assert not SlackChannel.objects.filter( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ).exists() + + step = slack_channel_scenarios.SlackChannelCreatedOrRenamedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + # Now the SlackChannel should exist with correct data + slack_channel = SlackChannel.objects.get( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ) + assert slack_channel.name == channel_name + assert slack_channel.last_populated == timezone.now().date() + + def test_process_scenario_channel_renamed( + self, + make_organization_and_user_with_slack_identities, + make_slack_channel, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel = make_slack_channel(slack_team_identity) + slack_channel_id = slack_channel.slack_id + new_name = "renamed-channel" + payload = { + "event": { + "channel": { + "id": slack_channel_id, + "name": new_name, + } + } + } + + step = slack_channel_scenarios.SlackChannelCreatedOrRenamedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + slack_channel.refresh_from_db() + assert slack_channel.name == new_name + assert slack_channel.last_populated == timezone.now().date() + + +@pytest.mark.django_db +class TestSlackChannelDeletedEventStep: + def test_process_scenario_channel_deleted( + self, + make_organization_and_user_with_slack_identities, + make_slack_channel, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel = make_slack_channel(slack_team_identity) + slack_channel_id = slack_channel.slack_id + + # Ensure the SlackChannel exists + assert SlackChannel.objects.filter( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ).exists() + + step = slack_channel_scenarios.SlackChannelDeletedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) + + # Now the SlackChannel should not exist + assert not SlackChannel.objects.filter( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ).exists() + + def test_process_scenario_channel_does_not_exist( + self, + make_organization_and_user_with_slack_identities, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel_id = "C12345678" + + # Ensure the SlackChannel does not exist + assert not SlackChannel.objects.filter( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ).exists() + + step = slack_channel_scenarios.SlackChannelDeletedEventStep(slack_team_identity, organization, user) + # The step should not raise an exception even if the channel does not exist + step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) + + # Still, the SlackChannel does not exist + assert not SlackChannel.objects.filter( + slack_id=slack_channel_id, + slack_team_identity=slack_team_identity, + ).exists() + + +@pytest.mark.django_db +class TestSlackChannelArchivedEventStep: + @patch("apps.slack.scenarios.slack_channel.clean_slack_channel_leftovers") + def test_process_scenario( + self, + mock_clean_slack_channel_leftovers, + make_organization_and_user_with_slack_identities, + make_slack_channel, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel = make_slack_channel(slack_team_identity) + slack_channel_id = slack_channel.slack_id + + assert slack_channel.is_archived is False + + step = slack_channel_scenarios.SlackChannelArchivedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) + + slack_channel.refresh_from_db() + + assert slack_channel.is_archived is True + mock_clean_slack_channel_leftovers.apply_async.assert_called_once_with( + (slack_team_identity.id, slack_channel_id) + ) + + +@pytest.mark.django_db +class TestSlackChannelUnarchivedEventStep: + def test_process_scenario_channel_unarchived( + self, + make_organization_and_user_with_slack_identities, + make_slack_channel, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel = make_slack_channel(slack_team_identity, is_archived=True) + slack_channel_id = slack_channel.slack_id + + assert slack_channel.is_archived is True + + step = slack_channel_scenarios.SlackChannelUnarchivedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) + + slack_channel.refresh_from_db() + assert slack_channel.is_archived is False + + def test_process_scenario_channel_already_unarchived( + self, + make_organization_and_user_with_slack_identities, + make_slack_channel, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + slack_channel = make_slack_channel(slack_team_identity, is_archived=False) + slack_channel_id = slack_channel.slack_id + + assert slack_channel.is_archived is False + + step = slack_channel_scenarios.SlackChannelUnarchivedEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) + + slack_channel.refresh_from_db() + # Ensure that is_archived remains False + assert slack_channel.is_archived is False diff --git a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py rename to engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py diff --git a/engine/apps/slack/tests/test_scenario_steps/test_slack_usergroup_steps.py b/engine/apps/slack/tests/scenario_steps/test_slack_usergroup_steps.py similarity index 100% rename from engine/apps/slack/tests/test_scenario_steps/test_slack_usergroup_steps.py rename to engine/apps/slack/tests/scenario_steps/test_slack_usergroup_steps.py diff --git a/engine/apps/slack/tests/test_reset_slack.py b/engine/apps/slack/tests/test_reset_slack.py index 8c5f57fa72..ed0a44c6e6 100644 --- a/engine/apps/slack/tests/test_reset_slack.py +++ b/engine/apps/slack/tests/test_reset_slack.py @@ -9,7 +9,11 @@ from apps.api.permissions import LegacyAccessControlRole from apps.schedules.models import OnCallScheduleWeb -from apps.slack.tasks import clean_slack_integration_leftovers, unpopulate_slack_user_identities +from apps.slack.tasks import ( + clean_slack_channel_leftovers, + clean_slack_integration_leftovers, + unpopulate_slack_user_identities, +) from apps.user_management.models import User @@ -38,6 +42,53 @@ def test_reset_slack_integration_permissions( assert response.status_code == expected_status +@pytest.mark.django_db +def test_clean_slack_channel_leftovers( + make_slack_team_identity, + make_slack_channel, + make_organization, + make_alert_receive_channel, + make_channel_filter, + make_slack_user_group, + make_schedule, +): + slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) + + # create channel filter with Slack channel + alert_receive_channel = make_alert_receive_channel(organization) + channel_filter = make_channel_filter(alert_receive_channel, slack_channel=slack_channel) + + # create schedule with Slack channel and user group + user_group = make_slack_user_group(slack_team_identity) + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + slack_channel=slack_channel, + user_group=user_group, + ) + + assert channel_filter.slack_channel == slack_channel + assert schedule.slack_channel == slack_channel + assert schedule.user_group == user_group + assert organization.default_slack_channel_slack_id == slack_channel.slack_id + + # clean Slack channel leftovers + clean_slack_channel_leftovers(slack_team_identity.pk, slack_channel.slack_id) + channel_filter.refresh_from_db() + schedule.refresh_from_db() + organization.refresh_from_db() + + # check that references to Slack objects are removed + assert channel_filter.slack_channel is None + assert organization.default_slack_channel is None + + # NOTE: user groups shouldn't be updated for schedules, only the channel + assert schedule.slack_channel is None + assert schedule.user_group == user_group + + @pytest.mark.django_db def test_clean_slack_integration_leftovers( make_slack_team_identity, @@ -58,11 +109,16 @@ def test_clean_slack_integration_leftovers( # create schedule with Slack channel and user group user_group = make_slack_user_group(slack_team_identity) - schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, channel="test", user_group=user_group) + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + slack_channel=slack_channel, + user_group=user_group, + ) - assert channel_filter.slack_channel is not None - assert schedule.channel is not None - assert schedule.user_group is not None + assert channel_filter.slack_channel == slack_channel + assert schedule.slack_channel == slack_channel + assert schedule.user_group == user_group # clean Slack integration leftovers clean_slack_integration_leftovers(organization.pk) @@ -71,7 +127,7 @@ def test_clean_slack_integration_leftovers( # check that references to Slack objects are removed assert channel_filter.slack_channel is None - assert schedule.channel is None + assert schedule.slack_channel is None assert schedule.user_group is None @@ -117,24 +173,22 @@ def test_delete_slack_channel_and_cascade_deletes( make_organization, make_alert_receive_channel, make_channel_filter, - # make_schedule, + make_schedule, ): - # TODO: add the schedule related bits once https://github.com/grafana/oncall/pull/5199 is merged - slack_team_identity = make_slack_team_identity() slack_channel = make_slack_channel(slack_team_identity) organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel) alert_receive_channel = make_alert_receive_channel(organization) channel_filter = make_channel_filter(alert_receive_channel, slack_channel=slack_channel) - # schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, slack_channel=slack_channel) assert channel_filter.slack_channel == slack_channel - # assert schedule.slack_channel == slack_channel + assert schedule.slack_channel == slack_channel slack_channel.delete() channel_filter.refresh_from_db() - # schedule.refresh_from_db() + schedule.refresh_from_db() assert channel_filter.slack_channel is None - # assert schedule.slack_channel is None + assert schedule.slack_channel is None