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

Add mobile app push notifications for shift swap requests #2717

Merged
merged 24 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Added

- Add mobile app push notifications for shift swap requests by @vadimkerr ([#2717](https://github.com/grafana/oncall/pull/2717))

### Changed

- Skip past due swap requests when calculating events ([2718](https://github.com/grafana/oncall/pull/2718))
Expand Down
2 changes: 1 addition & 1 deletion engine/apps/mobile_app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class VolumeType(models.TextChoices):

# Push notification settings for info notifications
# this is used for non escalation related push notifications such as the
# "You're going OnCall soon" push notification
# "You're going OnCall soon" and "You have a new shift swap request" push notifications
info_notifications_enabled = models.BooleanField(default=False)

info_notification_sound_name = models.CharField(max_length=100, default="default_sound", null=True)
Expand Down
202 changes: 196 additions & 6 deletions engine/apps/mobile_app/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from celery.utils.log import get_task_logger
from django.conf import settings
from django.core.cache import cache
from django.urls import reverse
from django.utils import timezone
from firebase_admin.exceptions import FirebaseError
from firebase_admin.messaging import AndroidConfig, APNSConfig, APNSPayload, Aps, ApsAlert, CriticalSound, Message
Expand All @@ -20,6 +21,7 @@
from apps.alerts.models import AlertGroup
from apps.base.utils import live_settings
from apps.mobile_app.alert_rendering import get_push_notification_subtitle
from apps.schedules.models import ShiftSwapRequest
from apps.schedules.models.on_call_schedule import OnCallSchedule, ScheduleEvent
from apps.user_management.models import User
from common.api_helpers.utils import create_engine_url
Expand Down Expand Up @@ -230,13 +232,12 @@ def _get_youre_going_oncall_notification_title(seconds_until_going_oncall: int)
return f"Your on-call shift starts in {humanize.naturaldelta(seconds_until_going_oncall)}"


def _get_youre_going_oncall_notification_subtitle(
def _get_shift_subtitle(
Copy link
Member Author

Choose a reason for hiding this comment

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

Reusing this method to generate subtitles both for on-call shifts and SSRs

schedule: OnCallSchedule,
schedule_event: ScheduleEvent,
shift_start: datetime.datetime,
shift_end: datetime.datetime,
mobile_app_user_settings: "MobileAppUserSettings",
) -> str:
shift_start = schedule_event["start"]
shift_end = schedule_event["end"]
shift_starts_and_ends_on_same_day = shift_start.date() == shift_end.date()
dt_formatter_func = format_localized_time if shift_starts_and_ends_on_same_day else format_localized_datetime

Expand Down Expand Up @@ -269,8 +270,8 @@ def _get_youre_going_oncall_fcm_message(
mobile_app_user_settings, _ = MobileAppUserSettings.objects.get_or_create(user=user)

notification_title = _get_youre_going_oncall_notification_title(seconds_until_going_oncall)
notification_subtitle = _get_youre_going_oncall_notification_subtitle(
schedule, schedule_event, mobile_app_user_settings
notification_subtitle = _get_shift_subtitle(
schedule, schedule_event["start"], schedule_event["end"], mobile_app_user_settings
)

data: FCMMessageData = {
Expand Down Expand Up @@ -499,3 +500,192 @@ def conditionally_send_going_oncall_push_notifications_for_schedule(schedule_pk)
def conditionally_send_going_oncall_push_notifications_for_all_schedules() -> None:
for schedule in OnCallSchedule.objects.all():
conditionally_send_going_oncall_push_notifications_for_schedule.apply_async((schedule.pk,))


# TODO: break down tasks.py into multiple files
Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving this TODO out of the scope of this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, good idea. Created #2722 to track this.


# Don't send notifications for shift swap requests that start more than 4 weeks in the future
SSR_EARLIEST_NOTIFICATION_OFFSET = datetime.timedelta(weeks=4)

# Once it's time to send out notifications, send them over the course of a week.
# This is because users can be in multiple timezones / have different working hours configured,
# so we can't just send all notifications at once, but need to wait for the users to be in their working hours.
# Once a notification is sent to a user, they won't be notified again for the same shift swap request for a week.
# After a week, the shift swap request won't be in the notification window anymore (see _get_shift_swap_requests_to_notify).
SSR_NOTIFICATION_WINDOW = datetime.timedelta(weeks=1)


@shared_dedicated_queue_retry_task()
def notify_shift_swap_requests() -> None:
"""
A periodic task that notifies users about shift swap requests.
"""

if not settings.FEATURE_SHIFT_SWAPS_ENABLED:
return

for shift_swap_request in _get_shift_swap_requests_to_notify(timezone.now()):
notify_shift_swap_request.delay(shift_swap_request.pk)


def _get_shift_swap_requests_to_notify(now: datetime.datetime) -> list[ShiftSwapRequest]:
"""
Returns shifts swap requests that are open and are in the notification window.
This method can return the same shift swap request multiple times while it's in the notification window,
but users are only notified once per shift swap request (see _mark_shift_swap_request_notified_for_user).
"""

shift_swap_requests_in_notification_window = []
for shift_swap_request in ShiftSwapRequest.objects.filter(benefactor__isnull=True, swap_start__gt=now):
notification_window_start = max(
shift_swap_request.created_at, shift_swap_request.swap_start - SSR_EARLIEST_NOTIFICATION_OFFSET
)
notification_window_end = min(notification_window_start + SSR_NOTIFICATION_WINDOW, shift_swap_request.swap_end)

if notification_window_start <= now <= notification_window_end:
shift_swap_requests_in_notification_window.append(shift_swap_request)

return shift_swap_requests_in_notification_window


@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=MAX_RETRIES)
def notify_shift_swap_request(shift_swap_request_pk: int) -> None:
"""
Notify relevant users for an individual shift swap request.
"""
try:
shift_swap_request = ShiftSwapRequest.objects.get(pk=shift_swap_request_pk)
except ShiftSwapRequest.DoesNotExist:
logger.info(f"ShiftSwapRequest {shift_swap_request_pk} does not exist")
return

now = timezone.now()
for user in shift_swap_request.possible_benefactors:
if _should_notify_user_about_shift_swap_request(shift_swap_request, user, now):
notify_user_about_shift_swap_request.delay(shift_swap_request.pk, user.pk)
_mark_shift_swap_request_notified_for_user(shift_swap_request, user)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this happen at the very end of notify_user_about_shift_swap_request? In case something inside there fails we probably don't want to mark the notification as sent. wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to do that to make sure the mobile app doesn't spam people. I'd rather not receive any SSR notifications at all than receive more than one (potentially once every 15 minutes if something goes wrong between sending the push notification and marking the notification sent). The notify_user_about_shift_swap_request task has a retry policy, so if something goes wrong while sending the push notification (e.g. network issue), the task will probably succeed after a retry. Lmk if this makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

task has a retry policy

ahh yes, forgot about this. Makes sense 👍



@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=MAX_RETRIES)
def notify_user_about_shift_swap_request(shift_swap_request_pk: int, user_pk: int) -> None:
"""
Send a push notification about a shift swap request to an individual user.
"""
# avoid circular import
from apps.mobile_app.models import FCMDevice, MobileAppUserSettings

try:
shift_swap_request = ShiftSwapRequest.objects.get(pk=shift_swap_request_pk)
except ShiftSwapRequest.DoesNotExist:
logger.info(f"ShiftSwapRequest {shift_swap_request_pk} does not exist")
return

try:
user = User.objects.get(pk=user_pk)
except User.DoesNotExist:
logger.info(f"User {user_pk} does not exist")
return

device_to_notify = FCMDevice.get_active_device_for_user(user)
if not device_to_notify:
logger.info(f"FCMDevice does not exist for user {user_pk}")
return

try:
mobile_app_user_settings = MobileAppUserSettings.objects.get(user=user)
except MobileAppUserSettings.DoesNotExist:
logger.info(f"MobileAppUserSettings does not exist for user {user_pk}")
return

if not mobile_app_user_settings.info_notifications_enabled:
logger.info(f"Info notifications are not enabled for user {user_pk}")
return

if shift_swap_request.status != ShiftSwapRequest.Statuses.OPEN:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we could create a @property on ShiftSwapRequest as such:

@property
def is_open(self) -> bool:
  return not any([self.is_deleted, self.is_taken, self.is_past_due])

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, added in f747ec1

logger.info(f"Shift swap request {shift_swap_request_pk} is not open anymore")
return

message = _shift_swap_request_fcm_message(shift_swap_request, user, device_to_notify, mobile_app_user_settings)
_send_push_notification(device_to_notify, message)


def _should_notify_user_about_shift_swap_request(
shift_swap_request: ShiftSwapRequest, user: User, now: datetime.datetime
) -> bool:
# avoid circular import
from apps.mobile_app.models import MobileAppUserSettings

try:
mobile_app_user_settings = MobileAppUserSettings.objects.get(user=user)
except MobileAppUserSettings.DoesNotExist:
return False # don't notify if the app is not configured

return (
mobile_app_user_settings.info_notifications_enabled # info notifications must be enabled
and user.is_in_working_hours(now, mobile_app_user_settings.time_zone) # user must be in working hours
and not _has_user_been_notified_for_shift_swap_request(shift_swap_request, user) # don't notify twice
)
Comment on lines +623 to +627
Copy link
Contributor

Choose a reason for hiding this comment

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

👍



def _mark_shift_swap_request_notified_for_user(shift_swap_request: ShiftSwapRequest, user: User) -> None:
key = _shift_swap_request_cache_key(shift_swap_request, user)
cache.set(key, True, timeout=SSR_NOTIFICATION_WINDOW.total_seconds())


def _has_user_been_notified_for_shift_swap_request(shift_swap_request: ShiftSwapRequest, user: User) -> bool:
key = _shift_swap_request_cache_key(shift_swap_request, user)
return cache.get(key) is True


def _shift_swap_request_cache_key(shift_swap_request: ShiftSwapRequest, user: User) -> str:
return f"ssr_push:{shift_swap_request.pk}:{user.pk}"


def _shift_swap_request_fcm_message(
shift_swap_request: ShiftSwapRequest,
user: User,
device_to_notify: "FCMDevice",
mobile_app_user_settings: "MobileAppUserSettings",
) -> Message:
from apps.mobile_app.models import MobileAppUserSettings

thread_id = f"{shift_swap_request.public_primary_key}:{user.public_primary_key}:ssr"
notification_title = "You have a new shift swap request" # TODO: decide on the exact wording
Copy link
Member Author

Choose a reason for hiding this comment

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

let me know if you have any ideas on notification title

Copy link
Contributor

Choose a reason for hiding this comment

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

What about "Your teammate has opened a shift swap request"? wdyt?

notification_subtitle = _get_shift_subtitle(
shift_swap_request.schedule,
shift_swap_request.swap_start,
shift_swap_request.swap_end,
mobile_app_user_settings,
)

# TODO: check with the mobile team
resource_url = reverse("api-internal:shift_swap-detail", kwargs={"pk": shift_swap_request.public_primary_key})
Copy link
Member Author

Choose a reason for hiding this comment

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

need to check this with @grafana/grafana-oncall-mobile, see here for the actual URL format


data: FCMMessageData = {
"title": notification_title,
"subtitle": notification_subtitle,
"resource_url": resource_url,
"info_notification_sound_name": (
mobile_app_user_settings.info_notification_sound_name + MobileAppUserSettings.ANDROID_SOUND_NAME_EXTENSION
),
"info_notification_volume_type": mobile_app_user_settings.info_notification_volume_type,
"info_notification_volume": str(mobile_app_user_settings.info_notification_volume),
"info_notification_volume_override": json.dumps(mobile_app_user_settings.info_notification_volume_override),
}

apns_payload = APNSPayload(
aps=Aps(
thread_id=thread_id,
alert=ApsAlert(title=notification_title, subtitle=notification_subtitle),
sound=CriticalSound(
critical=False,
name=mobile_app_user_settings.info_notification_sound_name
+ MobileAppUserSettings.IOS_SOUND_NAME_EXTENSION,
),
custom_data={
"interruption-level": "time-sensitive",
},
),
)

return _construct_fcm_message(MessageType.INFO, device_to_notify, thread_id, data, apns_payload)
Loading