Blogging Reminders: Add logic to schedule and display notifications#14719
Conversation
…678-blogging-reminders-scheduling-notification
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
|
You can test the changes on this Pull Request by downloading the APK here. |
6a18e9c to
b7a02a7
Compare
…678-blogging-reminders-scheduling-notification
| import java.time.temporal.TemporalAdjusters.next | ||
|
|
||
| sealed class ReminderConfig(val type: ReminderType) { | ||
| object DailyReminder : ReminderConfig(DAILY) |
There was a problem hiding this comment.
Do we need the DailyReminder? What I mean is that the current designs only allow weekly reminders and we can use weekly reminders for each day instead of daily reminders, right?
There was a problem hiding this comment.
You're right, we don't really need it right now. I just think having explicit models for each type of recurrence should make this much more flexible in the future, if we ever decide to build upon it with more advanced rules (interval, for example).
Also, as a bonus, it should make it a bit easier to translate rules into natural language for the UI.
planarvoid
left a comment
There was a problem hiding this comment.
The code looks good 👍 and as far as I could see this works well. We should spend some time reviewing the real functionality once we have everything in place 👍
I have one minor comment, let me know what you think
planarvoid
left a comment
There was a problem hiding this comment.
I tried to approve this before but Github kept on failing 👍
|
Moving this PR to next milestone, since 17.6 goes into code freeze today. |
Fixes #14678
This is a WIP and it's not ready to merge yet, as it depends on #14698.
This PR adds all the logic necessary to schedule and show reminder notifications using
WorkManager. The main classes added here are:ReminderConfig, which has all the logic to create daily and weekly reminders.ReminderScheduler, which uses aReminderConfigto create aWorkManagerrequest.ReminderNotifier, which is responsible for building and showing a blogging reminder notification.ReminderWorker, which is the actual worker implementation that will call both theReminderNotifierandReminderScheduler(to schedule the next reminder).A few things to note:
WorkManagerframework supports creating periodic works, these are not really suitable for our use case here. The reason is that we can never control the exact time in which a work would be triggered, we can only control its initial delay and the interval between each work. And sinceWorkManagerrespects things like Doze and battery management, we don’t know for sure at which time the following works would be triggered. So if our work is delayed a little bit every time, this means that over time the notification would appear much later than before. This video explains this in greater detail and suggests following the approach we're using here, which is to schedule a one time work that schedules another one time work by the end of it.WorkManagerframework already provides us tools to handle this. But this is something we can discuss later. For now, I've left this PR as simple as possible, just so we can move on with it.To test:
Follow the instructions in #14720.
Regression Notes
N/A
N/A
Unit tests for newly added classes.
PR submission checklist:
RELEASE-NOTES.txtif necessary.