fix: Prevent crash when showing account chooser (#1117)

Chooser dialog could start before any accounts have loaded. Fix by collecting the account flow and waiting for the first emission (convert the flow to shared instead of state so there's no initial empty list). Guard against the potential for a similar issue when fetching notifications. Order the list of accounts with active account first so that code that skips it by ignoring the first item works correctly.
pachli · Nov 20, 2024 · 632282d · 632282d
1 parent 5c04831
commit 632282d
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 37 deletions.
diff --git a/app/src/main/java/app/pachli/components/notifications/NotificationFetcher.kt b/app/src/main/java/app/pachli/components/notifications/NotificationFetcher.kt
@@ -40,6 +40,8 @@ import kotlin.collections.set
 import kotlin.math.min
 import kotlin.time.Duration.Companion.milliseconds
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.take
 import timber.log.Timber
 
 /**
@@ -57,19 +59,19 @@ class NotificationFetcher @Inject constructor(
     @ApplicationContext private val context: Context,
 ) {
     suspend fun fetchAndShow(pachliAccountId: Long) {
-        Timber.d("NotificationFetcher.fetchAndShow() started")
+        Timber.d("NotificationFetcher.fetchAndShow(%d) started", pachliAccountId)
 
         val accounts = buildList {
             if (pachliAccountId == NotificationWorker.ALL_ACCOUNTS) {
-                addAll(accountManager.accountsOrderedByActive)
+                addAll(accountManager.accountsOrderedByActiveFlow.take(1).first())
             } else {
                 accountManager.getAccountById(pachliAccountId)?.let { add(it) }
             }
         }
 
         for (account in accounts) {
             Timber.d(
-                "Checking %s$, notificationsEnabled = %s",
+                "Checking %s, notificationsEnabled = %s",
                 account.fullName,
                 account.notificationsEnabled,
             )

diff --git a/core/activity/src/main/kotlin/app/pachli/core/activity/BaseActivity.kt b/core/activity/src/main/kotlin/app/pachli/core/activity/BaseActivity.kt
@@ -59,6 +59,8 @@ import dagger.hilt.android.EntryPointAccessors.fromApplication
 import dagger.hilt.components.SingletonComponent
 import javax.inject.Inject
 import kotlin.properties.Delegates
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.take
 import kotlinx.coroutines.launch
 import timber.log.Timber
 
@@ -229,47 +231,55 @@ abstract class BaseActivity : AppCompatActivity(), MenuProvider {
         bar.show()
     }
 
+    /**
+     * Displays a dialog allowing the user to choose from the available accounts.
+     *
+     * @param dialogTitle
+     * @parma showActiveAccount True if the active account should be included in
+     * the list of accounts.
+     * @parma listener
+     */
     fun showAccountChooserDialog(
         dialogTitle: CharSequence?,
         showActiveAccount: Boolean,
         listener: AccountSelectionListener,
     ) {
-        val accounts = accountManager.accountsOrderedByActive.toMutableList()
-        val activeAccount = accounts.first()
-        when (accounts.size) {
-            1 -> {
-                listener.onAccountSelected(activeAccount)
-                return
-            }
+        lifecycleScope.launch {
+            val accounts = accountManager.accountsOrderedByActiveFlow.take(1).first().toMutableList()
+            val activeAccount = accounts.first()
+            when (accounts.size) {
+                1 -> {
+                    listener.onAccountSelected(activeAccount)
+                    return@launch
+                }
 
-            2 -> {
-                if (!showActiveAccount) {
-                    for (account in accounts) {
-                        if (activeAccount !== account) {
-                            listener.onAccountSelected(account)
-                            return
+                2 -> {
+                    if (!showActiveAccount) {
+                        for (account in accounts) {
+                            if (activeAccount !== account) {
+                                listener.onAccountSelected(account)
+                                return@launch
+                            }
                         }
                     }
                 }
             }
-        }
-        if (!showActiveAccount) {
-            accounts.remove(activeAccount)
-        }
-        val adapter = AccountSelectionAdapter(
-            this,
-            sharedPreferencesRepository.animateAvatars,
-            sharedPreferencesRepository.animateEmojis,
-        )
-        adapter.addAll(accounts)
-        AlertDialog.Builder(this)
-            .setTitle(dialogTitle)
-            .setAdapter(adapter) { _: DialogInterface?, index: Int ->
-                listener.onAccountSelected(
-                    accounts[index],
-                )
+            if (!showActiveAccount) {
+                accounts.remove(activeAccount)
             }
-            .show()
+            val adapter = AccountSelectionAdapter(
+                this@BaseActivity,
+                sharedPreferencesRepository.animateAvatars,
+                sharedPreferencesRepository.animateEmojis,
+            )
+            adapter.addAll(accounts)
+            AlertDialog.Builder(this@BaseActivity)
+                .setTitle(dialogTitle)
+                .setAdapter(adapter) { _: DialogInterface?, index: Int ->
+                    listener.onAccountSelected(accounts[index])
+                }
+                .show()
+        }
     }
 
     val openAsText: String?

diff --git a/core/data/src/main/kotlin/app/pachli/core/data/repository/AccountManager.kt b/core/data/src/main/kotlin/app/pachli/core/data/repository/AccountManager.kt
@@ -68,6 +68,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.launch
 import timber.log.Timber
@@ -190,11 +191,11 @@ class AccountManager @Inject constructor(
     val accounts: List<AccountEntity>
         get() = accountsFlow.value
 
-    private val accountsOrderedByActiveFlow = accountDao.getAccountsOrderedByActive()
-        .stateIn(externalScope, SharingStarted.Eagerly, emptyList())
+    val accountsOrderedByActiveFlow = accountDao.getAccountsOrderedByActive()
+        .shareIn(externalScope, SharingStarted.Eagerly, replay = 1)
 
     val accountsOrderedByActive: List<AccountEntity>
-        get() = accountsOrderedByActiveFlow.value
+        get() = accountsOrderedByActiveFlow.replayCache.first()
 
     @Deprecated("Caller should use getPachliAccountFlow with a specific account ID")
     val activePachliAccountFlow = accountDao.getActivePachliAccountFlow()

diff --git a/core/database/src/main/kotlin/app/pachli/core/database/dao/AccountDao.kt b/core/database/src/main/kotlin/app/pachli/core/database/dao/AccountDao.kt
@@ -86,7 +86,7 @@ interface AccountDao {
         """
         SELECT *
          FROM AccountEntity
-        ORDER BY isActive, id ASC
+        ORDER BY isActive DESC, id ASC
         """,
     )
     fun getAccountsOrderedByActive(): Flow<List<AccountEntity>>