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 @@ -202,24 +202,16 @@ interface SubscriptionsFeature {
/**
* Enables/Disables duckAi for subscribers (advanced models)
* This flag is used to hide the feature in the native client and FE.
* It will be used for the feature rollout and kill-switch if necessary.
*/
@Toggle.DefaultValue(DefaultFeatureValue.INTERNAL)
fun duckAiPlus(): Toggle

/**
* Android supports v2 token, but still relies on old v1 subscription messaging.
* We are introducing new JS messaging. Use this flag as kill-switch if necessary.
* It doesn't control which version of messaging FE uses.
*/
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
fun enableNewSubscriptionMessages(): Toggle
fun duckAiPlus(): Toggle
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.

duckAiPlus default changed from INTERNAL to TRUE

High Severity

The duckAiPlus() toggle default value changed from DefaultFeatureValue.INTERNAL to DefaultFeatureValue.TRUE. This was not mentioned in the PR description, which only lists refreshSubscriptionPlanFeatures and enableNewSubscriptionMessages as targets. It appears the @Toggle.DefaultValue(DefaultFeatureValue.TRUE) annotation that previously belonged to the removed enableNewSubscriptionMessages() was inadvertently reused for duckAiPlus(). This enables DuckAI Plus subscriber features (advanced models) for all production users by default, whereas previously it only defaulted to enabled for internal builds.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a748c98. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

diff merge not accurate:

  • Default value changed to True is intended.
  • the other FF has been removed, which creates a weird diff.


/**
* When enabled, we signal FE if v2 is available, enabling v2 messaging
* When disabled, FE works with old messaging (v1)
* This flag will be used to select FE subscription messaging mode.
* The value is added into GetFeatureConfig to allow FE to select the mode.
* Note: best to remove together with v1 clean up.
*/
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
fun enableSubscriptionFlowsV2(): Toggle
Expand All @@ -231,16 +223,14 @@ interface SubscriptionsFeature {
fun authApiV2JwksCache(): Toggle

/**
* As part of Duck.ai we are adding new supported JS messages.
* This is enabled by default, but can be disabled if necessary.
* Controls Duck.ai <> subscription JS messaging.
* Enabled by default.
* When Disabled, no subscription messaging supported.
* FF only controls native messaging (enabled/disabled).
*/
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
fun duckAISubscriptionMessaging(): Toggle

@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
fun refreshSubscriptionPlanFeatures(): Toggle

@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
fun supportsAlternateStripePaymentFlow(): Toggle

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,6 @@ class SubscriptionFeaturesFetcher @Inject constructor(
?.flatMap { it.subscriptionOfferDetails ?: emptyList() }
?.map { it.basePlanId }
?.distinct()
?.let { basePlanIds ->
logcat {
"fetchSubscriptionFeatures: found base plan ids: $basePlanIds"
}
if (subscriptionsFeature.refreshSubscriptionPlanFeatures().isEnabled()) {
basePlanIds
} else {
basePlanIds.filter {
authRepository.getFeatures(it).isEmpty()
}
}
}
?.forEach { basePlanId ->
runCatching {
if (subscriptionsFeature.tierMessagingEnabled().isEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,6 @@ class SubscriptionMessagingInterface @Inject constructor(
) {
val jsMessageId = jsMessage.id ?: return

if (subscriptionsFeature.enableNewSubscriptionMessages().isEnabled().not()) return

val authV2Enabled = subscriptionsFeature.enableSubscriptionFlowsV2().isEnabled()
val duckAiSubscriberModelsEnabled = subscriptionsFeature.duckAiPlus().isEnabled()
val supportsAlternateStripePaymentFlow = subscriptionsFeature.supportsAlternateStripePaymentFlow().isEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,41 +126,6 @@ class SubscriptionFeaturesFetcherTest {
verifyNoInteractions(subscriptionsCachedService)
}

@Test
fun `when features already stored and refresh features FF Disabled then does not fetch again`() = runTest {
givenRefreshSubscriptionPlanFeaturesEnabled(false)
givenTierMessagingEnabled(false)
val productDetails = mockProductDetails()
whenever(playBillingManager.productsFlow).thenReturn(flowOf(productDetails))
whenever(authRepository.getFeatures(any())).thenReturn(setOf(NETP, ITR))

processLifecycleOwner.currentState = CREATED

verify(playBillingManager).productsFlow
verify(authRepository).getFeatures(MONTHLY_PLAN_US)
verify(authRepository).getFeatures(YEARLY_PLAN_US)
verify(authRepository, never()).setFeatures(any(), any())
verifyNoInteractions(subscriptionsCachedService)
}

@Test
fun `when features already stored and refresh features FF enabled then does fetch again`() = runTest {
givenRefreshSubscriptionPlanFeaturesEnabled(true)
givenTierMessagingEnabled(false)
val productDetails = mockProductDetails()
whenever(playBillingManager.productsFlow).thenReturn(flowOf(productDetails))
whenever(authRepository.getFeatures(any())).thenReturn(setOf(NETP, ITR))
whenever(subscriptionsCachedService.features(any())).thenReturn(FeaturesResponse(listOf(NETP, ITR, DUCK_AI)))

processLifecycleOwner.currentState = CREATED

verify(playBillingManager).productsFlow
verify(subscriptionsCachedService).features(MONTHLY_PLAN_US)
verify(subscriptionsCachedService).features(YEARLY_PLAN_US)
verify(authRepository).setFeatures(MONTHLY_PLAN_US, setOf(NETP, ITR, DUCK_AI))
verify(authRepository).setFeatures(YEARLY_PLAN_US, setOf(NETP, ITR, DUCK_AI))
}

@Test
fun `when tierMessagingEnabled ON and V2 features empty then does not store anything`() = runTest {
givenTierMessagingEnabled(true)
Expand All @@ -178,11 +143,6 @@ class SubscriptionFeaturesFetcherTest {
verify(authRepository, never()).setFeaturesV2(any(), any())
}

@SuppressLint("DenyListedApi")
private fun givenRefreshSubscriptionPlanFeaturesEnabled(value: Boolean) {
subscriptionsFeature.refreshSubscriptionPlanFeatures().setRawStoredState(State(value))
}

@SuppressLint("DenyListedApi")
private fun givenTierMessagingEnabled(value: Boolean) {
subscriptionsFeature.tierMessagingEnabled().setRawStoredState(State(value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,6 @@ class SubscriptionMessagingInterfaceTest {
@Test
fun `when process and get feature config and messaging enabled then return response with feature flags`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = true)
givenAuthV2(enabled = true)
givenDuckAiPlus(enabled = true)
givenStripeSupported(enabled = true)
Expand Down Expand Up @@ -895,7 +894,6 @@ class SubscriptionMessagingInterfaceTest {
@Test
fun `when process and get feature config and messaging enabled but auth v2 disabled then return response with auth v2 false`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = true)
givenAuthV2(enabled = false)
givenDuckAiPlus(enabled = true)
givenStripeSupported(enabled = true)
Expand Down Expand Up @@ -930,7 +928,6 @@ class SubscriptionMessagingInterfaceTest {
@Test
fun `when process and get feature config and messaging enabled but duck ai plus disabled then return response with duck ai false`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = true)
givenAuthV2(enabled = true)
givenDuckAiPlus(enabled = false)
givenStripeSupported(enabled = true)
Expand Down Expand Up @@ -965,7 +962,6 @@ class SubscriptionMessagingInterfaceTest {
@Test
fun `when process and get feature config and tier options enabled then return response with tier options true`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = true)
givenAuthV2(enabled = true)
givenDuckAiPlus(enabled = true)
givenStripeSupported(enabled = true)
Expand Down Expand Up @@ -997,24 +993,9 @@ class SubscriptionMessagingInterfaceTest {
checkEquals(expected, jsMessage)
}

@Test
fun `when process and get feature config and messaging disabled then do nothing`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = false)

val message = """
{"context":"subscriptionPages","featureName":"useSubscription","method":"getFeatureConfig","id":"myId","params":{}}
""".trimIndent()

messagingInterface.process(message, "duckduckgo-android-messaging-secret")

verifyNoInteractions(jsMessageHelper)
}

@Test
fun `when process and get feature config if no id do nothing`() = runTest {
givenInterfaceIsRegistered()
givenSubscriptionMessaging(enabled = true)

val message = """
{"context":"subscriptionPages","featureName":"useSubscription","method":"getFeatureConfig","params":{}}
Expand Down Expand Up @@ -1064,12 +1045,6 @@ class SubscriptionMessagingInterfaceTest {
whenever(subscriptionsManager.getAccessToken()).thenReturn(AccessTokenResult.Failure(message = "something happened"))
}

private fun givenSubscriptionMessaging(enabled: Boolean) {
val subscriptionMessagingToggle = mock<com.duckduckgo.feature.toggles.api.Toggle>()
whenever(subscriptionMessagingToggle.isEnabled()).thenReturn(enabled)
whenever(subscriptionsFeature.enableNewSubscriptionMessages()).thenReturn(subscriptionMessagingToggle)
}

private fun givenAuthV2(enabled: Boolean) {
val v2SubscriptionFlow = mock<com.duckduckgo.feature.toggles.api.Toggle>()
whenever(v2SubscriptionFlow.isEnabled()).thenReturn(enabled)
Expand Down
Loading