diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index dbfd97ad..3fceaec0 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/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()], } diff --git a/engine/apps/alerts/models/alert_group_log_record.py b/engine/apps/alerts/models/alert_group_log_record.py index 7da4f247..3c4113a2 100644 --- a/engine/apps/alerts/models/alert_group_log_record.py +++ b/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) diff --git a/engine/apps/alerts/models/resolution_note.py b/engine/apps/alerts/models/resolution_note.py index b60f70d8..1624cb65 100644 --- a/engine/apps/alerts/models/resolution_note.py +++ b/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) diff --git a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py index 9294353b..e872623d 100644 --- a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py +++ b/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(): diff --git a/engine/apps/api/serializers/alert_group.py b/engine/apps/api/serializers/alert_group.py index 27c0910e..d4b2e65d 100644 --- a/engine/apps/api/serializers/alert_group.py +++ b/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): diff --git a/engine/apps/api/serializers/schedule_base.py b/engine/apps/api/serializers/schedule_base.py index 98ad615d..02de45d8 100644 --- a/engine/apps/api/serializers/schedule_base.py +++ b/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 diff --git a/engine/apps/api/serializers/schedule_polymorphic.py b/engine/apps/api/serializers/schedule_polymorphic.py index 29000e41..e6588652 100644 --- a/engine/apps/api/serializers/schedule_polymorphic.py +++ b/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" diff --git a/engine/apps/api/serializers/shift_swap.py b/engine/apps/api/serializers/shift_swap.py index 5cea7744..516958d8 100644 --- a/engine/apps/api/serializers/shift_swap.py +++ b/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 diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index 2133b80f..b9280e9b 100644 --- a/engine/apps/api/serializers/user.py +++ b/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 diff --git a/engine/apps/api/tests/test_alert_group.py b/engine/apps/api/tests/test_alert_group.py index e9564b86..42808e85 100644 --- a/engine/apps/api/tests/test_alert_group.py +++ b/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, diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index 9ebaa9f6..e8a3a6f6 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/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", diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index e0477ec0..65dcd588 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/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 = [ diff --git a/engine/apps/api/tests/test_shift_swaps.py b/engine/apps/api/tests/test_shift_swaps.py index 229fa1b1..ae384f40 100644 --- a/engine/apps/api/tests/test_shift_swaps.py +++ b/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) diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index a4468f89..900a48eb 100644 --- a/engine/apps/api/tests/test_user.py +++ b/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, }, diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 261851d6..b6d50c3d 100644 --- a/engine/apps/api/views/schedule.py +++ b/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) diff --git a/engine/apps/base/models/user_notification_policy.py b/engine/apps/base/models/user_notification_policy.py index 40855dcc..deb785f2 100644 --- a/engine/apps/base/models/user_notification_policy.py +++ b/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 diff --git a/engine/apps/base/models/user_notification_policy_log_record.py b/engine/apps/base/models/user_notification_policy_log_record.py index 2d2f8a2a..3d7ab44e 100644 --- a/engine/apps/base/models/user_notification_policy_log_record.py +++ b/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) diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 78636333..126b6204 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/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 diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index 4e249d67..77308544 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/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}, diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index 376dd82a..a5bff7aa 100644 --- a/engine/apps/user_management/models/user.py +++ b/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 diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 8399d3e1..43015b4a 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/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