Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

few add responders patches #3220

Merged
merged 10 commits into from
Oct 31, 2023
Merged

few add responders patches #3220

merged 10 commits into from
Oct 31, 2023

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Oct 30, 2023

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
  • 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
    Screenshot 2023-10-31 at 10 06 20
    After
    Screenshot 2023-10-31 at 10 48 12

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):
Screenshot 2023-10-30 at 18 14 59

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@joeyorlando joeyorlando added the pr:no public docs Added to a PR that does not require public documentation updates label Oct 30, 2023
@joeyorlando joeyorlando requested a review from a team October 30, 2023 22:13
Comment on lines 253 to 255
"schedules_with_oncall_users": self.schedules_with_oncall_users
if self._is_currently_oncall_or_short_request()
else {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main change, only set context["schedules_with_oncall_users"] for certain requests. Previously we were always setting it even if the serializer didn't need it (UserLongSerializer is the only serializer that needs this here).

@joeyorlando joeyorlando requested a review from a team October 31, 2023 14:05
@joeyorlando joeyorlando changed the title fix slow GET /users endpoint few add responders patches Oct 31, 2023
@joeyorlando joeyorlando enabled auto-merge October 31, 2023 15:09
@joeyorlando joeyorlando added this pull request to the merge queue Oct 31, 2023
@joeyorlando joeyorlando disabled auto-merge October 31, 2023 15:14
@joeyorlando joeyorlando removed this pull request from the merge queue due to a manual request Oct 31, 2023
@joeyorlando joeyorlando merged commit d568ad6 into dev Oct 31, 2023
17 checks passed
@joeyorlando joeyorlando deleted the jorlando/fix-slow-users-endpoint branch October 31, 2023 15:18
brojd pushed a commit that referenced this pull request Sep 18, 2024
# 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants