Speed up internal api endpoints (#4830)

# What this PR does
Reduces number of calls to db for `/schedules`, `/alertgroups` and
`/users` endpoints.
Fixes the issue when there was an additional call to db to get
organization url to build user avatar full link.

## Which issue(s) this PR closes

Related to [issue link here]

<!--
*Note*: If you want the issue to be auto-closed once the PR is merged,
change "Related to" to "Closes" in the line above.
If you have more than one GitHub issue that this PR closes, be sure to
preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## 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.

engine/apps/alerts/models/alert_group.py

@@ -559,6 +559,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
 
         user_ids: typing.Set[str] = set()
         users: typing.Dict[str, PagedUser] = {}
+        organization = self.channel.organization
 
         log_records = self.log_records.filter(
             type__in=(AlertGroupLogRecord.TYPE_DIRECT_PAGING, AlertGroupLogRecord.TYPE_UNPAGE_USER)
@@ -594,7 +595,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
                         "name": user.name,
                         "username": user.username,
                         "avatar": user.avatar_url,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(organization),
                         "important": important,
                         "teams": [{"pk": t.public_primary_key, "name": t.name} for t in user.teams.all()],
                     }

engine/apps/alerts/models/alert_group_log_record.py

@@ -230,9 +230,10 @@ class AlertGroupLogRecord(models.Model):
     def render_log_line_json(self):
         time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
         created_at = DateTimeField().to_representation(self.created_at)
-        author = self.author.short() if self.author is not None else None
+        organization = self.alert_group.channel.organization
+        author = self.author.short(organization) if self.author is not None else None
 
-        sf = SlackFormatter(self.alert_group.channel.organization)
+        sf = SlackFormatter(organization)
         action = sf.format(self.rendered_log_line_action(substitute_author_with_tag=True))
         action = clean_markup(action)
 

engine/apps/alerts/models/resolution_note.py

@@ -179,9 +179,10 @@ class ResolutionNote(models.Model):
     def render_log_line_json(self):
         time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
         created_at = DateTimeField().to_representation(self.created_at)
-        author = self.author.short() if self.author is not None else None
+        organization = self.alert_group.channel.organization
+        author = self.author.short(organization) if self.author is not None else None
 
-        sf = SlackFormatter(self.alert_group.channel.organization)
+        sf = SlackFormatter(organization)
         action = sf.format(self.text)
         action = clean_markup(action)
 

engine/apps/alerts/tasks/notify_ical_schedule_shift.py

@@ -37,7 +37,7 @@ def convert_prev_shifts_to_new_format(prev_shifts: dict, schedule: "OnCallSchedu
                 "display_name": user.username,
                 "email": user.email,
                 "pk": user.public_primary_key,
-                "avatar_full": user.avatar_full_url,
+                "avatar_full": user.avatar_full_url(schedule.organization),
             },
         )
     for uid, shift in prev_shifts.items():

engine/apps/api/serializers/alert_group.py

