diff --git a/app/src/main/kotlin/dev/privacyllc/period/MainActivity.kt b/app/src/main/kotlin/dev/privacyllc/period/MainActivity.kt index 209a082..7f6eda7 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/MainActivity.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/MainActivity.kt @@ -12,6 +12,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle import dagger.hilt.android.AndroidEntryPoint +import dev.privacyllc.period.core.notifications.PeriodNotifier import dev.privacyllc.period.core.security.AppLockRepository import dev.privacyllc.period.designsystem.PeriodTheme import dev.privacyllc.period.feature.export.ExportController @@ -139,6 +140,12 @@ class MainActivity : FragmentActivity() { } private companion object { - const val EXTRA_REMINDER_ACTION = "reminder_action" + /** + * The same key `core/notifications` writes, not a second copy of the + * literal: the two sides of a notification tap have to agree, and a + * repeated string is one rename away from an action that quietly stops + * arriving. + */ + const val EXTRA_REMINDER_ACTION = PeriodNotifier.EXTRA_REMINDER_ACTION } } diff --git a/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt index 384183d..860e0c3 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt @@ -2,7 +2,9 @@ package dev.privacyllc.period.notifications import dev.privacyllc.period.core.data.CycleRepository import dev.privacyllc.period.core.datastore.UserPreferencesRepository -import dev.privacyllc.period.core.notifications.PeriodNotifier +import dev.privacyllc.period.core.notifications.ReminderAction +import dev.privacyllc.period.domain.cycle.PeriodRecordSource +import kotlinx.coroutines.flow.first import java.time.Clock import java.time.LocalDate import javax.inject.Inject @@ -33,23 +35,52 @@ class NotificationActionHandler @Inject constructor( /** Returns true when the action was recognised and applied. */ suspend fun handle(action: String?): Boolean { val today = LocalDate.now(clock) - return when (action) { - PeriodNotifier.ACTION_STARTED -> { - repository.confirmPeriodStart(today) + // Anything this app did not write — including the action strings used + // before the buttons carried their own meaning — writes nothing. A + // notification already sitting in somebody's shade at upgrade time + // still opens the app; it just does not guess what she meant. + return when (ReminderAction.fromWireName(action)) { + ReminderAction.STARTED -> { + repository.confirmPeriodStart(today, PeriodRecordSource.NOTIFICATION_CONFIRMATION) // The question is answered, so the app is willing to ask again // next cycle rather than staying permanently quiet. preferences.resetCheckIns() true } - PeriodNotifier.ACTION_NOT_YET -> { + ReminderAction.NOT_YET -> { // Exactly what the Today screen's "Not yet" does — same call, // same censoring observation, same re-conditioned forecast. repository.recordNotYet(today) true } - else -> false + ReminderAction.ENDED -> { + // Closes the period that is running. NEVER opens one: this + // button answers "is it over?", and the write that used to sit + // here answered "did it start?" — inserting a fresh period in + // the middle of the one it was asking about. + // + // Nothing to close is not a failure. She may have ended it in + // the app between the notification being posted and being + // tapped, and the honest response to an answered question is + // silence rather than a second record. + val open = repository.confirmedPeriods.first() + .lastOrNull { it.endDate == null && !it.startDate.isAfter(today) } + open?.let { repository.setPeriodEnd(it.id, today) } + true + } + + ReminderAction.STILL_GOING -> { + // Deliberately writes nothing. "Still going" is the state the + // record is already in, and the Today screen's equivalent + // (setPeriodEnd(id, null)) is a no-op on an open period — one + // that would still move `updatedAt` and read, in the history, + // as an edit the user never made. + true + } + + null -> false } } } diff --git a/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt b/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt new file mode 100644 index 0000000..8eeb8f0 --- /dev/null +++ b/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt @@ -0,0 +1,177 @@ +package dev.privacyllc.period.notifications + +import androidx.test.core.app.ApplicationProvider +import dev.privacyllc.period.core.data.CycleData +import dev.privacyllc.period.core.data.CycleRepository +import dev.privacyllc.period.core.data.PeriodWriteResult +import dev.privacyllc.period.core.datastore.UserPreferencesRepository +import dev.privacyllc.period.core.notifications.NotificationCopy +import dev.privacyllc.period.core.notifications.ReminderAction +import dev.privacyllc.period.core.notifications.ReminderKind +import dev.privacyllc.period.core.datastore.NotificationPrivacy +import dev.privacyllc.period.domain.cycle.PeriodRecordSource +import dev.privacyllc.period.domain.prediction.PersonalPredictionEngine +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +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 two writes reachable from a locked phone. + * + * These had no test at all, which is how the period-end check-in came to record + * the opposite of what its buttons said: "Ended" inserted a new period starting + * today, in the middle of the period it was asking about. The button labels are + * chosen in one file and the writes happened in another, and nothing held them + * to each other. + * + * So the first test here is not about a write at all — it is about the two + * halves agreeing. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class NotificationActionHandlerTest { + + @get:Rule val temp = TemporaryFolder() + + private lateinit var repo: CycleRepository + private lateinit var prefs: UserPreferencesRepository + private lateinit var handler: NotificationActionHandler + + private val today = LocalDate.of(2026, 8, 20) + + @Before fun open() { + val clock = Clock.fixed(today.atStartOfDay(ZoneOffset.UTC).toInstant(), ZoneOffset.UTC) + // Through CycleData, like every other app test: this module deliberately + // cannot name PeriodDatabase, which is the boundary architecture/README + // describes and the compiler enforces. + repo = CycleData.repository(ApplicationProvider.getApplicationContext(), PersonalPredictionEngine(), clock) + runBlocking { repo.deleteAllHealthData() } + prefs = UserPreferencesRepository( + PreferenceDataStoreFactory.create(scope = CoroutineScope(Dispatchers.Unconfined)) { + temp.newFile("prefs.preferences_pb") + }, + ) + handler = NotificationActionHandler(repo, prefs, clock) + } + + // ----------------------------------------------------------------------- + // The pairing itself + // ----------------------------------------------------------------------- + + @Test + fun `a question about the end never offers an answer about the start`() { + // The defect, stated as a property. Whatever the labels say in whatever + // privacy mode, the period-end check-in must not be able to produce a + // start, and the did-it-start family must not be able to produce an end. + NotificationPrivacy.entries.forEach { privacy -> + val end = NotificationCopy.buttons(ReminderKind.PERIOD_END_CHECK_IN, privacy) + assertEquals(listOf(ReminderAction.ENDED, ReminderAction.STILL_GOING), end.map { it.action }) + + listOf(ReminderKind.DID_IT_START, ReminderKind.PERIOD_EXPECTED_TODAY).forEach { kind -> + val start = NotificationCopy.buttons(kind, privacy) + assertEquals(listOf(ReminderAction.STARTED, ReminderAction.NOT_YET), start.map { it.action }) + } + + (end + NotificationCopy.buttons(ReminderKind.DID_IT_START, privacy)).forEach { + assertTrue("a button in $privacy had a blank label", it.label.isNotBlank()) + } + } + } + + // ----------------------------------------------------------------------- + // What each button writes + // ----------------------------------------------------------------------- + + @Test + fun `Started records a period, sourced from the notification`() = runTest { + assertTrue(handler.handle(ReminderAction.STARTED.name)) + + val period = repo.confirmedPeriods.first().single() + assertEquals(today, period.startDate) + // Not MANUAL: how a record arrived is part of the record (§14). + assertEquals(PeriodRecordSource.NOTIFICATION_CONFIRMATION, period.source) + } + + @Test + fun `Not yet records a censoring observation and no period`() = runTest { + assertTrue(handler.handle(ReminderAction.NOT_YET.name)) + + assertEquals(today, repo.notYetObservations.first().single().date) + assertTrue(repo.confirmedPeriods.first().isEmpty()) + } + + @Test + fun `Ended closes the period that is running rather than starting another`() = runTest { + repo.confirmPeriodStart(LocalDate.of(2026, 8, 16)) + assertTrue(handler.handle(ReminderAction.ENDED.name)) + + // Regression: this used to insert a SECOND period starting today, in the + // middle of the one the notification was asking about. + val period = repo.confirmedPeriods.first().single() + assertEquals(LocalDate.of(2026, 8, 16), period.startDate) + assertEquals(today, period.endDate) + } + + @Test + fun `Ended with nothing running writes nothing`() = runTest { + assertTrue(handler.handle(ReminderAction.ENDED.name)) + assertTrue(repo.confirmedPeriods.first().isEmpty()) + } + + @Test + fun `Ended leaves a period that was already closed alone`() = runTest { + val id = (repo.confirmPeriodStart(LocalDate.of(2026, 8, 16)) as PeriodWriteResult.Added).id + repo.setPeriodEnd(id, LocalDate.of(2026, 8, 19)) + + assertTrue(handler.handle(ReminderAction.ENDED.name)) + + // She answered in the app first. The honest response to an answered + // question is silence, not a correction. + assertEquals(LocalDate.of(2026, 8, 19), repo.confirmedPeriods.first().single().endDate) + } + + @Test + fun `Still going writes nothing at all`() = runTest { + repo.confirmPeriodStart(LocalDate.of(2026, 8, 16)) + val before = repo.confirmedPeriods.first().single() + + assertTrue(handler.handle(ReminderAction.STILL_GOING.name)) + + val after = repo.confirmedPeriods.first().single() + assertEquals(before, after) + assertNull(after.endDate) + assertTrue(repo.notYetObservations.first().isEmpty()) + } + + // ----------------------------------------------------------------------- + // Anything else + // ----------------------------------------------------------------------- + + @Test + fun `an action this app did not write is not guessed at`() = runTest { + // Including the strings used before the buttons carried their own + // meaning — a notification sitting in the shade across an upgrade. + listOf(null, "", "dev.privacyllc.period.action.STARTED", "started", "ENDED_MAYBE").forEach { + assertFalse("$it was treated as an action", handler.handle(it)) + } + assertTrue(repo.confirmedPeriods.first().isEmpty()) + assertTrue(repo.notYetObservations.first().isEmpty()) + } +} diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt index e16a959..bf5cda5 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt @@ -113,24 +113,43 @@ object NotificationCopy { } /** - * Action labels, which are visible text on a lock screen too. + * The buttons a reminder offers: what each says, and what each does. * - * §31 is explicit about this and it is easy to miss: the notification body - * can be perfectly discreet while a button underneath it says "Started my - * period". These are chosen to mean nothing out of context. + * **One table, deliberately.** The label and the action are produced here + * together and travel together, because they were once produced apart — + * this returned bare strings and the worker paired them with writes by + * position — and the period-end check-in was paired with the wrong ones. Its + * "Ended" recorded a new period starting today. Nothing downstream now has + * an opportunity to re-pair them. + * + * Labels are visible text on a lock screen too. §31 is explicit about this + * and it is easy to miss: the body can be perfectly discreet while a button + * underneath it says "Started my period". These are chosen to mean nothing + * out of context. */ - fun actionLabels(kind: ReminderKind, privacy: NotificationPrivacy): List = - when (kind) { - ReminderKind.DID_IT_START, ReminderKind.PERIOD_EXPECTED_TODAY -> - if (privacy == NotificationPrivacy.DIRECT) listOf("Started", "Not yet") - else listOf("Yes", "Not yet") + fun buttons(kind: ReminderKind, privacy: NotificationPrivacy): List { + val direct = privacy == NotificationPrivacy.DIRECT + return when (kind) { + // "Has it started?" — a start, or a censoring observation. + ReminderKind.DID_IT_START, ReminderKind.PERIOD_EXPECTED_TODAY -> listOf( + ReminderButton(ReminderAction.STARTED, if (direct) "Started" else "Yes"), + ReminderButton(ReminderAction.NOT_YET, "Not yet"), + ) - ReminderKind.PERIOD_END_CHECK_IN -> - if (privacy == NotificationPrivacy.DIRECT) listOf("Ended", "Still going") - else listOf("Done", "Not yet") + // "Is it over?" — an end, or nothing at all. Never a start. + ReminderKind.PERIOD_END_CHECK_IN -> listOf( + ReminderButton(ReminderAction.ENDED, if (direct) "Ended" else "Done"), + ReminderButton(ReminderAction.STILL_GOING, if (direct) "Still going" else "Not yet"), + ) + // A heads-up asks nothing, so it offers nothing to answer. else -> emptyList() } + } + + /** The labels alone, for callers that only render text. */ + fun actionLabels(kind: ReminderKind, privacy: NotificationPrivacy): List = + buttons(kind, privacy).map { it.label } private fun discreetTitle(kind: ReminderKind) = when (kind) { ReminderKind.PERIOD_APPROACHING -> "A heads-up" 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 8a62955..37ee1e2 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 @@ -157,18 +157,24 @@ class PeriodNotifier(private val context: Context) { /** One id: a reminder replaces the previous one rather than stacking. */ const val NOTIFICATION_ID = 1001 - /** Actions come back through the app, so the tap lands on the check-in screen. */ - const val ACTION_STARTED = "dev.privacyllc.period.action.STARTED" - const val ACTION_NOT_YET = "dev.privacyllc.period.action.NOT_YET" const val EXTRA_FROM_NOTIFICATION = "from_notification" - @Suppress("UNUSED_PARAMETER") - fun launchIntent(context: Context, action: String? = null): Intent = + /** + * Which button was tapped, as [ReminderAction.name]. + * + * Declared here rather than in `app` because both sides of the tap need + * it and only one of them can see the other. `MainActivity` reads this + * constant; it used to repeat the string literal, which is one rename + * away from an action that silently stops arriving. + */ + const val EXTRA_REMINDER_ACTION = "reminder_action" + + fun launchIntent(context: Context, action: ReminderAction? = null): Intent = Intent().apply { setClassName(context, "dev.privacyllc.period.MainActivity") flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP putExtra(EXTRA_FROM_NOTIFICATION, true) - action?.let { putExtra("reminder_action", it) } + action?.let { putExtra(EXTRA_REMINDER_ACTION, it.name) } } } } diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt new file mode 100644 index 0000000..e99bc59 --- /dev/null +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt @@ -0,0 +1,53 @@ +package dev.privacyllc.period.core.notifications + +/** + * What a notification button actually does. + * + * ## Why this exists at all + * + * The labels and the writes used to live apart: [NotificationCopy] returned a + * list of strings, and `ReminderWorker` attached the first to "started" and the + * second to "not yet" — for every kind of reminder, by position. That is fine + * while every reminder asks the same question, and it was wrong the moment one + * did not. The period-end check-in asks *"Is your period over?"* and offers + * **Ended** and **Still going**; by position, "Ended" recorded a brand-new + * period starting today, in the middle of the period it was asking about, and + * "Still going" filed a censoring observation against a forecast that had + * already arrived. The user did nothing wrong and had no way to see it. + * + * So a button is now one thing carrying both halves — see [ReminderButton] — + * and the position of a label in a list means nothing. + * + * The name is the wire value: it travels in the PendingIntent that the tap + * delivers, and it is read back on the other side. Renaming a constant here + * changes what a notification already sitting in somebody's shade will say when + * they tap it, which is why the reader treats an unrecognised value as "do + * nothing" rather than guessing. + */ +enum class ReminderAction { + /** The period began. Confirms a start. */ + STARTED, + + /** It has not begun yet. Censors the forecast — §13. */ + NOT_YET, + + /** The period is over. Closes the open record; never opens a new one. */ + ENDED, + + /** The period is still going. Answered, and deliberately writes nothing. */ + STILL_GOING, + ; + + companion object { + /** Null for anything unrecognised — including the pre-2026-08 action strings. */ + fun fromWireName(name: String?): ReminderAction? = entries.firstOrNull { it.name == name } + } +} + +/** + * A button on a reminder: what it says, and what it does. + * + * The two are produced together by [NotificationCopy.buttons] precisely so that + * nothing downstream has to pair them up again. + */ +data class ReminderButton(val action: ReminderAction, val label: String) 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 99e63c0..18785cd 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 @@ -122,21 +122,24 @@ class ReminderWorker @AssistedInject constructor( is ReminderDecision.Send -> { val text = NotificationCopy.textFor(decision.kind, prefs.notificationPrivacy, decision.daysUntil) - val labels = NotificationCopy.actionLabels(decision.kind, prefs.notificationPrivacy) notifier.notify( text = text, privacy = prefs.notificationPrivacy, contentIntent = openApp(), - actions = labels.mapIndexed { index, label -> - NotificationCompat.Action.Builder( - 0, - label, - openApp( - if (index == 0) PeriodNotifier.ACTION_STARTED else PeriodNotifier.ACTION_NOT_YET, - ), - ).build() - }, + // Each button carries its own action. This used to attach + // the first label to "started" and the second to "not yet" + // by position, whatever the reminder was asking — which is + // how "Ended" came to record a new period. See + // NotificationCopy.buttons. + actions = NotificationCopy.buttons(decision.kind, prefs.notificationPrivacy) + .map { button -> + NotificationCompat.Action.Builder( + 0, + button.label, + openApp(button.action), + ).build() + }, ) // Only the "did it start" family counts toward the stopping @@ -151,10 +154,15 @@ class ReminderWorker @AssistedInject constructor( } } - private fun openApp(action: String? = null): PendingIntent = + /** + * The tap target. A distinct request code per action, so the actions do not + * overwrite one another's PendingIntent (they differ only in an extra, and + * FLAG_UPDATE_CURRENT would otherwise make the last one win for all). + */ + private fun openApp(action: ReminderAction? = null): PendingIntent = PendingIntent.getActivity( applicationContext, - action?.hashCode() ?: 0, + action?.let { it.ordinal + 1 } ?: 0, PeriodNotifier.launchIntent(applicationContext, action), PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) diff --git a/docs/design/README.md b/docs/design/README.md index 88e0652..9dc54d7 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -153,6 +153,27 @@ have replaced it. ## The lock screen is the one semi-public surface +### A button says what it does, and it does what it says + +A notification button is chosen twice: once as words on a lock screen, and once +as a write into the health record. Those two halves used to be produced in +different files — `NotificationCopy` returned a list of labels and the worker +attached the first to "started" and the second to "not yet", by position. That +holds while every reminder asks the same question. It stopped holding the moment +one did not: *"Is your period over?"* offers **Ended** and **Still going**, and +by position "Ended" recorded a brand-new period starting today, in the middle of +the period it was asking about. + +So the label and the action are now one thing, produced together by +`NotificationCopy.buttons` and carried together to the tap. Nothing downstream +pairs them up again, because nothing downstream is allowed to. + +The privacy rule that already governed the labels still governs them: a button +is visible text on a lock screen, and it says nothing out of context (§31). The +new rule beside it is that a button which asks about an ending may not produce a +beginning — stated as a property, and tested as one, rather than left to whoever +next edits the list. + Everything else in this app is seen only by somebody who already has it open. The lock screen is different: it is what appears when the owner opens the app in front of somebody else, and what anybody who picks the phone up sees. Three