Skip to content

Commit

Permalink
few add responders patches (#3220)
Browse files Browse the repository at this point in the history
# Which issue(s) this PR fixes

Closes grafana/support-escalations#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)
  • Loading branch information
joeyorlando authored Oct 31, 2023
1 parent 6693ed6 commit 415509d
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
}

.table {
max-height: 150px;
overflow: auto;
padding: 4px 0px;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
}
Expand Down
28 changes: 12 additions & 16 deletions src/models/grafana_team/grafana_team.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -29,37 +31,31 @@ 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',
},
});

this.items = {
...this.items,
...result.reduce(
(acc: { [key: number]: GrafanaTeam }, item: GrafanaTeam) => ({
...result.reduce<TeamItems>(
(acc, item) => ({
...acc,
[item.id]: item,
}),
{}
),
};

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]);
}
}
2 changes: 1 addition & 1 deletion src/models/grafana_team/grafana_team.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
4 changes: 2 additions & 2 deletions src/models/user/user.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

0 comments on commit 415509d

Please sign in to comment.