@@ -232,7 +232,7 @@ class AlertGroupListSerializer(
             if log_record.author is not None and log_record.author.public_primary_key not in users_ids:
                 users.append(log_record.author)
                 users_ids.add(log_record.author.public_primary_key)
-        return UserShortSerializer(users, many=True).data
+        return UserShortSerializer(users, context=self.context, many=True).data
 
 
 class AlertGroupSerializer(AlertGroupListSerializer):

engine/apps/api/serializers/schedule_base.py

@@ -69,7 +69,8 @@ class ScheduleBaseSerializer(EagerLoadingMixin, serializers.ModelSerializer):
     def get_on_call_now(self, obj):
         # Serializer context is set here: apps.api.views.schedule.ScheduleView.get_serializer_context
         users = self.context["oncall_users"].get(obj, [])
-        return [user.short() for user in users]
+        organization = self.context["request"].auth.organization
+        return [user.short(organization) for user in users]
 
     def get_number_of_escalation_chains(self, obj):
         # num_escalation_chains param added in queryset via annotate. Check ScheduleView.get_queryset

engine/apps/api/serializers/schedule_polymorphic.py

@@ -12,7 +12,7 @@ from common.api_helpers.mixins import EagerLoadingMixin
 
 
 class PolymorphicScheduleSerializer(EagerLoadingMixin, PolymorphicSerializer):
-    SELECT_RELATED = ["organization", "user_group"]
+    SELECT_RELATED = ["organization", "user_group", "team"]
 
     resource_type_field_name = "type"
 

engine/apps/api/serializers/shift_swap.py

@@ -103,11 +103,14 @@ class ShiftSwapRequestExpandedUsersListSerializer(BaseShiftSwapRequestListSerial
     def _serialize_user(self, user: "User") -> dict | None:
         user_data = None
         if user:
+            organization = (
+                self.context["request"].auth.organization if self.context.get("request") else user.organization
+            )
             user_data = {
                 "display_name": user.username,
                 "email": user.email,
                 "pk": user.public_primary_key,
-                "avatar_full": user.avatar_full_url,
+                "avatar_full": user.avatar_full_url(organization),
             }
         return user_data
 

engine/apps/api/serializers/user.py

@@ -83,7 +83,7 @@ class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin):
 
     timezone = TimeZoneField(allow_null=True, required=False)
     avatar = serializers.URLField(source="avatar_url", read_only=True)
-    avatar_full = serializers.URLField(source="avatar_full_url", read_only=True)
+    avatar_full = serializers.SerializerMethodField()
     notification_chain_verbal = serializers.SerializerMethodField()
     cloud_connection_status = serializers.SerializerMethodField()
     working_hours = WorkingHoursSerializer(required=False)
@@ -96,6 +96,7 @@ class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin):
         "mobileappauthtoken",
         "google_oauth2_user",
     ]
+    PREFETCH_RELATED = ["notification_policies"]
 
     class Meta:
         model = User
@@ -165,6 +166,10 @@ class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin):
         default, important = UserNotificationPolicy.get_short_verbals_for_user(user=obj)
         return {"default": " - ".join(default), "important": " - ".join(important)}
 
+    def get_avatar_full(self, obj):
+        organization = self.context["request"].auth.organization if self.context.get("request") else obj.organization
+        return obj.avatar_full_url(organization)
+
     def get_cloud_connection_status(self, obj: User) -> CloudSyncStatus | None:
         is_open_source_with_cloud_notifications = self.context.get("is_open_source_with_cloud_notifications", None)
         is_open_source_with_cloud_notifications = (
@@ -307,7 +312,7 @@ class UserShortSerializer(serializers.ModelSerializer):
     username = serializers.CharField()
     pk = serializers.CharField(source="public_primary_key")
     avatar = serializers.CharField(source="avatar_url")
-    avatar_full = serializers.CharField(source="avatar_full_url")
+    avatar_full = serializers.SerializerMethodField()
 
     class Meta:
         model = User
@@ -324,6 +329,10 @@ class UserShortSerializer(serializers.ModelSerializer):
             "avatar_full",
         ]
 
+    def get_avatar_full(self, obj):
+        organization = self.context["request"].auth.organization if self.context.get("request") else obj.organization
+        return obj.avatar_full_url(organization)
+
 
 class UserIsCurrentlyOnCallSerializer(UserShortSerializer, EagerLoadingMixin):
     context: UserSerializerContext

engine/apps/api/tests/test_alert_group.py

@@ -2083,7 +2083,7 @@ def test_alert_group_paged_users(
     assert response.json()["paged_users"] == [
         {
             "avatar": user2.avatar_url,
-            "avatar_full": user2.avatar_full_url,
+            "avatar_full": user2.avatar_full_url(user.organization),
             "id": user2.pk,
             "pk": user2.public_primary_key,
             "important": None,

engine/apps/api/tests/test_oncall_shift.py

@@ -1648,7 +1648,7 @@ def test_on_call_shift_preview(
                     "display_name": other_user.username,
                     "pk": other_user.public_primary_key,
                     "email": other_user.email,
-                    "avatar_full": other_user.avatar_full_url,
+                    "avatar_full": other_user.avatar_full_url(organization),
                 },
             ],
             "source": "web",
@@ -1978,7 +1978,7 @@ def test_on_call_shift_preview_update(
                 "display_name": other_user.username,
                 "pk": other_user.public_primary_key,
                 "email": other_user.email,
-                "avatar_full": other_user.avatar_full_url,
+                "avatar_full": other_user.avatar_full_url(organization),
             },
         ],
         "source": "web",
@@ -2093,7 +2093,7 @@ def test_on_call_shift_preview_update_not_started_reuse_pk(
                     "display_name": other_user.username,
                     "pk": other_user.public_primary_key,
                     "email": other_user.email,
-                    "avatar_full": other_user.avatar_full_url,
+                    "avatar_full": other_user.avatar_full_url(organization),
                 },
             ],
             "source": "web",

