"You're Going OnCall" mobile app push notification #1814
Conversation
mobile app settings
There was a problem hiding this comment.
this looks like a bigger diff than it actually is. I extracted the logic that is now in the following methods to avoid having to duplicate w/ the addition of this new push notification:
send_push_notification_to_fcm_relay_send_push_notification_construct_fcm_message
joeyorlando
changed the title
|
|
||
|
|
||
| ScheduleEvents = List[ScheduleEvent] | ||
| ScheduleEventIntervals = List[List[datetime.datetime]] |
engine/apps/mobile_app/tasks.py
Outdated
| def should_we_send_going_oncall_push_notification( | ||
| now: timezone.datetime, user_settings: "MobileAppUserSettings", schedule_event: ScheduleEvent | ||
| ) -> bool: | ||
| NOTIFICATION_TIMING_BUFFER = 15 * 60 # 15 minutes in seconds |
There was a problem hiding this comment.
Since the task is scheduled to run every 10 mins, is it possible we may send 2 notifications for the same user/shift combination?
There was a problem hiding this comment.
good point 👍 I ended up storing a key in redis for ~80mins to keep track of these notifications (change) and avoid duplicates
also, cache notifications sent to avoid resending
joeyorlando
added
the
pr:no public docs
|
discussed w/ @grafana/grafana-oncall-mobile, we will document this feature in a forthcoming PR |
…grafana/oncall into jorlando/youre-oncall-push-notification
engine/apps/mobile_app/tasks.py
Outdated
| logger.info("not sending going oncall push notification because info_notifications_enabled is false") | ||
| return | ||
|
|
||
| # 8 minute window where the notification could be sent (4 mins before or 4 mins after) |
There was a problem hiding this comment.
if we run the task every 10 mins, shouldn't the window cover (at least) 10 mins?
engine/apps/mobile_app/tasks.py
Outdated
| schedule_final_events = schedule.final_events("UTC", now, days=7) | ||
|
|
||
| relevant_cache_keys = [ | ||
| _generate_going_oncall_push_notification_cache_key(user["pk"], schedule_event, current_date_with_hour) |
There was a problem hiding this comment.
edge case, but using current_date_with_hour couldn't still lead to duplicates? (e.g. my shift starts at 16:02?) wouldn't be enough to use user and event keys? (and caching the key for 1h)
# What this PR does https://www.loom.com/share/c5deb35309604cfdab6176c44de7b15e ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] 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)
What this PR does
https://www.loom.com/share/c5deb35309604cfdab6176c44de7b15e
Checklist
pr:no public docsPR label added if not required)CHANGELOG.mdupdated (orpr:no changelogPR label added if not required)