Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import java.util.UUID
* and detect whether a provided ID was generated locally.
*/
object IDManager {
private const val LOCAL_PREFIX = "local-"
internal const val LOCAL_PREFIX = "local-"

/**
* Create a new local ID to be used temporarily prior to backend generation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ internal class SubscriptionManager(
args: ModelChangedArgs,
tag: String,
) {
val subscription = subscriptions.collection.firstOrNull { it.id == args.model.id }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, to use id as a getter, it would have to compare it.model.id == args.model.id

Copy link
Copy Markdown
Member Author

@jkasten2 jkasten2 Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, since the Susbcription.id returns "" if it starts with local- it was not finding it, using model always gives us the id value

val subscription =
subscriptions.collection.firstOrNull {
args.model == (it as Subscription).model
}

if (subscription == null) {
// this shouldn't happen, but create a new subscription if a model was updated and we
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
package com.onesignal.user.internal.subscriptions

import com.onesignal.common.IDManager.LOCAL_PREFIX
import com.onesignal.common.modeling.ModelChangeTags
import com.onesignal.common.modeling.ModelChangedArgs
import com.onesignal.core.internal.application.IApplicationService
import com.onesignal.session.internal.session.ISessionService
import com.onesignal.user.internal.Subscription
import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
import com.onesignal.user.subscriptions.ISmsSubscription
import io.kotest.core.spec.style.FunSpec
import io.kotest.matchers.should
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNotBe
import io.kotest.matchers.types.beInstanceOf
import io.kotest.matchers.types.shouldBeSameInstanceAs
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
Expand Down Expand Up @@ -328,6 +331,53 @@ class SubscriptionManagerTests : FunSpec({
verify(exactly = 1) { spySubscriptionChangedHandler.onSubscriptionChanged(any(), any()) }
}

// This is a common case where updates (such as optedIn) should
// still propagate even if we haven't sent the POST /users create
// call yet. Motivation for this test was a bug was discovered
// where calling OneSignal.User.pushSubscription.optIn() was not
// prompting for notification permission if it was called before
// the create User network call finished.
test("subscription modified when model updated, but with local-id") {
// Given
val pushSubscriptionModel = SubscriptionModel()
pushSubscriptionModel.id = "${LOCAL_PREFIX}subscription1"
pushSubscriptionModel.type = SubscriptionType.PUSH
pushSubscriptionModel.optedIn = true
pushSubscriptionModel.address = "my_push_token-org"

val mockSubscriptionModelStore = mockk<SubscriptionModelStore>()
val mockApplicationService = mockk<IApplicationService>()
val mockSessionService = mockk<ISessionService>(relaxed = true)
val listOfSubscriptions = listOf(pushSubscriptionModel)

every { mockSubscriptionModelStore.subscribe(any()) } just runs
every { mockSubscriptionModelStore.list() } returns listOfSubscriptions

val spySubscriptionChangedHandler = spyk<ISubscriptionChangedHandler>()

val subscriptionManager = SubscriptionManager(mockApplicationService, mockSessionService, mockSubscriptionModelStore)
subscriptionManager.subscribe(spySubscriptionChangedHandler)

// When
pushSubscriptionModel.address = "my_push_token-new"
subscriptionManager.onModelUpdated(
ModelChangedArgs(
pushSubscriptionModel,
SubscriptionModel::address.name,
SubscriptionModel::address.name,
"my_push_token-org",
"my_push_token-new",
),
ModelChangeTags.NORMAL,
)
val subscriptions = subscriptionManager.subscriptions

// Then
(subscriptions.push as Subscription).model shouldBeSameInstanceAs pushSubscriptionModel
subscriptions.push.token shouldBe "my_push_token-new"
verify(exactly = 1) { spySubscriptionChangedHandler.onSubscriptionChanged(any(), any()) }
}

test("subscription removed when model removed") {
// Given
val smsSubscription = SubscriptionModel()
Expand Down