Skip to content

Commit 64bf1e5

Browse files
FerrilFerril
authored
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.
1 parent 27c7412 commit 64bf1e5

21 files changed

+81
-55
lines changed

‎engine/apps/alerts/models/alert_group.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,7 @@ def get_paged_users(self) -> typing.List[PagedUser]:
559559

560560
user_ids: typing.Set[str] = set()
561561
users: typing.Dict[str, PagedUser] = {}
562+
organization = self.channel.organization
562563

563564
log_records = self.log_records.filter(
564565
type__in=(AlertGroupLogRecord.TYPE_DIRECT_PAGING, AlertGroupLogRecord.TYPE_UNPAGE_USER)
@@ -594,7 +595,7 @@ def get_paged_users(self) -> typing.List[PagedUser]:
594595
"name": user.name,
595596
"username": user.username,
596597
"avatar": user.avatar_url,
597-
"avatar_full": user.avatar_full_url,
598+
"avatar_full": user.avatar_full_url(organization),
598599
"important": important,
599600
"teams": [{"pk": t.public_primary_key, "name": t.name} for t in user.teams.all()],
600601
}

‎engine/apps/alerts/models/alert_group_log_record.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,10 @@ class AlertGroupLogRecord(models.Model):
230230
def render_log_line_json(self):
231231
time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
232232
created_at = DateTimeField().to_representation(self.created_at)
233-
author = self.author.short() if self.author is not None else None
233+
organization = self.alert_group.channel.organization
234+
author = self.author.short(organization) if self.author is not None else None
234235

235-
sf = SlackFormatter(self.alert_group.channel.organization)
236+
sf = SlackFormatter(organization)
236237
action = sf.format(self.rendered_log_line_action(substitute_author_with_tag=True))
237238
action = clean_markup(action)
238239

‎engine/apps/alerts/models/resolution_note.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,10 @@ def recreate(self):
179179
def render_log_line_json(self):
180180
time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
181181
created_at = DateTimeField().to_representation(self.created_at)
182-
author = self.author.short() if self.author is not None else None
182+
organization = self.alert_group.channel.organization
183+
author = self.author.short(organization) if self.author is not None else None
183184

184-
sf = SlackFormatter(self.alert_group.channel.organization)
185+
sf = SlackFormatter(organization)
185186
action = sf.format(self.text)
186187
action = clean_markup(action)
187188

‎engine/apps/alerts/tasks/notify_ical_schedule_shift.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def convert_prev_shifts_to_new_format(prev_shifts: dict, schedule: "OnCallSchedu
3737
"display_name": user.username,
3838
"email": user.email,
3939
"pk": user.public_primary_key,
40-
"avatar_full": user.avatar_full_url,
40+
"avatar_full": user.avatar_full_url(schedule.organization),
4141
},
4242
)
4343
for uid, shift in prev_shifts.items():

‎engine/apps/api/serializers/alert_group.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ def get_related_users(self, obj: "AlertGroup"):
232232
if log_record.author is not None and log_record.author.public_primary_key not in users_ids:
233233
users.append(log_record.author)
234234
users_ids.add(log_record.author.public_primary_key)
235-
return UserShortSerializer(users, many=True).data
235+
return UserShortSerializer(users, context=self.context, many=True).data
236236

237237

238238
class AlertGroupSerializer(AlertGroupListSerializer):

‎engine/apps/api/serializers/schedule_base.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ def get_warnings(self, obj):
6969
def get_on_call_now(self, obj):
7070
# Serializer context is set here: apps.api.views.schedule.ScheduleView.get_serializer_context
7171
users = self.context["oncall_users"].get(obj, [])
72-
return [user.short() for user in users]
72+
organization = self.context["request"].auth.organization
73+
return [user.short(organization) for user in users]
7374

7475
def get_number_of_escalation_chains(self, obj):
7576
# num_escalation_chains param added in queryset via annotate. Check ScheduleView.get_queryset

‎engine/apps/api/serializers/schedule_polymorphic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313

1414
class PolymorphicScheduleSerializer(EagerLoadingMixin, PolymorphicSerializer):
15-
SELECT_RELATED = ["organization", "user_group"]
15+
SELECT_RELATED = ["organization", "user_group", "team"]
1616

1717
resource_type_field_name = "type"
1818

