diff --git a/app/src/main/kotlin/dev/privacyllc/period/di/DataModule.kt b/app/src/main/kotlin/dev/privacyllc/period/di/DataModule.kt index ac00b39..aae782e 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/di/DataModule.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/di/DataModule.kt @@ -16,7 +16,11 @@ import dev.privacyllc.period.core.data.CycleData import dev.privacyllc.period.core.data.CycleRepository import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.notifications.ReminderScheduler +import dev.privacyllc.period.core.notifications.PeriodNotifier +import dev.privacyllc.period.core.notifications.ReminderNotifier import dev.privacyllc.period.core.security.AppLockRepository +import dev.privacyllc.period.notifications.AndroidNotificationPermissionState +import dev.privacyllc.period.notifications.NotificationPermissionState import dev.privacyllc.period.domain.prediction.PersonalPredictionEngine import dev.privacyllc.period.domain.prediction.PredictionEngine import dev.privacyllc.period.launcher.AndroidLauncherAliasSwitcher @@ -89,6 +93,22 @@ object DataModule { context.preferencesDataStoreFile("app_lock") } + /** + * Whether a reminder would actually arrive, behind an interface so the + * settings ViewModel can be tested against "no" — the answer an emulator + * makes hardest to reach and the one the screen has to handle. + */ + @Provides + @Singleton + fun reminderNotifier(@ApplicationContext context: Context): ReminderNotifier = + PeriodNotifier(context) + + @Provides + @Singleton + fun notificationPermissionState( + @ApplicationContext context: Context, + ): NotificationPermissionState = AndroidNotificationPermissionState(PeriodNotifier(context)) + @Provides @Singleton fun appLockRepository(@AppLockStore store: DataStore): AppLockRepository = diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt index c373353..c1fb660 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt @@ -1,6 +1,14 @@ package dev.privacyllc.period.feature.settings import android.Manifest +import androidx.lifecycle.compose.LocalLifecycleOwner +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.Lifecycle +import androidx.compose.ui.platform.LocalContext +import androidx.compose.runtime.DisposableEffect +import android.provider.Settings +import android.content.Intent +import android.content.Context import android.content.res.Configuration import android.os.Build import androidx.activity.compose.rememberLauncherForActivityResult @@ -57,9 +65,24 @@ fun NotificationSettingsScreen( val prefs by viewModel.state.collectAsStateWithLifecycle() val needsPermission by viewModel.needsPermission.collectAsStateWithLifecycle() + val blocked by viewModel.remindersBlocked.collectAsStateWithLifecycle() + val context = LocalContext.current + val launcher = rememberLauncherForActivityResult( ActivityResultContracts.RequestPermission(), - ) { viewModel.permissionHandled() } + ) { granted -> viewModel.onPermissionResult(granted) } + + // Re-read on resume, so somebody who leaves to switch notifications back on + // in Android settings is believed the moment they return rather than being + // told they are still blocked. + val owner = LocalLifecycleOwner.current + DisposableEffect(owner) { + val observer = LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_RESUME) viewModel.refresh() + } + owner.lifecycle.addObserver(observer) + onDispose { owner.lifecycle.removeObserver(observer) } + } // §31: asked when a reminder is switched on, never on first launch. A // permission dialog before the user has seen the app is one answered "deny" @@ -75,7 +98,12 @@ fun NotificationSettingsScreen( // The title is the Settings row she tapped to get here, so the bar names // where she is in the words she chose it by. SettingsSubpage(title = "Reminders and privacy", onBack = onNavigateBack) { - NotificationSettingsContent(prefs, viewModel) + NotificationSettingsContent( + prefs = prefs, + viewModel = viewModel, + blocked = blocked, + onOpenSystemSettings = { openAppNotificationSettings(context) }, + ) } } @@ -83,6 +111,8 @@ fun NotificationSettingsScreen( private fun NotificationSettingsContent( prefs: UserPreferences, viewModel: NotificationSettingsViewModel?, + blocked: Boolean = false, + onOpenSystemSettings: () -> Unit = {}, ) { Column( Modifier @@ -90,6 +120,21 @@ private fun NotificationSettingsContent( .verticalScroll(rememberScrollState()) .padding(16.dp), ) { + // Said once, at the top, where the toggles below it are about to look + // like they are working. + if (blocked) { + Card(Modifier.fillMaxWidth().padding(bottom = 16.dp)) { + Column(Modifier.padding(12.dp)) { + Text( + "Reminders are on here, but notifications are off for this app in " + + "Android settings. Nothing will arrive until they are switched on.", + style = MaterialTheme.typography.bodyMedium, + ) + TextButton(onClick = onOpenSystemSettings) { Text("Open system settings") } + } + } + } + Text("How reminders appear", style = MaterialTheme.typography.titleMedium) Spacer(Modifier.height(4.dp)) Text( @@ -220,3 +265,18 @@ private val timeFormat = DateTimeFormatter.ofPattern("HH:mm") private fun PreviewSettings() = PeriodTheme { NotificationSettingsContent(UserPreferences.Defaults, null) } + +/** + * The system's own notification settings for this app. + * + * `ACTION_APP_NOTIFICATION_SETTINGS` has existed since API 26, which is this + * project's floor. Wrapped anyway: an OEM that ships without the activity would + * otherwise turn a helpful button into a crash, and there is nothing useful to + * do about it beyond not crashing. + */ +private fun openAppNotificationSettings(context: Context) { + val intent = Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS) + .putExtra(Settings.EXTRA_APP_PACKAGE, context.packageName) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + runCatching { context.startActivity(intent) } +} diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModel.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModel.kt index 3e19b2c..13aa12b 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModel.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModel.kt @@ -7,6 +7,7 @@ import dev.privacyllc.period.core.datastore.NotificationPrivacy import dev.privacyllc.period.core.datastore.UserPreferences import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.notifications.ReminderScheduler +import dev.privacyllc.period.notifications.NotificationPermissionState import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -30,6 +31,7 @@ import javax.inject.Inject class NotificationSettingsViewModel @Inject constructor( private val preferences: UserPreferencesRepository, private val scheduler: ReminderScheduler, + private val permissions: NotificationPermissionState, ) : ViewModel() { val state: StateFlow = @@ -52,13 +54,55 @@ class NotificationSettingsViewModel @Inject constructor( fun permissionHandled() { _needsPermission.value = false } + /** + * True when reminders are switched on here but could not arrive. + * + * The screen says so rather than leaving a toggle looking on while nothing + * comes. The result of the permission dialog used to be discarded entirely — + * the toggle stayed on, no rationale appeared, and a later revocation was + * never noticed at all. + */ + private val _remindersBlocked = MutableStateFlow(false) + val remindersBlocked: StateFlow = _remindersBlocked.asStateFlow() + + /** + * Re-read the system's answer. Called when the screen resumes, which is how + * a user who left to change the setting is believed when they come back. + */ + fun refresh() { + viewModelScope.launch(handler) { + val prefs = preferences.preferences.first() + _remindersBlocked.value = anyEnabled(prefs) && !permissions.canPost(prefs.notificationPrivacy) + } + } + + /** + * The user answered the permission dialog. + * + * A denial does **not** switch the toggle back off. She said she wants the + * reminder; rewriting that would mean a later grant changes nothing and she + * has to find the toggle again. The screen shows the blocked row instead. + */ + fun onPermissionResult(@Suppress("UNUSED_PARAMETER") granted: Boolean) { + _needsPermission.value = false + refresh() + } + + private fun anyEnabled(p: UserPreferences) = + p.periodApproachingEnabled || p.periodExpectedTodayEnabled || p.didItStartEnabled || + p.periodEndCheckInEnabled || p.fertileWindowReminderEnabled || p.ovulationReminderEnabled + private val handler = CoroutineExceptionHandler { _, _ -> } private fun update(requestPermissionIfEnabling: Boolean = true, block: suspend () -> Unit) = viewModelScope.launch(handler) { block() - if (requestPermissionIfEnabling) _needsPermission.value = true + // Only ask when it is not already granted: a dialog the system will + // answer instantly is a flicker, and on a second denial it never + // appears at all. + if (requestPermissionIfEnabling && !permissions.granted()) _needsPermission.value = true reschedule() + refresh() } fun setPrivacy(value: NotificationPrivacy) = diff --git a/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationPermissionState.kt b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationPermissionState.kt new file mode 100644 index 0000000..0bd9734 --- /dev/null +++ b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationPermissionState.kt @@ -0,0 +1,33 @@ +package dev.privacyllc.period.notifications + +import dev.privacyllc.period.core.datastore.NotificationPrivacy +import dev.privacyllc.period.core.notifications.PeriodNotifier +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Whether a reminder would actually arrive. + * + * An interface so the settings ViewModel can be tested without a permission + * grant — the behaviour worth testing is what the screen says when the answer is + * *no*, and that is the state an emulator makes hardest to reach. + */ +interface NotificationPermissionState { + /** The runtime permission alone (API 33+); always true below it. */ + fun granted(): Boolean + + /** + * True when a reminder posted now would appear: the permission, plus + * notifications not switched off for the app, plus this mode's channel not + * blocked. + */ + fun canPost(privacy: NotificationPrivacy): Boolean +} + +@Singleton +class AndroidNotificationPermissionState @Inject constructor( + private val notifier: PeriodNotifier, +) : NotificationPermissionState { + override fun granted(): Boolean = notifier.hasPermission() + override fun canPost(privacy: NotificationPrivacy): Boolean = notifier.canPost(privacy) +} diff --git a/app/src/test/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModelTest.kt b/app/src/test/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModelTest.kt new file mode 100644 index 0000000..0b8c726 --- /dev/null +++ b/app/src/test/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsViewModelTest.kt @@ -0,0 +1,173 @@ +package dev.privacyllc.period.feature.settings + +import android.content.Context +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.preferencesDataStoreFile +import androidx.test.core.app.ApplicationProvider +import dev.privacyllc.period.core.datastore.NotificationPrivacy +import dev.privacyllc.period.core.datastore.UserPreferencesRepository +import dev.privacyllc.period.core.notifications.ReminderScheduler +import dev.privacyllc.period.notifications.NotificationPermissionState +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * What the reminders screen does when the system says no. + * + * The result of the permission dialog used to be discarded: the toggle stayed + * on, nothing explained why nothing arrived, and a revocation afterwards was + * never noticed. Worse, the reminder worker counted check-ins it had not been + * able to post, so the app could reach its "stop asking" limit having never + * asked once. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class NotificationSettingsViewModelTest { + + /** The answer an emulator makes hardest to reach, which is why it is a seam. */ + private class FakePermissions( + var granted: Boolean = true, + var canPost: Boolean = true, + ) : NotificationPermissionState { + override fun granted() = granted + override fun canPost(privacy: NotificationPrivacy) = canPost + } + + private val dispatcher = UnconfinedTestDispatcher() + private lateinit var permissions: FakePermissions + private lateinit var preferences: UserPreferencesRepository + private lateinit var vm: NotificationSettingsViewModel + + @Before fun setUp() { + Dispatchers.setMain(dispatcher) + val context = ApplicationProvider.getApplicationContext() + preferences = UserPreferencesRepository( + PreferenceDataStoreFactory.create { + context.preferencesDataStoreFile("notif_vm_test_${System.nanoTime()}") + }, + ) + permissions = FakePermissions() + vm = NotificationSettingsViewModel(preferences, ReminderScheduler(context), permissions) + } + + @After fun tearDown() = Dispatchers.resetMain() + + private fun await(predicate: suspend () -> Boolean) = runBlocking { + withTimeout(10_000) { while (!predicate()) delay(10) } + } + + /** + * Read the store, never `vm.state.value`. + * + * `state` is `stateIn(WhileSubscribed)`: with nobody collecting it, its + * value sits on `UserPreferences.Defaults` forever — where every reminder is + * already `true`. An assertion against it would pass whatever the ViewModel + * did, which is exactly what happened until a prove-guard run showed the + * test could not fail. + */ + private fun didItStartEnabled() = runBlocking { preferences.preferences.first().didItStartEnabled } + + @Test fun `switching a reminder on without the permission asks for it`() { + permissions.granted = false + + vm.setDidItStart(true) + + await { vm.needsPermission.value } + } + + @Test fun `switching a reminder on with the permission already granted asks for nothing`() { + permissions.granted = true + + vm.setDidItStart(true) + await { didItStartEnabled() } + + // A dialog the system answers instantly is a flicker, and on a second + // denial it never appears at all. + assertFalse(vm.needsPermission.value) + } + + @Test fun `a denied permission keeps the toggle on and says reminders are blocked`() { + permissions.granted = false + permissions.canPost = false + + vm.setDidItStart(true) + await { vm.needsPermission.value } + vm.onPermissionResult(granted = false) + + await { vm.remindersBlocked.value } + // Her stated preference is not rewritten. Turning it off here would mean + // a later grant changes nothing and she has to find the toggle again. + assertTrue("the denial switched the reminder back off", didItStartEnabled()) + assertFalse(vm.needsPermission.value) + } + + @Test fun `coming back after switching notifications on clears the blocked state`() { + permissions.granted = false + permissions.canPost = false + vm.setDidItStart(true) + vm.onPermissionResult(granted = false) + await { vm.remindersBlocked.value } + + // She left, changed it in Android settings, and came back. + permissions.granted = true + permissions.canPost = true + vm.refresh() + + await { !vm.remindersBlocked.value } + } + + @Test fun `nothing is blocked when every reminder is switched off`() { + // Reminders ship on, so they have to be turned off deliberately. + permissions.canPost = true + vm.setPeriodApproaching(false) + vm.setPeriodExpectedToday(false) + vm.setDidItStart(false) + vm.setPeriodEndCheckIn(false) + vm.setFertileWindow(false) + vm.setOvulation(false) + // Wait for all six, not just the last one asked for: the writes are + // independent coroutines and any still in flight would leave something + // enabled and make the assertion below measure the wrong thing. + await { + val p = preferences.preferences.first() + !p.periodApproachingEnabled && !p.periodExpectedTodayEnabled && !p.didItStartEnabled && + !p.periodEndCheckInEnabled && !p.fertileWindowReminderEnabled && !p.ovulationReminderEnabled + } + + permissions.canPost = false + vm.refresh() + + // Nothing to deliver is not a problem worth reporting. + await { !vm.remindersBlocked.value } + } + + @Test fun `a revoked permission is noticed on the next resume`() { + vm.setDidItStart(true) + await { didItStartEnabled() } + assertFalse(vm.remindersBlocked.value) + + // Revoked in system settings while the app was in the background, which + // nothing detected before: hasPermission() was called from nowhere. + permissions.granted = false + permissions.canPost = false + vm.refresh() + + await { vm.remindersBlocked.value } + assertEquals(true, didItStartEnabled()) + } +} diff --git a/core/notifications/build.gradle.kts b/core/notifications/build.gradle.kts index b38257f..954893f 100644 --- a/core/notifications/build.gradle.kts +++ b/core/notifications/build.gradle.kts @@ -13,6 +13,12 @@ android { testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } + testOptions { + // The worker builds a real Notification, which resolves + // R.drawable.ic_notification — Robolectric needs the resources for that. + unitTests.isIncludeAndroidResources = true + } + compileOptions { sourceCompatibility = JavaVersion.VERSION_17 targetCompatibility = JavaVersion.VERSION_17 @@ -34,6 +40,9 @@ dependencies { testImplementation(libs.junit) testImplementation(libs.kotlinx.coroutines.test) + testImplementation(libs.robolectric) + testImplementation(libs.androidx.test.core) + testImplementation(libs.androidx.work.testing) androidTestImplementation(libs.androidx.test.junit) androidTestImplementation(libs.androidx.test.runner) diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/PeriodNotifier.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/PeriodNotifier.kt index 37ee1e2..efab1e7 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/PeriodNotifier.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/PeriodNotifier.kt @@ -31,7 +31,7 @@ import dev.privacyllc.period.core.datastore.NotificationPrivacy * "Reminders" says nothing; "Period reminders" says everything, to anyone who * opens the notification settings on a shared phone. */ -class PeriodNotifier(private val context: Context) { +class PeriodNotifier(private val context: Context) : ReminderNotifier { /** * One channel per privacy mode, and that is not tidiness. @@ -77,11 +77,11 @@ class PeriodNotifier(private val context: Context) { * Post [text]. Returns false when the permission is absent — silently doing * nothing would look identical to a scheduling bug. */ - fun notify( + override fun notify( text: NotificationText, privacy: NotificationPrivacy, contentIntent: android.app.PendingIntent?, - actions: List = emptyList(), + actions: List, ): Boolean { // Checked inline rather than through hasPermission(), so lint can see // it. A helper method is equivalent to a human reader and invisible to @@ -133,6 +133,29 @@ class PeriodNotifier(private val context: Context) { ContextCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED + /** + * Whether a reminder posted right now would actually appear. + * + * Three ways it would not, and [hasPermission] only covers the first: + * the runtime permission (API 33+), notifications switched off for the whole + * app in system settings, and this privacy mode's channel blocked on its + * own. Below API 33 the permission is granted by definition, so an app whose + * notifications the user had switched off would post into nothing and count + * it as having asked. + * + * Used by the settings screen to say so, rather than leaving a toggle + * looking on while nothing arrives. + */ + fun canPost(privacy: NotificationPrivacy): Boolean { + if (!hasPermission()) return false + val manager = NotificationManagerCompat.from(context) + if (!manager.areNotificationsEnabled()) return false + // A channel the user has blocked, or set to no importance. Absent is + // fine: it is created on the first post. + val channel = manager.getNotificationChannelCompat(channelId(privacy)) ?: return true + return channel.importance != NotificationManagerCompat.IMPORTANCE_NONE + } + companion object { /** * Channel ids, and the names beside them. diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderNotifier.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderNotifier.kt new file mode 100644 index 0000000..f679102 --- /dev/null +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderNotifier.kt @@ -0,0 +1,36 @@ +package dev.privacyllc.period.core.notifications + +import androidx.core.app.NotificationCompat +import dev.privacyllc.period.core.datastore.NotificationPrivacy + +/** + * Posting a reminder, behind an interface so the worker can be tested. + * + * `ReminderWorker` built its own [PeriodNotifier] from the application context, + * which made it untestable — and it is the class that reads the history, applies + * §30's rules, posts, and counts the check-in. Every defect this batch fixed + * lived in that gap: a reminder counted as asked when it was never posted, a + * stop-asking notice that bypassed the tested copy, buttons wired to the wrong + * writes. + * + * Narrow on purpose. This is what the worker needs and nothing else; the channel + * setup, the permission checks and the public-version construction stay inside + * the implementation, where the tests that already cover them live. + */ +interface ReminderNotifier { + + /** + * Post [text], and say whether it actually appeared. + * + * **The return value is not advisory.** False means no permission, + * notifications switched off for the app, or a blocked channel — and a + * caller that counts a reminder it could not post will eventually stop + * asking having never asked. That is not hypothetical; it is what happened. + */ + fun notify( + text: NotificationText, + privacy: NotificationPrivacy, + contentIntent: android.app.PendingIntent?, + actions: List = emptyList(), + ): Boolean +} diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderWorker.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderWorker.kt index bc723f7..7ad7757 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderWorker.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderWorker.kt @@ -37,6 +37,7 @@ class ReminderWorker @AssistedInject constructor( private val repository: CycleRepository, private val preferences: UserPreferencesRepository, private val clock: Clock, + private val notifier: ReminderNotifier, ) : CoroutineWorker(appContext, params) { /** @@ -97,15 +98,13 @@ class ReminderWorker @AssistedInject constructor( today = today, ) - val notifier = PeriodNotifier(applicationContext) - when (decision) { ReminderDecision.Nothing -> return Result.success() ReminderDecision.StopAsking -> { // §30's exact sentiment, and it is the only notification in the // app that exists to say the app will now be quiet. - notifier.notify( + val posted = notifier.notify( text = NotificationText( publicTitle = if (prefs.notificationPrivacy == dev.privacyllc.period.core.datastore.NotificationPrivacy.DIRECT @@ -118,17 +117,16 @@ class ReminderWorker @AssistedInject constructor( privacy = prefs.notificationPrivacy, contentIntent = openApp(), ) - // Counted, so the next wake-up sees a number past the limit and - // sends nothing. Against the period it was asked about, so the - // next period starts the count over on its own. - latestPeriodId?.let { preferences.recordCheckIn(it) } + // Counted only if it was actually shown. See the check-in + // comment below — the same rule, for the same reason. + if (posted) latestPeriodId?.let { preferences.recordCheckIn(it) } return Result.success() } is ReminderDecision.Send -> { val text = NotificationCopy.textFor(decision.kind, prefs.notificationPrivacy, decision.daysUntil) - notifier.notify( + val posted = notifier.notify( text = text, privacy = prefs.notificationPrivacy, contentIntent = openApp(), @@ -149,9 +147,16 @@ class ReminderWorker @AssistedInject constructor( // Only the "did it start" family counts toward the stopping // rule. A heads-up three days out is not the app pestering. - if (decision.kind == ReminderKind.DID_IT_START || + // + // And only if the notification was actually posted. `notify` + // returns false when it cannot — no permission, notifications + // switched off, the channel blocked — and the return value used + // to be discarded here. With notifications denied the counter + // climbed to the limit and the app stopped asking, permanently, + // having never once asked. + val counts = decision.kind == ReminderKind.DID_IT_START || decision.kind == ReminderKind.PERIOD_EXPECTED_TODAY - ) { + if (posted && counts) { latestPeriodId?.let { preferences.recordCheckIn(it) } } return Result.success() diff --git a/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/ReminderWorkerTest.kt b/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/ReminderWorkerTest.kt new file mode 100644 index 0000000..370eec2 --- /dev/null +++ b/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/ReminderWorkerTest.kt @@ -0,0 +1,187 @@ +package dev.privacyllc.period.core.notifications + +import android.content.Context +import androidx.core.app.NotificationCompat +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.preferencesDataStoreFile +import androidx.test.core.app.ApplicationProvider +import androidx.work.ListenableWorker +import androidx.work.WorkerFactory +import androidx.work.WorkerParameters +import androidx.work.testing.TestListenableWorkerBuilder +import dev.privacyllc.period.core.data.CycleData +import dev.privacyllc.period.core.data.CycleRepository +import dev.privacyllc.period.core.datastore.NotificationPrivacy +import dev.privacyllc.period.core.datastore.UserPreferencesRepository +import dev.privacyllc.period.domain.prediction.PersonalPredictionEngine +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.time.Clock +import java.time.LocalDate +import java.time.ZoneOffset + +/** + * The class that decides whether the app speaks, and remembers that it did. + * + * It had no test of any kind, and every defect this batch fixed lived in that + * gap: buttons wired to the wrong writes by position, and a check-in counted + * whether or not the notification could actually be posted — which let the app + * reach its "stop asking" limit having never asked once. + * + * Runs the real worker over a real repository and a real preference store, with + * only the notifier faked, because what is being asserted is precisely what the + * worker does with the notifier's answer. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class ReminderWorkerTest { + + /** Records what was posted, and can refuse to post — the case that mattered. */ + private class FakeNotifier(var canPost: Boolean = true) : ReminderNotifier { + val posted = mutableListOf() + var lastActions: List = emptyList() + + override fun notify( + text: NotificationText, + privacy: NotificationPrivacy, + contentIntent: android.app.PendingIntent?, + actions: List, + ): Boolean { + if (!canPost) return false + posted += text + lastActions = actions + return true + } + } + + private val today = LocalDate.of(2026, 8, 20) + private lateinit var context: Context + private lateinit var repo: CycleRepository + private lateinit var prefs: UserPreferencesRepository + private lateinit var notifier: FakeNotifier + + @Before fun setUp() { + context = ApplicationProvider.getApplicationContext() + val clock = Clock.fixed(today.atStartOfDay(ZoneOffset.UTC).toInstant(), ZoneOffset.UTC) + repo = CycleData.repository(context, PersonalPredictionEngine(), clock) + runBlocking { repo.deleteAllHealthData() } + prefs = UserPreferencesRepository( + PreferenceDataStoreFactory.create { + context.preferencesDataStoreFile("worker_test_${System.nanoTime()}") + }, + ) + notifier = FakeNotifier() + } + + private fun runWorker(): ListenableWorker.Result { + val clock = Clock.fixed(today.atStartOfDay(ZoneOffset.UTC).toInstant(), ZoneOffset.UTC) + val worker = TestListenableWorkerBuilder(context) + .setWorkerFactory(object : WorkerFactory() { + override fun createWorker( + appContext: Context, + workerClassName: String, + workerParameters: WorkerParameters, + ): ListenableWorker = ReminderWorker(appContext, workerParameters, repo, prefs, clock, notifier) + }) + .build() + return runBlocking { worker.doWork() } + } + + /** A period expected today: the forecast lands on the day the worker runs. */ + private fun seedPeriodDueToday() = runBlocking { + repo.confirmPeriodStart(today.minusDays(84)) + repo.confirmPeriodStart(today.minusDays(56)) + repo.confirmPeriodStart(today.minusDays(28)) + } + + private fun tally() = runBlocking { + val latest = repo.confirmedPeriods.first().maxByOrNull { it.startDate }?.id + prefs.checkInTally.first().countFor(latest) + } + + // ----------------------------------------------------------------------- + // Counting only what was said + // ----------------------------------------------------------------------- + + @Test fun `a check-in that could not be posted is not counted as having asked`() { + seedPeriodDueToday() + notifier.canPost = false + + repeat(5) { runWorker() } + + // The regression: notify() returns false without the permission, the + // return value was discarded, and the counter climbed to the limit + // regardless. The app then stopped asking, permanently, having never + // asked once. + assertTrue(notifier.posted.isEmpty()) + assertEquals(0, tally()) + } + + @Test fun `a posted check-in is counted`() { + seedPeriodDueToday() + + runWorker() + + assertEquals(1, notifier.posted.size) + assertEquals(1, tally()) + } + + @Test fun `the count survives a fresh worker, which is a fresh process`() { + seedPeriodDueToday() + + runWorker() + runWorker() + + // Each doWork() is what a separate WorkManager process does. The count + // used to be wiped by the coordinator's first emission in every one of + // them, which is why §30's stopping rule was unreachable. + assertEquals(2, tally()) + } + + // ----------------------------------------------------------------------- + // What it says, and what it offers + // ----------------------------------------------------------------------- + + @Test fun `the period-due reminder offers a start and a not-yet, never an end`() { + seedPeriodDueToday() + + runWorker() + + assertEquals(1, notifier.posted.size) + assertEquals(2, notifier.lastActions.size) + // The actions themselves are asserted in NotificationCopyTest; here the + // point is that the worker attaches the ones the kind actually calls for. + val labels = notifier.lastActions.map { it.title.toString() } + assertTrue("offered $labels", labels.any { it == "Yes" || it == "Started" }) + assertTrue("offered $labels", labels.any { it == "Not yet" }) + } + + @Test fun `a wake-up with no history says nothing and does not fail`() { + val result = runWorker() + + assertTrue(notifier.posted.isEmpty()) + assertEquals(ListenableWorker.Result.success(), result) + } + + @Test fun `a wake-up with every reminder switched off says nothing`() = runBlocking { + seedPeriodDueToday() + prefs.setPeriodApproachingEnabled(false) + prefs.setPeriodExpectedTodayEnabled(false) + prefs.setDidItStartEnabled(false) + prefs.setPeriodEndCheckInEnabled(false) + prefs.setFertileWindowReminderEnabled(false) + prefs.setOvulationReminderEnabled(false) + + runWorker() + + assertTrue(notifier.posted.isEmpty()) + assertEquals(0, tally()) + } +} diff --git a/docs/design/README.md b/docs/design/README.md index b93499d..ca007e5 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -153,6 +153,26 @@ have replaced it. ## The lock screen is the one semi-public surface +### A switch that is on should mean something is happening + +Reminders can be switched on here and still never arrive: the notification +permission can be denied, notifications can be off for the whole app, or the +channel can be blocked. The screen used to show none of that. The permission +dialog's result was discarded, the toggle stayed on, and a revocation afterwards +was never noticed at all — so the honest reading of that screen was a promise the +app could not keep. + +It says so now, in one row above the toggles, with a button to the system +setting that would fix it. And it is re-read when the screen resumes, so somebody +who leaves to switch notifications back on is believed the moment she returns +rather than told she is still blocked. + +**A denial does not switch the toggle back off.** That is the tempting fix and it +is wrong: she said she wants the reminder, and rewriting her answer means a later +grant changes nothing and she has to find the toggle again to discover that. +The preference records what she asked for; the row records what the system is +doing about it. + ### Back is a promise, and the top bar is where it is kept **Every screen that is not a tab has a top bar with a back arrow, and it is one diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ee62df8..e945e26 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -71,6 +71,11 @@ androidx-room-runtime = { group = "androidx.room", name = "room-runtime", versio androidx-room-ktx = { group = "androidx.room", name = "room-ktx", version.ref = "room" } androidx-room-compiler = { group = "androidx.room", name = "room-compiler", version.ref = "room" } androidx-room-testing = { group = "androidx.room", name = "room-testing", version.ref = "room" } +# WorkManager's test harness: TestListenableWorkerBuilder runs a CoroutineWorker +# on the JVM without a scheduler. Added because the reminder worker -- which +# reads the history, applies the rules, posts, and counts the check-in -- had no +# test of any kind, and the defects in Batch 12 all lived in exactly that gap. +androidx-work-testing = { group = "androidx.work", name = "work-testing", version.ref = "work" } androidx-work-runtime = { group = "androidx.work", name = "work-runtime-ktx", version.ref = "work" } androidx-datastore-preferences = { group = "androidx.datastore", name = "datastore-preferences", version.ref = "datastore" } androidx-biometric = { group = "androidx.biometric", name = "biometric", version.ref = "biometric" }