1
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2025-03-03 04:55:54 +03:00

PM-15380 set up storage and retrieval of data to determine if we show the review prompt

This commit is contained in:
Dave Severns 2024-12-03 12:14:41 -05:00
parent 382597f356
commit c817921499
10 changed files with 532 additions and 0 deletions
app/src
main/java/com/x8bit/bitwarden
test/java/com/x8bit/bitwarden

View file

@ -325,4 +325,44 @@ interface SettingsDiskSource {
* Emits updates that track [getVaultRegisteredForExport] for the given [userId].
*/
fun getVaultRegisteredForExportFlow(userId: String): Flow<Boolean?>
/**
* Gets the number of qualifying add actions for the given [userId].
*/
fun getAddActionCount(userId: String): Int?
/**
* Stores the given [count] completed "add" actions for the given [userId].
*/
fun storeAddActionCount(userId: String, count: Int?)
/**
* Gets the number of qualifying copy actions for the given [userId].
*/
fun getCopyActionCount(userId: String): Int?
/**
* Stores the given [count] completed "copy" actions for the given [userId].
*/
fun storeCopyActionCount(userId: String, count: Int?)
/**
* Gets the number of qualifying create actions for the given [userId].
*/
fun getCreateActionCount(userId: String): Int?
/**
* Stores the given [count] completed "create" actions for the given [userId].
*/
fun storeCreateActionCount(userId: String, count: Int?)
/**
* Gets whether the given [userId] has been prompted for an app review.
*/
fun getUserHasBeenPromptedForReview(userId: String): Boolean?
/**
* Stores whether the given [userId] has been prompted for an app review.
*/
fun storeUserHasBeenPromptedForReview(userId: String, value: Boolean?)
}

View file

@ -37,6 +37,10 @@ private const val SHOW_AUTOFILL_SETTING_BADGE = "showAutofillSettingBadge"
private const val SHOW_UNLOCK_SETTING_BADGE = "showUnlockSettingBadge"
private const val SHOW_IMPORT_LOGINS_SETTING_BADGE = "showImportLoginsSettingBadge"
private const val IS_VAULT_REGISTERED_FOR_EXPORT = "isVaultRegisteredForExport"
private const val ADD_ACTION_COUNT = "addActionCount"
private const val COPY_ACTION_COUNT = "copyActionCount"
private const val CREATE_ACTION_COUNT = "createActionCount"
private const val HAS_BEEN_PROMPTED_FOR_REVIEW = "hasBeenPromptedForReview"
/**
* Primary implementation of [SettingsDiskSource].
@ -172,6 +176,10 @@ class SettingsDiskSourceImpl(
storeClearClipboardFrequencySeconds(userId = userId, frequency = null)
removeWithPrefix(prefix = ACCOUNT_BIOMETRIC_INTEGRITY_VALID_KEY.appendIdentifier(userId))
storeVaultRegisteredForExport(userId = userId, isRegistered = null)
storeAddActionCount(userId = userId, count = null)
storeCopyActionCount(userId = userId, count = null)
storeCreateActionCount(userId = userId, count = null)
storeUserHasBeenPromptedForReview(userId = userId, value = null)
// The following are intentionally not cleared so they can be
// restored after logging out and back in:
@ -446,6 +454,49 @@ class SettingsDiskSourceImpl(
getMutableVaultRegisteredForExportFlow(userId)
.onSubscription { emit(getVaultRegisteredForExport(userId)) }
override fun getAddActionCount(userId: String): Int? = getInt(
key = ADD_ACTION_COUNT.appendIdentifier(userId),
)
override fun storeAddActionCount(userId: String, count: Int?) {
putInt(
key = ADD_ACTION_COUNT.appendIdentifier(userId),
value = count,
)
}
override fun getCopyActionCount(userId: String): Int? = getInt(
key = COPY_ACTION_COUNT.appendIdentifier(userId),
)
override fun storeCopyActionCount(userId: String, count: Int?) {
putInt(
key = COPY_ACTION_COUNT.appendIdentifier(userId),
value = count,
)
}
override fun getCreateActionCount(userId: String): Int? = getInt(
key = CREATE_ACTION_COUNT.appendIdentifier(userId),
)
override fun storeCreateActionCount(userId: String, count: Int?) {
putInt(
key = CREATE_ACTION_COUNT.appendIdentifier(userId),
value = count,
)
}
override fun getUserHasBeenPromptedForReview(userId: String): Boolean? =
getBoolean(key = HAS_BEEN_PROMPTED_FOR_REVIEW.appendIdentifier(userId))
override fun storeUserHasBeenPromptedForReview(userId: String, value: Boolean?) {
putBoolean(
key = HAS_BEEN_PROMPTED_FOR_REVIEW.appendIdentifier(userId),
value = value,
)
}
private fun getMutableLastSyncFlow(
userId: String,
): MutableSharedFlow<Instant?> =

View file

@ -0,0 +1,27 @@
package com.x8bit.bitwarden.data.platform.manager
/**
* Responsible for managing whether or not the app review prompt should be shown.
*/
interface ReviewPromptManager {
/**
* Increments the add action count for the active user.
*/
fun incrementAddActionCount()
/**
* Increments the copy action count for the active user.
*/
fun incrementCopyActionCount()
/**
* Increments the create action count for the active user.
*/
fun incrementCreateActionCount()
/**
* Returns a boolean value indicating whether or not the user should be prompted to
* review the app.
*/
fun shouldPromptForAppReview(): Boolean
}

