From d90cf0f59325770dc452d1e618a5a45ed4354aba Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Thu, 7 Sep 2023 09:42:30 +0100 Subject: [PATCH] Fix Slack integration leftovers after disconnecting (#2986) # What this PR does Improve Slack disconnect cleanup: - Reset Slack user group for schedules - Reset Slack user identity for deleted users ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/1962 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 1 + engine/apps/slack/tasks.py | 17 ++---- engine/apps/slack/tests/test_reset_slack.py | 58 +++++++++++++++++++++ 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 117fa1e1..2d364df6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Don't update Slack user groups for deleted organizations by @vadimkerr ([#2985](https://github.com/grafana/oncall/pull/2985)) +- Fix Slack integration leftovers after disconnecting by @vadimkerr ([#2986](https://github.com/grafana/oncall/pull/2986)) ## v1.3.35 (2023-09-05) diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index 4c00a4d9..4addb985 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -162,17 +162,13 @@ def unpopulate_slack_user_identities(organization_pk, force=False, ts=None): organization = Organization.objects.get(pk=organization_pk) - users_to_update = [] - for user in organization.users.filter(slack_user_identity__isnull=False): - user.slack_user_identity = None - users_to_update.append(user) - - User.objects.bulk_update(users_to_update, ["slack_user_identity"], batch_size=5000) + # Reset slack_user_identity for organization users (make sure to include deleted users in the queryset) + User.objects.filter_with_deleted(organization=organization).update(slack_user_identity=None) if force: organization.slack_team_identity = None organization.general_log_channel_id = None - organization.save() + organization.save(update_fields=["slack_team_identity", "general_log_channel_id"]) @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=0) @@ -626,12 +622,9 @@ def clean_slack_integration_leftovers(organization_id, *args, **kwargs): from apps.alerts.models import ChannelFilter from apps.schedules.models import OnCallSchedule - logger.info(f"Start clean slack leftovers for organization {organization_id}") + logger.info(f"Cleaning up for organization {organization_id}") ChannelFilter.objects.filter(alert_receive_channel__organization_id=organization_id).update(slack_channel_id=None) - logger.info(f"Cleaned ChannelFilters slack_channel_id for organization {organization_id}") - OnCallSchedule.objects.filter(organization_id=organization_id).update(channel=None) - logger.info(f"Cleaned OnCallSchedule slack_channel_id for organization {organization_id}") - logger.info(f"Finish clean slack leftovers for organization {organization_id}") + OnCallSchedule.objects.filter(organization_id=organization_id).update(channel=None, user_group=None) @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=10) diff --git a/engine/apps/slack/tests/test_reset_slack.py b/engine/apps/slack/tests/test_reset_slack.py index b64e7b3b..df572177 100644 --- a/engine/apps/slack/tests/test_reset_slack.py +++ b/engine/apps/slack/tests/test_reset_slack.py @@ -8,6 +8,9 @@ from rest_framework.response import Response from rest_framework.test import APIClient 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.user_management.models import User @pytest.mark.django_db @@ -32,3 +35,58 @@ def test_reset_slack_integration_permissions( response = client.post(url, format="json", **make_user_auth_headers(user, token)) assert response.status_code == expected_status + + +@pytest.mark.django_db +def test_clean_slack_integration_leftovers( + make_organization_with_slack_team_identity, + make_alert_receive_channel, + make_channel_filter, + make_slack_user_group, + make_schedule, +): + organization, slack_team_identity = make_organization_with_slack_team_identity() + + # create channel filter with Slack channel + alert_receive_channel = make_alert_receive_channel(organization) + channel_filter = make_channel_filter(alert_receive_channel, slack_channel_id="test") + + # 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) + + # clean Slack integration leftovers + clean_slack_integration_leftovers(organization.pk) + channel_filter.refresh_from_db() + schedule.refresh_from_db() + + # check that references to Slack objects are removed + assert channel_filter.slack_channel_id is None + assert schedule.channel is None + assert schedule.user_group is None + + +@pytest.mark.django_db +def test_unpopulate_slack_user_identities( + make_organization_and_user_with_slack_identities, make_user_with_slack_user_identity +): + # create organization and user with Slack connected + organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() + + # create & delete user with Slack connected + deleted_user, _ = make_user_with_slack_user_identity(slack_team_identity, organization) + User.objects.filter(pk=deleted_user.pk).delete() + + # unpopulate Slack user identities + unpopulate_slack_user_identities(organization.pk, force=True) + user.refresh_from_db() + deleted_user.refresh_from_db() + organization.refresh_from_db() + + # check that references to Slack user identities are removed + assert user.slack_user_identity is None + assert deleted_user.slack_user_identity is None + + # check that Slack specific info is reset for organization + assert organization.slack_team_identity is None + assert organization.general_log_channel_id is None