‎engine/apps/api/serializers/shift_swap.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,14 @@ class ShiftSwapRequestExpandedUsersListSerializer(BaseShiftSwapRequestListSerial
103103
def _serialize_user(self, user: "User") -> dict | None:
104104
user_data = None
105105
if user:
106+
organization = (
107+
self.context["request"].auth.organization if self.context.get("request") else user.organization
108+
)
106109
user_data = {
107110
"display_name": user.username,
108111
"email": user.email,
109112
"pk": user.public_primary_key,
110-
"avatar_full": user.avatar_full_url,
113+
"avatar_full": user.avatar_full_url(organization),
111114
}
112115
return user_data
113116

‎engine/apps/api/serializers/user.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin):
8383

8484
timezone = TimeZoneField(allow_null=True, required=False)
8585
avatar = serializers.URLField(source="avatar_url", read_only=True)
86-
avatar_full = serializers.URLField(source="avatar_full_url", read_only=True)
86+
avatar_full = serializers.SerializerMethodField()
8787
notification_chain_verbal = serializers.SerializerMethodField()
8888
cloud_connection_status = serializers.SerializerMethodField()
8989
working_hours = WorkingHoursSerializer(required=False)
@@ -96,6 +96,7 @@ class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin):
9696
"mobileappauthtoken",
9797
"google_oauth2_user",
9898
]
99+
PREFETCH_RELATED = ["notification_policies"]
99100

100101
class Meta:
101102
model = User
@@ -165,6 +166,10 @@ def get_notification_chain_verbal(self, obj: User) -> NotificationChainVerbal:
165166
default, important = UserNotificationPolicy.get_short_verbals_for_user(user=obj)
166167
return {"default": " - ".join(default), "important": " - ".join(important)}
167168

169+
def get_avatar_full(self, obj):
170+
organization = self.context["request"].auth.organization if self.context.get("request") else obj.organization
171+
return obj.avatar_full_url(organization)
172+
168173
def get_cloud_connection_status(self, obj: User) -> CloudSyncStatus | None:
169174
is_open_source_with_cloud_notifications = self.context.get("is_open_source_with_cloud_notifications", None)
170175
is_open_source_with_cloud_notifications = (
@@ -307,7 +312,7 @@ class UserShortSerializer(serializers.ModelSerializer):
307312
username = serializers.CharField()
308313
pk = serializers.CharField(source="public_primary_key")
309314
avatar = serializers.CharField(source="avatar_url")
310-
avatar_full = serializers.CharField(source="avatar_full_url")
315+
avatar_full = serializers.SerializerMethodField()
311316

312317
class Meta:
313318
model = User
@@ -324,6 +329,10 @@ class Meta:
324329
"avatar_full",
325330
]
326331

332+
def get_avatar_full(self, obj):
333+
organization = self.context["request"].auth.organization if self.context.get("request") else obj.organization
334+
return obj.avatar_full_url(organization)
335+
327336

328337
class UserIsCurrentlyOnCallSerializer(UserShortSerializer, EagerLoadingMixin):
329338
context: UserSerializerContext

‎engine/apps/api/tests/test_alert_group.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2083,7 +2083,7 @@ def test_alert_group_paged_users(
20832083
assert response.json()["paged_users"] == [
20842084
{
20852085
"avatar": user2.avatar_url,
2086-
"avatar_full": user2.avatar_full_url,
2086+
"avatar_full": user2.avatar_full_url(user.organization),
20872087
"id": user2.pk,
20882088
"pk": user2.public_primary_key,
20892089
"important": None,

‎engine/apps/api/tests/test_oncall_shift.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1648,7 +1648,7 @@ def test_on_call_shift_preview(
16481648
"display_name": other_user.username,
16491649
"pk": other_user.public_primary_key,
16501650
"email": other_user.email,
1651-
"avatar_full": other_user.avatar_full_url,
1651+
"avatar_full": other_user.avatar_full_url(organization),
16521652
},
16531653
],
16541654
"source": "web",
@@ -1978,7 +1978,7 @@ def test_on_call_shift_preview_update(
19781978
"display_name": other_user.username,
19791979
"pk": other_user.public_primary_key,
19801980
"email": other_user.email,
1981-
"avatar_full": other_user.avatar_full_url,
1981+
"avatar_full": other_user.avatar_full_url(organization),
19821982
},
19831983
],
19841984
"source": "web",
@@ -2093,7 +2093,7 @@ def test_on_call_shift_preview_update_not_started_reuse_pk(
20932093
"display_name": other_user.username,
20942094
"pk": other_user.public_primary_key,
20952095
"email": other_user.email,
2096-
"avatar_full": other_user.avatar_full_url,
2096+
"avatar_full": other_user.avatar_full_url(organization),
20972097
},
20982098
],
20992099
"source": "web",