engine/apps/api/tests/test_schedules.py

@@ -612,7 +612,7 @@ def test_get_detail_schedule_oncall_now_multipage_objects(
                 "pk": user.public_primary_key,
                 "username": user.username,
                 "avatar": user.avatar_url,
-                "avatar_full": user.avatar_full_url,
+                "avatar_full": user.avatar_full_url(organization),
             }
         ],
         "has_gaps": False,
@@ -916,7 +916,7 @@ def test_events_calendar(
                         "display_name": user.username,
                         "pk": user.public_primary_key,
                         "email": user.email,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(organization),
                     },
                 ],
                 "missing_users": [],
@@ -989,7 +989,7 @@ def test_filter_events_calendar(
                         "display_name": user.username,
                         "pk": user.public_primary_key,
                         "email": user.email,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(organization),
                     },
                 ],
                 "missing_users": [],
@@ -1014,7 +1014,7 @@ def test_filter_events_calendar(
                         "display_name": user.username,
                         "pk": user.public_primary_key,
                         "email": user.email,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(organization),
                     }
                 ],
                 "missing_users": [],
@@ -1106,7 +1106,7 @@ def test_filter_events_range_calendar(
                         "display_name": user.username,
                         "pk": user.public_primary_key,
                         "email": user.email,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(organization),
                     },
                 ],
                 "missing_users": [],
@@ -1197,7 +1197,7 @@ def test_filter_events_overrides(
                         "display_name": other_user.username,
                         "pk": other_user.public_primary_key,
                         "email": other_user.email,
-                        "avatar_full": other_user.avatar_full_url,
+                        "avatar_full": other_user.avatar_full_url(organization),
                     }
                 ],
                 "missing_users": [],
@@ -1395,7 +1395,7 @@ def test_filter_swap_requests(
                 "display_name": u.username,
                 "email": u.email,
                 "pk": u.public_primary_key,
-                "avatar_full": u.avatar_full_url,
+                "avatar_full": u.avatar_full_url(organization),
             }
 
     expected = [

engine/apps/api/tests/test_shift_swaps.py

@@ -61,7 +61,7 @@ def _construct_serialized_object(
                     "display_name": u.username,
                     "email": u.email,
                     "pk": u.public_primary_key,
-                    "avatar_full": u.avatar_full_url,
+                    "avatar_full": u.avatar_full_url(u.organization),
                 }
 
         data["beneficiary"] = _serialized_user(ssr.beneficiary)

engine/apps/api/tests/test_user.py

@@ -63,7 +63,7 @@ def test_current_user(
         "notification_chain_verbal": {"default": "", "important": ""},
         "slack_user_identity": None,
         "avatar": user.avatar_url,
-        "avatar_full": user.avatar_full_url,
+        "avatar_full": user.avatar_full_url(organization),
         "has_google_oauth2_connected": False,
         "google_calendar_settings": None,
         "google_oauth2_token_is_missing_scopes": False,
@@ -212,7 +212,7 @@ def test_update_user_cant_change_email_and_username(
         "notification_chain_verbal": {"default": "", "important": ""},
         "slack_user_identity": None,
         "avatar": admin.avatar_url,
-        "avatar_full": admin.avatar_full_url,
+        "avatar_full": admin.avatar_full_url(organization),
         "has_google_oauth2_connected": False,
         "google_calendar_settings": None,
     }
@@ -264,7 +264,7 @@ def test_list_users(
                 "notification_chain_verbal": {"default": "", "important": ""},
                 "slack_user_identity": None,
                 "avatar": admin.avatar_url,
-                "avatar_full": admin.avatar_full_url,
+                "avatar_full": admin.avatar_full_url(organization),
                 "cloud_connection_status": None,
                 "has_google_oauth2_connected": False,
             },
@@ -290,7 +290,7 @@ def test_list_users(
                 "notification_chain_verbal": {"default": "", "important": ""},
                 "slack_user_identity": None,
                 "avatar": editor.avatar_url,
-                "avatar_full": editor.avatar_full_url,
+                "avatar_full": editor.avatar_full_url(organization),
                 "cloud_connection_status": None,
                 "has_google_oauth2_connected": False,
             },

engine/apps/api/views/schedule.py

@@ -385,7 +385,9 @@ class ScheduleView(
 
         swap_requests = schedule.filter_swap_requests(datetime_start, datetime_end)
 
-        serialized_swap_requests = ShiftSwapRequestExpandedUsersListSerializer(swap_requests, many=True)
+        serialized_swap_requests = ShiftSwapRequestExpandedUsersListSerializer(
+            swap_requests, context={"request": self.request}, many=True
+        )
         result = {"shift_swaps": serialized_swap_requests.data}
 
         return Response(result, status=status.HTTP_200_OK)

engine/apps/base/models/user_notification_policy.py

@@ -7,7 +7,6 @@ from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.core.validators import MinLengthValidator
 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
@@ -128,13 +127,18 @@ class UserNotificationPolicy(OrderedModel):
 
     @classmethod
     def get_short_verbals_for_user(cls, user: User) -> Tuple[Tuple[str, ...], Tuple[str, ...]]:
-        is_wait_step = Q(step=cls.Step.WAIT)
-        is_wait_step_configured = Q(wait_delay__isnull=False)
+        policies = user.notification_policies.all()
 
-        policies = cls.objects.filter(Q(user=user, step__isnull=False) & (~is_wait_step | is_wait_step_configured))
+        default = ()
+        important = ()
 
-        default = tuple(str(policy.short_verbal) for policy in policies if policy.important is False)
-        important = tuple(str(policy.short_verbal) for policy in policies if policy.important is True)
+        for policy in policies:
+            if policy.step is None or (policy.step == cls.Step.WAIT and policy.wait_delay is None):
+                continue
+            if policy.important:
+                important += (policy.short_verbal,)
+            else:
+                default += (policy.short_verbal,)
 
         return default, important
 

engine/apps/base/models/user_notification_policy_log_record.py

@@ -171,9 +171,10 @@ class UserNotificationPolicyLogRecord(models.Model):
     def rendered_notification_log_line_json(self):
         time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
         created_at = DateTimeField().to_representation(self.created_at)
-        author = self.author.short() if self.author is not None else None
+        organization = self.alert_group.channel.organization
+        author = self.author.short(organization) if self.author is not None else None
 
-        sf = SlackFormatter(self.alert_group.channel.organization)
+        sf = SlackFormatter(organization)
         action = sf.format(self.render_log_line_action(substitute_author_with_tag=True))
         action = clean_markup(action)
 

engine/apps/schedules/models/on_call_schedule.py

@@ -407,7 +407,7 @@ class OnCallSchedule(PolymorphicModel):
                         "display_name": user.username,
                         "email": user.email,
                         "pk": user.public_primary_key,
-                        "avatar_full": user.avatar_full_url,
+                        "avatar_full": user.avatar_full_url(self.organization),
                     }
                     for user in shift["users"]
                 ],
@@ -780,13 +780,13 @@ class OnCallSchedule(PolymorphicModel):
                         user_to_swap["pk"] = swap.benefactor.public_primary_key
                         user_to_swap["display_name"] = swap.benefactor.username
                         user_to_swap["email"] = swap.benefactor.email
-                        user_to_swap["avatar_full"] = swap.benefactor.avatar_full_url
+                        user_to_swap["avatar_full"] = swap.benefactor.avatar_full_url(self.organization)
                         # add beneficiary user to details
                         swap_details["user"] = {
                             "display_name": swap.beneficiary.username,
                             "email": swap.beneficiary.email,
                             "pk": swap.beneficiary.public_primary_key,
-                            "avatar_full": swap.beneficiary.avatar_full_url,
+                            "avatar_full": swap.beneficiary.avatar_full_url(self.organization),
                         }
                     user_to_swap["swap_request"] = swap_details
 

engine/apps/schedules/tests/test_on_call_schedule.py

@@ -98,7 +98,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched
                     "display_name": user.username,
                     "pk": user.public_primary_key,
                     "email": user.email,
-                    "avatar_full": user.avatar_full_url,
+                    "avatar_full": user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": on_call_shift.public_primary_key},
@@ -127,7 +127,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched
                     "display_name": user.username,
                     "pk": user.public_primary_key,
                     "email": user.email,
-                    "avatar_full": user.avatar_full_url,
+                    "avatar_full": user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": override.public_primary_key},
@@ -199,7 +199,7 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio
                     "display_name": user.username,
                     "pk": user.public_primary_key,
                     "email": user.email,
-                    "avatar_full": user.avatar_full_url,
+                    "avatar_full": user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": on_call_shift.public_primary_key},
@@ -284,7 +284,7 @@ def test_filter_events_include_shift_info(
                     "display_name": user.username,
                     "pk": user.public_primary_key,
                     "email": user.email,
-                    "avatar_full": user.avatar_full_url,
+                    "avatar_full": user.avatar_full_url(organization),
                 },
             ],
             "shift": {
@@ -899,7 +899,7 @@ def test_preview_shift(make_organization, make_user_for_organization, make_sched
                     "display_name": other_user.username,
                     "pk": other_user.public_primary_key,
                     "email": other_user.email,
-                    "avatar_full": other_user.avatar_full_url,
+                    "avatar_full": other_user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": new_shift.public_primary_key},
@@ -1001,7 +1001,7 @@ def test_preview_shift_do_not_change_rotation_events(
                     "display_name": user.username,
                     "pk": user.public_primary_key,
                     "email": user.email,
-                    "avatar_full": user.avatar_full_url,
+                    "avatar_full": user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": on_call_shift.public_primary_key},
@@ -1137,7 +1137,7 @@ def test_preview_override_shift(make_organization, make_user_for_organization, m
                     "display_name": other_user.username,
                     "pk": other_user.public_primary_key,
                     "email": other_user.email,
-                    "avatar_full": other_user.avatar_full_url,
+                    "avatar_full": other_user.avatar_full_url(organization),
                 },
             ],
             "shift": {"pk": new_shift.public_primary_key},

engine/apps/user_management/models/user.py

@@ -195,9 +195,12 @@ class User(models.Model):
             return False
         return not google_utils.user_granted_all_required_scopes(self.google_oauth2_user.oauth_scope)
 
-    @property
-    def avatar_full_url(self):
-        return urljoin(self.organization.grafana_url, self.avatar_url)
+    def avatar_full_url(self, organization: "Organization"):
+        """
+        Use arg `organization` instead of `self.organization` to avoid multiple requests to db when getting avatar for
+        users list
+        """
+        return urljoin(organization.grafana_url, self.avatar_url)
 
     @property
     def verified_phone_number(self) -> str | None:
@@ -295,12 +298,12 @@ class User(models.Model):
 
         return day_start <= dt <= day_end
 
-    def short(self):
+    def short(self, organization):
         return {
             "username": self.username,
             "pk": self.public_primary_key,
             "avatar": self.avatar_url,
-            "avatar_full": self.avatar_full_url,
+            "avatar_full": self.avatar_full_url(organization),
         }
 
     # Insight logs

engine/apps/user_management/tests/test_sync.py

@@ -113,14 +113,14 @@ def test_sync_users_for_organization(make_organization, make_user_for_organizati
     assert updated_user is not None
     assert updated_user.name == api_users[0]["name"]
     assert updated_user.email == api_users[0]["email"]
-    assert updated_user.avatar_full_url == "https://test.test/test/1234"
+    assert updated_user.avatar_full_url(organization) == "https://test.test/test/1234"
 
     # check that missing users are created
     created_user = organization.users.filter(user_id=api_users[1]["userId"]).first()
     assert created_user is not None
     assert created_user.user_id == api_users[1]["userId"]
     assert created_user.name == api_users[1]["name"]
-    assert created_user.avatar_full_url == "https://test.test/test/1234"
+    assert created_user.avatar_full_url(organization) == "https://test.test/test/1234"
 
 
 @pytest.mark.django_db
@@ -532,7 +532,7 @@ def test_get_or_create_user(make_organization, make_team, make_user_for_organiza
     assert user.user_id == sync_user.id
     assert user.name == sync_user.name
     assert user.email == sync_user.email
-    assert user.avatar_full_url == sync_user.avatar_url
+    assert user.avatar_full_url(organization) == sync_user.avatar_url
     assert organization.users.count() == 2
     assert team.users.count() == 1