diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManager.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManager.kt index 3efee4851a..0df74700f7 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManager.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManager.kt @@ -23,13 +23,4 @@ interface VaultMigrationManager { * @param cipherList List of ciphers from the sync response to check for personal items. */ fun verifyAndUpdateMigrationState(userId: String, cipherList: List) - - /** - * Checks if the user should migrate their vault based on policies, feature flags, - * network connectivity, and whether they have personal items. - * - * @param hasPersonalItems Callback to check if the user has personal items. - * @return true if migration conditions are met, false otherwise. - */ - fun shouldMigrateVault(hasPersonalItems: () -> Boolean): Boolean } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerImpl.kt index 5f07a792b4..3bb1c29a8c 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerImpl.kt @@ -33,7 +33,7 @@ class VaultMigrationManagerImpl( cipherList: List, ) { mutableVaultMigrationDataStateFlow.update { - if (shouldMigrateVault { cipherList.any { it.organizationId == null } }) { + if (shouldMigrateVault(cipherList)) { val orgId = policyManager.getPersonalOwnershipPolicyOrganizationId() val orgName = authDiskSource .getOrganizations(userId = userId) @@ -54,12 +54,19 @@ class VaultMigrationManagerImpl( } } - override fun shouldMigrateVault(hasPersonalItems: () -> Boolean): Boolean { + /** + * Checks if the user should migrate their vault based on policies, feature flags, + * network connectivity, and whether they have personal items. + * + * @param cipherList List of ciphers to check for personal items. + * @return true if migration conditions are met, false otherwise. + */ + private fun shouldMigrateVault(cipherList: List): Boolean { return policyManager .getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) .any() && featureFlagManager.getFeatureFlag(FlagKey.MigrateMyVaultToMyItems) && connectionManager.isNetworkConnected && - hasPersonalItems() + cipherList.any { it.organizationId == null } } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerTest.kt index aa7cbc49ae..af23ca5740 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultMigrationManagerTest.kt @@ -348,79 +348,6 @@ class VaultMigrationManagerTest { expectNoEvents() } } - - @Test - fun `shouldMigrateVault returns true when all conditions are met`() { - fakeAuthDiskSource.userState = MOCK_USER_STATE - - val mockPolicy = createMockPolicy(number = 1, type = PolicyTypeJson.PERSONAL_OWNERSHIP) - every { - policyManager.getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) - } returns listOf(mockPolicy) - - val result = vaultMigrationManager.shouldMigrateVault { true } - - assertEquals(true, result) - } - - @Test - fun `shouldMigrateVault returns false when no policies`() { - fakeAuthDiskSource.userState = MOCK_USER_STATE - - every { - policyManager.getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) - } returns emptyList() - - val result = vaultMigrationManager.shouldMigrateVault { true } - - assertEquals(false, result) - } - - @Test - fun `shouldMigrateVault returns false when feature flag is disabled`() { - fakeAuthDiskSource.userState = MOCK_USER_STATE - - val mockPolicy = createMockPolicy(number = 1, type = PolicyTypeJson.PERSONAL_OWNERSHIP) - every { - policyManager.getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) - } returns listOf(mockPolicy) - every { - mockFeatureFlagManager.getFeatureFlag(FlagKey.MigrateMyVaultToMyItems) - } returns false - - val result = vaultMigrationManager.shouldMigrateVault { true } - - assertEquals(false, result) - } - - @Test - fun `shouldMigrateVault returns false when no network`() { - fakeAuthDiskSource.userState = MOCK_USER_STATE - - val mockPolicy = createMockPolicy(number = 1, type = PolicyTypeJson.PERSONAL_OWNERSHIP) - every { - policyManager.getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) - } returns listOf(mockPolicy) - every { connectionManager.isNetworkConnected } returns false - - val result = vaultMigrationManager.shouldMigrateVault { true } - - assertEquals(false, result) - } - - @Test - fun `shouldMigrateVault returns false when no personal items`() { - fakeAuthDiskSource.userState = MOCK_USER_STATE - - val mockPolicy = createMockPolicy(number = 1, type = PolicyTypeJson.PERSONAL_OWNERSHIP) - every { - policyManager.getActivePolicies(PolicyTypeJson.PERSONAL_OWNERSHIP) - } returns listOf(mockPolicy) - - val result = vaultMigrationManager.shouldMigrateVault { false } - - assertEquals(false, result) - } } private val MOCK_USER_STATE = UserStateJson( diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt index 79af9644bc..8a3f43750c 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt @@ -146,7 +146,6 @@ class VaultSyncManagerTest { private val vaultMigrationManager: VaultMigrationManager = mockk { every { vaultMigrationDataStateFlow } returns mutableVaultMigrationDataStateFlow every { verifyAndUpdateMigrationState(userId = any(), cipherList = any()) } just runs - every { shouldMigrateVault(any()) } returns false } private val vaultSyncManager: VaultSyncManager = VaultSyncManagerImpl(