‎engine/apps/api/tests/test_schedules.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ def test_get_detail_schedule_oncall_now_multipage_objects(
612612
"pk": user.public_primary_key,
613613
"username": user.username,
614614
"avatar": user.avatar_url,
615-
"avatar_full": user.avatar_full_url,
615+
"avatar_full": user.avatar_full_url(organization),
616616
}
617617
],
618618
"has_gaps": False,
@@ -916,7 +916,7 @@ def test_events_calendar(
916916
"display_name": user.username,
917917
"pk": user.public_primary_key,
918918
"email": user.email,
919-
"avatar_full": user.avatar_full_url,
919+
"avatar_full": user.avatar_full_url(organization),
920920
},
921921
],
922922
"missing_users": [],
@@ -989,7 +989,7 @@ def test_filter_events_calendar(
989989
"display_name": user.username,
990990
"pk": user.public_primary_key,
991991
"email": user.email,
992-
"avatar_full": user.avatar_full_url,
992+
"avatar_full": user.avatar_full_url(organization),
993993
},
994994
],
995995
"missing_users": [],
@@ -1014,7 +1014,7 @@ def test_filter_events_calendar(
10141014
"display_name": user.username,
10151015
"pk": user.public_primary_key,
10161016
"email": user.email,
1017-
"avatar_full": user.avatar_full_url,
1017+
"avatar_full": user.avatar_full_url(organization),
10181018
}
10191019
],
10201020
"missing_users": [],
@@ -1106,7 +1106,7 @@ def test_filter_events_range_calendar(
11061106
"display_name": user.username,
11071107
"pk": user.public_primary_key,
11081108
"email": user.email,
1109-
"avatar_full": user.avatar_full_url,
1109+
"avatar_full": user.avatar_full_url(organization),
11101110
},
11111111
],
11121112
"missing_users": [],
@@ -1197,7 +1197,7 @@ def test_filter_events_overrides(
11971197
"display_name": other_user.username,
11981198
"pk": other_user.public_primary_key,
11991199
"email": other_user.email,
1200-
"avatar_full": other_user.avatar_full_url,
1200+
"avatar_full": other_user.avatar_full_url(organization),
12011201
}
12021202
],
12031203
"missing_users": [],
@@ -1395,7 +1395,7 @@ def _serialized_user(u):
13951395
"display_name": u.username,
13961396
"email": u.email,
13971397
"pk": u.public_primary_key,
1398-
"avatar_full": u.avatar_full_url,
1398+
"avatar_full": u.avatar_full_url(organization),
13991399
}
14001400

14011401
expected = [

‎engine/apps/api/tests/test_shift_swaps.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def _serialized_user(u):
6161
"display_name": u.username,
6262
"email": u.email,
6363
"pk": u.public_primary_key,
64-
"avatar_full": u.avatar_full_url,
64+
"avatar_full": u.avatar_full_url(u.organization),
6565
}
6666

6767
data["beneficiary"] = _serialized_user(ssr.beneficiary)

