pr feedback

bitwarden · Sep 10, 2024 · 79fe37c · 79fe37c
1 parent bd6cc1e
commit 79fe37c
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 47 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutofillScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutofillScreen.kt
@@ -94,7 +94,6 @@ fun SetupAutoFillScreen(
         null -> Unit
     }
 
-    BackHandler(onBack = handler.onTurnOnLaterClick)
     val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior()
     BitwardenScaffold(
         modifier = Modifier
@@ -163,11 +162,9 @@ private fun SetupAutoFillContent(
         )
         Spacer(modifier = Modifier.height(24.dp))
         BitwardenWideSwitch(
-            label =
-            stringResource(
+            label = stringResource(
                 R.string.autofill_services,
             ),
-
             isChecked = autofillEnabled,
             onCheckedChange = onAutofillServiceChanged,
             modifier = Modifier

diff --git a/.../test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt b/.../test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt
@@ -1,40 +1,32 @@
 package com.x8bit.bitwarden.ui.auth.feature.accountsetup
 
 import app.cash.turbine.test
-import com.google.common.base.Verify.verify
 import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
 import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
 import io.mockk.every
+import io.mockk.just
 import io.mockk.mockk
+import io.mockk.runs
 import io.mockk.verify
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertNull
 import org.junit.jupiter.api.Assertions.assertTrue
-import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 
 class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     private val mutableAutoFillEnabledStateFlow = MutableStateFlow(false)
     private val settingsRepository = mockk<SettingsRepository>(relaxed = true) {
         every { isAutofillEnabledStateFlow } returns mutableAutoFillEnabledStateFlow
-        every { disableAutofill() } answers {
-            mutableAutoFillEnabledStateFlow.value = false
-        }
-    }
-
-    private lateinit var viewModel: SetupAutoFillViewModel
-
-    @BeforeEach
-    fun setup() {
-        viewModel = SetupAutoFillViewModel(settingsRepository)
+        every { disableAutofill() } just runs
     }
 
     @Test
     fun `handleAutofillEnabledUpdateReceive updates autofillEnabled state`() {
+        val viewModel = createViewModel()
         assertFalse(viewModel.stateFlow.value.autofillEnabled)
         mutableAutoFillEnabledStateFlow.value = true
 
@@ -44,6 +36,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
     @Test
     fun `handleAutofillServiceChanged with autofillEnabled true navigates to autofill settings`() =
         runTest {
+            val viewModel = createViewModel()
             viewModel.eventFlow.test {
                 viewModel.trySendAction(SetupAutoFillAction.AutofillServiceChanged(true))
                 assertEquals(
@@ -55,20 +48,21 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     @Suppress("MaxLineLength")
     @Test
-    fun `handleAutofillServiceChanged with autofillEnabled false disables autofill and cause state update`() =
+    fun `handleAutofillServiceChanged with autofillEnabled false disables autofill`() =
         runTest {
+            val viewModel = createViewModel()
             mutableAutoFillEnabledStateFlow.value = true
             assertTrue(viewModel.stateFlow.value.autofillEnabled)
             viewModel.eventFlow.test {
                 viewModel.trySendAction(SetupAutoFillAction.AutofillServiceChanged(false))
                 expectNoEvents()
             }
             verify { settingsRepository.disableAutofill() }
-            assertFalse(viewModel.stateFlow.value.autofillEnabled)
         }
 
     @Test
     fun `handleContinueClick sends NavigateToCompleteSetup event`() = runTest {
+        val viewModel = createViewModel()
         viewModel.eventFlow.test {
             viewModel.trySendAction(SetupAutoFillAction.ContinueClick)
             assertEquals(SetupAutoFillEvent.NavigateToCompleteSetup, awaitItem())
@@ -77,6 +71,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     @Test
     fun `handleTurnOnLater click sets dialogState to TurnOnLaterDialog`() {
+        val viewModel = createViewModel()
         viewModel.trySendAction(SetupAutoFillAction.TurnOnLaterClick)
         assertEquals(
             SetupAutoFillDialogState.TurnOnLaterDialog,
@@ -86,6 +81,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     @Test
     fun `handleTurnOnLaterConfirmClick sends NavigateToCompleteSetup event`() = runTest {
+        val viewModel = createViewModel()
         viewModel.eventFlow.test {
             viewModel.trySendAction(SetupAutoFillAction.TurnOnLaterConfirmClick)
             assertEquals(SetupAutoFillEvent.NavigateToCompleteSetup, awaitItem())
@@ -94,6 +90,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     @Test
     fun `handleDismissDialog sets dialogState to null`() {
+        val viewModel = createViewModel()
         viewModel.trySendAction(SetupAutoFillAction.TurnOnLaterClick)
         assertEquals(
             SetupAutoFillDialogState.TurnOnLaterDialog,
@@ -105,10 +102,13 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
 
     @Test
     fun `handleAutoFillServiceFallback sets dialogState to AutoFillFallbackDialog`() {
+        val viewModel = createViewModel()
         viewModel.trySendAction(SetupAutoFillAction.AutoFillServiceFallback)
         assertEquals(
             SetupAutoFillDialogState.AutoFillFallbackDialog,
             viewModel.stateFlow.value.dialogState,
         )
     }
+
+    private fun createViewModel() = SetupAutoFillViewModel(settingsRepository)
 }
diff --git a/...src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutofillScreenTest.kt b/...src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutofillScreenTest.kt
@@ -16,6 +16,7 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.update
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -24,8 +25,7 @@ class SetupAutofillScreenTest : BaseComposeTest() {
 
     private var onNavigateToCompleteSetupCalled = false
     private val mutableEventFlow = bufferedMutableSharedFlow<SetupAutoFillEvent>()
-    private val mutableStateFlow =
-        MutableStateFlow(SetupAutoFillState(dialogState = null, autofillEnabled = false))
+    private val mutableStateFlow = MutableStateFlow(DEFAULT_STATE)
 
     private val viewModel = mockk<SetupAutoFillViewModel>(relaxed = true) {
         every { eventFlow } returns mutableEventFlow
@@ -36,7 +36,7 @@ class SetupAutofillScreenTest : BaseComposeTest() {
 
     @Before
     fun setup() {
-        setContentWithBackDispatcher {
+        composeTestRule.setContent {
             SetupAutoFillScreen(
                 onNavigateToCompleteSetup = { onNavigateToCompleteSetupCalled = true },
                 intentManager = intentManager,
@@ -58,7 +58,9 @@ class SetupAutofillScreenTest : BaseComposeTest() {
 
     @Test
     fun `Turning off autofill should send AutofillServiceChanged with value of false`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(autofillEnabled = true)
+        mutableStateFlow.update {
+            it.copy(autofillEnabled = true)
+        }
         composeTestRule
             .onNodeWithText("Auto-fill services", ignoreCase = true)
             .performScrollTo()
@@ -90,14 +92,6 @@ class SetupAutofillScreenTest : BaseComposeTest() {
         }
     }
 
-    @Test
-    fun `System back press should send turn on later action`() {
-        backDispatcher?.onBackPressed()
-        verify {
-            viewModel.trySendAction(SetupAutoFillAction.TurnOnLaterClick)
-        }
-    }
-
     @Test
     fun `NavigateToAutoFillSettings should start system autofill settings activity`() {
         every { intentManager.startSystemAutofillSettingsActivity() } returns true
@@ -123,9 +117,11 @@ class SetupAutofillScreenTest : BaseComposeTest() {
 
     @Test
     fun `Show autofill fallback dialog when dialog state is AutoFillFallbackDialog`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(
-            dialogState = SetupAutoFillDialogState.AutoFillFallbackDialog,
-        )
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = SetupAutoFillDialogState.AutoFillFallbackDialog,
+            )
+        }
         composeTestRule
             .onNode(isDialog())
             .assertIsDisplayed()
@@ -141,9 +137,11 @@ class SetupAutofillScreenTest : BaseComposeTest() {
     @Suppress("MaxLineLength")
     @Test
     fun `When autofill fallback dialog is dismissed, sends action to dismiss dialog and is removed when state is null`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(
-            dialogState = SetupAutoFillDialogState.AutoFillFallbackDialog,
-        )
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = SetupAutoFillDialogState.AutoFillFallbackDialog,
+            )
+        }
         composeTestRule
             .onNode(isDialog())
             .assertIsDisplayed()
@@ -152,15 +150,21 @@ class SetupAutofillScreenTest : BaseComposeTest() {
             .filterToOne(hasAnyAncestor(isDialog()))
             .performClick()
         verify { viewModel.trySendAction(SetupAutoFillAction.DismissDialog) }
-        mutableStateFlow.value = mutableStateFlow.value.copy(dialogState = null)
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = null,
+            )
+        }
         composeTestRule.assertNoDialogExists()
     }
 
     @Test
     fun `Show turn on later dialog when dialog state is TurnOnLaterDialog`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(
-            dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
-        )
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
+            )
+        }
         composeTestRule
             .onNode(isDialog())
             .assertIsDisplayed()
@@ -172,9 +176,11 @@ class SetupAutofillScreenTest : BaseComposeTest() {
 
     @Test
     fun `On confirm click on TurnOnLaterDialog, sends action to turn on later`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(
-            dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
-        )
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
+            )
+        }
         composeTestRule
             .onNode(isDialog())
             .assertIsDisplayed()
@@ -189,9 +195,11 @@ class SetupAutofillScreenTest : BaseComposeTest() {
     @Suppress("MaxLineLength")
     @Test
     fun `When turn on later dialog is dismissed, sends action to dismiss dialog and is removed when state is null`() {
-        mutableStateFlow.value = mutableStateFlow.value.copy(
-            dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
-        )
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = SetupAutoFillDialogState.TurnOnLaterDialog,
+            )
+        }
         composeTestRule
             .onNode(isDialog())
             .assertIsDisplayed()
@@ -200,7 +208,13 @@ class SetupAutofillScreenTest : BaseComposeTest() {
             .filterToOne(hasAnyAncestor(isDialog()))
             .performClick()
         verify { viewModel.trySendAction(SetupAutoFillAction.DismissDialog) }
-        mutableStateFlow.value = mutableStateFlow.value.copy(dialogState = null)
+        mutableStateFlow.update {
+            it.copy(
+                dialogState = null,
+            )
+        }
         composeTestRule.assertNoDialogExists()
     }
 }
+
+private val DEFAULT_STATE = SetupAutoFillState(dialogState = null, autofillEnabled = false)