Don't send notifications about past SSRs when turning on info notifications (#2783)
# What this PR does Fixes a bug when a user is receiving push notifications about past SSRs after turning on the info notifications in the mobile app. ## 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)
This commit is contained in:
parent
3e703d354b
commit
a9bf4f5521
3 changed files with 36 additions and 6 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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"])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue