From 31759fdf8082af486f3c9dfc5d52a99718a9a0cc Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Wed, 6 Sep 2023 17:02:01 +0100 Subject: [PATCH] Don't update Slack user groups for deleted organizations (#2985) # What this PR does Make sure that Slack user groups are not getting updated for deleted orgs. Without this change, there could be issues with backends in multiple clusters trying to update a single Slack user group after migrating an org to another cluster (organizations get soft deleted from the original cluster after migration). ## Which issue(s) this PR fixes https://github.com/grafana/support-escalations/issues/6936 ## 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 | 4 ++++ engine/apps/slack/tasks.py | 7 +++++- engine/apps/slack/tests/test_user_group.py | 26 +++++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d43f36ad..117fa1e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Enable email notification step by default on Helm by @vadimkerr ([#2975](https://github.com/grafana/oncall/pull/2975)) - Handle slack resolution note errors consistently ([#2976](https://github.com/grafana/oncall/pull/2976)) +### Fixed + +- Don't update Slack user groups for deleted organizations by @vadimkerr ([#2985](https://github.com/grafana/oncall/pull/2985)) + ## v1.3.35 (2023-09-05) ### Fixed diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index d669be75..4c00a4d9 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -451,7 +451,12 @@ def start_update_slack_user_group_for_schedules(): from apps.slack.models import SlackUserGroup user_group_pks = ( - SlackUserGroup.objects.filter(oncall_schedules__isnull=False).distinct().values_list("pk", flat=True) + SlackUserGroup.objects.filter( + oncall_schedules__isnull=False, # has oncall schedules connected + oncall_schedules__organization__deleted_at__isnull=True, # organization is not deleted + ) + .distinct() + .values_list("pk", flat=True) ) for user_group_pk in user_group_pks: diff --git a/engine/apps/slack/tests/test_user_group.py b/engine/apps/slack/tests/test_user_group.py index 614b6a44..6626f862 100644 --- a/engine/apps/slack/tests/test_user_group.py +++ b/engine/apps/slack/tests/test_user_group.py @@ -2,9 +2,11 @@ from unittest.mock import PropertyMock, patch import pytest -from apps.schedules.models.on_call_schedule import OnCallScheduleQuerySet +from apps.schedules.models.on_call_schedule import OnCallScheduleQuerySet, OnCallScheduleWeb from apps.slack.client import SlackClientWithErrorHandling from apps.slack.models import SlackUserGroup +from apps.slack.tasks import start_update_slack_user_group_for_schedules, update_slack_user_group_for_schedules +from apps.user_management.models import Organization @pytest.mark.django_db @@ -67,3 +69,25 @@ def test_update_oncall_members( with patch.object(SlackUserGroup, "update_members") as update_members_mock: user_group.update_oncall_members() update_members_mock.assert_called() + + +@pytest.mark.django_db +def test_start_update_slack_user_group_for_schedules_organization_deleted( + make_organization_with_slack_team_identity, make_slack_user_group, make_schedule +): + organization, slack_team_identity = make_organization_with_slack_team_identity() + user_group = make_slack_user_group(slack_team_identity) + make_schedule(organization, schedule_class=OnCallScheduleWeb, user_group=user_group) + + # check user group is updated + with patch.object(update_slack_user_group_for_schedules, "delay") as mock: + start_update_slack_user_group_for_schedules() + mock.assert_called_once_with(user_group_pk=user_group.pk) + + # soft delete the organization + Organization.objects.filter(pk=organization.pk).delete() + + # check user group is not updated for deleted organization + with patch.object(update_slack_user_group_for_schedules, "delay") as mock: + start_update_slack_user_group_for_schedules() + mock.assert_not_called()