few add responders patches (#3220)

# Which issue(s) this PR fixes

Closes https://github.com/grafana/support-escalations/issues/8143

Fix a few minor issues introduced in #3128:

- Fix slow `GET /users` internal API endpoint related to [this
change](https://github.com/grafana/oncall/blob/dev/engine/apps/api/views/user.py#L239)
- Fix slow `GET /teams` internal API endpoint. Introduced a `short`
query parameter that only invokes
`apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules` when
`short=false`.
- Order results from `GET /teams` internal API endpoint by name
(ascending)
- Fix search issue when searching for teams in the add responders popup
window (this was strictly a frontend issue)
- CSS changes to add responders dropdown to fix lonnnggg results list:
  **Before**
<img width="377" alt="Screenshot 2023-10-31 at 10 06 20"
src="https://github.com/grafana/oncall/assets/9406895/246c7c3b-7bea-44a1-afec-a38144c2c2d1">
  **After**
<img width="444" alt="Screenshot 2023-10-31 at 10 48 12"
src="https://github.com/grafana/oncall/assets/9406895/b5602a22-c691-4dc7-bd3d-e4d6b76d04a0">



## Still todo

The `apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules`
method is still very slow when an instance has a lot of users (ex.
`ops`). Ideally we should refactor this method to be more efficient
because we still need to call this method under some circumstances. Ex.
to populate this dropdown when Direct Paging a user (note that it didn't
finish loading here on `ops`):
<img width="1037" alt="Screenshot 2023-10-30 at 18 14 59"
src="https://github.com/grafana/oncall/assets/9406895/9d91a57c-5db8-4ff9-862a-cd3755f52690">



## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)

CHANGELOG.md

@@ -12,9 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removed the hardcoding of page size on frontend ([#3205](https://github.com/grafana/oncall/pull/3205))
 - Prevent additional polling on Incidents if the previous request didn't complete
   ([#3205](https://github.com/grafana/oncall/pull/3205))
+- Order results from `GET /teams` internal API endpoint by ascending name by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220))
 
 ### Fixed
 
+- Improve slow `GET /users` + `GET /teams` internal API endpoints by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220))
+- Fix search issue when searching for teams in the add responders popup window by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220))
+- CSS changes to add responders dropdown to fix long search results list by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220))
 - Do not allow to update terraform-based shifts in web UI schedule API ([#3224](https://github.com/grafana/oncall/pull/3224))
 
 ## v1.3.48 (2023-10-30)

engine/apps/api/serializers/team.py

@@ -19,21 +19,17 @@ class FastTeamSerializer(serializers.ModelSerializer):
 
 
 class TeamSerializer(serializers.ModelSerializer):
-    context: TeamSerializerContext
-
     id = serializers.CharField(read_only=True, source="public_primary_key")
-    number_of_users_currently_oncall = serializers.SerializerMethodField()
 
     class Meta:
         model = Team
-        fields = (
+        fields = [
             "id",
             "name",
             "email",
             "avatar_url",
             "is_sharing_resources_to_all",
-            "number_of_users_currently_oncall",
-        )
+        ]
 
         read_only_fields = [
             "id",
@@ -42,6 +38,17 @@ class TeamSerializer(serializers.ModelSerializer):
             "avatar_url",
         ]
 
+
+class TeamLongSerializer(TeamSerializer):
+    context: TeamSerializerContext
+
+    number_of_users_currently_oncall = serializers.SerializerMethodField()
+
+    class Meta(TeamSerializer.Meta):
+        fields = TeamSerializer.Meta.fields + [
+            "number_of_users_currently_oncall",
+        ]
+
     def get_number_of_users_currently_oncall(self, obj: Team) -> int:
         num_of_users_oncall_for_team = 0
 

engine/apps/api/tests/test_team.py

@@ -14,16 +14,19 @@ from apps.user_management.models import Team
 GENERAL_TEAM = Team(public_primary_key="null", name="No team", email=None, avatar_url=None)
 
 
-def get_payload_from_team(team):
-    return {
+def get_payload_from_team(team, long=False):
+    payload = {
         "id": team.public_primary_key,
         "name": team.name,
         "email": team.email,
         "avatar_url": team.avatar_url,
         "is_sharing_resources_to_all": team.is_sharing_resources_to_all,
-        "number_of_users_currently_oncall": 0,
     }
 
+    if long:
+        payload.update({"number_of_users_currently_oncall": 0})
+    return payload
+
 
 @pytest.mark.django_db
 def test_list_teams(
@@ -40,22 +43,36 @@ def test_list_teams(
     team = make_team(organization)
     team.users.add(user)
 
+    auth_headers = make_user_auth_headers(user, token)
+
     general_team_payload = get_payload_from_team(GENERAL_TEAM)
+    general_team_long_payload = get_payload_from_team(GENERAL_TEAM, long=True)
     team_payload = get_payload_from_team(team)
+    team_long_payload = get_payload_from_team(team, long=True)
 
     client = APIClient()
     url = reverse("api-internal:team-list")
-    response = client.get(url, format="json", **make_user_auth_headers(user, token))
+    response = client.get(url, format="json", **auth_headers)
 
     assert response.status_code == status.HTTP_200_OK
     assert response.json() == [general_team_payload, team_payload]
 
+    response = client.get(f"{url}?short=false", format="json", **auth_headers)
+
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json() == [general_team_long_payload, team_long_payload]
+
     url = reverse("api-internal:team-list")
-    response = client.get(f"{url}?include_no_team=false", format="json", **make_user_auth_headers(user, token))
+    response = client.get(f"{url}?include_no_team=false", format="json", **auth_headers)
 
     assert response.status_code == status.HTTP_200_OK
     assert response.json() == [team_payload]
 
+    response = client.get(f"{url}?include_no_team=false&short=false", format="json", **auth_headers)
+
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json() == [team_long_payload]
+
 
 @pytest.mark.django_db
 def test_list_teams_only_include_notifiable_teams(
@@ -146,7 +163,7 @@ def test_teams_number_of_users_currently_oncall_attribute_works_properly(
     _make_schedule(team=team3, oncall_users=[])
 
     client = APIClient()
-    url = reverse("api-internal:team-list")
+    url = f"{reverse('api-internal:team-list')}?short=false"
 
     response = client.get(url, format="json", **make_user_auth_headers(user1, token))
 

engine/apps/api/views/team.py

@@ -6,7 +6,7 @@ from rest_framework.response import Response
 
 from apps.alerts.paging import integration_is_notifiable
 from apps.api.permissions import RBACPermission
-from apps.api.serializers.team import TeamSerializer
+from apps.api.serializers.team import TeamLongSerializer, TeamSerializer
 from apps.auth_token.auth import PluginAuthentication
 from apps.mobile_app.auth import MobileAppAuthTokenAuthentication
 from apps.schedules.ical_utils import get_oncall_users_for_multiple_schedules
@@ -33,6 +33,9 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod
     def get_queryset(self):
         return self.request.user.available_teams
 
+    def _is_long_request(self) -> bool:
+        return self.request.query_params.get("short", "true").lower() == "false"
+
     @cached_property
     def schedules_with_oncall_users(self):
         """
@@ -45,9 +48,14 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod
 
     def get_serializer_context(self):
         context = super().get_serializer_context()
-        context.update({"schedules_with_oncall_users": self.schedules_with_oncall_users})
+        context.update(
+            {"schedules_with_oncall_users": self.schedules_with_oncall_users if self._is_long_request() else {}}
+        )
         return context
 
+    def get_serializer_class(self):
+        return TeamLongSerializer if self._is_long_request() else TeamSerializer
+
     def list(self, request, *args, **kwargs):
         general_team = [Team(public_primary_key="null", name="No team", email=None, avatar_url=None)]
         queryset = self.filter_queryset(self.get_queryset())
@@ -62,6 +70,8 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod
 
             queryset = queryset.filter(pk__in=team_ids)
 
+        queryset = queryset.order_by("name")
+
         teams = list(queryset)
         if self.request.query_params.get("include_no_team", "true") != "false":
             # Adds general team to the queryset in a way that it always shows up first (even when not searched for).

engine/apps/api/views/user.py

@@ -234,9 +234,27 @@ class UserView(
         """
         return get_oncall_users_for_multiple_schedules(self.request.user.organization.oncall_schedules.all())
 
+    def _get_is_currently_oncall_query_param(self) -> str:
+        return self.request.query_params.get("is_currently_oncall", "").lower()
+
+    def _is_currently_oncall_request(self) -> bool:
+        return self._get_is_currently_oncall_query_param() in ["true", "false"]
+
+    def _is_long_request(self) -> bool:
+        return self.request.query_params.get("short", "true").lower() == "false"
+
+    def _is_currently_oncall_or_long_request(self) -> bool:
+        return self._is_currently_oncall_request() or self._is_long_request()
+
     def get_serializer_context(self):
         context = super().get_serializer_context()
-        context.update({"schedules_with_oncall_users": self.schedules_with_oncall_users})
+        context.update(
+            {
+                "schedules_with_oncall_users": self.schedules_with_oncall_users
+                if self._is_currently_oncall_or_long_request()
+                else {}
+            }
+        )
         return context
 
     def get_serializer_class(self):
@@ -247,12 +265,10 @@ class UserView(
 
         is_list_request = self.action in ["list"]
         is_filters_request = query_params.get("filters", "false") == "true"
-        is_short_request = query_params.get("short", "true") == "false"
-        is_currently_oncall_request = query_params.get("is_currently_oncall", "").lower() in ["true", "false"]
 
         if is_list_request and is_filters_request:
             return self.get_filter_serializer_class()
-        elif is_list_request and (is_short_request or is_currently_oncall_request):
+        elif is_list_request and self._is_currently_oncall_or_long_request():
             return UserLongSerializer
 
         is_users_own_data = kwargs.get("pk") is not None and kwargs.get("pk") == user.public_primary_key
@@ -277,11 +293,10 @@ class UserView(
     def list(self, request, *args, **kwargs) -> Response:
         queryset = self.filter_queryset(self.get_queryset())
 
-        is_currently_oncall_query_param = request.query_params.get("is_currently_oncall", "").lower()
-
         def _get_oncall_user_ids():
             return {user.pk for _, users in self.schedules_with_oncall_users.items() for user in users}
 
+        is_currently_oncall_query_param = self._get_is_currently_oncall_query_param()
         if is_currently_oncall_query_param == "true":
             # client explicitly wants to filter out users that are on-call
             queryset = queryset.filter(pk__in=_get_oncall_user_ids())

grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss

@@ -31,6 +31,7 @@
 }
 
 .table {
+  max-height: 150px;
   overflow: auto;
   padding: 4px 0px;
 

grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx

@@ -115,7 +115,7 @@ const AddRespondersPopup = observer(
 
     const handleSearchTermChange = useDebouncedCallback(() => {
       if (isCreateMode && activeOption === TabOptions.Teams) {
-        grafanaTeamStore.updateItems(searchTerm, false, true);
+        grafanaTeamStore.updateItems(searchTerm, false, true, false);
       } else {
         userStore.updateItems({ searchTerm, short: 'false' });
       }

grafana-plugin/src/models/grafana_team/grafana_team.ts

@@ -5,12 +5,14 @@ import { GrafanaTeam } from 'models/grafana_team/grafana_team.types';
 import { makeRequest } from 'network';
 import { RootStore } from 'state';
 
+type TeamItems = { [id: string]: GrafanaTeam };
+
 export class GrafanaTeamStore extends BaseStore {
   @observable
-  searchResult: { [key: string]: Array<GrafanaTeam['id']> } = {};
+  searchResult: Array<GrafanaTeam['id']> = [];
 
   @observable.shallow
-  items: { [id: string]: GrafanaTeam } = {};
+  items: TeamItems = {};
 
   constructor(rootStore: RootStore) {
     super(rootStore);
@@ -29,10 +31,11 @@ export class GrafanaTeamStore extends BaseStore {
   }
 
   @action
-  async updateItems(query = '', includeNoTeam = true, onlyIncludeNotifiableTeams = false) {
-    const result = await makeRequest(`${this.path}`, {
+  async updateItems(query = '', includeNoTeam = true, onlyIncludeNotifiableTeams = false, short = true) {
+    const result = await makeRequest<GrafanaTeam[]>(`${this.path}`, {
       params: {
         search: query,
+        short: short ? 'true' : 'false',
         include_no_team: includeNoTeam ? 'true' : 'false',
         only_include_notifiable_teams: onlyIncludeNotifiableTeams ? 'true' : 'false',
       },
@@ -40,8 +43,8 @@ export class GrafanaTeamStore extends BaseStore {
 
     this.items = {
       ...this.items,
-      ...result.reduce(
-        (acc: { [key: number]: GrafanaTeam }, item: GrafanaTeam) => ({
+      ...result.reduce<TeamItems>(
+        (acc, item) => ({
           ...acc,
           [item.id]: item,
         }),
@@ -49,17 +52,10 @@ export class GrafanaTeamStore extends BaseStore {
       ),
     };
 
-    this.searchResult = {
-      ...this.searchResult,
-      [query]: result.map((item: GrafanaTeam) => item.id),
-    };
+    this.searchResult = result.map((item: GrafanaTeam) => item.id);
   }
 
-  getSearchResult(query = '') {
-    if (!this.searchResult[query]) {
-      return [];
-    }
-
-    return this.searchResult[query].map((teamId: GrafanaTeam['id']) => this.items[teamId]);
+  getSearchResult() {
+    return this.searchResult.map((teamId: GrafanaTeam['id']) => this.items[teamId]);
   }
 }

grafana-plugin/src/models/grafana_team/grafana_team.types.ts

@@ -4,5 +4,5 @@ export interface GrafanaTeam {
   email: string;
   avatar_url: string;
   is_sharing_resources_to_all: boolean;
-  number_of_users_currently_oncall: number;
+  number_of_users_currently_oncall?: number;
 }

grafana-plugin/src/models/user/user.types.ts

@@ -43,6 +43,6 @@ export interface User {
   hidden_fields?: boolean;
   timezone: Timezone;
   working_hours: { [key: string]: [] };
-  is_currently_oncall: boolean;
-  teams: GrafanaTeam[];
+  is_currently_oncall?: boolean;
+  teams?: GrafanaTeam[];
 }