Fix #1660: Show monthly ad grant reminders #1988
Conversation
kylehickinson
force-pushed
the
quiet-monthly-notification
branch
2 times, most recently
from
3fa3e45 to
bf54017
Compare
There was a problem hiding this comment.
I feel like we should set Calendar to always use gregorian format.
In the past we had few date related bugs in DAU that happened because Arabic and Japanese calendars were used by users
Since Date() is locale independent, just changing the calendar alone should be enough to prevent bugs, might need to do more research on it though, esp with handling next month logic
Let me know what do you think about it
| return | ||
| } | ||
| let center = UNUserNotificationCenter.current() | ||
| center.requestAuthorization(options: [.provisional, .alert, .sound, .badge]) { (granted, error) in |
There was a problem hiding this comment.
Can drop brackets around granted, error
| content.body = Strings.MonthlyAdsClaimNotificationBody | ||
|
|
||
| let trigger = UNCalendarNotificationTrigger( | ||
| dateMatching: .init(month: month, day: 7, hour: 13, minute: 21), |
There was a problem hiding this comment.
Why this date precisely? Will it work correctly for 12-hour formats with the hour = 13?
There was a problem hiding this comment.
@iccub Basically just was asked for that to be the time it goes off in the ticket (1:21pm or 13:21 in 24-hour clocks). But yes, it definitely works with 12 hour formats, tested on my device which is in 12-hour mode
| /// this would return `1` for January. | ||
| static private func nextMonth() -> Int? { | ||
| guard let nextMonthsDate = Calendar.current.date(byAdding: .month, value: 1, to: Date()) else { | ||
| // Apocalypse... |
There was a problem hiding this comment.
what about assertionFailure("Apocalypse")
|
Some tests
|
These notifications are scheduled based on when a user sees ads are always scheduled for the following month. They are only cancelled if the user successfully claims an ad grant
kylehickinson
force-pushed
the
quiet-monthly-notification
branch
from
bf54017 to
f6ed34e
Compare
These notifications are scheduled based on when a user sees ads are always scheduled for the following month. They are only cancelled if the user successfully claims an ad grant
Summary of Changes
This pull request fixes issue #1660
Submitter Checklist:
NSLocalizableString()Test Plan:
Reviewer Checklist:
QA/(Yes|No)release-notes/(include|exclude)bug/enhancement