Activate: Store creation done local notification #9085
Conversation
…ut it in the beginning of a work chain
…on permission, only when requesting it
You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:
|
| override fun doWork(): Result { | ||
| return if (canDisplayNotifications) { | ||
| val type = inputData.getString(LOCAL_NOTIFICATION_TYPE) | ||
| if (type != null) { | ||
| when (type) { | ||
| LocalNotificationType.STORE_CREATION_FINISHED.value, | ||
| LocalNotificationType.STORE_CREATION_INCOMPLETE.value, | ||
| LocalNotificationType.FREE_TRIAL_EXPIRING.value, | ||
| LocalNotificationType.FREE_TRIAL_EXPIRED.value -> { | ||
| Result.success() | ||
| } | ||
| else -> { | ||
| cancelWork("Unknown notification $type. Cancelling work.") | ||
| } | ||
| } | ||
| } else { | ||
| cancelWork("Notification check data is invalid") | ||
| } | ||
| } else { | ||
| cancelWork("Notifications permission not granted. Cancelling work.") | ||
| } | ||
| } |
There was a problem hiding this comment.
Np, you can maybe reduce some nesting here with early return approach:
override fun doWork(): Result {
if (!canDisplayNotifications) cancelWork("Notifications permission not granted. Cancelling work.")
val type = inputData.getString(LOCAL_NOTIFICATION_TYPE)
if (type == null) cancelWork("Notification check data is invalid")
return when (type) {
LocalNotificationType.STORE_CREATION_FINISHED.value,
LocalNotificationType.STORE_CREATION_INCOMPLETE.value,
LocalNotificationType.FREE_TRIAL_EXPIRING.value,
LocalNotificationType.FREE_TRIAL_EXPIRED.value -> {
Result.success()
}
else -> {
cancelWork("Unknown notification $type. Cancelling work.")
}
}
}There was a problem hiding this comment.
Thanks for the tip, I like it! 👍 I modified it a bit because the !canDisplayNotifications and type == null cases did not return in your example, which would log multiple cancellation messages.
| fun onTryForFreeButtonPressed() { | ||
| tracker.track(AnalyticsEvent.SITE_CREATION_TRY_FOR_FREE_TAPPED) | ||
|
|
||
| manageDeferredNotifications() |
There was a problem hiding this comment.
Np, I think I'd move this scheduling down to line 77-78. Once we know the new site was created successfully but before the installation phase begins. Wdyt?
There was a problem hiding this comment.
Nice catch, I wanted to do that but I missed it! 👍
| wasPreviouslyStopped = false | ||
| } | ||
|
|
||
| myStoreViewModel.onStoreOpened() |
There was a problem hiding this comment.
Do you need this? Can we not handle everything from MyStoreViewModel. For example by calling notificationScheduler.cancelScheduledNotification(STORE_CREATION_FINISHED) from the viewmodel's init block?
There was a problem hiding this comment.
I wasn't sure but you're right, init works 👍
|
Excellent job @0nko 🥇 I like the I left a few minor nitpicks, but apart from that, the PR is good to go after conflicts are resolved. |
# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/AppInitializer.kt
Generated by 🚫 dangerJS |
|
Thanks for adding the request matcher for WireMock @0nko 👍 |
Fixes #9060, a subtask of #8999.
This PR integrates the WP.com feature flag repository and fetches the flags on app initialization.
LOCAL_NOTIFICATION_STORE_CREATION_READYis then used to check if a local notification should be scheduled 5 minutes after the store creation is started.The
WorkManagerworker chain has been updated and it now contains 2 steps:PreconditionWorker- This worker is delayed by the configured amount of time, upon which a precondition check is performed. If the precondition is satisfied, the final step is executed, otherwise the work is canceled.LocalNotificationWorker- This is the point when a local notification is displayed.To test: