fix: stop counting reminders the app could not send
A reminder can be switched on and still never arrive: the permission denied, notifications off for the whole app, or the channel blocked. notify() returns false in all three cases, and both call sites discarded it while still counting the check-in. So with notifications denied the counter climbed to §30's limit and the app stopped asking -- permanently, having never once asked. Counted only when posted now. canPost() also asks whether notifications are enabled at all and whether this mode's channel is blocked; below API 33 the permission is granted by definition, so an app whose notifications the user had switched off posted into nothing and called it asking. The settings screen says so, in one row above the toggles, with a button to the system setting that would fix it -- and re-reads on resume, so somebody who leaves to switch notifications back on is believed when she returns. Revocation after the fact was previously undetectable: hasPermission() was called from nowhere in main. 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. ReminderWorker now takes a ReminderNotifier rather than building one from the application context, which is what made it testable. It had no test of any kind -- the class that reads the history, applies the rules, posts, and counts -- and every defect in this batch lived in that gap. Six now, over a real repository and a real preference store with only the notifier faked, since what is asserted is precisely what the worker does with the notifier's answer. Two things the prove-guard discipline caught that a green suite did not. The posted-and-counts guard reddened nothing at first, because the worker had no tests to redden -- the fix was unproven until the harness existed. And an assertion of mine read vm.state.value, which is stateIn(WhileSubscribed): with nobody collecting, it sits on the defaults, where every reminder is already true. That test could not have failed. It reads the store now. closes #71 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7447156cbc
commit
65e8e574b7
|
|
@ -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<Preferences>): AppLockRepository =
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<UserPreferences> =
|
||||
|
|
@ -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<Boolean> = _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) =
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
@ -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<Context>()
|
||||
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())
|
||||
}
|
||||
}
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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<NotificationCompat.Action> = emptyList(),
|
||||
actions: List<NotificationCompat.Action>,
|
||||
): 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.
|
||||
|
|
|
|||
|
|
@ -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<NotificationCompat.Action> = emptyList(),
|
||||
): Boolean
|
||||
}
|
||||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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<NotificationText>()
|
||||
var lastActions: List<NotificationCompat.Action> = emptyList()
|
||||
|
||||
override fun notify(
|
||||
text: NotificationText,
|
||||
privacy: NotificationPrivacy,
|
||||
contentIntent: android.app.PendingIntent?,
|
||||
actions: List<NotificationCompat.Action>,
|
||||
): 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<ReminderWorker>(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())
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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" }
|
||||
|
|
|
|||
Loading…
Reference in New Issue