Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor notifications #4883

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
+1,072 −1,217
Open

Refactor notifications #4883

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fee2ad5
Move all pull notification code to a new helper class
Lakoja Jan 18, 2025
975f390
4858: Fix notification permission check and result
Lakoja Jan 18, 2025
2824c3f
Use Kotlin style programming
Lakoja Jan 18, 2025
111a2cb
Move push code to NotificationService
Lakoja Jan 18, 2025
5428a1b
Use newer ActivityResultContracts for notification permission
Lakoja Jan 18, 2025
c7b0f36
Remove last usage of a separate context
Lakoja Jan 18, 2025
dc16b04
Appease linter
Lakoja Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions app/src/main/java/com/keylesspalace/tusky/MainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,6 +75,7 @@ import com.keylesspalace.tusky.components.login.LoginActivity
import com.keylesspalace.tusky.components.preference.PreferencesActivity
import com.keylesspalace.tusky.components.scheduled.ScheduledStatusActivity
import com.keylesspalace.tusky.components.search.SearchActivity
import com.keylesspalace.tusky.components.systemnotifications.NotificationService
Lakoja marked this conversation as resolved.
Show resolved Hide resolved
import com.keylesspalace.tusky.components.trending.TrendingActivity
import com.keylesspalace.tusky.databinding.ActivityMainBinding
import com.keylesspalace.tusky.db.DraftsAlert
Expand Down Expand Up @@ -138,6 +138,9 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvider {
@Inject
lateinit var eventHub: EventHub

@Inject
lateinit var notificationService: NotificationService

@Inject
lateinit var cacheUpdater: CacheUpdater

Expand Down Expand Up @@ -198,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,
Expand Down Expand Up @@ -291,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) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug fix.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use the newer ActivityResultContracts api instead. See ViewMediaActivity for an example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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)
Expand Down Expand Up @@ -1078,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
Expand Down
27 changes: 16 additions & 11 deletions app/src/main/java/com/keylesspalace/tusky/MainViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import com.keylesspalace.tusky.appstore.ConversationsLoadingEvent
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.NewNotificationsEvent
import com.keylesspalace.tusky.appstore.NotificationsLoadingEvent
import com.keylesspalace.tusky.components.systemnotifications.NotificationHelper
import com.keylesspalace.tusky.components.systemnotifications.NotificationService
import com.keylesspalace.tusky.components.systemnotifications.disableAllNotifications
import com.keylesspalace.tusky.components.systemnotifications.enablePushNotificationsWithFallback
import com.keylesspalace.tusky.db.AccountManager
Expand All @@ -52,7 +52,8 @@ class MainViewModel @Inject constructor(
private val api: MastodonApi,
private val eventHub: EventHub,
private val accountManager: AccountManager,
private val shareShortcutHelper: ShareShortcutHelper
private val shareShortcutHelper: ShareShortcutHelper,
private val notificationService: NotificationService,
) : ViewModel() {

private val activeAccount = accountManager.activeAccount!!
Expand Down Expand Up @@ -98,15 +99,7 @@ class MainViewModel @Inject constructor(

shareShortcutHelper.updateShortcuts()

NotificationHelper.createNotificationChannelsForAccount(activeAccount, context)

if (NotificationHelper.areNotificationsEnabled(context, accountManager)) {
viewModelScope.launch {
enablePushNotificationsWithFallback(context, api, accountManager)
}
} else {
disableAllNotifications(context, accountManager)
}
setupNotifications()
},
{ throwable ->
Log.w(TAG, "Failed to fetch user info.", throwable)
Expand Down Expand Up @@ -169,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"
}
Expand Down
5 changes: 2 additions & 3 deletions app/src/main/java/com/keylesspalace/tusky/TuskyApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.keylesspalace.tusky

import android.app.Application
import android.content.Context
import android.content.SharedPreferences
import android.util.Log
import androidx.hilt.work.HiltWorkerFactory
Expand All @@ -24,7 +25,6 @@ import androidx.work.Constraints
import androidx.work.ExistingPeriodicWorkPolicy
import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import com.keylesspalace.tusky.components.systemnotifications.NotificationHelper
import com.keylesspalace.tusky.settings.AppTheme
import com.keylesspalace.tusky.settings.NEW_INSTALL_SCHEMA_VERSION
import com.keylesspalace.tusky.settings.PrefKeys
Expand Down Expand Up @@ -75,6 +75,7 @@ class TuskyApplication : Application(), Configuration.Provider {
NEW_INSTALL_SCHEMA_VERSION
)
if (oldVersion != SCHEMA_VERSION) {
// TODO SCHEMA_VERSION is outdated / not updated in code
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collateral find.

upgradeSharedPreferences(oldVersion, SCHEMA_VERSION)
}

Expand All @@ -89,8 +90,6 @@ class TuskyApplication : Application(), Configuration.Provider {

localeManager.setLocale()

NotificationHelper.createWorkerNotificationChannel(this)

// Prune the database every ~ 12 hours when the device is idle.
val pruneCacheWorker = PeriodicWorkRequestBuilder<PruneCacheWorker>(12, TimeUnit.HOURS)
.setConstraints(Constraints.Builder().setRequiresDeviceIdle(true).build())
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.PreferenceChangedEvent
import com.keylesspalace.tusky.components.notifications.requests.NotificationRequestsActivity
import com.keylesspalace.tusky.components.preference.PreferencesFragment.ReadingOrder
import com.keylesspalace.tusky.components.systemnotifications.NotificationHelper
import com.keylesspalace.tusky.components.systemnotifications.NotificationService
import com.keylesspalace.tusky.databinding.FragmentTimelineNotificationsBinding
import com.keylesspalace.tusky.databinding.NotificationsFilterBinding
import com.keylesspalace.tusky.entity.Notification
Expand Down Expand Up @@ -97,6 +97,9 @@ class NotificationsFragment :
@Inject
lateinit var eventHub: EventHub

@Inject
lateinit var notificationService: NotificationService

private val binding by viewBinding(FragmentTimelineNotificationsBinding::bind)

private val viewModel: NotificationsViewModel by viewModels()
Expand Down Expand Up @@ -259,7 +262,7 @@ class NotificationsFragment :
viewLifecycleOwner.lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) {
accountManager.activeAccount?.let { account ->
NotificationHelper.clearNotificationsForAccount(requireContext(), account)
notificationService.clearNotificationsForAccount(account)
}

val useAbsoluteTime = preferences.getBoolean(PrefKeys.ABSOLUTE_TIME_VIEW, false)
Expand Down
11 changes: 7 additions & 4 deletions ...ain/java/com/keylesspalace/tusky/components/preference/NotificationPreferencesFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import android.os.Bundle
import androidx.lifecycle.lifecycleScope
import androidx.preference.PreferenceFragmentCompat
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.components.systemnotifications.NotificationHelper
import com.keylesspalace.tusky.components.systemnotifications.NotificationService
import com.keylesspalace.tusky.db.AccountManager
import com.keylesspalace.tusky.db.entity.AccountEntity
import com.keylesspalace.tusky.settings.PrefKeys
Expand All @@ -36,6 +36,9 @@ class NotificationPreferencesFragment : PreferenceFragmentCompat() {
@Inject
lateinit var accountManager: AccountManager

@Inject
lateinit var notificationService: NotificationService

override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
val activeAccount = accountManager.activeAccount ?: return
val context = requireContext()
Expand All @@ -47,10 +50,10 @@ class NotificationPreferencesFragment : PreferenceFragmentCompat() {
isChecked = activeAccount.notificationsEnabled
setOnPreferenceChangeListener { _, newValue ->
updateAccount { copy(notificationsEnabled = newValue as Boolean) }
if (NotificationHelper.areNotificationsEnabled(context, accountManager)) {
NotificationHelper.enablePullNotifications(context)
if (notificationService.areNotificationsEnabled()) {
notificationService.enablePullNotifications(context)
} else {
NotificationHelper.disablePullNotifications(context)
notificationService.disablePullNotifications(context)
}
true
}
Expand Down
69 changes: 12 additions & 57 deletions ...c/main/java/com/keylesspalace/tusky/components/systemnotifications/NotificationFetcher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,16 @@
package com.keylesspalace.tusky.components.systemnotifications

import android.Manifest
import android.app.NotificationManager
import android.content.Context
import android.content.pm.PackageManager
import android.util.Log
import androidx.annotation.WorkerThread
import androidx.core.app.ActivityCompat
import androidx.core.app.NotificationManagerCompat
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.NewNotificationsEvent
import com.keylesspalace.tusky.components.systemnotifications.NotificationHelper.filterNotification
import com.keylesspalace.tusky.db.AccountManager
import com.keylesspalace.tusky.db.entity.AccountEntity
import com.keylesspalace.tusky.entity.Marker
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 */
Expand Down Expand Up @@ -50,65 +42,26 @@ 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 eventHub: EventHub,
private val notificationService: NotificationService,
) {
suspend fun fetchAndShow() {
if (ActivityCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) != PackageManager.PERMISSION_GRANTED) {
return
}

for (account in accountManager.accounts) {
if (account.notificationsEnabled) {
try {
val notificationManager = context.getSystemService(
Context.NOTIFICATION_SERVICE
) as NotificationManager
// 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 notificationManagerCompat = NotificationManagerCompat.from(context)

// Create sorted list of new notifications
val notifications = fetchNewNotifications(account)
.filter { filterNotification(notificationManager, account, it) }
.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))

val newNotifications = ArrayList<NotificationManagerCompat.NotificationWithIdAndTag>()

val notificationsByType: Map<Notification.Type, List<Notification>> = notifications.groupBy { it.type }
notificationsByType.forEach { notificationsGroup: Map.Entry<Notification.Type, List<Notification>> ->
// NOTE Enqueue the summary first: Needed to avoid rate limit problems:
// ie. single notification is enqueued but that later summary one is filtered and thus no grouping
// takes place.
newNotifications.add(
NotificationHelper.makeSummaryNotification(
context,
notificationManager,
account,
notificationsGroup.key,
notificationsGroup.value
)
)

notificationsGroup.value.forEach { notification ->
newNotifications.add(
NotificationHelper.make(
context,
notificationManager,
notification,
account
)
)
}
}

// NOTE having multiple summary notifications this here should still collapse them in only one occurrence
notificationManagerCompat.notify(newNotifications)
notificationService.show(account, notifications)
} catch (e: Exception) {
Log.e(TAG, "Error while fetching notifications", e)
}
Expand Down Expand Up @@ -142,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(
Expand All @@ -160,10 +113,10 @@ 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 = buildList {
val notifications: List<Notification> = buildList {
while (minId != null) {
val response = mastodonApi.notificationsWithAuth(
authHeader,
Expand Down Expand Up @@ -197,6 +150,8 @@ class NotificationFetcher @Inject constructor(
accountManager.updateAccount(account) { copy(notificationMarkerId = newMarkerId) }
}

Log.d(TAG, "Got ${notifications.size} Notifications.")

return notifications
}

Expand Down
Loading
Loading