-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Push] Show notification on push notification (until sync is started) #1043
[Push] Show notification on push notification (until sync is started) #1043
Conversation
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
…fication-until-sync-is-started
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, will be cool to finally get the notifications :)
A few comments though…
app/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/ui/NotificationRegistry.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/sync/worker/OneTimeSyncWorker.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rfc2822 I'm not sure how to get an instance of PushNotificationManager
from here, since it's in the companion, should we pass it as an argument? Doesn't sound really good
Edit: here*
davx5-ose/app/src/main/kotlin/at/bitfire/davdroid/sync/worker/OneTimeSyncWorker.kt
Lines 154 to 160 in e210448
// Show notification if called by push | |
// FIXME: Inject PushNotificationManager | |
if (isPush) { | |
PushNotificationManager.notify(context, account, authority) | |
} | |
We could have used a custom entry point for the companion object. However I think there shouldn't be too much logic in companion object anyway, so I have prepared a new class in #1056. That one is useable with DI. |
…fication-until-sync-is-started # Conflicts: # app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt # app/src/main/kotlin/at/bitfire/davdroid/sync/worker/OneTimeSyncWorker.kt
Signed-off-by: Arnau Mora Gras <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very promising – only minor comments.
app/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/sync/worker/SyncWorkerManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt
Outdated
Show resolved
Hide resolved
…fication-until-sync-is-started
Signed-off-by: Arnau Mora Gras <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, looking forward to (not) see the notifications 👍🏻
Purpose
It can happen that a PUSH notification is received, but the app actually doesn't have access to the internet, so the synchronization is not started immediately. A notification will be shown to tell the user that the sync is pending. It should be hidden when the synchronization starts.
Short description
Added a new parameter to the enqueue functions, to tell them whether the schedule is from a push notification or not. If it is, a notification is shown using
PushNotificationManager
, which is shown until the sync is started.Checklist
Depends on: #1056