diff --git a/CHANGELOG.md b/CHANGELOG.md index c4dfc9a2..e24a957f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ignore ical cancelled events when calculating shifts ([#2776](https://github.com/grafana/oncall/pull/2776)) - Fix Slack acknowledgment reminders by @vadimkerr ([#2769](https://github.com/grafana/oncall/pull/2769)) - Fix issue with updating "Require resolution note" setting by @Ferril ([#2782](https://github.com/grafana/oncall/pull/2782)) +- Don't send notifications about past SSRs when turning on info notifications by @vadimkerr ([#2783](https://github.com/grafana/oncall/pull/2783)) ## v1.3.23 (2023-08-10) diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index d4eb354e..d6dabdc4 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -595,10 +595,10 @@ def _should_notify_user_about_shift_swap_request( 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 + return user.is_in_working_hours( # user must be in working hours + now, mobile_app_user_settings.time_zone + ) and not _has_user_been_notified_for_shift_swap_request( # don't notify twice + shift_swap_request, user ) diff --git a/engine/apps/mobile_app/tests/test_shift_swap_request.py b/engine/apps/mobile_app/tests/test_shift_swap_request.py index afd216a7..38b3c213 100644 --- a/engine/apps/mobile_app/tests/test_shift_swap_request.py +++ b/engine/apps/mobile_app/tests/test_shift_swap_request.py @@ -245,7 +245,7 @@ def test_notify_user_about_shift_swap_request(make_organization, make_user, make device_to_notify = FCMDevice.objects.create(user=benefactor, registration_id="test_device_id") MobileAppUserSettings.objects.create(user=benefactor, info_notifications_enabled=True) - now = timezone.datetime(2023, 8, 1, 19, 38, tzinfo=timezone.utc) + now = timezone.now() swap_start = now + timezone.timedelta(days=100) swap_end = swap_start + timezone.timedelta(days=1) @@ -270,6 +270,32 @@ def test_notify_user_about_shift_swap_request(make_organization, make_user, make assert message.apns.payload.aps.sound.critical is False +@pytest.mark.django_db +def test_notify_user_about_shift_swap_request_info_notifications_disabled( + make_organization, make_user, make_schedule, make_shift_swap_request +): + organization = make_organization() + beneficiary = make_user(organization=organization) + benefactor = make_user(organization=organization) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + + FCMDevice.objects.create(user=benefactor, registration_id="test_device_id") + MobileAppUserSettings.objects.create(user=benefactor, info_notifications_enabled=False) + + now = timezone.now() + swap_start = now + timezone.timedelta(days=100) + swap_end = swap_start + timezone.timedelta(days=1) + + shift_swap_request = make_shift_swap_request( + schedule, beneficiary, swap_start=swap_start, swap_end=swap_end, created_at=now + ) + + with patch("apps.mobile_app.tasks._send_push_notification") as mock_send_push_notification: + notify_user_about_shift_swap_request(shift_swap_request.pk, benefactor.pk) + + mock_send_push_notification.assert_not_called() + + @pytest.mark.django_db def test_should_notify_user(make_organization, make_user, make_schedule, make_shift_swap_request): organization = make_organization() @@ -288,8 +314,11 @@ def test_should_notify_user(make_organization, make_user, make_schedule, make_sh assert not MobileAppUserSettings.objects.exists() assert _should_notify_user_about_shift_swap_request(shift_swap_request, benefactor, now) is False + # check _should_notify_user_about_shift_swap_request is True when info notifications are disabled mobile_app_settings = MobileAppUserSettings.objects.create(user=benefactor, info_notifications_enabled=False) - assert _should_notify_user_about_shift_swap_request(shift_swap_request, benefactor, now) is False + with patch.object(benefactor, "is_in_working_hours", return_value=True): + with patch("apps.mobile_app.tasks._has_user_been_notified_for_shift_swap_request", return_value=False): + assert _should_notify_user_about_shift_swap_request(shift_swap_request, benefactor, now) is True mobile_app_settings.info_notifications_enabled = True mobile_app_settings.save(update_fields=["info_notifications_enabled"])