diff --git a/CHANGELOG.md b/CHANGELOG.md index f035ec91c5..52c8e2f510 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/engine/apps/api/serializers/team.py b/engine/apps/api/serializers/team.py index dd7e172dec..b7263bcd57 100644 --- a/engine/apps/api/serializers/team.py +++ b/engine/apps/api/serializers/team.py @@ -19,21 +19,17 @@ class Meta: 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 Meta: "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 diff --git a/engine/apps/api/tests/test_team.py b/engine/apps/api/tests/test_team.py index a5a93a9a82..69a8afbe9d 100644 --- a/engine/apps/api/tests/test_team.py +++ b/engine/apps/api/tests/test_team.py @@ -14,16 +14,19 @@ 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 _make_schedule(team=None, oncall_users=[]): _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)) diff --git a/engine/apps/api/views/team.py b/engine/apps/api/views/team.py index 4e6dd48126..47c10e118a 100644 --- a/engine/apps/api/views/team.py +++ b/engine/apps/api/views/team.py @@ -6,7 +6,7 @@ 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 @@ def schedules_with_oncall_users(self): 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 @@ def list(self, request, *args, **kwargs): 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). diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index b6df3df3a3..c6a51fc71e 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -234,9 +234,27 @@ def schedules_with_oncall_users(self): """ 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 @@ def get_serializer_class(self): 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 @@ def get_queryset(self): 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()) diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss index 2c2f92f501..fbbd2163d3 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss @@ -31,6 +31,7 @@ } .table { + max-height: 150px; overflow: auto; padding: 4px 0px; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx index d36c3532a2..3aa328dd7e 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx +++ b/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' }); } diff --git a/grafana-plugin/src/models/grafana_team/grafana_team.ts b/grafana-plugin/src/models/grafana_team/grafana_team.ts index bb0b3cee80..023150be43 100644 --- a/grafana-plugin/src/models/grafana_team/grafana_team.ts +++ b/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 } = {}; + searchResult: Array = []; @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(`${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( + (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]); } } diff --git a/grafana-plugin/src/models/grafana_team/grafana_team.types.ts b/grafana-plugin/src/models/grafana_team/grafana_team.types.ts index 8b0af3070d..97210b8b6c 100644 --- a/grafana-plugin/src/models/grafana_team/grafana_team.types.ts +++ b/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; } diff --git a/grafana-plugin/src/models/user/user.types.ts b/grafana-plugin/src/models/user/user.types.ts index a1bad61be3..11d6247b7d 100644 --- a/grafana-plugin/src/models/user/user.types.ts +++ b/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[]; }