🐞 Fix adding task with alarm doesn't schedule the notification #671
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Having the last inserted row in the Task table will allow the app to schedule an alarm right after inserting the task. The lack of this support is what is causing the bug where the alarm is not schedule when using the botom sheet.
The logic that handles what to do with the alarm, such as schedule/cancel and set the repeating frequency were moved from the UI layer to the domain to be reusable.
Now that we have a use case that handles the alarm update, the logic and tests were simplified in the Ui layer. I don't love the solution to have to load the task before sending a copy to be modified, but for now, that's not going to impact the code too much and don't require a big refactor.
Now that all the structure were ported to be more reusable, the use case was updated to schedule the alarm and the interval. This fixes the bug where the alarm was not being scheduled through the bottom sheet.
The UpdateAlarm use case will be the only public one for the other clients. That allows better control on how to properly update the alarms without missing any requirement.
Unit tests update with the new injection in AddTask use case and remove unused test fakes.
- KDoc improved
To ensure that the "select last row" will work properly after inserting the task, the code was updated to run inside a transaction. This is required because it "only works on the connection where the write happened". Despite working fine in Android, this was not working on iOS. Source: sqldelight/sqldelight#1828
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When I implemented this feature, I didn't test it properly. It ended up that a some code connections were missing to enable the alarm to be schedule while the task is being inserted. This PR does the following
TaskAlarmViewModelto a dedicated use caseinsertTaskquery to return the latest inserted entryAddTaskuse case to schedule the notifications if an alarm is set