From 3fd9a73a524421c2dedc10c31ff823ff4b3ef6ae Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Thu, 25 Apr 2024 14:16:42 -0400 Subject: [PATCH] GCal autogenerated shift swap requests - don't recreate if one was previously created and deleted (#4281) # What this PR does Addresses two issues: - addresses an internal feature request ([in Slack](https://raintank-corp.slack.com/archives/C03KS498VGV/p1713550543916289?thread_ts=1713546008.831749&cid=C03KS498VGV)) to not have a new shift swap request auto-generated if one previously was, but than the user decided to delete that SSR - when disconnecting the GCal integration from your user, I've seen cases where Google will return HTTP 400 as such ([example logs](https://ops.grafana-ops.net/goto/8vX76pBSg?orgId=1)): ``` {'error': 'invalid_token', 'error_description': 'Token expired or revoked'} ``` I can't seem to find detailed documentation on the revoke endpoint (`GET https://accounts.google.com/o/oauth2/revoke?token=`) to try and better understand the possible `error` values.. but I think our best bet here is to just continue forward w/ `user.finish_google_oauth2_disconnection_flow()` (which deletes the `GoogleOAuth2User` associated with the user and sets `user.google_calendar_settings = None`) ## 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] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- .../on-call-schedules/shift-swaps/index.md | 3 ++ engine/apps/google/tasks.py | 5 ++- ..._out_of_office_calendar_events_for_user.py | 15 +++++++-- engine/apps/social_auth/pipeline/google.py | 32 +++++++++++++++++-- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/docs/sources/manage/on-call-schedules/shift-swaps/index.md b/docs/sources/manage/on-call-schedules/shift-swaps/index.md index f4c3a4e9..950cfc2c 100644 --- a/docs/sources/manage/on-call-schedules/shift-swaps/index.md +++ b/docs/sources/manage/on-call-schedules/shift-swaps/index.md @@ -114,6 +114,9 @@ are involved in). If you would like to have Grafana OnCall ignore a specific Out of Office event from being considered for Shift Swap Request generation, simply add `#grafana-oncall-ignore` to the Out of Office event's title. +Additionally, if we generate a shift swap request for you from a Google Calendar event, and you delete the shift swap +request, we will not attempt to regenerate a new shift swap request. + ### Configuring for open source 1. Follow the instructions [here](https://developers.google.com/identity/protocols/oauth2) to setup your Google OAuth2 diff --git a/engine/apps/google/tasks.py b/engine/apps/google/tasks.py index dd7ab9a9..b1b601d4 100644 --- a/engine/apps/google/tasks.py +++ b/engine/apps/google/tasks.py @@ -64,7 +64,10 @@ def sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk: int) -> N f"for user {user_id} during the out of office event {event_id}" ) - shift_swap_request_exists = ShiftSwapRequest.objects.filter( + # also consider deleted shift swap requests.. this can be useful in the event + # that we autogenerated a shift swap request for the user but the user decided to delete it + # in this case, we shouldn't recreate a new one + shift_swap_request_exists = ShiftSwapRequest.objects_with_deleted.filter( beneficiary=user, schedule=schedule, swap_start=start_time_utc, diff --git a/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py b/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py index 0a58f03a..d930af13 100644 --- a/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py +++ b/engine/apps/google/tests/test_sync_out_of_office_calendar_events_for_user.py @@ -325,6 +325,7 @@ def test_sync_out_of_office_calendar_events_for_user_preexisting_shift_swap_requ } google_oauth2_user, schedule = test_setup(out_of_office_events) + google_oauth2_user_pk = google_oauth2_user.pk user = google_oauth2_user.user make_shift_swap_request( @@ -334,7 +335,17 @@ def test_sync_out_of_office_calendar_events_for_user_preexisting_shift_swap_requ swap_end=end_time, ) - tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user.pk) + def _fetch_shift_swap_requests(): + return ShiftSwapRequest.objects_with_deleted.filter(beneficiary=user, schedule=schedule) + + tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk) # should be 1 because we just created a shift swap request above via the fixture - assert ShiftSwapRequest.objects.filter(beneficiary=user, schedule=schedule).count() == 1 + ssrs = _fetch_shift_swap_requests() + assert ssrs.count() == 1 + + # lets delete the shift swap request and run the task again, it should recognize that there was already + # a shift swap request and shouldn't recreate a new one + ssrs.first().delete() + tasks.sync_out_of_office_calendar_events_for_user(google_oauth2_user_pk) + assert _fetch_shift_swap_requests().count() == 1 diff --git a/engine/apps/social_auth/pipeline/google.py b/engine/apps/social_auth/pipeline/google.py index 0d5fc609..37702b39 100644 --- a/engine/apps/social_auth/pipeline/google.py +++ b/engine/apps/social_auth/pipeline/google.py @@ -1,6 +1,7 @@ import logging import typing +import requests from rest_framework.response import Response from social_core.backends.base import BaseAuth @@ -28,10 +29,35 @@ def disconnect_user_google_oauth2_settings(backend: typing.Type[BaseAuth], user: https://stackoverflow.com/a/18578660/3902555 """ - logger.info(f"Disconnecting user {user.pk} from Google OAuth2") + user_pk = user.pk + google_oauth2_user = user.google_oauth2_user + + logger.info(f"Disconnecting user {user_pk} from Google OAuth2") + + try: + backend.revoke_token(google_oauth2_user.refresh_token, google_oauth2_user.google_user_id) + except requests.exceptions.HTTPError as e: + response = e.response + + logger.error(f"There was an HTTP error when trying to revoke Google OAuth2 token for user={user_pk}") + + if response.status_code == 400: + error_details = response.json() + error_code = error_details["error"] + error_description = error_details["error_description"] + + logger.error( + f"There was an HTTP 400 error when trying to revoke Google OAuth2 token for user={user_pk} " + f"error_code={error_code} error_description={error_description}" + ) + + error_codes_to_ignore = ["invalid_token"] + + if error_code not in error_codes_to_ignore: + raise e + else: + logger.info(f"Google OAuth2 token for user {user_pk} is already invalid or revoked, ignoring error") - # 2nd argument, uid, is not needed for GoogleOauth2 backend - backend.revoke_token(user.google_oauth2_user.refresh_token, "") user.finish_google_oauth2_disconnection_flow() logger.info(f"Successfully disconnected user {user.pk} from Google OAuth2")