‎engine/apps/api/tests/test_user.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def test_current_user(
6363
"notification_chain_verbal": {"default": "", "important": ""},
6464
"slack_user_identity": None,
6565
"avatar": user.avatar_url,
66-
"avatar_full": user.avatar_full_url,
66+
"avatar_full": user.avatar_full_url(organization),
6767
"has_google_oauth2_connected": False,
6868
"google_calendar_settings": None,
6969
"google_oauth2_token_is_missing_scopes": False,
@@ -212,7 +212,7 @@ def test_update_user_cant_change_email_and_username(
212212
"notification_chain_verbal": {"default": "", "important": ""},
213213
"slack_user_identity": None,
214214
"avatar": admin.avatar_url,
215-
"avatar_full": admin.avatar_full_url,
215+
"avatar_full": admin.avatar_full_url(organization),
216216
"has_google_oauth2_connected": False,
217217
"google_calendar_settings": None,
218218
}
@@ -264,7 +264,7 @@ def test_list_users(
264264
"notification_chain_verbal": {"default": "", "important": ""},
265265
"slack_user_identity": None,
266266
"avatar": admin.avatar_url,
267-
"avatar_full": admin.avatar_full_url,
267+
"avatar_full": admin.avatar_full_url(organization),
268268
"cloud_connection_status": None,
269269
"has_google_oauth2_connected": False,
270270
},
@@ -290,7 +290,7 @@ def test_list_users(
290290
"notification_chain_verbal": {"default": "", "important": ""},
291291
"slack_user_identity": None,
292292
"avatar": editor.avatar_url,
293-
"avatar_full": editor.avatar_full_url,
293+
"avatar_full": editor.avatar_full_url(organization),
294294
"cloud_connection_status": None,
295295
"has_google_oauth2_connected": False,
296296
},

‎engine/apps/api/views/schedule.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,9 @@ def filter_shift_swaps(self, request: Request, pk: str) -> Response:
385385

386386
swap_requests = schedule.filter_swap_requests(datetime_start, datetime_end)
387387

388-
serialized_swap_requests = ShiftSwapRequestExpandedUsersListSerializer(swap_requests, many=True)
388+
serialized_swap_requests = ShiftSwapRequestExpandedUsersListSerializer(
389+
swap_requests, context={"request": self.request}, many=True
390+
)
389391
result = {"shift_swaps": serialized_swap_requests.data}
390392

391393
return Response(result, status=status.HTTP_200_OK)

‎engine/apps/base/models/user_notification_policy.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from django.core.exceptions import ValidationError
88
from django.core.validators import MinLengthValidator
99
from django.db import models
10-
from django.db.models import Q
1110

1211
from apps.base.messaging import get_messaging_backends
1312
from apps.user_management.models import User
@@ -128,13 +127,18 @@ def __str__(self):
128127

129128
@classmethod
130129
def get_short_verbals_for_user(cls, user: User) -> Tuple[Tuple[str, ...], Tuple[str, ...]]:
131-
is_wait_step = Q(step=cls.Step.WAIT)
132-
is_wait_step_configured = Q(wait_delay__isnull=False)
130+
policies = user.notification_policies.all()
133131

134-
policies = cls.objects.filter(Q(user=user, step__isnull=False) & (~is_wait_step | is_wait_step_configured))
132+
default = ()
133+
important = ()
135134

136-
default = tuple(str(policy.short_verbal) for policy in policies if policy.important is False)
137-
important = tuple(str(policy.short_verbal) for policy in policies if policy.important is True)
135+
for policy in policies:
136+
if policy.step is None or (policy.step == cls.Step.WAIT and policy.wait_delay is None):
137+
continue
138+
if policy.important:
139+
important += (policy.short_verbal,)
140+
else:
141+
default += (policy.short_verbal,)
138142

139143
return default, important
140144

‎engine/apps/base/models/user_notification_policy_log_record.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ def rendered_notification_log_line(self, for_slack=False, html=False):
171171
def rendered_notification_log_line_json(self):
172172
time = humanize.naturaldelta(self.alert_group.started_at - self.created_at)
173173
created_at = DateTimeField().to_representation(self.created_at)
174-
author = self.author.short() if self.author is not None else None
174+
organization = self.alert_group.channel.organization
175+
author = self.author.short(organization) if self.author is not None else None
175176

176-
sf = SlackFormatter(self.alert_group.channel.organization)
177+
sf = SlackFormatter(organization)
177178
action = sf.format(self.render_log_line_action(substitute_author_with_tag=True))
178179
action = clean_markup(action)
179180

‎engine/apps/schedules/models/on_call_schedule.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ def filter_events(
407407
"display_name": user.username,
408408
"email": user.email,
409409
"pk": user.public_primary_key,
410-
"avatar_full": user.avatar_full_url,
410+
"avatar_full": user.avatar_full_url(self.organization),
411411
}
412412
for user in shift["users"]
413413
],
@@ -780,13 +780,13 @@ def _insert_event(index: int, event: ScheduleEvent) -> int:
780780
user_to_swap["pk"] = swap.benefactor.public_primary_key
781781
user_to_swap["display_name"] = swap.benefactor.username
782782
user_to_swap["email"] = swap.benefactor.email
783-
user_to_swap["avatar_full"] = swap.benefactor.avatar_full_url
783+
user_to_swap["avatar_full"] = swap.benefactor.avatar_full_url(self.organization)
784784
# add beneficiary user to details
785785
swap_details["user"] = {
786786
"display_name": swap.beneficiary.username,
787787
"email": swap.beneficiary.email,
788788
"pk": swap.beneficiary.public_primary_key,
789-
"avatar_full": swap.beneficiary.avatar_full_url,
789+
"avatar_full": swap.beneficiary.avatar_full_url(self.organization),
790790
}
791791
user_to_swap["swap_request"] = swap_details
792792

0 commit comments

Comments
 (0)