Skip to content

Commit a170ae0

Browse files
committed
Bug 2040784 - Introduce AIFeatureState to handle the "Unknown" states… r=boek
… of AI controls This fixes the issue in bug 2040784 by ensuring that we have an unknown state where we don't know yet if the feature is enabled or not. This becomes important when we need to know if the user turned a feature on explicitly, or it was on by default, or they just never never interacted with the feature. I commandeered this from Segun. Only downstream changes from me were to swap to the IO dispatcher while collecting the storage flow in the registry and a cosmetic change to remove the text fixtures he introduced in favor of the existing `inMemory` factories. Pull request: mozilla-firefox/firefox#274 UltraBlame original commit: 2f6f9d8fa5263e68eb27d229e5ace31c90a0166f
1 parent 53c75bd commit a170ae0

20 files changed

Lines changed: 312 additions & 57 deletions

File tree

mobile/android/android-components/components/concept/ai-controls/src/main/java/mozilla/components/concept/ai/controls/AIControllableFeature.kt

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ package mozilla.components.concept.ai.controls
66

77
import kotlinx.coroutines.flow.Flow
88
import kotlinx.coroutines.flow.MutableStateFlow
9+
import kotlinx.coroutines.flow.mapNotNull
10+
import kotlinx.coroutines.flow.update
11+
import mozilla.components.concept.ai.controls.AIFeatureState.Disabled
12+
import mozilla.components.concept.ai.controls.AIFeatureState.Enabled
13+
import mozilla.components.concept.ai.controls.AIFeatureState.Unknown
914

