4858: Fix notification permission check and result

tuskyapp · Jan 18, 2025 · 975f390 · 975f390
1 parent fee2ad5
commit 975f390
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 29 deletions.
diff --git a/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt b/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt
@@ -45,7 +45,6 @@ import androidx.appcompat.app.AlertDialog
 import androidx.appcompat.content.res.AppCompatResources
 import androidx.coordinatorlayout.widget.CoordinatorLayout
 import androidx.core.app.ActivityCompat
-import androidx.core.content.ContextCompat
 import androidx.core.content.pm.ShortcutManagerCompat
 import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen
 import androidx.core.view.MenuProvider
@@ -202,6 +201,16 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvider {
         // will be redirected to LoginActivity by BaseActivity
         activeAccount = accountManager.activeAccount ?: return
 
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU &&
+            ActivityCompat.checkSelfPermission(this, Manifest.permission.POST_NOTIFICATIONS) != PackageManager.PERMISSION_GRANTED
+        ) {
+            ActivityCompat.requestPermissions(
+                this,
+                arrayOf(Manifest.permission.POST_NOTIFICATIONS),
+                ALLOW_NOTIFICATIONS_REQUEST_CODE
+            )
+        }
+
         if (explodeAnimationWasRequested()) {
             overrideActivityTransitionCompat(
                 ActivityConstants.OVERRIDE_TRANSITION_OPEN,
@@ -295,21 +304,30 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvider {
 
         onBackPressedDispatcher.addCallback(this@MainActivity, onBackPressedCallback)
 
-        if (
-            Build.VERSION.SDK_INT >= 33 &&
-            ContextCompat.checkSelfPermission(this@MainActivity, Manifest.permission.POST_NOTIFICATIONS) != PackageManager.PERMISSION_GRANTED
-        ) {
-            ActivityCompat.requestPermissions(
-                this@MainActivity,
-                arrayOf(Manifest.permission.POST_NOTIFICATIONS),
-                1
-            )
-        }
-
         // "Post failed" dialog should display in this activity
         draftsAlert.observeInContext(this@MainActivity, true)
     }
 
+    override fun onRequestPermissionsResult(requestCode: Int, permissions: Array<out String>, grantResults: IntArray) {
+        super.onRequestPermissionsResult(requestCode, permissions, grantResults)
+
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
+            return
+        }
+
+        if (requestCode != ALLOW_NOTIFICATIONS_REQUEST_CODE) {
+            return
+        }
+
+        val idx = permissions.indexOfFirst { it == Manifest.permission.POST_NOTIFICATIONS }
+
+        if (idx < 0 || grantResults[idx] != PackageManager.PERMISSION_GRANTED) {
+            return
+        }
+
+        viewModel.setupNotifications()
+    }
+
     override fun onNewIntent(intent: Intent) {
         super.onNewIntent(intent)
         val showNotificationTab = handleIntent(intent, activeAccount)
@@ -1082,6 +1100,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvider {
     override fun getActionButton() = binding.composeButton
 
     companion object {
+        const val ALLOW_NOTIFICATIONS_REQUEST_CODE = 2389668
         const val OPEN_WITH_EXPLODE_ANIMATION = "explode"
 
         private const val TAG = "MainActivity" // logging tag

diff --git a/app/src/main/java/com/keylesspalace/tusky/MainViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/MainViewModel.kt
@@ -99,15 +99,7 @@ class MainViewModel @Inject constructor(
 
                     shareShortcutHelper.updateShortcuts()
 
-                    notificationService.createNotificationChannelsForAccount(activeAccount)
-
-                    if (notificationService.areNotificationsEnabled()) {
-                        viewModelScope.launch {
-                            enablePushNotificationsWithFallback(context, api, accountManager, notificationService)
-                        }
-                    } else {
-                        disableAllNotifications(context, accountManager, notificationService)
-                    }
+                    setupNotifications()
                 },
                 { throwable ->
                     Log.w(TAG, "Failed to fetch user info.", throwable)
@@ -170,6 +162,18 @@ class MainViewModel @Inject constructor(
         }
     }
 
+    fun setupNotifications() {
+        notificationService.createNotificationChannelsForAccount(activeAccount)
+
+        if (notificationService.areNotificationsEnabled()) {
+            viewModelScope.launch {
+                enablePushNotificationsWithFallback(context, api, accountManager, notificationService)
+            }
+        } else {
+            disableAllNotifications(context, accountManager, notificationService)
+        }
+    }
+
     companion object {
         private const val TAG = "MainViewModel"
     }

diff --git a/...c/main/java/com/keylesspalace/tusky/components/systemnotifications/NotificationFetcher.kt b/...c/main/java/com/keylesspalace/tusky/components/systemnotifications/NotificationFetcher.kt
@@ -1,6 +1,5 @@
 package com.keylesspalace.tusky.components.systemnotifications
 
-import android.content.Context
 import android.util.Log
 import androidx.annotation.WorkerThread
 import com.keylesspalace.tusky.appstore.EventHub
@@ -12,7 +11,6 @@ import com.keylesspalace.tusky.entity.Notification
 import com.keylesspalace.tusky.network.MastodonApi
 import com.keylesspalace.tusky.util.HttpHeaderLink
 import com.keylesspalace.tusky.util.isLessThan
-import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject
 
 /** Models next/prev links from the "Links" header in an API response */
@@ -44,22 +42,23 @@ data class Links(val next: String?, val prev: String?) {
 class NotificationFetcher @Inject constructor(
     private val mastodonApi: MastodonApi,
     private val accountManager: AccountManager,
-    @ApplicationContext private val context: Context,
     private val eventHub: EventHub,
     private val notificationService: NotificationService,
 ) {
     suspend fun fetchAndShow() {
         for (account in accountManager.accounts) {
             if (account.notificationsEnabled) {
                 try {
+                    // TODO one could do a checkSelfPermission here every time. It is possible that we got
+                    //   notification permissions in the meantime. There are no "positive" broadcasts on changes.
+                    //   see https://stackoverflow.com/questions/32718933/broadcast-action-on-permission-change-in-android-m
+
                     val notifications = fetchNewNotifications(account)
                         .filter { notificationService.filterNotification(account, it.type) }
                         .sortedWith(
                             compareBy({ it.id.length }, { it.id })
                         ) // oldest notifications first
 
-                    // TODO do this before filter above? But one could argue that (for example) a tab badge is also a notification
-                    //   (and should therefore adhere to the notification config).
                     eventHub.dispatch(NewNotificationsEvent(account.accountId, notifications))
 
                     notificationService.show(account, notifications)
@@ -96,7 +95,7 @@ class NotificationFetcher @Inject constructor(
         // - The Mastodon marker API (if the server supports it)
         // - account.notificationMarkerId
         // - account.lastNotificationId
-        Log.d(TAG, "getting notification marker for ${account.fullName}")
+        Log.d(TAG, "Getting notification marker for ${account.fullName}.")
         val remoteMarkerId = fetchMarker(authHeader, account)?.lastReadId ?: "0"
         val localMarkerId = account.notificationMarkerId
         val markerId = if (remoteMarkerId.isLessThan(
@@ -114,7 +113,7 @@ class NotificationFetcher @Inject constructor(
         Log.d(TAG, "  localMarkerId: $localMarkerId")
         Log.d(TAG, "  readingPosition: $readingPosition")
 
-        Log.d(TAG, "getting Notifications for ${account.fullName}, min_id: $minId")
+        Log.d(TAG, "Getting Notifications for ${account.fullName}, min_id: $minId.")
 
         // Fetch all outstanding notifications
         val notifications: List<Notification> = buildList {
@@ -151,7 +150,7 @@ class NotificationFetcher @Inject constructor(
             accountManager.updateAccount(account) { copy(notificationMarkerId = newMarkerId) }
         }
 
-        Log.d(TAG, "got ${notifications.size} Notifications")
+        Log.d(TAG, "Got ${notifications.size} Notifications.")
 
         return notifications
     }