Bug 1824379 - When deleting a login delete it from the shared LoginsFragmentStore

This reverts the recent patch for bug 1799622 and reworks the implementation to
avoid using the Fragment Result API for a simpler functionality using the
already shared LoginsFragmentStore.

app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt

@@ -54,6 +54,7 @@ sealed class LoginsAction : Action {
     data class FilterLogins(val newText: String?) : LoginsAction()
     data class UpdateLoginsList(val list: List<SavedLogin>) : LoginsAction()
     data class AddLogin(val newLogin: SavedLogin) : LoginsAction()
+    data class DeleteLogin(val loginId: String) : LoginsAction()
     object LoginsListUpToDate : LoginsAction()
     data class UpdateCurrentLogin(val item: SavedLogin) : LoginsAction()
     data class SortLogins(val sortingStrategy: SortingStrategy) : LoginsAction()
@@ -117,6 +118,13 @@ private fun savedLoginsStateReducer(
                 filteredItems = state.sortingStrategy(updatedLogins),
             )
         }
+        is LoginsAction.DeleteLogin -> {
+            val updatedLogins = state.loginList.filterNot { it.guid == action.loginId }
+            state.copy(
+                loginList = updatedLogins,
+                filteredItems = state.sortingStrategy(updatedLogins),
+            )
+        }
         is LoginsAction.FilterLogins -> {
             filterItems(
                 action.newText,

app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt

@@ -49,6 +49,7 @@ open class SavedLoginsStorageController(
                 passwordsStorage.delete(loginId)
             }
             deleteLoginJob?.await()
+            deleteLoginFromState(loginId)
             withContext(Dispatchers.Main) {
                 navController.popBackStack(R.id.savedLoginsFragment, false)
             }
@@ -177,6 +178,12 @@ open class SavedLoginsStorageController(
         )
     }
 
+    private fun deleteLoginFromState(loginId: String) {
+        loginsFragmentStore.dispatch(
+            LoginsAction.DeleteLogin(loginId),
+        )
+    }
+
     fun findDuplicateForAdd(originText: String, usernameText: String, passwordText: String) {
         lifecycleScope.launch(ioDispatcher) {
             findDuplicate(

app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt

@@ -15,9 +15,7 @@ import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
 import androidx.appcompat.app.AlertDialog
-import androidx.core.os.bundleOf
 import androidx.core.view.MenuProvider
-import androidx.fragment.app.setFragmentResult
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
@@ -220,10 +218,6 @@ class LoginDetailFragment : SecureFragment(R.layout.fragment_login_detail), Menu
                 setPositiveButton(R.string.dialog_delete_positive) { dialog: DialogInterface, _ ->
                     Logins.deleteSavedLogin.record(NoExtras())
                     interactor.onDeleteLogin(args.savedLoginId)
-                    setFragmentResult(
-                        LOGIN_REQUEST_KEY,
-                        bundleOf(LOGIN_BUNDLE_ARGS to args.savedLoginId),
-                    )
                     dialog.dismiss()
                 }
                 create().withCenterAlignedButtons()
@@ -238,7 +232,5 @@ class LoginDetailFragment : SecureFragment(R.layout.fragment_login_detail), Menu
 
     companion object {
         private const val BUTTON_INCREASE_DPS = 24
-        const val LOGIN_REQUEST_KEY = "logins"
-        const val LOGIN_BUNDLE_ARGS = "loginsBundle"
     }
 }

app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt

@@ -18,11 +18,9 @@ import androidx.appcompat.widget.SearchView
 import androidx.appcompat.widget.Toolbar
 import androidx.constraintlayout.widget.ConstraintLayout
 import androidx.core.view.MenuProvider
-import androidx.fragment.app.setFragmentResultListener
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
-import androidx.navigation.navGraphViewModels
 import mozilla.components.concept.menu.MenuController
 import mozilla.components.concept.menu.Orientation
 import mozilla.components.lib.state.ext.consumeFrom
@@ -36,10 +34,8 @@ import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.redirectToReAuth
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.ext.showToolbar
-import org.mozilla.fenix.home.SharedViewModel
 import org.mozilla.fenix.settings.logins.LoginsAction
 import org.mozilla.fenix.settings.logins.LoginsFragmentStore
-import org.mozilla.fenix.settings.logins.LoginsListState
 import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu
 import org.mozilla.fenix.settings.logins.SortingStrategy
 import org.mozilla.fenix.settings.logins.controller.LoginsListController
@@ -59,9 +55,6 @@ class SavedLoginsFragment : SecureFragment(), MenuProvider {
     private lateinit var sortLoginsMenuRoot: ConstraintLayout
     private lateinit var loginsListController: LoginsListController
     private lateinit var savedLoginsStorageController: SavedLoginsStorageController
-    private lateinit var loginState: LoginsListState
-    private var removedLoginGuid: String? = null
-    private var deletedGuid = mutableSetOf<String>()
 
     override fun onResume() {
         super.onResume()
@@ -114,25 +107,9 @@ class SavedLoginsFragment : SecureFragment(), MenuProvider {
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        setFragmentResultListener(LoginDetailFragment.LOGIN_REQUEST_KEY) { _, bundle ->
-            removedLoginGuid = bundle.getString(LoginDetailFragment.LOGIN_BUNDLE_ARGS)
-            deletedGuid.add(removedLoginGuid.toString())
-        }
-        consumeFrom(savedLoginsStore) { loginsListState ->
+        consumeFrom(savedLoginsStore) {
             sortingStrategyMenu.updateMenu(savedLoginsStore.state.highlightedItem)
-
-            loginState = loginsListState
-            val currentList = loginState.filteredItems.toMutableList()
-
-            if (removedLoginGuid != null) {
-                val newList = currentList.filter { !deletedGuid.contains(it.guid) }
-
-                loginState = loginState.copy(
-                    loginList = newList,
-                    filteredItems = newList,
-                )
-            }
-            savedLoginsListView.update(loginState)
+            savedLoginsListView.update(it)
         }
     }
 

app/src/test/java/org/mozilla/fenix/settings/logins/LoginsFragmentStoreTest.kt

@@ -24,8 +24,8 @@ class LoginsFragmentStoreTest {
         password = "",
         timeLastUsed = 0L,
     )
-    private val exampleLogin = baseLogin.copy(origin = "example.com", timeLastUsed = 10)
-    private val firefoxLogin = baseLogin.copy(origin = "firefox.com", timeLastUsed = 20)
+    private val exampleLogin = baseLogin.copy(guid = "example", origin = "example.com", timeLastUsed = 10)
+    private val firefoxLogin = baseLogin.copy(guid = "firefox", origin = "firefox.com", timeLastUsed = 20)
     private val loginList = listOf(exampleLogin, firefoxLogin)
     private val baseState = LoginsListState(
         loginList = emptyList(),
@@ -101,6 +101,33 @@ class LoginsFragmentStoreTest {
         assertEquals(listOf(firefoxLogin, exampleLogin), store.state.filteredItems)
     }
 
+    @Test
+    fun `GIVEN logins already exist WHEN asked to remove one THEN update the state to not contain it`() {
+        val store = LoginsFragmentStore(
+            baseState.copy(
+                isLoading = true,
+                loginList = loginList,
+            ),
+        )
+
+        store.dispatch(LoginsAction.DeleteLogin("not_existing")).joinBlocking()
+        assertEquals(loginList, store.state.loginList)
+        assertEquals(listOf(firefoxLogin, exampleLogin), store.state.filteredItems)
+
+        store.dispatch(LoginsAction.DeleteLogin(exampleLogin.guid)).joinBlocking()
+        assertEquals(listOf(firefoxLogin), store.state.loginList)
+        assertEquals(listOf(firefoxLogin), store.state.filteredItems)
+
+        store.dispatch(LoginsAction.DeleteLogin(firefoxLogin.guid)).joinBlocking()
+        assertEquals(emptyList<SavedLogin>(), store.state.loginList)
+        assertEquals(emptyList<SavedLogin>(), store.state.filteredItems)
+
+        // Test deleting from an empty store
+        store.dispatch(LoginsAction.DeleteLogin(firefoxLogin.guid)).joinBlocking()
+        assertEquals(emptyList<SavedLogin>(), store.state.loginList)
+        assertEquals(emptyList<SavedLogin>(), store.state.filteredItems)
+    }
+
     @Test
     fun `FilterLogins action`() {
         val store = LoginsFragmentStore(

app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsStorageControllerTest.kt

@@ -68,6 +68,7 @@ class SavedLoginsStorageControllerTest {
 
         coVerify {
             passwordsStorage.delete(loginId)
+            loginsFragmentStore.dispatch(LoginsAction.DeleteLogin(loginId))
             navController.popBackStack(R.id.savedLoginsFragment, false)
         }
     }

detekt-baseline.xml

@@ -252,6 +252,7 @@
     <ID>UndocumentedPublicClass:LoginsFragmentStore.kt$LoginsAction$UpdateCurrentLogin : LoginsAction</ID>
     <ID>UndocumentedPublicClass:LoginsFragmentStore.kt$LoginsAction$UpdateLoginsList : LoginsAction</ID>
     <ID>UndocumentedPublicClass:LoginsFragmentStore.kt$LoginsAction$AddLogin : LoginsAction</ID>
+    <ID>UndocumentedPublicClass:LoginsFragmentStore.kt$LoginsAction$DeleteLogin : LoginsAction</ID>
     <ID>UndocumentedPublicClass:LoginsListViewHolder.kt$LoginsListViewHolder : ViewHolder</ID>
     <ID>UndocumentedPublicClass:MenuPresenter.kt$MenuPresenter : OnAttachStateChangeListener</ID>
     <ID>UndocumentedPublicClass:MessageMetadataStorage.kt$MessageMetadataStorage</ID>