1015
/**
1116
* Metadata defining an AI Feature.
@@ -30,11 +35,39 @@ interface AIFeatureMetadata {
3035
val description: Description
3136
}
3237

38+
/**
39+
* Describes the state of the AI feature. Represents all the possible states as
40+
* [Enabled], [Disabled] and [Unknown].
41+
*/
42+
sealed class AIFeatureState {
43+
44+
/**
45+
* Describes the state when the AI feature has been enabled.
46+
*/
47+
object Enabled : AIFeatureState()
48+
49+
/**
50+
* Describes the state where the AI feature has been disabled.
51+
*/
52+
object Disabled : AIFeatureState()
53+
54+
/**
55+
* Describes the state where it is not known if the AI feature is enabled or disabled.
56+
* This state is likely to occur if we check the AI controls state before the underlying
57+
* AI feature is able to say whether it's enabled or not.
58+
*
59+
* Because it is possible for the [AIFeatureBlock.isBlocked] to be true, for a specific feature
60+
* to be turned on, we need to know if that feature is explicitly turned on or if the user
61+
* has simply never interacted with the AI controls, and that is when this comes in handy.
62+
*/
63+
object Unknown : AIFeatureState()
64+
}
65+
3366
/**
3467
* A feature that can be enabled or disabled by AI controls.
3568
*/
3669
interface AIControllableFeature : AIFeatureMetadata {
37-
val isEnabled: Flow<Boolean>
70+
val featureState: Flow<AIFeatureState>
3871

3972
/**
4073
* Enables or disables this feature.
@@ -48,20 +81,38 @@ interface AIControllableFeature : AIFeatureMetadata {
4881
fun inMemory(
4982
id: AIFeatureMetadata.FeatureId = AIFeatureMetadata.FeatureId("inMemory"),
5083
description: AIFeatureMetadata.Description = AIFeatureMetadata.Description(0, 0, 0),
51-
initialEnabled: Boolean = false,
52-
): AIControllableFeature = InMemoryAIControllableFeature(id, description, initialEnabled)
84+
initialFeatureState: AIFeatureState = AIFeatureState.Unknown,
85+
): AIControllableFeature = InMemoryAIControllableFeature(id, description, initialFeatureState)
5386
}
5487
}
5588

89+
/**
90+
* Convenience function for mapping [AIControllableFeature.featureState] to a [Flow] of non-null [Boolean] values.
91+
* [AIFeatureState.Unknown] is resolved as not enabled
92+
*/
93+
val AIControllableFeature.isEnabled: Flow<Boolean>
94+
get() = featureState.mapNotNull {
95+
when (it) {
96+
is Enabled -> true
97+
is Disabled -> false
98+
is Unknown -> null
99+
}
100+
}
101+
56102
private class InMemoryAIControllableFeature(
57103
override val id: AIFeatureMetadata.FeatureId,
58104
override val description: AIFeatureMetadata.Description,
59-
initialEnabled: Boolean,
105+
initialFeatureState: AIFeatureState,
60106
) : AIControllableFeature {
61-
private val _isEnabled = MutableStateFlow(initialEnabled)
62-
override val isEnabled: Flow<Boolean> = _isEnabled
107+
108+
private val _featureState =
109+
MutableStateFlow(initialFeatureState)
110+
override val featureState: Flow<AIFeatureState>
111+
get() = _featureState
63112

64113
override suspend fun set(enabled: Boolean) {
65-
_isEnabled.value = enabled
114+
_featureState.update {
115+
if (enabled) Enabled else Disabled
116+
}
66117
}
67118
}

mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/PageSummaryFeature.kt

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
package mozilla.components.feature.summarize
66

77
import kotlinx.coroutines.flow.Flow
8-
import kotlinx.coroutines.flow.emitAll
9-
import kotlinx.coroutines.flow.flow
8+
import kotlinx.coroutines.flow.map
109
import mozilla.components.concept.ai.controls.AIControllableFeature
1110
import mozilla.components.concept.ai.controls.AIFeatureMetadata
11+
import mozilla.components.concept.ai.controls.AIFeatureState
1212
import mozilla.components.feature.summarize.settings.SummarizationSettings
1313
import mozilla.components.ui.icons.R as iconsR
1414

@@ -19,7 +19,15 @@ class PageSummaryFeature(
1919
private val settings: SummarizationSettings,
2020
) : AIControllableFeature, AIFeatureMetadata by Companion {
2121

22-
override val isEnabled: Flow<Boolean> = flow { emitAll(settings.getFeatureEnabledUserStatus()) }
22+
override val featureState: Flow<AIFeatureState>
23+
get() = settings.getFeatureEnabledUserStatus()
24+
.map { enabledState ->
25+
when {
26+
enabledState == null -> AIFeatureState.Unknown
27+
enabledState -> AIFeatureState.Enabled
28+
else -> AIFeatureState.Disabled
29+
}
30+
}
2331

2432
override suspend fun set(enabled: Boolean) {
2533
settings.setFeatureEnabledUserStatus(enabled)

mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/settings/SummarizationSettings.kt

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,13 @@ interface SummarizationSettings {
2828
/**
2929
* @return A [Flow] emitting the user's current preference for whether the summarization
3030
* feature is enabled.
31+
*
32+
* - It emits true when the feature is enabled
33+
* - false when it's not, or
34+
* - null when we don't know this means that the preference for this feature has never been set -
35+
* either by the user, or by the app.
3136
*/
32-
suspend fun getFeatureEnabledUserStatus(): Flow<Boolean>
37+
fun getFeatureEnabledUserStatus(): Flow<Boolean?>
3338

3439
/**
3540
* Persists the user's preference for whether the summarization feature is enabled.
@@ -81,7 +86,7 @@ interface SummarizationSettings {
8186
* @return An in-memory [SummarizationSettings] instance.
8287
*/
8388
fun inMemory(
84-
isFeatureEnabled: Boolean = false,
89+
isFeatureEnabled: Boolean? = null,
8590
isGestureEnabled: Boolean = false,
8691
hasConsentedToShake: Boolean = false,
8792
shakeConsentRejectedCount: Int = 0,
@@ -94,7 +99,7 @@ interface SummarizationSettings {
9499
MutableStateFlow(hasConsentedToShake)
95100
private var shakeConsentRejectedCount = 0
96101

97-
override suspend fun getFeatureEnabledUserStatus(): Flow<Boolean> = isFeatureEnabledFlow
102+
override fun getFeatureEnabledUserStatus(): Flow<Boolean?> = isFeatureEnabledFlow
98103

99104
override suspend fun setFeatureEnabledUserStatus(newValue: Boolean) {
100105
isFeatureEnabledFlow.emit(newValue)
@@ -140,8 +145,8 @@ internal class DataStoreBackedSettings(private val dataStore: DataStore<Preferen
140145
private val hasConsentedToShakeKey = booleanPreferencesKey("has_consented_to_shake_key")
141146
private val shakeConsentRejectedCountKey = intPreferencesKey("shake_consent_rejected_count_key")
142147

143-
override suspend fun getFeatureEnabledUserStatus(): Flow<Boolean> = dataStore.data.map { preferences ->
144-
preferences[featureEnabledKey] ?: true
148+
override fun getFeatureEnabledUserStatus(): Flow<Boolean?> = dataStore.data.map { preferences ->
149+
preferences[featureEnabledKey]
145150
}
146151

147152
override suspend fun setFeatureEnabledUserStatus(newValue: Boolean) {
@@ -182,7 +187,7 @@ internal class DataStoreBackedSettings(private val dataStore: DataStore<Preferen
182187
val updatedCount = (preferences[shakeConsentRejectedCountKey] ?: 0) + 1
183188
preferences[shakeConsentRejectedCountKey] = updatedCount
184189
if (updatedCount >= MAX_SHAKE_CONSENT_REJECTION) {
185-
setGestureEnabledUserStatus(false)
190+
preferences[gestureEnabledKey] = false
186191
}
187192
}
188193
}

mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/settings/SummarizeSettingsMiddleware.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import mozilla.components.lib.state.Store
1313
/**
1414
* Middleware for the summarize settings screen that persists preference changes.
1515
*
16-
* @param settings The [SummarizationFeatureSettings] to persist preference changes to.
16+
* @param settings The [SummarizationSettings] to persist preference changes to.
1717
* @param onLearnMoreClicked Callback invoked when the learn more link is clicked.
1818
*/
1919
class SummarizeSettingsMiddleware(
@@ -35,7 +35,7 @@ class SummarizeSettingsMiddleware(
3535
ViewAppeared -> scope.launch {
3636
store.dispatch(
3737
SettingsLoaded(
38-
isFeatureEnabled = settings.getFeatureEnabledUserStatus().first(),
38+
isFeatureEnabled = settings.getFeatureEnabledUserStatus().first() == true,
3939
isGestureEnabled = settings.getGestureEnabledUserStatus().first(),
4040
),
4141
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
package mozilla.components.feature.summarize
6+
7+
import kotlinx.coroutines.flow.first
8+
import kotlinx.coroutines.test.runTest
9+
import mozilla.components.concept.ai.controls.isEnabled
10+
import mozilla.components.feature.summarize.settings.SummarizationSettings
11+
import org.junit.Assert.assertFalse
12+
import org.junit.Assert.assertTrue
13+
import org.junit.Test
14+
15+
class PageSummaryFeatureTest {
16+
17+
@Test
18+
fun `isEnabled reflects feature enabled status from settings`() = runTest {
19+
val settings = SummarizationSettings.inMemory(isFeatureEnabled = true)
20+
val feature = PageSummaryFeature(settings)
21+
22+
assertTrue(feature.isEnabled.first())
23+
}
24+
25+
@Test
26+
fun `isEnabled reflects feature disabled status from settings`() = runTest {
27+
val settings = SummarizationSettings.inMemory(isFeatureEnabled = false)
28+
val feature = PageSummaryFeature(settings)
29+
30+
assertFalse(feature.isEnabled.first())
31+
}
32+
33+
@Test
34+
fun `set enabled persists to settings`() = runTest {
35+
val settings = SummarizationSettings.inMemory(isFeatureEnabled = false)
36+
val feature = PageSummaryFeature(settings)
37+
38+
feature.set(true)
39+
40+
assertTrue(feature.isEnabled.first())
41+
}
42+
43+
@Test
44+
fun `set disabled persists to settings`() = runTest {
45+
val settings = SummarizationSettings.inMemory(isFeatureEnabled = true)
46+
val feature = PageSummaryFeature(settings)
47+
48+
feature.set(false)
49+
50+
assertFalse(feature.isEnabled.first())
51+
}
52+
}

mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/settings/PageSummariesSettingsMiddlewareTest.kt

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ package mozilla.components.feature.summarize.settings
77
import kotlinx.coroutines.CoroutineScope
88
import kotlinx.coroutines.ExperimentalCoroutinesApi
99
import kotlinx.coroutines.flow.first
10-
import kotlinx.coroutines.flow.last
11-
import kotlinx.coroutines.flow.take
1210
import kotlinx.coroutines.test.runCurrent
1311
import kotlinx.coroutines.test.runTest
1412
import org.junit.Assert.assertFalse
@@ -41,7 +39,7 @@ class PageSummariesSettingsMiddlewareTest {
4139
store.dispatch(SummarizePagesPreferenceToggled)
4240
this.runCurrent()
4341

44-
assertTrue(settings.getFeatureEnabledUserStatus().first())
42+
assertTrue(settings.getFeatureEnabledUserStatus().first() == true)
4543
}
4644

4745
@Test
@@ -59,7 +57,7 @@ class PageSummariesSettingsMiddlewareTest {
5957
store.dispatch(SummarizePagesPreferenceToggled)
6058
this.runCurrent()
6159

62-
assertFalse(settings.getFeatureEnabledUserStatus().first())
60+
assertFalse(settings.getFeatureEnabledUserStatus().first() == true)
6361
}
6462

6563
@Test
@@ -77,7 +75,7 @@ class PageSummariesSettingsMiddlewareTest {
7775
store.dispatch(ShakeToSummarizePreferenceToggled)
7876
this.runCurrent()
7977

80-
assertTrue(settings.getFeatureEnabledUserStatus().first())
78+
assertTrue(settings.getFeatureEnabledUserStatus().first() == true)
8179
}
8280

8381
@Test
@@ -113,7 +111,7 @@ class PageSummariesSettingsMiddlewareTest {
113111
store.dispatch(SummarizePagesPreferenceToggled)
114112
this.runCurrent()
115113

116-
assertFalse(settings.getFeatureEnabledUserStatus().first())
114+
assertFalse(settings.getFeatureEnabledUserStatus().first() == true)
117115
assertTrue(settings.getGestureEnabledUserStatus().first())
118116
}
119117

mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/settings/SummarizationSettingsTest.kt

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,27 @@ import mozilla.components.support.test.fakes.android.FakePreferencesDataStore
1010
import org.junit.Assert.assertFalse
1111
import org.junit.Assert.assertTrue
1212
import org.junit.Test
13+
import kotlin.test.assertNull
1314

1415
class SummarizationSettingsTest {
16+
17+
@Test
18+
fun `that user preference for feature returns null if the value has never been set`() =
19+
runTest {
20+
val dataStore = FakePreferencesDataStore()
21+
val settings = DataStoreBackedSettings(dataStore)
22+
23+
assertNull(
24+
settings.getFeatureEnabledUserStatus().first(),
25+
"Expected initial preference to be null because it has not been previous set",
26+
)
27+
}
28+
1529
@Test
1630
fun `that user preference for feature is persisted`() = runTest {
1731
val dataStore = FakePreferencesDataStore()
1832
val settings = DataStoreBackedSettings(dataStore)
1933

20-
assertTrue(settings.getFeatureEnabledUserStatus().first())
2134
settings.setFeatureEnabledUserStatus(false)
2235
assertFalse(settings.getHasConsentedToShake().first())
2336
}
@@ -27,7 +40,6 @@ class SummarizationSettingsTest {
2740
val dataStore = FakePreferencesDataStore()
2841
val settings = DataStoreBackedSettings(dataStore)
2942

30-
assertTrue(settings.getGestureEnabledUserStatus().first())
3143
settings.setGestureEnabledUserStatus(false)
3244
assertFalse(settings.getHasConsentedToShake().first())
3345
}
@@ -37,7 +49,6 @@ class SummarizationSettingsTest {
3749
val dataStore = FakePreferencesDataStore()
3850
val settings = DataStoreBackedSettings(dataStore)
3951

40-
assertFalse(settings.getHasConsentedToShake().first())
4152
settings.setHasConsentedToShake(true)
4253
assertTrue(settings.getHasConsentedToShake().first())
4354
}
@@ -47,7 +58,6 @@ class SummarizationSettingsTest {
4758
val dataStore = FakePreferencesDataStore()
4859
val settings = DataStoreBackedSettings(dataStore)
4960

50-
assertTrue(settings.getGestureEnabledUserStatus().first())
5161
settings.incrementShakeConsentRejectedCount()
5262
settings.incrementShakeConsentRejectedCount()
5363
settings.incrementShakeConsentRejectedCount()

0 commit comments

Comments
 (0)