Skip to content

Commit e58090b

Browse files
authored
Merge pull request #2037 from OneSignal/fix/optIn-not-prompting-if-called-before-sub-created
[Fix] optIn() not prompting if called before push subscription is created on backend
2 parents a83b990 + a92bcee commit e58090b

File tree

3 files changed

+55
-2
lines changed

3 files changed

+55
-2
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/IDManager.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import java.util.UUID
77
* and detect whether a provided ID was generated locally.
88
*/
99
object IDManager {
10-
private const val LOCAL_PREFIX = "local-"
10+
internal const val LOCAL_PREFIX = "local-"
1111

1212
/**
1313
* Create a new local ID to be used temporarily prior to backend generation.

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/subscriptions/impl/SubscriptionManager.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ internal class SubscriptionManager(
157157
args: ModelChangedArgs,
158158
tag: String,
159159
) {
160-
val subscription = subscriptions.collection.firstOrNull { it.id == args.model.id }
160+
val subscription =
161+
subscriptions.collection.firstOrNull {
162+
args.model == (it as Subscription).model
163+
}
161164

162165
if (subscription == null) {
163166
// this shouldn't happen, but create a new subscription if a model was updated and we

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
package com.onesignal.user.internal.subscriptions
22

3+
import com.onesignal.common.IDManager.LOCAL_PREFIX
34
import com.onesignal.common.modeling.ModelChangeTags
45
import com.onesignal.common.modeling.ModelChangedArgs
56
import com.onesignal.core.internal.application.IApplicationService
67
import com.onesignal.session.internal.session.ISessionService
8+
import com.onesignal.user.internal.Subscription
79
import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
810
import com.onesignal.user.subscriptions.ISmsSubscription
911
import io.kotest.core.spec.style.FunSpec
1012
import io.kotest.matchers.should
1113
import io.kotest.matchers.shouldBe
1214
import io.kotest.matchers.shouldNotBe
1315
import io.kotest.matchers.types.beInstanceOf
16+
import io.kotest.matchers.types.shouldBeSameInstanceAs
1417
import io.mockk.every
1518
import io.mockk.just
1619
import io.mockk.mockk
@@ -328,6 +331,53 @@ class SubscriptionManagerTests : FunSpec({
328331
verify(exactly = 1) { spySubscriptionChangedHandler.onSubscriptionChanged(any(), any()) }
329332
}
330333

334+
// This is a common case where updates (such as optedIn) should
335+
// still propagate even if we haven't sent the POST /users create
336+
// call yet. Motivation for this test was a bug was discovered
337+
// where calling OneSignal.User.pushSubscription.optIn() was not
338+
// prompting for notification permission if it was called before
339+
// the create User network call finished.
340+
test("subscription modified when model updated, but with local-id") {
341+
// Given
342+
val pushSubscriptionModel = SubscriptionModel()
343+
pushSubscriptionModel.id = "${LOCAL_PREFIX}subscription1"
344+
pushSubscriptionModel.type = SubscriptionType.PUSH
345+
pushSubscriptionModel.optedIn = true
346+
pushSubscriptionModel.address = "my_push_token-org"
347+
348+
val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
349+
val mockApplicationService = mockk<IApplicationService>()
350+
val mockSessionService = mockk<ISessionService>(relaxed = true)
351+
val listOfSubscriptions = listOf(pushSubscriptionModel)
352+
353+
every { mockSubscriptionModelStore.subscribe(any()) } just runs
354+
every { mockSubscriptionModelStore.list() } returns listOfSubscriptions
355+
356+
val spySubscriptionChangedHandler = spyk<ISubscriptionChangedHandler>()
357+
358+
val subscriptionManager = SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
359+
subscriptionManager.subscribe(spySubscriptionChangedHandler)
360+
361+
// When
362+
pushSubscriptionModel.address = "my_push_token-new"
363+
subscriptionManager.onModelUpdated(
364+
ModelChangedArgs(
365+
pushSubscriptionModel,
366+
SubscriptionModel::address.name,
367+
SubscriptionModel::address.name,
368+
"my_push_token-org",
369+
"my_push_token-new",
370+
),
371+
ModelChangeTags.NORMAL,
372+
)
373+
val subscriptions = subscriptionManager.subscriptions
374+
375+
// Then
376+
(subscriptions.push as Subscription).model shouldBeSameInstanceAs pushSubscriptionModel
377+
subscriptions.push.token shouldBe "my_push_token-new"
378+
verify(exactly = 1) { spySubscriptionChangedHandler.onSubscriptionChanged(any(), any()) }
379+
}
380+
331381
test("subscription removed when model removed") {
332382
// Given
333383
val smsSubscription = SubscriptionModel()

0 commit comments

Comments
 (0)