Write notification account information to the correct account (#3697)

Don't use `accountManager.activeAccount` throughout the code. Instead, get the active account once, and use that over the life of the viewmodel. As shown in #3689 (comment) the active account can change before the view model is destroyed, and if that happens account information for the account will be written to the wrong account.
tuskyapp · Jun 1, 2023 · 346dabf · 346dabf
1 parent 61374c5
commit 346dabf
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 22 deletions.
diff --git a/app/src/main/java/com/keylesspalace/tusky/BaseActivity.java b/app/src/main/java/com/keylesspalace/tusky/BaseActivity.java
@@ -212,6 +212,13 @@ public void showAccountChooserDialog(CharSequence dialogTitle, boolean showActiv
         }
     }
 
+    // TODO: This changes the accountManager's activeAccount property, but does not do any
+    // of the work that AccountManager.setActiveAccount() does. In particular:
+    //
+    // - The current active account is not saved
+    // - The account passed as parameter here goes not have its `isActive` property set
+    //
+    // Is that deliberate? Or is this a bug?
     public void openAsAccount(@NonNull String url, @NonNull AccountEntity account) {
         accountManager.setActiveAccount(account);
         Intent intent = new Intent(this, MainActivity.class);

diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java
@@ -606,9 +606,7 @@ public static void disablePullNotifications(Context context) {
         Log.d(TAG, "disabled notification checks");
     }
 
-    public static void clearNotificationsForActiveAccount(@NonNull Context context, @NonNull AccountManager accountManager) {
-        AccountEntity account = accountManager.getActiveAccount();
-        if (account == null) return;
+    public static void clearNotificationsForAccount(@NonNull Context context, @NonNull AccountEntity account) {
         int accountId = (int) account.getId();
 
         NotificationManager notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);

diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt
@@ -106,7 +106,7 @@ class NotificationsFragment :
 
         adapter = NotificationsPagingAdapter(
             notificationDiffCallback,
-            accountId = accountManager.activeAccount!!.accountId,
+            accountId = viewModel.account.accountId,
             statusActionListener = this,
             notificationActionListener = this,
             accountActionListener = this,
@@ -460,7 +460,7 @@ class NotificationsFragment :
     override fun onRefresh() {
         binding.progressBar.isVisible = false
         adapter.refresh()
-        NotificationHelper.clearNotificationsForActiveAccount(requireContext(), accountManager)
+        NotificationHelper.clearNotificationsForAccount(requireContext(), viewModel.account)
     }
 
     override fun onPause() {
@@ -477,7 +477,7 @@ class NotificationsFragment :
 
     override fun onResume() {
         super.onResume()
-        NotificationHelper.clearNotificationsForActiveAccount(requireContext(), accountManager)
+        NotificationHelper.clearNotificationsForAccount(requireContext(), viewModel.account)
     }
 
     override fun onReply(position: Int) {
@@ -606,7 +606,7 @@ class NotificationsFragment :
 
     override fun onViewReport(reportId: String) {
         requireContext().openLink(
-            "https://${accountManager.activeAccount!!.domain}/admin/reports/$reportId"
+            "https://${viewModel.account.domain}/admin/reports/$reportId"
         )
     }
 
@@ -622,7 +622,7 @@ class NotificationsFragment :
     }
 
     companion object {
-        private const val TAG = "NotificationF"
+        private const val TAG = "NotificationsFragment"
         fun newInstance() = NotificationsFragment()
 
         private val notificationDiffCallback: DiffUtil.ItemCallback<NotificationViewData> =

diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt
@@ -282,6 +282,8 @@ class NotificationsViewModel @Inject constructor(
     private val timelineCases: TimelineCases,
     private val eventHub: EventHub
 ) : ViewModel() {
+    /** The account to display notifications for */
+    val account = accountManager.activeAccount!!
 
     val uiState: StateFlow<UiState>
 
@@ -316,16 +318,14 @@ class NotificationsViewModel @Inject constructor(
             // Save each change back to the active account
             .onEach { action ->
                 Log.d(TAG, "notificationFilter: $action")
-                accountManager.activeAccount?.let { account ->
-                    account.notificationsFilter = serialize(action.filter)
-                    accountManager.saveAccount(account)
-                }
+                account.notificationsFilter = serialize(action.filter)
+                accountManager.saveAccount(account)
             }
             // Load the initial filter from the active account
             .onStart {
                 emit(
                     InfallibleUiAction.ApplyFilter(
-                        filter = deserialize(accountManager.activeAccount?.notificationsFilter)
+                        filter = deserialize(account.notificationsFilter)
                     )
                 )
             }
@@ -336,11 +336,9 @@ class NotificationsViewModel @Inject constructor(
                 .filterIsInstance<InfallibleUiAction.SaveVisibleId>()
                 .distinctUntilChanged()
                 .collectLatest { action ->
-                    Log.d(TAG, "Saving visible ID: ${action.visibleId}")
-                    accountManager.activeAccount?.let { account ->
-                        account.lastNotificationId = action.visibleId
-                        accountManager.saveAccount(account)
-                    }
+                    Log.d(TAG, "Saving visible ID: ${action.visibleId}, active account = ${account.id}")
+                    account.lastNotificationId = action.visibleId
+                    accountManager.saveAccount(account)
                 }
         }
 
@@ -351,7 +349,7 @@ class NotificationsViewModel @Inject constructor(
         statusDisplayOptions = MutableStateFlow(
             StatusDisplayOptions.from(
                 preferences,
-                accountManager.activeAccount!!
+                account
             )
         )
 
@@ -363,7 +361,7 @@ class NotificationsViewModel @Inject constructor(
                     statusDisplayOptions.value.make(
                         preferences,
                         it.preferenceKey,
-                        accountManager.activeAccount!!
+                        account
                     )
                 }
                 .collect {
@@ -494,7 +492,7 @@ class NotificationsViewModel @Inject constructor(
     // The database stores "0" as the last notification ID if notifications have not been
     // fetched. Convert to null to ensure a full fetch in this case
     private fun getInitialKey(): String? {
-        val initialKey = when (val id = accountManager.activeAccount?.lastNotificationId) {
+        val initialKey = when (val id = account.lastNotificationId) {
             "0" -> null
             else -> id
         }

diff --git a/app/src/main/java/com/keylesspalace/tusky/db/AccountManager.kt b/app/src/main/java/com/keylesspalace/tusky/db/AccountManager.kt
@@ -52,7 +52,7 @@ class AccountManager @Inject constructor(db: AppDatabase) {
     /**
      * Adds a new account and makes it the active account.
      * @param accessToken the access token for the new account
-     * @param domain the domain of the accounts Mastodon instance
+     * @param domain the domain of the account's Mastodon instance
      * @param clientId the oauth client id used to sign in the account
      * @param clientSecret the oauth client secret used to sign in the account
      * @param oauthScopes the oauth scopes granted to the account