don't force create default user notification policies (#4608)

# What this PR does

Related to #4410

The changes in this PR are a prerequisite to
grafana/terraform-provider-grafana#1653. See the
conversation
[here](https://raintank-corp.slack.com/archives/C04JCU51NF8/p1719806995902499?thread_ts=1719520920.744319&cid=C04JCU51NF8)
for more context on why we decided to move away from always creating
default personal notification rules for users.

## 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] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.

docs/sources/oncall-api-reference/personal_notification_rules.md

@@ -35,8 +35,8 @@ The above command returns JSON structured in the following way:
 | ----------- | :------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `user_id`   |   Yes    | User ID                                                                                                                                                                                                                                                                                             |
 | `position`  | Optional | Personal notification rules execute one after another starting from `position=0`. `Position=-1` will put the escalation policy to the end of the list. A new escalation policy created with a position of an existing escalation policy will move the old one (and all following) down on the list. |
-| `type`      |   Yes    | One of: `wait`, `notify_by_slack`, `notify_by_sms`, `notify_by_phone_call`, `notify_by_telegram`, `notify_by_email`.                                                                                                                                                                                |
-| `duration`  | Optional | A time in secs when type `wait` is chosen for `type`.                                                                                                                                                                                                                                               |
+| `type`      |   Yes    | One of: `wait`, `notify_by_slack`, `notify_by_sms`, `notify_by_phone_call`, `notify_by_telegram`, `notify_by_email`, `notify_by_mobile_app`, `notify_by_mobile_app_critical`.                                                                                                                                                                                |
+| `duration`  | Optional | A time in seconds to wait (when `type=wait`). Can be one of 60, 300, 900, 1800, or 3600.                                                                                                                                                                                                                                               |
 | `important` | Optional | Boolean value indicates if a rule is "important". Default is `false`.                                                                                                                                                                                                                               |
 
 **HTTP request**

engine/apps/alerts/incident_log_builder/incident_log_builder.py

@@ -10,7 +10,8 @@
     from django.db.models.manager import RelatedManager
 
     from apps.alerts.models import AlertGroup, AlertGroupLogRecord, ResolutionNote
-    from apps.base.models import UserNotificationPolicyLogRecord
+    from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord
+    from apps.user_management.models import User
 
 
 class IncidentLogBuilder:
@@ -578,7 +579,9 @@ def _render_escalation_step_plan_from_escalation_policy_snapshot(
                 escalation_plan_dict.setdefault(timedelta, []).append(plan)
         return escalation_plan_dict
 
-    def _render_user_notification_line(self, user_to_notify, notification_policy, for_slack=False):
+    def _render_user_notification_line(
+        self, user_to_notify: "User", notification_policy: "UserNotificationPolicy", for_slack=False
+    ):
         """
         Renders user notification plan line
         :param user_to_notify:
@@ -611,7 +614,9 @@ def _render_user_notification_line(self, user_to_notify, notification_policy, fo
             result += f"inviting {user_verbal} but notification channel is unspecified"
         return result
 
-    def _get_notification_plan_for_user(self, user_to_notify, future_step=False, important=False, for_slack=False):
+    def _get_notification_plan_for_user(
+        self, user_to_notify: "User", future_step=False, important=False, for_slack=False
+    ):
         """
         Renders user notification plan
         :param user_to_notify:
@@ -665,7 +670,7 @@ def _get_notification_plan_for_user(self, user_to_notify, future_step=False, imp
                     # last passed step order + 1
                     notification_policy_order = last_user_log.notification_policy.order + 1
 
-        notification_policies = user_to_notify.get_or_create_notification_policies(important=important)
+        notification_policies = user_to_notify.get_notification_policies_or_use_default_fallback(important=important)
 
         for notification_policy in notification_policies:
             future_notification = notification_policy.order >= notification_policy_order

engine/apps/alerts/tasks/notify_group.py

@@ -83,22 +83,22 @@ def notify_group_task(alert_group_pk, escalation_policy_snapshot_order=None):
                 continue
 
             important = escalation_policy_step == EscalationPolicy.STEP_NOTIFY_GROUP_IMPORTANT
-            notification_policies = user.get_or_create_notification_policies(important=important)
+            notification_policies = user.get_notification_policies_or_use_default_fallback(important=important)
 
             if notification_policies:
                 usergroup_notification_plan += "\n_{} (".format(
-                    step.get_user_notification_message_for_thread_for_usergroup(user, notification_policies.first())
+                    step.get_user_notification_message_for_thread_for_usergroup(user, notification_policies[0])
                 )
-
-            notification_channels = []
-            if notification_policies.filter(step=UserNotificationPolicy.Step.NOTIFY).count() == 0:
+            else:
                 usergroup_notification_plan += "Empty notifications"
 
+            notification_channels = []
             for notification_policy in notification_policies:
                 if notification_policy.step == UserNotificationPolicy.Step.NOTIFY:
                     notification_channels.append(
                         UserNotificationPolicy.NotificationChannel(notification_policy.notify_by).label
                     )
+
             usergroup_notification_plan += "→".join(notification_channels) + ")_"
             reason = f"Membership in <!subteam^{usergroup.slack_id}> User Group"
 

engine/apps/alerts/tasks/notify_user.py

@@ -72,20 +72,20 @@ def notify_user_task(
         user_has_notification = UserHasNotification.objects.filter(pk=user_has_notification.pk).select_for_update()[0]
 
         if previous_notification_policy_pk is None:
-            notification_policy = user.get_or_create_notification_policies(important=important).first()
-            if notification_policy is None:
+            notification_policies = user.get_notification_policies_or_use_default_fallback(important=important)
+            if not notification_policies:
                 task_logger.info(
                     f"notify_user_task: Failed to notify. No notification policies. user_id={user_pk} alert_group_id={alert_group_pk} important={important}"
                 )
                 return
+
             # Here we collect a brief overview of notification steps configured for user to send it to thread.
             collected_steps_ids = []
-            next_notification_policy = notification_policy.next()
-            while next_notification_policy is not None:
-                if next_notification_policy.step == UserNotificationPolicy.Step.NOTIFY:
-                    if next_notification_policy.notify_by not in collected_steps_ids:
-                        collected_steps_ids.append(next_notification_policy.notify_by)
-                next_notification_policy = next_notification_policy.next()
+            for notification_policy in notification_policies:
+                if notification_policy.step == UserNotificationPolicy.Step.NOTIFY:
+                    if notification_policy.notify_by not in collected_steps_ids:
+                        collected_steps_ids.append(notification_policy.notify_by)
+
             collected_steps = ", ".join(
                 UserNotificationPolicy.NotificationChannel(step_id).label for step_id in collected_steps_ids
             )

engine/apps/api/views/user_notification_policy.py

@@ -17,7 +17,6 @@
 from apps.user_management.models import User
 from common.api_helpers.exceptions import BadRequest
 from common.api_helpers.mixins import UpdateSerializerMixin
-from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
 from common.insight_log import EntityEvent, write_resource_insight_log
 from common.ordered_model.viewset import OrderedModelViewSet
 
@@ -73,7 +72,7 @@ def get_queryset(self):
                 target_user = User.objects.get(public_primary_key=user_id)
             except User.DoesNotExist:
                 raise BadRequest(detail="User does not exist")
-        queryset = target_user.get_or_create_notification_policies(important=important)
+        queryset = UserNotificationPolicy.objects.filter(user=target_user, important=important)
         return self.serializer_class.setup_eager_loading(queryset)
 
     def get_object(self):
@@ -119,10 +118,7 @@ def perform_update(self, serializer):
     def perform_destroy(self, instance):
         user = instance.user
         prev_state = user.insight_logs_serialized
-        try:
-            instance.delete()
-        except UserNotificationPolicyCouldNotBeDeleted:
-            raise BadRequest(detail="Can't delete last user notification policy")
+        instance.delete()
         new_state = user.insight_logs_serialized
         write_resource_insight_log(
             instance=user,

engine/apps/base/models/user_notification_policy.py

@@ -5,12 +5,11 @@
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.core.validators import MinLengthValidator
-from django.db import IntegrityError, models
+from django.db import models
 from django.db.models import Q
 
 from apps.base.messaging import get_messaging_backends
 from apps.user_management.models import User
-from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
 from common.ordered_model.ordered_model import OrderedModel
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
@@ -66,32 +65,7 @@ def validate_channel_choice(value):
         raise ValidationError("%(value)s is not a valid option", params={"value": value})
 
 
-class UserNotificationPolicyQuerySet(models.QuerySet):
-    def create_default_policies_for_user(self, user: User) -> None:
-        if user.notification_policies.filter(important=False).exists():
-            return
-
-        policies_to_create = user.default_notification_policies_defaults
-
-        try:
-            super().bulk_create(policies_to_create)
-        except IntegrityError:
-            pass
-
-    def create_important_policies_for_user(self, user: User) -> None:
-        if user.notification_policies.filter(important=True).exists():
-            return
-
-        policies_to_create = user.important_notification_policies_defaults
-
-        try:
-            super().bulk_create(policies_to_create)
-        except IntegrityError:
-            pass
-
-
 class UserNotificationPolicy(OrderedModel):
-    objects = UserNotificationPolicyQuerySet.as_manager()
     order_with_respect_to = ("user_id", "important")
 
     public_primary_key = models.CharField(
@@ -171,12 +145,6 @@ def short_verbal(self) -> str:
         else:
             return "Not set"
 
-    def delete(self):
-        if UserNotificationPolicy.objects.filter(important=self.important, user=self.user).count() == 1:
-            raise UserNotificationPolicyCouldNotBeDeleted("Can't delete last user notification policy")
-        else:
-            super().delete()
-
 
 class NotificationChannelOptions:
     """

engine/apps/base/tests/test_user_notification_policy.py

@@ -9,7 +9,6 @@
     validate_channel_choice,
 )
 from apps.base.tests.messaging_backend import TestOnlyBackend
-from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
 
 
 @pytest.mark.parametrize(
@@ -84,7 +83,7 @@ def test_extra_messaging_backends_details():
 
 
 @pytest.mark.django_db
-def test_unable_to_delete_last_notification_policy(
+def test_can_delete_last_notification_policy(
     make_organization,
     make_user_for_organization,
     make_user_notification_policy,
@@ -101,5 +100,4 @@ def test_unable_to_delete_last_notification_policy(
     )
 
     first_policy.delete()
-    with pytest.raises(UserNotificationPolicyCouldNotBeDeleted):
-        second_policy.delete()
+    second_policy.delete()

engine/apps/public_api/views/personal_notifications.py

@@ -12,7 +12,6 @@
 from common.api_helpers.exceptions import BadRequest
 from common.api_helpers.mixins import RateLimitHeadersMixin, UpdateSerializerMixin
 from common.api_helpers.paginators import FiftyPageSizePaginator
-from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
 from common.insight_log import EntityEvent, write_resource_insight_log
 
 
@@ -75,10 +74,7 @@ def destroy(self, request, *args, **kwargs):
     def perform_destroy(self, instance):
         user = self.request.user
         prev_state = user.insight_logs_serialized
-        try:
-            instance.delete()
-        except UserNotificationPolicyCouldNotBeDeleted:
-            raise BadRequest(detail="Can't delete last user notification policy")
+        instance.delete()
         new_state = user.insight_logs_serialized
         write_resource_insight_log(
             instance=user,

engine/apps/user_management/models/user.py

@@ -9,7 +9,7 @@
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
 from django.core.validators import MinLengthValidator
-from django.db import models, transaction
+from django.db import models
 from django.db.models import Q
 from django.db.models.signals import post_save
 from django.dispatch import receiver
@@ -84,8 +84,6 @@ def sync_for_team(team, api_members: list[dict]):
 
     @staticmethod
     def sync_for_organization(organization, api_users: list[dict]):
-        from apps.base.models import UserNotificationPolicy
-
         grafana_users = {user["userId"]: user for user in api_users}
         existing_user_ids = set(organization.users.all().values_list("user_id", flat=True))
 
@@ -105,21 +103,7 @@ def sync_for_organization(organization, api_users: list[dict]):
             if user["userId"] not in existing_user_ids
         )
 
-        with transaction.atomic():
-            organization.users.bulk_create(users_to_create, batch_size=5000)
-            # Retrieve primary keys for the newly created users
-            #
-            # If the model’s primary key is an AutoField, the primary key attribute can only be retrieved
-            # on certain databases (currently PostgreSQL, MariaDB 10.5+, and SQLite 3.35+).
-            # On other databases, it will not be set.
-            # https://docs.djangoproject.com/en/4.1/ref/models/querysets/#django.db.models.query.QuerySet.bulk_create
-            created_users = organization.users.exclude(user_id__in=existing_user_ids)
-
-            policies_to_create = ()
-            for user in created_users:
-                policies_to_create = policies_to_create + user.default_notification_policies_defaults
-                policies_to_create = policies_to_create + user.important_notification_policies_defaults
-            UserNotificationPolicy.objects.bulk_create(policies_to_create, batch_size=5000)
+        organization.users.bulk_create(users_to_create, batch_size=5000)
 
         # delete excess users
         user_ids_to_delete = existing_user_ids - grafana_users.keys()
@@ -429,43 +413,25 @@ def build_permissions_query(
             return PermissionsQuery(permissions__contains=[required_permission])
         return RoleInQuery(role__lte=permission.fallback_role.value)
 
-    def get_or_create_notification_policies(self, important=False):
-        if not self.notification_policies.filter(important=important).exists():
-            if important:
-                self.notification_policies.create_important_policies_for_user(self)
-            else:
-                self.notification_policies.create_default_policies_for_user(self)
-        notification_policies = self.notification_policies.filter(important=important)
-        return notification_policies
-
-    @property
-    def default_notification_policies_defaults(self):
-        from apps.base.models import UserNotificationPolicy
-
-        print(self)
-
-        return (
-            UserNotificationPolicy(
-                user=self,
-                step=UserNotificationPolicy.Step.NOTIFY,
-                notify_by=settings.EMAIL_BACKEND_INTERNAL_ID,
-                order=0,
-            ),
-        )
-
-    @property
-    def important_notification_policies_defaults(self):
+    def get_notification_policies_or_use_default_fallback(
+        self, important=False
+    ) -> typing.List["UserNotificationPolicy"]:
+        """
+        If the user has no notification policies defined, fallback to using e-mail as the notification channel.
+        """
         from apps.base.models import UserNotificationPolicy
 
-        return (
-            UserNotificationPolicy(
-                user=self,
-                step=UserNotificationPolicy.Step.NOTIFY,
-                notify_by=settings.EMAIL_BACKEND_INTERNAL_ID,
-                important=True,
-                order=0,
-            ),
-        )
+        if not self.notification_policies.filter(important=important).exists():
+            return [
+                UserNotificationPolicy(
+                    user=self,
+                    step=UserNotificationPolicy.Step.NOTIFY,
+                    notify_by=settings.EMAIL_BACKEND_INTERNAL_ID,
+                    important=important,
+                    order=0,
+                ),
+            ]
+        return list(self.notification_policies.filter(important=important).all())
 
     def update_alert_group_table_selected_columns(self, columns: typing.List[AlertGroupTableColumn]) -> None:
         if self.alert_group_table_selected_columns != columns:
@@ -498,9 +464,6 @@ def finish_google_oauth2_disconnection_flow(self) -> None:
 # TODO: check whether this signal can be moved to save method of the model
 @receiver(post_save, sender=User)
 def listen_for_user_model_save(sender: User, instance: User, created: bool, *args, **kwargs) -> None:
-    if created:
-        instance.notification_policies.create_default_policies_for_user(instance)
-        instance.notification_policies.create_important_policies_for_user(instance)
     drop_cached_ical_for_custom_events_for_organization.apply_async(
         (instance.organization_id,),
     )

engine/apps/user_management/tests/test_sync.py

@@ -118,18 +118,6 @@ def test_sync_users_for_organization(make_organization, make_user_for_organizati
     assert created_user.name == api_users[1]["name"]
     assert created_user.avatar_full_url == "https://test.test/test/1234"
 
-    assert created_user.notification_policies.filter(important=False).count() == 1
-    assert (
-        created_user.notification_policies.filter(important=False).first().notify_by
-        == settings.EMAIL_BACKEND_INTERNAL_ID
-    )
-
-    assert created_user.notification_policies.filter(important=True).count() == 1
-    assert (
-        created_user.notification_policies.filter(important=True).first().notify_by
-        == settings.EMAIL_BACKEND_INTERNAL_ID
-    )
-
 
 @pytest.mark.django_db
 def test_sync_users_for_organization_role_none(make_organization, make_user_for_organization):

engine/common/exceptions/__init__.py

@@ -3,5 +3,4 @@
     MaintenanceCouldNotBeStartedError,
     TeamCanNotBeChangedError,
     UnableToSendDemoAlert,
-    UserNotificationPolicyCouldNotBeDeleted,
 )

engine/common/exceptions/exceptions.py

@@ -19,10 +19,6 @@ class UnableToSendDemoAlert(OperationCouldNotBePerformedError):
     pass
 
 
-class UserNotificationPolicyCouldNotBeDeleted(OperationCouldNotBePerformedError):
-    pass
-
-
 class BacksyncIntegrationRequestError(Exception):
     """Error making request to alert receive channel backsync connection."""