View file

@ -0,0 +1,78 @@
package com.x8bit.bitwarden.data.platform.manager
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.ui.platform.util.orZero
private const val ADD_ACTION_REQUIREMENT = 3
private const val COPY_ACTION_REQUIREMENT = 3
private const val CREATE_ACTION_REQUIREMENT = 3
/**
* Default implementation of [ReviewPromptManager].
*/
class ReviewPromptManagerImpl(
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
private val autofillEnabledManager: AutofillEnabledManager,
private val accessibilityEnabledManager: AccessibilityEnabledManager,
) : ReviewPromptManager {
override fun incrementAddActionCount() {
val activeUserId = authDiskSource.userState?.activeUserId ?: return
if (isMinimumAddActionsMet(activeUserId)) return
val currentValue = settingsDiskSource.getAddActionCount(activeUserId) ?: 0
settingsDiskSource.storeAddActionCount(
userId = activeUserId,
count = currentValue + 1,
)
}
override fun incrementCopyActionCount() {
val activeUserId = authDiskSource.userState?.activeUserId ?: return
if (isMinimumCopyActionsMet(activeUserId)) return
val currentValue = settingsDiskSource.getCopyActionCount(activeUserId) ?: 0
settingsDiskSource.storeCopyActionCount(
userId = activeUserId,
count = currentValue + 1,
)
}
override fun incrementCreateActionCount() {
val activeUserId = authDiskSource.userState?.activeUserId ?: return
if (isMinimumCreateActionsMet(activeUserId)) return
val currentValue = settingsDiskSource.getCreateActionCount(activeUserId) ?: 0
settingsDiskSource.storeCreateActionCount(
userId = activeUserId,
count = currentValue + 1,
)
}
override fun shouldPromptForAppReview(): Boolean {
val activeUserId = authDiskSource.userState?.activeUserId ?: return false
val promptHasNotBeenShown =
settingsDiskSource.getUserHasBeenPromptedForReview(activeUserId) != true
val autofillEnabled = autofillEnabledManager.isAutofillEnabledStateFlow.value
val accessibilityEnabled = accessibilityEnabledManager.isAccessibilityEnabledStateFlow.value
val minAddActionsMet = isMinimumAddActionsMet(activeUserId)
val minCopyActionsMet = isMinimumCopyActionsMet(activeUserId)
val minCreateActionsMet = isMinimumCreateActionsMet(activeUserId)
return (autofillEnabled || accessibilityEnabled) &&
(minAddActionsMet || minCopyActionsMet || minCreateActionsMet) &&
promptHasNotBeenShown
}
private fun isMinimumAddActionsMet(userId: String): Boolean {
return settingsDiskSource.getAddActionCount(userId).orZero() >= ADD_ACTION_REQUIREMENT
}
private fun isMinimumCopyActionsMet(userId: String): Boolean {
return settingsDiskSource.getCopyActionCount(userId).orZero() >= COPY_ACTION_REQUIREMENT
}
private fun isMinimumCreateActionsMet(userId: String): Boolean {
return settingsDiskSource.getCreateActionCount(userId).orZero() >= CREATE_ACTION_REQUIREMENT
}
}

