Update slack user group update not to retry on some errors (#3363)

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Populate `users` field of the public Shift GET API with `rolling_users` from the type override created from web UI([#3303](https://github.com/grafana/oncall/pull/3303))
+- Do not retry to update slack user group on every API error ([#3363](https://github.com/grafana/oncall/pull/3363))
 
 ## v1.3.58 (2023-11-14)
 

engine/apps/slack/errors.py

@@ -53,7 +53,11 @@ class SlackAPIInvalidAuthError(SlackAPIError):
 
 
 class SlackAPIUsergroupNotFoundError(SlackAPIError):
-    errors = ("no_such_subteam",)
+    errors = ("no_such_subteam", "subteam_not_found")
+
+
+class SlackAPIInvalidUsersError(SlackAPIError):
+    errors = ("invalid_users",)
 
 
 class SlackAPIChannelNotFoundError(SlackAPIError):

engine/apps/slack/models/slack_usergroup.py

@@ -10,7 +10,13 @@ from django.utils import timezone
 
 from apps.api.permissions import RBACPermission
 from apps.slack.client import SlackClient
-from apps.slack.errors import SlackAPIError, SlackAPIPermissionDeniedError
+from apps.slack.errors import (
+    SlackAPIError,
+    SlackAPIInvalidUsersError,
+    SlackAPIPermissionDeniedError,
+    SlackAPITokenError,
+    SlackAPIUsergroupNotFoundError,
+)
 from apps.slack.models import SlackTeamIdentity
 from apps.user_management.models.user import User
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
@@ -112,10 +118,15 @@ class SlackUserGroup(models.Model):
     def update_members(self, slack_ids):
         sc = SlackClient(self.slack_team_identity)
 
-        sc.usergroups_users_update(usergroup=self.slack_id, users=slack_ids)
-
-        self.members = slack_ids
-        self.save(update_fields=("members",))
+        try:
+            sc.usergroups_users_update(usergroup=self.slack_id, users=slack_ids)
+        except (SlackAPITokenError, SlackAPIUsergroupNotFoundError, SlackAPIInvalidUsersError) as err:
+            logger.warning(f"Slack usergroup update failed: {err}")
+        except SlackAPIError:
+            raise
+        else:
+            self.members = slack_ids
+            self.save(update_fields=("members",))
 
     def get_users_from_members_for_organization(self, organization):
         return organization.users.filter(

engine/apps/slack/tests/test_user_group.py

@@ -4,6 +4,12 @@ import pytest
 
 from apps.schedules.models.on_call_schedule import OnCallScheduleQuerySet, OnCallScheduleWeb
 from apps.slack.client import SlackClient
+from apps.slack.errors import (
+    SlackAPIError,
+    SlackAPIInvalidUsersError,
+    SlackAPITokenError,
+    SlackAPIUsergroupNotFoundError,
+)
 from apps.slack.models import SlackUserGroup
 from apps.slack.tasks import (
     populate_slack_usergroups_for_team,
@@ -20,14 +26,49 @@ def test_update_members(make_organization_with_slack_team_identity, make_slack_u
     user_group = make_slack_user_group(slack_team_identity)
 
     slack_ids = ["slack_id_1", "slack_id_2"]
-
-    with patch.object(SlackClient, "api_call") as mock:
+    with patch("apps.slack.client.SlackClient.usergroups_users_update") as mock_update:
         user_group.update_members(slack_ids)
-        mock.assert_called()
 
+    mock_update.assert_called_once()
     assert user_group.members == slack_ids
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize("exception", [SlackAPITokenError, SlackAPIUsergroupNotFoundError, SlackAPIInvalidUsersError])
+def test_slack_user_group_update_errors(
+    make_organization_with_slack_team_identity,
+    make_slack_user_group,
+    exception,
+):
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+    user_group = make_slack_user_group(slack_team_identity=slack_team_identity)
+
+    slack_ids = ["slack_id_1", "slack_id_2"]
+    with patch("apps.slack.client.SlackClient.usergroups_users_update", side_effect=exception("Error")) as mock_update:
+        user_group.update_members(slack_ids)
+
+    mock_update.assert_called_once()
+    assert user_group.members is None
+
+
+@pytest.mark.django_db
+def test_slack_user_group_update_errors_raise(
+    make_organization_with_slack_team_identity,
+    make_slack_user_group,
+):
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+    user_group = make_slack_user_group(slack_team_identity=slack_team_identity)
+
+    slack_ids = ["slack_id_1", "slack_id_2"]
+    with patch(
+        "apps.slack.client.SlackClient.usergroups_users_update", side_effect=SlackAPIError("Error")
+    ) as mock_update:
+        with pytest.raises(SlackAPIError):
+            user_group.update_members(slack_ids)
+
+    mock_update.assert_called_once()
+
+
 @pytest.mark.django_db
 def test_oncall_slack_user_identities(
     make_organization_with_slack_team_identity,