From dae3f287f247ea1695a740dcf520fafe1f0cd0b3 Mon Sep 17 00:00:00 2001 From: LYK Date: Fri, 17 Apr 2026 14:10:38 +0900 Subject: [PATCH 1/9] Add FCM push notification support - Add firebase-messaging dependency - Implement HackersPubMessagingService for receiving FCM data messages - Implement FcmTokenManager for token registration/unregistration via registerFcmDeviceToken/unregisterFcmDeviceToken GraphQL mutations - Register FCM token on login, unregister on logout - Request notification permission on login (not just app startup) - Remove foreground polling (FCM replaces it; 15-min WorkManager polling remains as fallback) - Handle GraphQL union error types in mutation responses - Add FCM mutation types to local GraphQL schema Requires server-side FCM support: hackers-pub/hackerspub#251 Co-Authored-By: Claude Opus 4.6 (1M context) --- app/build.gradle.kts | 1 + app/src/main/AndroidManifest.xml | 7 ++ .../pub/hackers/android/operations.graphql | 35 ++++++ .../pub/hackers/android/schema.graphqls | 50 +++++++++ .../java/pub/hackers/android/MainActivity.kt | 14 +-- .../android/data/messaging/FcmTokenManager.kt | 106 ++++++++++++++++++ .../messaging/HackersPubMessagingService.kt | 90 +++++++++++++++ .../pub/hackers/android/ui/AppViewModel.kt | 49 ++------ .../pub/hackers/android/ui/HackersPubApp.kt | 27 +---- .../ui/screens/settings/SettingsViewModel.kt | 3 + gradle/libs.versions.toml | 1 + 11 files changed, 310 insertions(+), 73 deletions(-) create mode 100644 app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt create mode 100644 app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 3c02e60b..17b80a0b 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -130,6 +130,7 @@ dependencies { implementation(platform(libs.firebase.bom)) implementation(libs.firebase.crashlytics) implementation(libs.firebase.analytics) + implementation(libs.firebase.messaging) implementation("androidx.browser:browser:1.8.0") implementation("androidx.credentials:credentials:1.5.0-rc01") diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 157ea7b0..29adb7a4 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -66,6 +66,13 @@ android:pathPrefix="/notifications" /> + + + + + = Build.VERSION_CODES.TIRAMISU) { lifecycleScope.launch { - val isLoggedIn = sessionManager.isLoggedIn.first() - if (isLoggedIn && ContextCompat.checkSelfPermission( - this@MainActivity, Manifest.permission.POST_NOTIFICATIONS - ) != PackageManager.PERMISSION_GRANTED - ) { - requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) + sessionManager.isLoggedIn.collect { isLoggedIn -> + if (isLoggedIn && ContextCompat.checkSelfPermission( + this@MainActivity, Manifest.permission.POST_NOTIFICATIONS + ) != PackageManager.PERMISSION_GRANTED + ) { + requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) + } } } } diff --git a/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt new file mode 100644 index 00000000..52b6a318 --- /dev/null +++ b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt @@ -0,0 +1,106 @@ +package pub.hackers.android.data.messaging + +import android.util.Log +import com.apollographql.apollo.ApolloClient +import com.google.firebase.messaging.FirebaseMessaging +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.tasks.await +import pub.hackers.android.data.local.SessionManager +import pub.hackers.android.graphql.RegisterFcmDeviceTokenMutation +import pub.hackers.android.graphql.UnregisterFcmDeviceTokenMutation +import pub.hackers.android.graphql.type.RegisterFcmDeviceTokenInput +import pub.hackers.android.graphql.type.UnregisterFcmDeviceTokenInput +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class FcmTokenManager @Inject constructor( + private val apolloClient: ApolloClient, + private val sessionManager: SessionManager, +) { + companion object { + private const val TAG = "FcmTokenManager" + } + + suspend fun registerCurrentToken() { + val isLoggedIn = sessionManager.isLoggedIn.first() + if (!isLoggedIn) return + + val token = try { + FirebaseMessaging.getInstance().token.await() + } catch (e: Exception) { + Log.w(TAG, "Failed to get FCM token", e) + return + } + registerToken(token) + } + + suspend fun registerToken(token: String) { + val isLoggedIn = sessionManager.isLoggedIn.first() + if (!isLoggedIn) return + + try { + val response = apolloClient.mutation( + RegisterFcmDeviceTokenMutation( + RegisterFcmDeviceTokenInput(deviceToken = token) + ) + ).execute() + + if (response.hasErrors()) { + Log.w(TAG, "FCM token registration returned errors: ${response.errors}") + return + } + + val result = response.data?.registerFcmDeviceToken + when { + result?.onRegisterFcmDeviceTokenPayload != null -> + Log.d(TAG, "FCM token registered") + result?.onRegisterFcmDeviceTokenFailedError != null -> + Log.w(TAG, "FCM token registration failed: ${result.onRegisterFcmDeviceTokenFailedError.message}") + result?.onInvalidInputError != null -> + Log.w(TAG, "FCM token registration invalid input: ${result.onInvalidInputError.inputPath}") + result?.onNotAuthenticatedError != null -> + Log.w(TAG, "FCM token registration not authenticated") + else -> + Log.w(TAG, "FCM token registration unexpected result") + } + } catch (e: Exception) { + Log.w(TAG, "Failed to register FCM token", e) + } + } + + suspend fun unregisterCurrentToken() { + val token = try { + FirebaseMessaging.getInstance().token.await() + } catch (e: Exception) { + return + } + + try { + val response = apolloClient.mutation( + UnregisterFcmDeviceTokenMutation( + UnregisterFcmDeviceTokenInput(deviceToken = token) + ) + ).execute() + + if (response.hasErrors()) { + Log.w(TAG, "FCM token unregistration returned errors: ${response.errors}") + return + } + + val result = response.data?.unregisterFcmDeviceToken + when { + result?.onUnregisterFcmDeviceTokenPayload != null -> + Log.d(TAG, "FCM token unregistered") + result?.onInvalidInputError != null -> + Log.w(TAG, "FCM token unregistration invalid input: ${result.onInvalidInputError.inputPath}") + result?.onNotAuthenticatedError != null -> + Log.w(TAG, "FCM token unregistration not authenticated") + else -> + Log.w(TAG, "FCM token unregistration unexpected result") + } + } catch (e: Exception) { + Log.w(TAG, "Failed to unregister FCM token", e) + } + } +} diff --git a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt new file mode 100644 index 00000000..ff8e6cc6 --- /dev/null +++ b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt @@ -0,0 +1,90 @@ +package pub.hackers.android.data.messaging + +import android.Manifest +import android.app.NotificationManager +import android.app.PendingIntent +import android.content.Context +import android.content.Intent +import android.content.pm.PackageManager +import android.os.Build +import androidx.core.app.NotificationCompat +import androidx.core.content.ContextCompat +import com.google.firebase.messaging.FirebaseMessagingService +import com.google.firebase.messaging.RemoteMessage +import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import pub.hackers.android.HackersPubApplication +import pub.hackers.android.MainActivity +import pub.hackers.android.R +import pub.hackers.android.data.local.NotificationStateManager +import javax.inject.Inject + +@AndroidEntryPoint +class HackersPubMessagingService : FirebaseMessagingService() { + + @Inject + lateinit var fcmTokenManager: FcmTokenManager + + @Inject + lateinit var notificationStateManager: NotificationStateManager + + private val serviceScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + + override fun onNewToken(token: String) { + serviceScope.launch { + fcmTokenManager.registerToken(token) + } + } + + override fun onMessageReceived(message: RemoteMessage) { + val data = message.data + if (data.isEmpty()) return + + val alert = data["alert"] ?: return + val notificationId = data["notificationId"] ?: return + + kotlinx.coroutines.runBlocking { + notificationStateManager.updateLastPolledId(notificationId) + } + + if (hasNotificationPermission()) { + showNotification(alert) + } + } + + private fun hasNotificationPermission(): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + ContextCompat.checkSelfPermission( + this, + Manifest.permission.POST_NOTIFICATIONS + ) == PackageManager.PERMISSION_GRANTED + } else { + true + } + } + + private fun showNotification(text: String) { + val intent = Intent(this, MainActivity::class.java).apply { + flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP + putExtra("navigate_to", "notifications") + } + val pendingIntent = PendingIntent.getActivity( + this, 0, intent, + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE + ) + + val notification = NotificationCompat.Builder(this, HackersPubApplication.NOTIFICATION_CHANNEL_ID) + .setSmallIcon(R.drawable.ic_launcher_foreground) + .setContentTitle("Hackers' Pub") + .setContentText(text) + .setAutoCancel(true) + .setContentIntent(pendingIntent) + .build() + + val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + manager.notify(System.currentTimeMillis().toInt(), notification) + } +} diff --git a/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt b/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt index ded669b6..c8536f8b 100644 --- a/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt +++ b/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt @@ -8,19 +8,16 @@ import androidx.work.NetworkType import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.Job -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.stateIn -import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import pub.hackers.android.data.local.NotificationStateManager import pub.hackers.android.data.local.PreferencesManager import pub.hackers.android.data.local.SessionManager -import pub.hackers.android.data.repository.HackersPubRepository +import pub.hackers.android.data.messaging.FcmTokenManager import pub.hackers.android.data.worker.NotificationWorker import pub.hackers.android.ui.screens.timeline.TimelineRefreshTrigger import java.util.concurrent.TimeUnit @@ -32,13 +29,12 @@ class AppViewModel @Inject constructor( val preferencesManager: PreferencesManager, private val notificationStateManager: NotificationStateManager, private val workManager: WorkManager, - private val repository: HackersPubRepository, val timelineRefreshTrigger: TimelineRefreshTrigger, + private val fcmTokenManager: FcmTokenManager, ) : ViewModel() { companion object { const val NOTIFICATION_WORK_NAME = "notification_poll" - const val FOREGROUND_POLL_INTERVAL_MS = 60_000L // 1 minute } val isLoggedIn: Flow = sessionManager.isLoggedIn @@ -46,8 +42,6 @@ class AppViewModel @Inject constructor( val hasUnread: StateFlow = notificationStateManager.hasUnread .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), false) - private var foregroundPollJob: Job? = null - init { ensureNotificationPolling() } @@ -57,11 +51,16 @@ class AppViewModel @Inject constructor( val loggedIn = sessionManager.isLoggedIn.first() if (loggedIn) { enqueueNotificationPolling() - startForegroundPolling() } } } + fun registerFcmToken() { + viewModelScope.launch { + fcmTokenManager.registerCurrentToken() + } + } + fun enqueueNotificationPolling() { val constraints = Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) @@ -77,36 +76,4 @@ class AppViewModel @Inject constructor( workRequest ) } - - fun startForegroundPolling() { - if (foregroundPollJob?.isActive == true) return - foregroundPollJob = viewModelScope.launch { - while (isActive) { - delay(FOREGROUND_POLL_INTERVAL_MS) - pollNotifications() - } - } - } - - fun stopForegroundPolling() { - foregroundPollJob?.cancel() - foregroundPollJob = null - } - - private suspend fun pollNotifications() { - val loggedIn = sessionManager.isLoggedIn.first() - if (!loggedIn) return - - repository.getNotifications(refresh = true) - .onSuccess { result -> - val notifications = result.notifications - if (notifications.isNotEmpty()) { - val newestId = notifications.first().id - val previousId = notificationStateManager.getLastPolledId() - if (newestId != previousId) { - notificationStateManager.updateLastPolledId(newestId) - } - } - } - } } diff --git a/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt b/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt index 43501ac2..9ca2677a 100644 --- a/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt +++ b/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt @@ -15,14 +15,10 @@ import androidx.compose.material.icons.outlined.Search import androidx.compose.material.icons.outlined.Settings import androidx.compose.material3.Scaffold import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.remember -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleEventObserver -import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource @@ -200,30 +196,11 @@ fun HackersPubApp( } } - // Enqueue notification polling when user logs in + // Enqueue background polling fallback and register FCM token when user logs in LaunchedEffect(isLoggedIn) { if (isLoggedIn) { viewModel.enqueueNotificationPolling() - viewModel.startForegroundPolling() - } else { - viewModel.stopForegroundPolling() - } - } - - // Start/stop foreground polling based on app lifecycle - val lifecycleOwner = LocalLifecycleOwner.current - DisposableEffect(lifecycleOwner, isLoggedIn) { - val observer = LifecycleEventObserver { _, event -> - if (!isLoggedIn) return@LifecycleEventObserver - when (event) { - Lifecycle.Event.ON_START -> viewModel.startForegroundPolling() - Lifecycle.Event.ON_STOP -> viewModel.stopForegroundPolling() - else -> {} - } - } - lifecycleOwner.lifecycle.addObserver(observer) - onDispose { - lifecycleOwner.lifecycle.removeObserver(observer) + viewModel.registerFcmToken() } } diff --git a/app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsViewModel.kt b/app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsViewModel.kt index 755edb10..4c86f9c3 100644 --- a/app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsViewModel.kt +++ b/app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsViewModel.kt @@ -26,6 +26,7 @@ import kotlinx.coroutines.withContext import pub.hackers.android.data.auth.PasskeyManager import pub.hackers.android.data.local.PreferencesManager import pub.hackers.android.data.local.SessionManager +import pub.hackers.android.data.messaging.FcmTokenManager import pub.hackers.android.data.repository.HackersPubRepository import pub.hackers.android.domain.model.Passkey import javax.inject.Inject @@ -57,6 +58,7 @@ class SettingsViewModel @Inject constructor( private val apolloClient: ApolloClient, private val notificationStateManager: NotificationStateManager, private val passkeyManager: PasskeyManager, + private val fcmTokenManager: FcmTokenManager, private val workManager: WorkManager, @ApplicationContext private val context: Context ) : ViewModel() { @@ -148,6 +150,7 @@ class SettingsViewModel @Inject constructor( fun signOut() { viewModelScope.launch { + fcmTokenManager.unregisterCurrentToken() val sessionId = sessionManager.sessionToken.first() if (sessionId != null) { repository.revokeSession(sessionId) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b7b725c2..b0e55538 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -69,6 +69,7 @@ compose-ui-test-manifest = { group = "androidx.compose.ui", name = "ui-test-mani firebase-bom = { group = "com.google.firebase", name = "firebase-bom", version.ref = "firebaseBom" } firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" } firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics" } +firebase-messaging = { group = "com.google.firebase", name = "firebase-messaging" } androidx-paging-runtime = { group = "androidx.paging", name = "paging-runtime-ktx", version.ref = "paging" } androidx-paging-compose = { group = "androidx.paging", name = "paging-compose", version.ref = "paging" } From b087143930f1e78aa3a961fc9edb80e4e0bcb1e9 Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 16:43:42 +0900 Subject: [PATCH 2/9] Fix AndroidManifest.xml after merge The merge commit silently dropped two manifest changes from main: - c38339e (Instantiatable suppression on MainActivity) - e8ef8ec (removed /notifications deep link intent-filter; the router side of that change did land in the merge) Reapply both, plus add the same Instantiatable suppression to the new HackersPubMessagingService (FCM) since it triggers the same Hilt @AndroidEntryPoint false positive that c38339e narrowed for MainActivity. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/src/main/AndroidManifest.xml | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 29adb7a4..b0b994d3 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -8,15 +8,18 @@ + android:theme="@style/Theme.HackersPub" + tools:targetApi="33"> + android:theme="@style/Theme.HackersPub" + tools:ignore="Instantiatable"> @@ -56,19 +59,11 @@ android:host="hackers.pub" android:pathPrefix="/tags/" /> - - - - - - + android:exported="false" + tools:ignore="Instantiatable"> From ecbc8f4c64174a590b9b96cc3b1b02b84badacd0 Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 17:20:08 +0900 Subject: [PATCH 3/9] Address Copilot review feedback on FCM flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - HackersPubMessagingService.onMessageReceived: replace runBlocking with serviceScope.launch so the FCM callback thread is not blocked on DataStore writes (avoids ANR risk under load) - HackersPubMessagingService.showNotification: derive notification id from server notificationId (hashCode) instead of currentTimeMillis, so re-deliveries update an existing notification and the int doesn't overflow - MainActivity.requestNotificationPermissionIfNeeded: gate the isLoggedIn collect with repeatOnLifecycle(STARTED) and react only on the false→true transition (distinctUntilChanged + filter), so the permission dialog can't be launched while the activity isn't visible - HackersPubApp: unregister the FCM token on the true→false logged-in transition so logout paths beyond Settings sign-out also have a chance to clear the token. Track the previous value in remember to avoid firing the unauthenticated mutation on cold start while logged out - FcmTokenManager: split unregisterCurrentToken into a token-fetch wrapper plus unregisterToken(token), mirroring the register path, and log the token-fetch failure that was previously swallowed Co-Authored-By: Claude Opus 4.7 (1M context) --- .../java/pub/hackers/android/MainActivity.kt | 23 +++++++++++++------ .../android/data/messaging/FcmTokenManager.kt | 4 ++++ .../messaging/HackersPubMessagingService.kt | 8 +++---- .../pub/hackers/android/ui/AppViewModel.kt | 6 +++++ .../pub/hackers/android/ui/HackersPubApp.kt | 10 +++++++- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/pub/hackers/android/MainActivity.kt b/app/src/main/java/pub/hackers/android/MainActivity.kt index 0d26c5ed..d69dff2f 100644 --- a/app/src/main/java/pub/hackers/android/MainActivity.kt +++ b/app/src/main/java/pub/hackers/android/MainActivity.kt @@ -21,8 +21,12 @@ import androidx.compose.runtime.setValue import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Modifier import androidx.core.content.ContextCompat +import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.launch import pub.hackers.android.data.local.PreferencesManager import pub.hackers.android.data.local.SessionManager @@ -142,13 +146,18 @@ class MainActivity : ComponentActivity() { private fun requestNotificationPermissionIfNeeded() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { lifecycleScope.launch { - sessionManager.isLoggedIn.collect { isLoggedIn -> - if (isLoggedIn && ContextCompat.checkSelfPermission( - this@MainActivity, Manifest.permission.POST_NOTIFICATIONS - ) != PackageManager.PERMISSION_GRANTED - ) { - requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) - } + repeatOnLifecycle(Lifecycle.State.STARTED) { + sessionManager.isLoggedIn + .distinctUntilChanged() + .filter { it } + .collect { + if (ContextCompat.checkSelfPermission( + this@MainActivity, Manifest.permission.POST_NOTIFICATIONS + ) != PackageManager.PERMISSION_GRANTED + ) { + requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) + } + } } } } diff --git a/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt index 52b6a318..306a8a21 100644 --- a/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt +++ b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt @@ -73,9 +73,13 @@ class FcmTokenManager @Inject constructor( val token = try { FirebaseMessaging.getInstance().token.await() } catch (e: Exception) { + Log.w(TAG, "Failed to get FCM token", e) return } + unregisterToken(token) + } + suspend fun unregisterToken(token: String) { try { val response = apolloClient.mutation( UnregisterFcmDeviceTokenMutation( diff --git a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt index ff8e6cc6..408cc263 100644 --- a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt +++ b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt @@ -46,12 +46,12 @@ class HackersPubMessagingService : FirebaseMessagingService() { val alert = data["alert"] ?: return val notificationId = data["notificationId"] ?: return - kotlinx.coroutines.runBlocking { + serviceScope.launch { notificationStateManager.updateLastPolledId(notificationId) } if (hasNotificationPermission()) { - showNotification(alert) + showNotification(notificationId, alert) } } @@ -66,7 +66,7 @@ class HackersPubMessagingService : FirebaseMessagingService() { } } - private fun showNotification(text: String) { + private fun showNotification(notificationId: String, text: String) { val intent = Intent(this, MainActivity::class.java).apply { flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP putExtra("navigate_to", "notifications") @@ -85,6 +85,6 @@ class HackersPubMessagingService : FirebaseMessagingService() { .build() val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager - manager.notify(System.currentTimeMillis().toInt(), notification) + manager.notify(notificationId.hashCode(), notification) } } diff --git a/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt b/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt index c8536f8b..cfc4e26d 100644 --- a/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt +++ b/app/src/main/java/pub/hackers/android/ui/AppViewModel.kt @@ -61,6 +61,12 @@ class AppViewModel @Inject constructor( } } + fun unregisterFcmToken() { + viewModelScope.launch { + fcmTokenManager.unregisterCurrentToken() + } + } + fun enqueueNotificationPolling() { val constraints = Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) diff --git a/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt b/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt index 8920a953..0def6b38 100644 --- a/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt +++ b/app/src/main/java/pub/hackers/android/ui/HackersPubApp.kt @@ -18,7 +18,9 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource @@ -204,12 +206,18 @@ fun HackersPubApp( } } - // Enqueue background polling fallback and register FCM token when user logs in + // Enqueue background polling fallback and register FCM token on login; + // unregister on the true→false transition so cold starts while logged + // out don't fire a guaranteed-to-fail unauthenticated mutation. + var prevLoggedIn by remember { mutableStateOf(null) } LaunchedEffect(isLoggedIn) { if (isLoggedIn) { viewModel.enqueueNotificationPolling() viewModel.registerFcmToken() + } else if (prevLoggedIn == true) { + viewModel.unregisterFcmToken() } + prevLoggedIn = isLoggedIn } ProvideInAppBrowserUriHandler( From d5a22b2c7c2d87c671af0e466481449ccb08766f Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:17:55 +0900 Subject: [PATCH 4/9] Add FcmTokenManager unit tests Cover the testable surface of register/unregister flows without having to mockkStatic FirebaseMessaging: - registerToken returns early without calling Apollo when logged out - registerToken sends the mutation with the captured token when logged in - registerToken survives hasErrors responses, every union variant (success payload, FailedError, InvalidInput, NotAuthenticated, plus an unknown __typename), and Apollo execute() throwing - unregisterToken counterpart tests for the same shape (no FailedError variant since the schema doesn't have one) ApolloResponse exposes data/errors/requestUuid as @JvmField, which mockk cannot intercept, so the helpers build real ApolloResponse instances via ApolloResponse.Builder + uuid4 instead of mocking. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../data/messaging/FcmTokenManagerTest.kt | 255 ++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt diff --git a/app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt b/app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt new file mode 100644 index 00000000..9470c99b --- /dev/null +++ b/app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt @@ -0,0 +1,255 @@ +package pub.hackers.android.data.messaging + +import com.apollographql.apollo.ApolloCall +import com.apollographql.apollo.ApolloClient +import com.apollographql.apollo.api.ApolloResponse +import com.apollographql.apollo.api.Error +import com.apollographql.apollo.api.Operation +import com.benasher44.uuid.uuid4 +import io.mockk.CapturingSlot +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import pub.hackers.android.data.local.SessionManager +import pub.hackers.android.graphql.RegisterFcmDeviceTokenMutation +import pub.hackers.android.graphql.UnregisterFcmDeviceTokenMutation + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [35]) +class FcmTokenManagerTest { + + private val apolloClient = mockk() + private val sessionManager = mockk() + + private fun newManager() = FcmTokenManager(apolloClient, sessionManager) + + private fun stubLoggedIn(value: Boolean) { + every { sessionManager.isLoggedIn } returns flowOf(value) + } + + private fun buildResponse( + operation: com.apollographql.apollo.api.Operation, + data: D?, + errors: List? = null, + ): ApolloResponse = ApolloResponse.Builder(operation, uuid4()) + .data(data) + .errors(errors) + .build() + + private fun stubRegisterMutation( + data: RegisterFcmDeviceTokenMutation.Data?, + errors: List? = null, + ): CapturingSlot { + val slot = slot() + val call = mockk>() + every { apolloClient.mutation(capture(slot)) } answers { + coEvery { call.execute() } returns buildResponse(slot.captured, data, errors) + call + } + return slot + } + + private fun stubUnregisterMutation( + data: UnregisterFcmDeviceTokenMutation.Data?, + errors: List? = null, + ): CapturingSlot { + val slot = slot() + val call = mockk>() + every { apolloClient.mutation(capture(slot)) } answers { + coEvery { call.execute() } returns buildResponse(slot.captured, data, errors) + call + } + return slot + } + + private fun registerData( + typename: String, + payload: RegisterFcmDeviceTokenMutation.OnRegisterFcmDeviceTokenPayload? = null, + failed: RegisterFcmDeviceTokenMutation.OnRegisterFcmDeviceTokenFailedError? = null, + invalidInput: RegisterFcmDeviceTokenMutation.OnInvalidInputError? = null, + notAuthenticated: RegisterFcmDeviceTokenMutation.OnNotAuthenticatedError? = null, + ) = RegisterFcmDeviceTokenMutation.Data( + registerFcmDeviceToken = RegisterFcmDeviceTokenMutation.RegisterFcmDeviceToken( + __typename = typename, + onRegisterFcmDeviceTokenPayload = payload, + onRegisterFcmDeviceTokenFailedError = failed, + onInvalidInputError = invalidInput, + onNotAuthenticatedError = notAuthenticated, + ) + ) + + private fun unregisterData( + typename: String, + payload: UnregisterFcmDeviceTokenMutation.OnUnregisterFcmDeviceTokenPayload? = null, + invalidInput: UnregisterFcmDeviceTokenMutation.OnInvalidInputError? = null, + notAuthenticated: UnregisterFcmDeviceTokenMutation.OnNotAuthenticatedError? = null, + ) = UnregisterFcmDeviceTokenMutation.Data( + unregisterFcmDeviceToken = UnregisterFcmDeviceTokenMutation.UnregisterFcmDeviceToken( + __typename = typename, + onUnregisterFcmDeviceTokenPayload = payload, + onInvalidInputError = invalidInput, + onNotAuthenticatedError = notAuthenticated, + ) + ) + + @Test + fun `registerToken returns early without calling Apollo when logged out`() = runTest { + stubLoggedIn(false) + + newManager().registerToken("token-x") + + verify(exactly = 0) { apolloClient.mutation(any()) } + } + + @Test + fun `registerToken sends mutation with provided token when logged in`() = runTest { + stubLoggedIn(true) + val slot = stubRegisterMutation( + registerData( + typename = "RegisterFcmDeviceTokenPayload", + payload = RegisterFcmDeviceTokenMutation.OnRegisterFcmDeviceTokenPayload( + deviceToken = "token-x", + created = "2026-01-01T00:00:00Z", + updated = "2026-01-01T00:00:00Z", + ), + ) + ) + + newManager().registerToken("token-x") + + assertEquals("token-x", slot.captured.input.deviceToken) + } + + @Test + fun `registerToken handles hasErrors response without throwing`() = runTest { + stubLoggedIn(true) + stubRegisterMutation( + data = null, + errors = listOf(Error.Builder(message = "boom").build()), + ) + + newManager().registerToken("token-x") + } + + @Test + fun `registerToken handles all union variants without throwing`() = runTest { + val cases = listOf( + registerData( + "RegisterFcmDeviceTokenPayload", + payload = RegisterFcmDeviceTokenMutation.OnRegisterFcmDeviceTokenPayload( + "t", "c", "u" + ), + ), + registerData( + "RegisterFcmDeviceTokenFailedError", + failed = RegisterFcmDeviceTokenMutation.OnRegisterFcmDeviceTokenFailedError( + message = "limit reached", limit = 5 + ), + ), + registerData( + "InvalidInputError", + invalidInput = RegisterFcmDeviceTokenMutation.OnInvalidInputError( + inputPath = "input.deviceToken" + ), + ), + registerData( + "NotAuthenticatedError", + notAuthenticated = RegisterFcmDeviceTokenMutation.OnNotAuthenticatedError( + notAuthenticated = "login required" + ), + ), + registerData("UnknownTypename"), + ) + for (data in cases) { + stubLoggedIn(true) + stubRegisterMutation(data) + newManager().registerToken("token-x") + } + } + + @Test + fun `registerToken does not throw when Apollo execute throws`() = runTest { + stubLoggedIn(true) + val call = mockk>() + coEvery { call.execute() } throws RuntimeException("network down") + every { apolloClient.mutation(any()) } returns call + + newManager().registerToken("token-x") + } + + @Test + fun `unregisterToken sends mutation with provided token`() = runTest { + val slot = stubUnregisterMutation( + unregisterData( + typename = "UnregisterFcmDeviceTokenPayload", + payload = UnregisterFcmDeviceTokenMutation.OnUnregisterFcmDeviceTokenPayload( + deviceToken = "token-x", + unregistered = true, + ), + ) + ) + + newManager().unregisterToken("token-x") + + assertEquals("token-x", slot.captured.input.deviceToken) + } + + @Test + fun `unregisterToken handles hasErrors response without throwing`() = runTest { + stubUnregisterMutation( + data = null, + errors = listOf(Error.Builder(message = "boom").build()), + ) + + newManager().unregisterToken("token-x") + } + + @Test + fun `unregisterToken handles all union variants without throwing`() = runTest { + val cases = listOf( + unregisterData( + "UnregisterFcmDeviceTokenPayload", + payload = UnregisterFcmDeviceTokenMutation.OnUnregisterFcmDeviceTokenPayload( + "t", true + ), + ), + unregisterData( + "InvalidInputError", + invalidInput = UnregisterFcmDeviceTokenMutation.OnInvalidInputError( + "input.deviceToken" + ), + ), + unregisterData( + "NotAuthenticatedError", + notAuthenticated = UnregisterFcmDeviceTokenMutation.OnNotAuthenticatedError( + "login required" + ), + ), + unregisterData("UnknownTypename"), + ) + for (data in cases) { + stubUnregisterMutation(data) + newManager().unregisterToken("token-x") + } + } + + @Test + fun `unregisterToken does not throw when Apollo execute throws`() = runTest { + val call = mockk>() + coEvery { call.execute() } throws RuntimeException("network down") + every { apolloClient.mutation(any()) } returns call + + newManager().unregisterToken("token-x") + } +} From 74bb06ffe5342fd94d4b3450916f472e456b8302 Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:25:05 +0900 Subject: [PATCH 5/9] Cancel HackersPubMessagingService scope on destroy The CoroutineScope(SupervisorJob() + Dispatchers.IO) was never cancelled. FirebaseMessagingService instances are destroyed and recreated by the system, so without an onDestroy() hook the launched jobs (token registration, DataStore writes) would outlive their service instance and pile up across recreations. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../android/data/messaging/HackersPubMessagingService.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt index 408cc263..02dbbf72 100644 --- a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt +++ b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt @@ -15,6 +15,7 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import pub.hackers.android.HackersPubApplication import pub.hackers.android.MainActivity @@ -33,6 +34,11 @@ class HackersPubMessagingService : FirebaseMessagingService() { private val serviceScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + override fun onDestroy() { + serviceScope.cancel() + super.onDestroy() + } + override fun onNewToken(token: String) { serviceScope.launch { fcmTokenManager.registerToken(token) From 5da821f6e2e0e7695d4176c991e6dcf9c74d7a6f Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:35:34 +0900 Subject: [PATCH 6/9] Use a monochrome notification icon and localize the title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous setSmallIcon target ic_launcher_foreground is a colored adaptive vector (#1A1A2E / #E94560). On API 21+ the system tints the notification small icon based on alpha only, so a colored asset renders as a white square on many devices. Add a dedicated alpha-only ic_notification.xml and switch the FCM service to it. Also replace the hardcoded "Hackers' Pub" title with getString(R.string.app_name) — this string is already provided per buildType via build.gradle.kts resValue (debug shows "Hackers' Pub Dev", release shows "Hackers' Pub"), matching the manifest's android:label source. Scope is limited to HackersPubMessagingService; the same pattern in NotificationWorker is pre-existing tech debt outside this PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../data/messaging/HackersPubMessagingService.kt | 4 ++-- app/src/main/res/drawable/ic_notification.xml | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 app/src/main/res/drawable/ic_notification.xml diff --git a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt index 02dbbf72..6171ee24 100644 --- a/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt +++ b/app/src/main/java/pub/hackers/android/data/messaging/HackersPubMessagingService.kt @@ -83,8 +83,8 @@ class HackersPubMessagingService : FirebaseMessagingService() { ) val notification = NotificationCompat.Builder(this, HackersPubApplication.NOTIFICATION_CHANNEL_ID) - .setSmallIcon(R.drawable.ic_launcher_foreground) - .setContentTitle("Hackers' Pub") + .setSmallIcon(R.drawable.ic_notification) + .setContentTitle(getString(R.string.app_name)) .setContentText(text) .setAutoCancel(true) .setContentIntent(pendingIntent) diff --git a/app/src/main/res/drawable/ic_notification.xml b/app/src/main/res/drawable/ic_notification.xml new file mode 100644 index 00000000..2998bc39 --- /dev/null +++ b/app/src/main/res/drawable/ic_notification.xml @@ -0,0 +1,13 @@ + + + + From e1d34ae448f163b2655270e7d6287e8d240ab216 Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:35:44 +0900 Subject: [PATCH 7/9] Roll back FCM token registration if session ended mid-flight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerToken reads sessionManager.isLoggedIn before awaiting the GraphQL mutation. SettingsViewModel.signOut() unregisters the current token then calls clearSession(), but a register call already in flight when sign-out begins can complete after clearSession() runs, leaving the device token registered on the server for a now logged-out user. After the success arm of the mutation response, re-check sessionManager.isLoggedIn.first(); if it has flipped to false, call unregisterToken(token) to roll back. Only the success arm runs the check — error responses (FailedError, InvalidInputError, NotAuthenticatedError, hasErrors) never committed a registration on the server, so a rollback there would just be a guaranteed-fail extra call. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../pub/hackers/android/data/messaging/FcmTokenManager.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt index 306a8a21..bdb72132 100644 --- a/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt +++ b/app/src/main/java/pub/hackers/android/data/messaging/FcmTokenManager.kt @@ -53,8 +53,13 @@ class FcmTokenManager @Inject constructor( val result = response.data?.registerFcmDeviceToken when { - result?.onRegisterFcmDeviceTokenPayload != null -> + result?.onRegisterFcmDeviceTokenPayload != null -> { Log.d(TAG, "FCM token registered") + if (!sessionManager.isLoggedIn.first()) { + Log.w(TAG, "Session ended during registration; rolling back") + unregisterToken(token) + } + } result?.onRegisterFcmDeviceTokenFailedError != null -> Log.w(TAG, "FCM token registration failed: ${result.onRegisterFcmDeviceTokenFailedError.message}") result?.onInvalidInputError != null -> From 8f2eca9de51faa6a19476105975eca6c528e8b6f Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:41:57 +0900 Subject: [PATCH 8/9] Document Apollo response testing pattern in CONVENTION MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add §9.4 covering the gotcha discovered while writing FcmTokenManagerTest: ApolloResponse exposes data/errors/requestUuid as @JvmField, so mockk can't intercept the field reads. The fix is to construct real instances via the public ApolloResponse.Builder (plus uuid4() from com.benasher44.uuid). Reference: app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt Co-Authored-By: Claude Opus 4.7 (1M context) --- CONVENTION.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONVENTION.md b/CONVENTION.md index 74359416..d36fb2a5 100644 --- a/CONVENTION.md +++ b/CONVENTION.md @@ -276,6 +276,22 @@ assertEquals(expected, outer.sharedPost!!.field) // setUp guarantees sharedPost New or substantially refactored ViewModels ship with tests in the same PR. Reference tests live in `app/src/test/java/pub/hackers/android/ui/screens/**/*ViewModelTest.kt`. +### §9.4 Build real `ApolloResponse` instead of mocking it + +`ApolloResponse` exposes `data`, `errors`, and `requestUuid` as `@JvmField`s, so mockk cannot intercept the field reads — `every { response.data } returns ...` fails with "missing mocked calls" because there is no getter to record. Construct real instances via the public Builder: + +```kotlin +import com.apollographql.apollo.api.ApolloResponse +import com.benasher44.uuid.uuid4 + +val response = ApolloResponse.Builder(operation = mutation, requestUuid = uuid4()) + .data(data) + .errors(errors) + .build() +``` + +Mock the `ApolloCall` chain (`apolloClient.mutation(any())`) and have `coEvery { call.execute() } returns response`. Reference: `app/src/test/java/pub/hackers/android/data/messaging/FcmTokenManagerTest.kt`. + --- ## §10 Build and variants From ed0d546fb784a68bd126103ed957a425963bb23e Mon Sep 17 00:00:00 2001 From: LYK Date: Sat, 25 Apr 2026 19:42:50 +0900 Subject: [PATCH 9/9] Document merge-conflict pitfalls in AGENTS Two functional pitfalls bit this exact PR while resolving conflicts against main: 1. Merging a stale local `main` (one commit behind `origin/main`) produced a merge that silently lacked the latest commit's changes; recovering required `git merge --abort` and remerging against the synced main. 2. Even after the synced merge succeeded with no conflict markers, AndroidManifest.xml lost two hunks that main had added (the MainActivity Instantiatable suppression and the /notifications intent-filter removal). git's 3-way auto-merge picked one side silently for those regions. Add a one-line expectation so contributors know to sync local main first and to verify intersection files against `main` after the merge instead of trusting the "Auto-merging" output. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index d13a6e24..8c56ebd8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,3 +11,4 @@ Minimum expectations for any change: - Run `./gradlew :app:lintDebug` before pushing. - Do not introduce new `!!` in production code. - For paged post feeds, keep using the existing Paging overlay and deduplication patterns. +- When resolving merge conflicts, fast-forward local `main` from `origin/main` first (otherwise the merge silently uses a stale base), then after the merge inspect files touched by both branches against `main` directly — git's 3-way auto-merge can drop one side's hunks without surfacing a conflict marker.