View file

@ -6,6 +6,7 @@ import androidx.core.content.getSystemService
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.manager.AddTotpItemFromAuthenticatorManager
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.datasource.disk.EventDiskSource
import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource
@ -39,6 +40,8 @@ import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.platform.manager.PushManagerImpl
import com.x8bit.bitwarden.data.platform.manager.ResourceCacheManager
import com.x8bit.bitwarden.data.platform.manager.ResourceCacheManagerImpl
import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager
import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManagerImpl
import com.x8bit.bitwarden.data.platform.manager.SdkClientManager
import com.x8bit.bitwarden.data.platform.manager.SdkClientManagerImpl
import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingManager
@ -310,4 +313,18 @@ object PlatformManagerModule {
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
)
@Provides
@Singleton
fun provideReviewPromptManager(
authDiskSource: AuthDiskSource,
settingsDiskSource: SettingsDiskSource,
autofillEnabledManager: AutofillEnabledManager,
accessibilityEnabledManager: AccessibilityEnabledManager,
): ReviewPromptManager = ReviewPromptManagerImpl(
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
autofillEnabledManager = autofillEnabledManager,
accessibilityEnabledManager = accessibilityEnabledManager,
)
}

View file

@ -0,0 +1,7 @@
package com.x8bit.bitwarden.ui.platform.util
/**
* Extension to be applied to a nullable [Int] type where if the value is null, a default
* value of "0" is returned.
*/
fun Int?.orZero() = this ?: 0

View file

@ -1193,4 +1193,76 @@ class SettingsDiskSourceTest {
assertFalse(awaitItem() ?: true)
}
}
@Test
fun `getAddActionCount should pull from SharedPreferences`() {
val mockUserId = "mockUserId"
val addActionCountKey = "bwPreferencesStorage:addActionCount_$mockUserId"
fakeSharedPreferences.edit { putInt(addActionCountKey, 1) }
assertEquals(
1, settingsDiskSource.getAddActionCount(mockUserId),
)
}
@Test
fun `storeAddActionCount should update SharedPreferences`() {
val mockUserId = "mockUserId"
val addActionCountKey = "bwPreferencesStorage:addActionCount_$mockUserId"
settingsDiskSource.storeAddActionCount(mockUserId, 1)
assertEquals(1, fakeSharedPreferences.getInt(addActionCountKey, 0))
}
@Test
fun `getCopyActionCount should pull from SharedPreferences`() {
val mockUserId = "mockUserId"
val copyActionCountKey = "bwPreferencesStorage:copyActionCount_$mockUserId"
fakeSharedPreferences.edit { putInt(copyActionCountKey, 1) }
assertEquals(
1, settingsDiskSource.getCopyActionCount(mockUserId),
)
}
@Test
fun `storeCopyActionCount should update SharedPreferences`() {
val mockUserId = "mockUserId"
val copyActionCountKey = "bwPreferencesStorage:copyActionCount_$mockUserId"
settingsDiskSource.storeCopyActionCount(mockUserId, 1)
assertEquals(1, fakeSharedPreferences.getInt(copyActionCountKey, 0))
}
@Test
fun `getCreateActionCount should pull from SharedPreferences`() {
val mockUserId = "mockUserId"
val createActionCountKey = "bwPreferencesStorage:createActionCount_$mockUserId"
fakeSharedPreferences.edit { putInt(createActionCountKey, 1) }
assertEquals(1, settingsDiskSource.getCreateActionCount(mockUserId))
}
@Test
fun `storeCreateActionCount should update SharedPreferences`() {
val mockUserId = "mockUserId"
val createActionCountKey = "bwPreferencesStorage:createActionCount_$mockUserId"
settingsDiskSource.storeCreateActionCount(mockUserId, 1)
assertEquals(1, fakeSharedPreferences.getInt(createActionCountKey, 0))
}
@Test
fun `getUserHasBeenPromptedForReview should pull from SharedPreferences`() {
val mockUserId = "mockUserId"
val hasUserBeenPromptedToReviewKey =
"bwPreferencesStorage:hasBeenPromptedForReview_$mockUserId"
fakeSharedPreferences.edit { putBoolean(hasUserBeenPromptedToReviewKey, true) }
assertTrue(settingsDiskSource.getUserHasBeenPromptedForReview(mockUserId)!!)
}
@Test
fun `storeUserHasBeenPromptedForReview should update SharedPreferences`() {
val mockUserId = "mockUserId"
val hasUserBeenPromptedToReviewKey =
"bwPreferencesStorage:hasBeenPromptedForReview_$mockUserId"
settingsDiskSource.storeUserHasBeenPromptedForReview(mockUserId, true)
assertTrue(
fakeSharedPreferences.getBoolean(hasUserBeenPromptedToReviewKey, false),
)
}
}

View file

@ -65,6 +65,10 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val userShowUnlockBadge = mutableMapOf<String, Boolean?>()
private val userShowImportLoginsBadge = mutableMapOf<String, Boolean?>()
private val vaultRegisteredForExport = mutableMapOf<String, Boolean?>()
private val addActionCount = mutableMapOf<String, Int?>()
private val copyActionCount = mutableMapOf<String, Int?>()
private val createActionCount = mutableMapOf<String, Int?>()
private val userHasBeenPromptedForReview = mutableMapOf<String, Boolean?>()
private val mutableShowAutoFillSettingBadgeFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>()
@ -353,6 +357,38 @@ class FakeSettingsDiskSource : SettingsDiskSource {
emit(getVaultRegisteredForExport(userId = userId))
}
override fun getAddActionCount(userId: String): Int? {
return addActionCount[userId]
}
override fun storeAddActionCount(userId: String, count: Int?) {
addActionCount[userId] = count
}
override fun getCopyActionCount(userId: String): Int? {
return copyActionCount[userId]
}
override fun storeCopyActionCount(userId: String, count: Int?) {
copyActionCount[userId] = count
}
override fun getCreateActionCount(userId: String): Int? {
return createActionCount[userId]
}
override fun storeCreateActionCount(userId: String, count: Int?) {
createActionCount[userId] = count
}
override fun getUserHasBeenPromptedForReview(userId: String): Boolean? {
return userHasBeenPromptedForReview[userId]
}
override fun storeUserHasBeenPromptedForReview(userId: String, value: Boolean?) {
userHasBeenPromptedForReview[userId] = value
}
//region Private helper functions
private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow<Boolean?> {
return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) {

View file

@ -0,0 +1,183 @@
package com.x8bit.bitwarden.data.platform.manager
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson
import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource
import com.x8bit.bitwarden.data.autofill.accessibility.manager.FakeAccessibilityEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManagerImpl
import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeSettingsDiskSource
import io.mockk.every
import io.mockk.mockk
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
class ReviewPromptManagerTest {
private val autofillEnabledManager: AutofillEnabledManager = AutofillEnabledManagerImpl()
private val fakeAccessibilityEnabledManager = FakeAccessibilityEnabledManager()
private val fakeAuthDiskSource = FakeAuthDiskSource()
private val fakeSettingsDiskSource = FakeSettingsDiskSource()
private val reviewPromptManager = ReviewPromptManagerImpl(
autofillEnabledManager = autofillEnabledManager,
accessibilityEnabledManager = fakeAccessibilityEnabledManager,
authDiskSource = fakeAuthDiskSource,
settingsDiskSource = fakeSettingsDiskSource,
)
@Test
fun `incrementAddActionCount increments stored value as expected`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
reviewPromptManager.incrementAddActionCount()
assertEquals(
1,
fakeSettingsDiskSource.getAddActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementAddActionCount()
assertEquals(
2,
fakeSettingsDiskSource.getAddActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementAddActionCount()
assertEquals(
3,
fakeSettingsDiskSource.getAddActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementAddActionCount()
// Should not increment over 3.
assertEquals(
3,
fakeSettingsDiskSource.getAddActionCount(
userId = USER_ID,
),
)
}
@Test
fun `incrementCopyActionCount increments stored value as expected`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
reviewPromptManager.incrementCopyActionCount()
assertEquals(
1,
fakeSettingsDiskSource.getCopyActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCopyActionCount()
assertEquals(
2,
fakeSettingsDiskSource.getCopyActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCopyActionCount()
assertEquals(
3,
fakeSettingsDiskSource.getCopyActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCopyActionCount()
assertEquals(
3,
fakeSettingsDiskSource.getCopyActionCount(
userId = USER_ID,
),
)
}
@Test
fun `incrementCreateActionCount increments stored value as expected`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
reviewPromptManager.incrementCreateActionCount()
assertEquals(
1,
fakeSettingsDiskSource.getCreateActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCreateActionCount()
assertEquals(
2,
fakeSettingsDiskSource.getCreateActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCreateActionCount()
assertEquals(
3,
fakeSettingsDiskSource.getCreateActionCount(
userId = USER_ID,
),
)
reviewPromptManager.incrementCreateActionCount()
assertEquals(
3,
fakeSettingsDiskSource.getCreateActionCount(
userId = USER_ID,
),
)
}
@Test
fun `shouldPromptForAppReview should default to false if no active user`() {
assertFalse(reviewPromptManager.shouldPromptForAppReview())
}
@Suppress("MaxLineLength")
@Test
fun `shouldPromptForAppReview should return true if one auto fill service is enabled and one actions requirement is met`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
fakeAccessibilityEnabledManager.isAccessibilityEnabled = true
autofillEnabledManager.isAutofillEnabled = false
fakeSettingsDiskSource.storeCopyActionCount(USER_ID, 0)
fakeSettingsDiskSource.storeCreateActionCount(USER_ID, 0)
fakeSettingsDiskSource.storeAddActionCount(USER_ID, 4)
assertTrue(reviewPromptManager.shouldPromptForAppReview())
}
@Suppress("MaxLineLength")
@Test
fun `shouldPromptForAppReview should return false if prompt has been shown but other criteria is met`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
fakeAccessibilityEnabledManager.isAccessibilityEnabled = true
fakeSettingsDiskSource.storeUserHasBeenPromptedForReview(USER_ID, true)
fakeSettingsDiskSource.storeAddActionCount(USER_ID, 4)
assertFalse(reviewPromptManager.shouldPromptForAppReview())
}
@Test
fun `shouldPromptForAppReview should return false if no auto fill service is enabled`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
fakeAccessibilityEnabledManager.isAccessibilityEnabled = false
autofillEnabledManager.isAutofillEnabled = false
fakeSettingsDiskSource.storeCopyActionCount(USER_ID, 0)
fakeSettingsDiskSource.storeCreateActionCount(USER_ID, 0)
fakeSettingsDiskSource.storeAddActionCount(USER_ID, 4)
assertFalse(reviewPromptManager.shouldPromptForAppReview())
}
@Test
fun `shouldPromptForAppReview should return false if no action count is met`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
fakeAccessibilityEnabledManager.isAccessibilityEnabled = true
autofillEnabledManager.isAutofillEnabled = true
fakeSettingsDiskSource.storeCopyActionCount(USER_ID, 1)
fakeSettingsDiskSource.storeCreateActionCount(USER_ID, 0)
fakeSettingsDiskSource.storeAddActionCount(USER_ID, 2)
assertFalse(reviewPromptManager.shouldPromptForAppReview())
}
}
private const val USER_ID = "user_id"
private val MOCK_USER_STATE = mockk<UserStateJson>() {
every { activeUserId } returns USER_ID
}

View file

@ -0,0 +1,21 @@
package com.x8bit.bitwarden.ui.platform.util
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
class IntExtensionsTest {
@Test
fun `orZero returns zero when null`() {
val nullInt: Int? = null
assertEquals(0, nullInt.orZero())
}
@Test
fun `orZero returns value when not null`() {
val nonNullInt = 42
assertEquals(42, nonNullInt.orZero())
val negativeNonNullInt = -42
assertEquals(-42, negativeNonNullInt.orZero())
assertEquals(0, 0.orZero())
}
}