fix: apply a reminder's answer to the day it asked about
A notification waits in the shade until somebody deals with it. The handler used LocalDate.now(), so a reminder posted on Friday and tapped on Monday recorded Monday -- and a period start is the single input the whole prediction engine is built on. Being wrong by a weekend there is worse than never asking. The day the question was about now travels in the PendingIntent, written when the notification is built rather than read when it is tapped, and the parked action carries it through the app lock too. ReminderActionRules then decides whether the answer is still worth writing: nothing dated in the future, nothing older than a day, nothing already settled by a start she has logged since, and ENDED only where something is actually open to close. The bias is towards writing nothing -- a stale tap still opens the app, which is where she can see what is recorded and change it, and that beats a confident write against the wrong day. MainActivity consumes the extras after parking, and only parks when savedInstanceState is null. Android redelivers the original Intent after process death with its extras intact, so a restore would otherwise apply a days-old answer a second time; a rotation would too. The writes are idempotent today, which is the only reason that was survivable. Anything unrecognised -- including the action strings from before buttons carried their own meaning -- writes nothing. A notification sitting in a shade across an upgrade opens the app and records nothing, rather than guessing. Handler tests go from 8 to 12: the next-morning case, the days-late case, the already-answered-in-the-app case, and a legacy notification with no date at all. Also gives the app-lock test's await a diagnosis. It went red once in a full-module run and passed alone, and "timed out" said nothing about whether the write never happened, the callback never fired, or the state had simply not arrived. It reports busy, message and hasPin now. closes #69 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d31bfbd0e7
commit
cf09c7ab97
|
|
@ -13,6 +13,7 @@ import androidx.lifecycle.lifecycleScope
|
||||||
import androidx.lifecycle.repeatOnLifecycle
|
import androidx.lifecycle.repeatOnLifecycle
|
||||||
import dagger.hilt.android.AndroidEntryPoint
|
import dagger.hilt.android.AndroidEntryPoint
|
||||||
import dev.privacyllc.period.core.notifications.PeriodNotifier
|
import dev.privacyllc.period.core.notifications.PeriodNotifier
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
import dev.privacyllc.period.core.security.AppLockRepository
|
import dev.privacyllc.period.core.security.AppLockRepository
|
||||||
import dev.privacyllc.period.designsystem.PeriodTheme
|
import dev.privacyllc.period.designsystem.PeriodTheme
|
||||||
import dev.privacyllc.period.feature.export.ExportController
|
import dev.privacyllc.period.feature.export.ExportController
|
||||||
|
|
@ -77,7 +78,10 @@ class MainActivity : FragmentActivity() {
|
||||||
|
|
||||||
// Parked, never applied here. With a lock on, the write waits for the
|
// Parked, never applied here. With a lock on, the write waits for the
|
||||||
// unlock; the gate delivers it. See AppLockController.
|
// unlock; the gate delivers it. See AppLockController.
|
||||||
lockController.holdNotificationAction(intent?.getStringExtra(EXTRA_REMINDER_ACTION))
|
// Only on a genuine launch: after process death the original Intent is
|
||||||
|
// redelivered with its extras intact, and re-parking it would apply an
|
||||||
|
// answer the user gave days ago a second time.
|
||||||
|
if (savedInstanceState == null) parkActionFrom(intent)
|
||||||
|
|
||||||
lifecycleScope.launch {
|
lifecycleScope.launch {
|
||||||
repeatOnLifecycle(Lifecycle.State.STARTED) {
|
repeatOnLifecycle(Lifecycle.State.STARTED) {
|
||||||
|
|
@ -117,7 +121,8 @@ class MainActivity : FragmentActivity() {
|
||||||
override fun onNewIntent(intent: Intent) {
|
override fun onNewIntent(intent: Intent) {
|
||||||
super.onNewIntent(intent)
|
super.onNewIntent(intent)
|
||||||
setIntent(intent)
|
setIntent(intent)
|
||||||
lockController.holdNotificationAction(intent.getStringExtra(EXTRA_REMINDER_ACTION))
|
setIntent(intent)
|
||||||
|
parkActionFrom(intent)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -139,6 +144,20 @@ class MainActivity : FragmentActivity() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Take the answer out of the Intent and hold it for the gate.
|
||||||
|
*
|
||||||
|
* `removeExtra` matters: without it a rotation or a theme change re-runs
|
||||||
|
* `onCreate` on the same Intent and parks the same answer again. The writes
|
||||||
|
* are idempotent today, which is the only reason that was survivable.
|
||||||
|
*/
|
||||||
|
private fun parkActionFrom(intent: Intent?) {
|
||||||
|
val request = intent?.let(ReminderActionRequest::fromIntent) ?: return
|
||||||
|
intent.removeExtra(PeriodNotifier.EXTRA_REMINDER_ACTION)
|
||||||
|
intent.removeExtra(PeriodNotifier.EXTRA_REMINDER_DATE)
|
||||||
|
lockController.holdNotificationAction(request)
|
||||||
|
}
|
||||||
|
|
||||||
private companion object {
|
private companion object {
|
||||||
/**
|
/**
|
||||||
* The same key `core/notifications` writes, not a second copy of the
|
* The same key `core/notifications` writes, not a second copy of the
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@ package dev.privacyllc.period.lock
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
import javax.inject.Singleton
|
import javax.inject.Singleton
|
||||||
|
|
||||||
|
|
@ -42,13 +43,13 @@ class AppLockController @Inject constructor() {
|
||||||
@Volatile
|
@Volatile
|
||||||
var authInProgress: Boolean = false
|
var authInProgress: Boolean = false
|
||||||
|
|
||||||
private val _pendingNotificationAction = MutableStateFlow<String?>(null)
|
private val _pendingNotificationAction = MutableStateFlow<ReminderActionRequest?>(null)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Observed rather than read once, so an action arriving while the app is
|
* Observed rather than read once, so an action arriving while the app is
|
||||||
* already open — `onNewIntent`, not `onCreate` — is delivered too.
|
* already open — `onNewIntent`, not `onCreate` — is delivered too.
|
||||||
*/
|
*/
|
||||||
val pendingNotificationAction: StateFlow<String?> = _pendingNotificationAction.asStateFlow()
|
val pendingNotificationAction: StateFlow<ReminderActionRequest?> = _pendingNotificationAction.asStateFlow()
|
||||||
|
|
||||||
fun unlock() {
|
fun unlock() {
|
||||||
_unlocked.value = true
|
_unlocked.value = true
|
||||||
|
|
@ -67,11 +68,11 @@ class AppLockController @Inject constructor() {
|
||||||
* has to wait for the unlock, or the lock is decorative for the one action
|
* has to wait for the unlock, or the lock is decorative for the one action
|
||||||
* that modifies data.
|
* that modifies data.
|
||||||
*/
|
*/
|
||||||
fun holdNotificationAction(action: String?) {
|
fun holdNotificationAction(action: ReminderActionRequest?) {
|
||||||
if (action != null) _pendingNotificationAction.value = action
|
if (action != null) _pendingNotificationAction.value = action
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Returns the held action once, and forgets it. Null when there is none. */
|
/** Returns the held action once, and forgets it. Null when there is none. */
|
||||||
fun takeNotificationAction(): String? =
|
fun takeNotificationAction(): ReminderActionRequest? =
|
||||||
_pendingNotificationAction.value.also { _pendingNotificationAction.value = null }
|
_pendingNotificationAction.value.also { _pendingNotificationAction.value = null }
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
import dev.privacyllc.period.core.security.AppLockRepository
|
import dev.privacyllc.period.core.security.AppLockRepository
|
||||||
import dev.privacyllc.period.core.security.UnlockResult
|
import dev.privacyllc.period.core.security.UnlockResult
|
||||||
import dev.privacyllc.period.notifications.NotificationActionHandler
|
import dev.privacyllc.period.notifications.NotificationActionHandler
|
||||||
|
|
@ -58,7 +59,7 @@ class AppLockViewModel @Inject constructor(
|
||||||
) : ViewModel() {
|
) : ViewModel() {
|
||||||
|
|
||||||
/** Non-null while a notification action is waiting to be applied. */
|
/** Non-null while a notification action is waiting to be applied. */
|
||||||
val pendingNotificationAction: StateFlow<String?> = controller.pendingNotificationAction
|
val pendingNotificationAction: StateFlow<ReminderActionRequest?> = controller.pendingNotificationAction
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Apply a parked notification action, now that somebody has authenticated.
|
* Apply a parked notification action, now that somebody has authenticated.
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,8 @@ package dev.privacyllc.period.notifications
|
||||||
import dev.privacyllc.period.core.data.CycleRepository
|
import dev.privacyllc.period.core.data.CycleRepository
|
||||||
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
||||||
import dev.privacyllc.period.core.notifications.ReminderAction
|
import dev.privacyllc.period.core.notifications.ReminderAction
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRules
|
||||||
import dev.privacyllc.period.domain.cycle.PeriodRecordSource
|
import dev.privacyllc.period.domain.cycle.PeriodRecordSource
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import java.time.Clock
|
import java.time.Clock
|
||||||
|
|
@ -32,55 +34,60 @@ class NotificationActionHandler @Inject constructor(
|
||||||
private val clock: Clock,
|
private val clock: Clock,
|
||||||
) {
|
) {
|
||||||
|
|
||||||
/** Returns true when the action was recognised and applied. */
|
/**
|
||||||
suspend fun handle(action: String?): Boolean {
|
* Apply an answer to the day it was about.
|
||||||
|
*
|
||||||
|
* Returns true when something was recognised — including the cases that
|
||||||
|
* deliberately write nothing, because "understood, and there is nothing to
|
||||||
|
* record" is not a failure.
|
||||||
|
*/
|
||||||
|
suspend fun handle(request: ReminderActionRequest?): Boolean {
|
||||||
|
request ?: return false
|
||||||
|
|
||||||
val today = LocalDate.now(clock)
|
val today = LocalDate.now(clock)
|
||||||
// Anything this app did not write — including the action strings used
|
val periods = repository.confirmedPeriods.first()
|
||||||
// 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 -> {
|
|
||||||
// The question is answered. Nothing resets a counter here: the
|
|
||||||
// count is stored against the period it was asked about, so this
|
|
||||||
// new start makes the old count read as zero on its own.
|
|
||||||
repository.confirmPeriodStart(today, PeriodRecordSource.NOTIFICATION_CONFIRMATION)
|
|
||||||
true
|
|
||||||
}
|
|
||||||
|
|
||||||
ReminderAction.NOT_YET -> {
|
return when (val verdict = ReminderActionRules.verdict(request, today, periods)) {
|
||||||
// Exactly what the Today screen's "Not yet" does — same call,
|
// Too old, already answered, or nothing left to close. The tap has
|
||||||
// same censoring observation, same re-conditioned forecast.
|
// opened the app, which is where she can see what is recorded and
|
||||||
repository.recordNotYet(today)
|
// change it — better than a confident write against the wrong day.
|
||||||
true
|
ReminderActionRules.Verdict.Stale -> true
|
||||||
}
|
|
||||||
|
|
||||||
ReminderAction.ENDED -> {
|
is ReminderActionRules.Verdict.EndPeriod -> {
|
||||||
// Closes the period that is running. NEVER opens one: this
|
// Closes the period that is running. NEVER opens one: this
|
||||||
// button answers "is it over?", and the write that used to sit
|
// button answers "is it over?", and the write that used to sit
|
||||||
// here answered "did it start?" — inserting a fresh period in
|
// here answered "did it start?" — inserting a fresh period in
|
||||||
// the middle of the one it was asking about.
|
// the middle of the one it was asking about.
|
||||||
//
|
repository.setPeriodEnd(verdict.id, request.date!!)
|
||||||
// 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
|
true
|
||||||
}
|
}
|
||||||
|
|
||||||
ReminderAction.STILL_GOING -> {
|
ReminderActionRules.Verdict.Apply -> {
|
||||||
// Deliberately writes nothing. "Still going" is the state the
|
when (request.action) {
|
||||||
// record is already in, and the Today screen's equivalent
|
ReminderAction.STARTED ->
|
||||||
// (setPeriodEnd(id, null)) is a no-op on an open period — one
|
repository.confirmPeriodStart(
|
||||||
// that would still move `updatedAt` and read, in the history,
|
request.date!!,
|
||||||
// as an edit the user never made.
|
PeriodRecordSource.NOTIFICATION_CONFIRMATION,
|
||||||
|
)
|
||||||
|
|
||||||
|
// Exactly what the Today screen's "Not yet" does — same
|
||||||
|
// call, same censoring observation, same re-conditioned
|
||||||
|
// forecast — but dated to the day that was asked about.
|
||||||
|
ReminderAction.NOT_YET -> repository.recordNotYet(request.date!!)
|
||||||
|
|
||||||
|
// Deliberately writes nothing. "Still going" is the state
|
||||||
|
// the record is already in, and the in-app equivalent is a
|
||||||
|
// no-op that would still move `updatedAt` and read, in the
|
||||||
|
// history, as an edit she never made.
|
||||||
|
ReminderAction.STILL_GOING -> Unit
|
||||||
|
|
||||||
|
// Handled above, and unreachable: ENDED always resolves to
|
||||||
|
// EndPeriod or Stale.
|
||||||
|
ReminderAction.ENDED -> Unit
|
||||||
|
}
|
||||||
true
|
true
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
null -> false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,7 @@ import kotlinx.coroutines.runBlocking
|
||||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||||
import kotlinx.coroutines.test.resetMain
|
import kotlinx.coroutines.test.resetMain
|
||||||
import kotlinx.coroutines.test.setMain
|
import kotlinx.coroutines.test.setMain
|
||||||
|
import kotlinx.coroutines.TimeoutCancellationException
|
||||||
import kotlinx.coroutines.withTimeout
|
import kotlinx.coroutines.withTimeout
|
||||||
import org.junit.After
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
|
|
@ -118,8 +119,28 @@ class LockSettingsViewModelTest {
|
||||||
* and time out when the whole suite runs beside it. This budget is for
|
* and time out when the whole suite runs beside it. This budget is for
|
||||||
* catching a genuine hang, not for measuring the crypto.
|
* catching a genuine hang, not for measuring the crypto.
|
||||||
*/
|
*/
|
||||||
private fun await(predicate: suspend () -> Boolean) = runBlocking {
|
/**
|
||||||
withTimeout(30_000) { while (!predicate()) delay(10) }
|
* Generous, and it reports what it was waiting on when it gives up.
|
||||||
|
*
|
||||||
|
* Every PIN here costs a real PBKDF2 derivation at 210,000 iterations, and
|
||||||
|
* one test asks for three, so a budget tuned to a lone run turns red when
|
||||||
|
* the whole module runs beside it — a flaky guard is one people learn to
|
||||||
|
* ignore. The budget is for catching a hang, not for measuring the crypto.
|
||||||
|
*
|
||||||
|
* The message matters as much as the number: a bare "timed out" says
|
||||||
|
* nothing about whether the write never happened, the callback never fired,
|
||||||
|
* or the state simply had not arrived yet.
|
||||||
|
*/
|
||||||
|
private fun await(what: String = "a condition", predicate: suspend () -> Boolean) = runBlocking {
|
||||||
|
try {
|
||||||
|
withTimeout(60_000) { while (!predicate()) delay(10) }
|
||||||
|
} catch (timeout: TimeoutCancellationException) {
|
||||||
|
throw AssertionError(
|
||||||
|
"gave up waiting for $what — busy=${vm.state.value.busy}, " +
|
||||||
|
"message=${vm.state.value.message}, hasPin=${lock.hasPin.first()}",
|
||||||
|
timeout,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun pin(value: String) = value.toCharArray()
|
private fun pin(value: String) = value.toCharArray()
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,10 @@ import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertNull
|
import org.junit.Assert.assertNull
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderAction
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
|
import java.time.LocalDate
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The session flag, and the parked notification action.
|
* The session flag, and the parked notification action.
|
||||||
|
|
@ -16,6 +19,13 @@ import org.junit.Test
|
||||||
*/
|
*/
|
||||||
class AppLockControllerTest {
|
class AppLockControllerTest {
|
||||||
|
|
||||||
|
/** A parked answer now carries the day it was about, not just its name. */
|
||||||
|
private fun request(action: String) = ReminderActionRequest(
|
||||||
|
ReminderAction.valueOf(action),
|
||||||
|
LocalDate.of(2026, 8, 20),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@Test fun `a new process starts locked`() = runBlocking {
|
@Test fun `a new process starts locked`() = runBlocking {
|
||||||
assertFalse(
|
assertFalse(
|
||||||
"a cold start must never begin unlocked — that is the whole reason " +
|
"a cold start must never begin unlocked — that is the whole reason " +
|
||||||
|
|
@ -39,27 +49,27 @@ class AppLockControllerTest {
|
||||||
*/
|
*/
|
||||||
@Test fun `a notification action is held rather than applied`() = runBlocking {
|
@Test fun `a notification action is held rather than applied`() = runBlocking {
|
||||||
val controller = AppLockController()
|
val controller = AppLockController()
|
||||||
controller.holdNotificationAction("not_yet")
|
controller.holdNotificationAction(request("NOT_YET"))
|
||||||
|
|
||||||
assertEquals("not_yet", controller.pendingNotificationAction.first())
|
assertEquals(request("NOT_YET"), controller.pendingNotificationAction.first())
|
||||||
assertEquals("not_yet", controller.takeNotificationAction())
|
assertEquals(request("NOT_YET"), controller.takeNotificationAction())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test fun `taking the action consumes it, so it cannot be applied twice`() {
|
@Test fun `taking the action consumes it, so it cannot be applied twice`() {
|
||||||
val controller = AppLockController()
|
val controller = AppLockController()
|
||||||
controller.holdNotificationAction("not_yet")
|
controller.holdNotificationAction(request("NOT_YET"))
|
||||||
|
|
||||||
assertEquals("not_yet", controller.takeNotificationAction())
|
assertEquals(request("NOT_YET"), controller.takeNotificationAction())
|
||||||
assertNull("a second take must find nothing", controller.takeNotificationAction())
|
assertNull("a second take must find nothing", controller.takeNotificationAction())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test fun `holding null does not clear an action already waiting`() {
|
@Test fun `holding null does not clear an action already waiting`() {
|
||||||
val controller = AppLockController()
|
val controller = AppLockController()
|
||||||
controller.holdNotificationAction("started")
|
controller.holdNotificationAction(request("STARTED"))
|
||||||
// Every launch delivers an intent; most carry no action. That must not
|
// Every launch delivers an intent; most carry no action. That must not
|
||||||
// discard one that is genuinely waiting.
|
// discard one that is genuinely waiting.
|
||||||
controller.holdNotificationAction(null)
|
controller.holdNotificationAction(null)
|
||||||
assertEquals("started", controller.takeNotificationAction())
|
assertEquals(request("STARTED"), controller.takeNotificationAction())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test fun `the auth-in-progress flag defaults to false`() {
|
@Test fun `the auth-in-progress flag defaults to false`() {
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ import dev.privacyllc.period.core.data.PeriodWriteResult
|
||||||
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
||||||
import dev.privacyllc.period.core.notifications.NotificationCopy
|
import dev.privacyllc.period.core.notifications.NotificationCopy
|
||||||
import dev.privacyllc.period.core.notifications.ReminderAction
|
import dev.privacyllc.period.core.notifications.ReminderAction
|
||||||
|
import dev.privacyllc.period.core.notifications.ReminderActionRequest
|
||||||
import dev.privacyllc.period.core.notifications.ReminderKind
|
import dev.privacyllc.period.core.notifications.ReminderKind
|
||||||
import dev.privacyllc.period.core.datastore.NotificationPrivacy
|
import dev.privacyllc.period.core.datastore.NotificationPrivacy
|
||||||
import dev.privacyllc.period.domain.cycle.PeriodRecordSource
|
import dev.privacyllc.period.domain.cycle.PeriodRecordSource
|
||||||
|
|
@ -101,7 +102,7 @@ class NotificationActionHandlerTest {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `Started records a period, sourced from the notification`() = runTest {
|
fun `Started records a period, sourced from the notification`() = runTest {
|
||||||
assertTrue(handler.handle(ReminderAction.STARTED.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STARTED, today)))
|
||||||
|
|
||||||
val period = repo.confirmedPeriods.first().single()
|
val period = repo.confirmedPeriods.first().single()
|
||||||
assertEquals(today, period.startDate)
|
assertEquals(today, period.startDate)
|
||||||
|
|
@ -111,7 +112,7 @@ class NotificationActionHandlerTest {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `Not yet records a censoring observation and no period`() = runTest {
|
fun `Not yet records a censoring observation and no period`() = runTest {
|
||||||
assertTrue(handler.handle(ReminderAction.NOT_YET.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.NOT_YET, today)))
|
||||||
|
|
||||||
assertEquals(today, repo.notYetObservations.first().single().date)
|
assertEquals(today, repo.notYetObservations.first().single().date)
|
||||||
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
||||||
|
|
@ -120,7 +121,7 @@ class NotificationActionHandlerTest {
|
||||||
@Test
|
@Test
|
||||||
fun `Ended closes the period that is running rather than starting another`() = runTest {
|
fun `Ended closes the period that is running rather than starting another`() = runTest {
|
||||||
repo.confirmPeriodStart(LocalDate.of(2026, 8, 16))
|
repo.confirmPeriodStart(LocalDate.of(2026, 8, 16))
|
||||||
assertTrue(handler.handle(ReminderAction.ENDED.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.ENDED, today)))
|
||||||
|
|
||||||
// Regression: this used to insert a SECOND period starting today, in the
|
// Regression: this used to insert a SECOND period starting today, in the
|
||||||
// middle of the one the notification was asking about.
|
// middle of the one the notification was asking about.
|
||||||
|
|
@ -131,7 +132,7 @@ class NotificationActionHandlerTest {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `Ended with nothing running writes nothing`() = runTest {
|
fun `Ended with nothing running writes nothing`() = runTest {
|
||||||
assertTrue(handler.handle(ReminderAction.ENDED.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.ENDED, today)))
|
||||||
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -140,7 +141,7 @@ class NotificationActionHandlerTest {
|
||||||
val id = (repo.confirmPeriodStart(LocalDate.of(2026, 8, 16)) as PeriodWriteResult.Added).id
|
val id = (repo.confirmPeriodStart(LocalDate.of(2026, 8, 16)) as PeriodWriteResult.Added).id
|
||||||
repo.setPeriodEnd(id, LocalDate.of(2026, 8, 19))
|
repo.setPeriodEnd(id, LocalDate.of(2026, 8, 19))
|
||||||
|
|
||||||
assertTrue(handler.handle(ReminderAction.ENDED.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.ENDED, today)))
|
||||||
|
|
||||||
// She answered in the app first. The honest response to an answered
|
// She answered in the app first. The honest response to an answered
|
||||||
// question is silence, not a correction.
|
// question is silence, not a correction.
|
||||||
|
|
@ -152,7 +153,7 @@ class NotificationActionHandlerTest {
|
||||||
repo.confirmPeriodStart(LocalDate.of(2026, 8, 16))
|
repo.confirmPeriodStart(LocalDate.of(2026, 8, 16))
|
||||||
val before = repo.confirmedPeriods.first().single()
|
val before = repo.confirmedPeriods.first().single()
|
||||||
|
|
||||||
assertTrue(handler.handle(ReminderAction.STILL_GOING.name))
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STILL_GOING, today)))
|
||||||
|
|
||||||
val after = repo.confirmedPeriods.first().single()
|
val after = repo.confirmedPeriods.first().single()
|
||||||
assertEquals(before, after)
|
assertEquals(before, after)
|
||||||
|
|
@ -166,12 +167,58 @@ class NotificationActionHandlerTest {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `an action this app did not write is not guessed at`() = runTest {
|
fun `an action this app did not write is not guessed at`() = runTest {
|
||||||
// Including the strings used before the buttons carried their own
|
// Nothing to apply at all.
|
||||||
// meaning — a notification sitting in the shade across an upgrade.
|
assertFalse(handler.handle(null))
|
||||||
listOf(null, "", "dev.privacyllc.period.action.STARTED", "started", "ENDED_MAYBE").forEach {
|
|
||||||
assertFalse("$it was treated as an action", handler.handle(it))
|
// And a notification posted before the day travelled with the answer —
|
||||||
}
|
// one sitting in somebody's shade across an upgrade. It opens the app
|
||||||
|
// and writes nothing, because there is no day it could honestly use.
|
||||||
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STARTED, date = null)))
|
||||||
|
|
||||||
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
||||||
assertTrue(repo.notYetObservations.first().isEmpty())
|
assertTrue(repo.notYetObservations.first().isEmpty())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
// The day it was asking about
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an answer given the next morning is recorded against the day it was asked`() = runTest {
|
||||||
|
// Posted yesterday evening, tapped this morning. The handler used
|
||||||
|
// LocalDate.now(), so this recorded today — and a start date is the one
|
||||||
|
// input the whole prediction engine is built on.
|
||||||
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STARTED, today.minusDays(1))))
|
||||||
|
|
||||||
|
assertEquals(today.minusDays(1), repo.confirmedPeriods.first().single().startDate)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an answer left for days writes nothing`() = runTest {
|
||||||
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STARTED, today.minusDays(5))))
|
||||||
|
|
||||||
|
// She has lived days the app knows nothing about. Opening the app, where
|
||||||
|
// she can see and fix what is recorded, beats a confident wrong write.
|
||||||
|
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a question already answered in the app is not answered twice`() = runTest {
|
||||||
|
repo.confirmPeriodStart(today)
|
||||||
|
|
||||||
|
// The notification from yesterday is still in the shade.
|
||||||
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.NOT_YET, today.minusDays(1))))
|
||||||
|
|
||||||
|
// A "not yet" for a day before a start she has since logged would
|
||||||
|
// censor a forecast that has already arrived.
|
||||||
|
assertTrue(repo.notYetObservations.first().isEmpty())
|
||||||
|
assertEquals(1, repo.confirmedPeriods.first().size)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an answer dated in the future writes nothing`() = runTest {
|
||||||
|
assertTrue(handler.handle(ReminderActionRequest(ReminderAction.STARTED, today.plusDays(1))))
|
||||||
|
|
||||||
|
assertTrue(repo.confirmedPeriods.first().isEmpty())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -192,12 +192,24 @@ class PeriodNotifier(private val context: Context) : ReminderNotifier {
|
||||||
*/
|
*/
|
||||||
const val EXTRA_REMINDER_ACTION = "reminder_action"
|
const val EXTRA_REMINDER_ACTION = "reminder_action"
|
||||||
|
|
||||||
fun launchIntent(context: Context, action: ReminderAction? = null): Intent =
|
/**
|
||||||
|
* The day the reminder was asking about, as an epoch day.
|
||||||
|
*
|
||||||
|
* Not the day it is tapped. A notification waits in the shade, and the
|
||||||
|
* handler used to write against "now" — so an answer given on Monday to
|
||||||
|
* a question asked on Friday recorded Monday.
|
||||||
|
*/
|
||||||
|
const val EXTRA_REMINDER_DATE = "reminder_date"
|
||||||
|
|
||||||
|
fun launchIntent(
|
||||||
|
context: Context,
|
||||||
|
request: ReminderActionRequest? = null,
|
||||||
|
): Intent =
|
||||||
Intent().apply {
|
Intent().apply {
|
||||||
setClassName(context, "dev.privacyllc.period.MainActivity")
|
setClassName(context, "dev.privacyllc.period.MainActivity")
|
||||||
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP
|
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP
|
||||||
putExtra(EXTRA_FROM_NOTIFICATION, true)
|
putExtra(EXTRA_FROM_NOTIFICATION, true)
|
||||||
action?.let { putExtra(EXTRA_REMINDER_ACTION, it.name) }
|
request?.writeTo(this)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,56 @@
|
||||||
|
package dev.privacyllc.period.core.notifications
|
||||||
|
|
||||||
|
import android.content.Intent
|
||||||
|
import java.time.LocalDate
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A tapped notification button, and the day it was asking about.
|
||||||
|
*
|
||||||
|
* ## Why the date travels with the action
|
||||||
|
*
|
||||||
|
* A notification sits in the shade until it is dealt with. The handler used
|
||||||
|
* `LocalDate.now()`, so a reminder posted on Friday and tapped on Monday
|
||||||
|
* recorded Monday — and a period start is the single input the whole prediction
|
||||||
|
* engine is built on. Being wrong by a weekend there is worse than not asking.
|
||||||
|
*
|
||||||
|
* The date is the day the *question* was about, not the day the answer was
|
||||||
|
* given, which is why it is written when the notification is built rather than
|
||||||
|
* read when it is tapped.
|
||||||
|
*
|
||||||
|
* ## Unknown is not a guess
|
||||||
|
*
|
||||||
|
* [fromIntent] returns null for anything it does not recognise, including the
|
||||||
|
* action strings this app used before buttons carried their own meaning. A
|
||||||
|
* notification already in somebody's shade across an upgrade still opens the
|
||||||
|
* app; it simply does not write anything, because there is nothing it can know
|
||||||
|
* for certain about what she meant.
|
||||||
|
*/
|
||||||
|
data class ReminderActionRequest(
|
||||||
|
val action: ReminderAction,
|
||||||
|
/** Null for a notification posted before the date travelled — see the KDoc. */
|
||||||
|
val date: LocalDate?,
|
||||||
|
) {
|
||||||
|
fun writeTo(intent: Intent) {
|
||||||
|
intent.putExtra(PeriodNotifier.EXTRA_REMINDER_ACTION, action.name)
|
||||||
|
date?.let { intent.putExtra(PeriodNotifier.EXTRA_REMINDER_DATE, it.toEpochDay()) }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Dateless: it records which day was asked about, and that is a cycle date. */
|
||||||
|
override fun toString(): String = "ReminderActionRequest(action=$action)"
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
fun fromIntent(intent: Intent): ReminderActionRequest? {
|
||||||
|
val action = ReminderAction.fromWireName(
|
||||||
|
intent.getStringExtra(PeriodNotifier.EXTRA_REMINDER_ACTION),
|
||||||
|
) ?: return null
|
||||||
|
val epochDay = intent.getLongExtra(PeriodNotifier.EXTRA_REMINDER_DATE, NO_DATE)
|
||||||
|
return ReminderActionRequest(
|
||||||
|
action = action,
|
||||||
|
date = if (epochDay == NO_DATE) null else LocalDate.ofEpochDay(epochDay),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Not a plausible epoch day, and distinct from 1970-01-01 (which is 0). */
|
||||||
|
private const val NO_DATE = Long.MIN_VALUE
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,72 @@
|
||||||
|
package dev.privacyllc.period.core.notifications
|
||||||
|
|
||||||
|
import dev.privacyllc.period.domain.cycle.PeriodRecord
|
||||||
|
import java.time.LocalDate
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether an answer is still worth writing down.
|
||||||
|
*
|
||||||
|
* Pure, and separate from the handler, because the interesting decisions here
|
||||||
|
* are all about time and none of them need a database: a notification can be
|
||||||
|
* answered days late, answered twice, or answered after the user has already
|
||||||
|
* said the same thing in the app.
|
||||||
|
*
|
||||||
|
* The bias is towards writing nothing. A stale answer still opens the app —
|
||||||
|
* which is where she can see what is actually recorded and fix it — and that is
|
||||||
|
* a better outcome than a confident write against the wrong day.
|
||||||
|
*/
|
||||||
|
object ReminderActionRules {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How old an answer may be and still count.
|
||||||
|
*
|
||||||
|
* One day: a reminder asked in the evening and answered the next morning is
|
||||||
|
* the ordinary case, and the ordinary case has to work. Beyond that she has
|
||||||
|
* lived a day the app knows nothing about, and the app should not pretend
|
||||||
|
* otherwise on the strength of a tap.
|
||||||
|
*/
|
||||||
|
const val MAX_ACTION_AGE_DAYS = 1L
|
||||||
|
|
||||||
|
sealed interface Verdict {
|
||||||
|
/** Write nothing; the tap has already opened the app, which is enough. */
|
||||||
|
data object Stale : Verdict
|
||||||
|
|
||||||
|
/** Apply the action to [ReminderActionRequest.date]. */
|
||||||
|
data object Apply : Verdict
|
||||||
|
|
||||||
|
/** Close this period, rather than opening one. */
|
||||||
|
data class EndPeriod(val id: Long) : Verdict
|
||||||
|
}
|
||||||
|
|
||||||
|
fun verdict(
|
||||||
|
request: ReminderActionRequest,
|
||||||
|
today: LocalDate,
|
||||||
|
periods: List<PeriodRecord>,
|
||||||
|
): Verdict {
|
||||||
|
val date = request.date ?: return Verdict.Stale
|
||||||
|
if (date.isAfter(today)) return Verdict.Stale
|
||||||
|
if (today.toEpochDay() - date.toEpochDay() > MAX_ACTION_AGE_DAYS) return Verdict.Stale
|
||||||
|
|
||||||
|
val latest = periods.filter { it.isConfirmed }.maxByOrNull { it.startDate }
|
||||||
|
|
||||||
|
return when (request.action) {
|
||||||
|
// Already answered. She logged a start on or after the day the
|
||||||
|
// notification was asking about, so the question is settled and a
|
||||||
|
// second write would either duplicate it or contradict it.
|
||||||
|
ReminderAction.STARTED, ReminderAction.NOT_YET ->
|
||||||
|
if (latest != null && !latest.startDate.isBefore(date)) Verdict.Stale else Verdict.Apply
|
||||||
|
|
||||||
|
// Ending needs something open that began on or before the day in
|
||||||
|
// question. Nothing open is not a failure: she may have ended it in
|
||||||
|
// the app between the notification being posted and being tapped.
|
||||||
|
ReminderAction.ENDED -> {
|
||||||
|
val open = periods.filter { it.isConfirmed }
|
||||||
|
.lastOrNull { it.endDate == null && !it.startDate.isAfter(date) }
|
||||||
|
open?.let { Verdict.EndPeriod(it.id) } ?: Verdict.Stale
|
||||||
|
}
|
||||||
|
|
||||||
|
// Answered, and deliberately writes nothing either way.
|
||||||
|
ReminderAction.STILL_GOING -> Verdict.Apply
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -140,7 +140,9 @@ class ReminderWorker @AssistedInject constructor(
|
||||||
NotificationCompat.Action.Builder(
|
NotificationCompat.Action.Builder(
|
||||||
0,
|
0,
|
||||||
button.label,
|
button.label,
|
||||||
openApp(button.action),
|
// The day the question is about travels with the
|
||||||
|
// answer, so a tap tomorrow still means today.
|
||||||
|
openApp(ReminderActionRequest(button.action, today)),
|
||||||
).build()
|
).build()
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
@ -169,11 +171,11 @@ class ReminderWorker @AssistedInject constructor(
|
||||||
* overwrite one another's PendingIntent (they differ only in an extra, and
|
* overwrite one another's PendingIntent (they differ only in an extra, and
|
||||||
* FLAG_UPDATE_CURRENT would otherwise make the last one win for all).
|
* FLAG_UPDATE_CURRENT would otherwise make the last one win for all).
|
||||||
*/
|
*/
|
||||||
private fun openApp(action: ReminderAction? = null): PendingIntent =
|
private fun openApp(request: ReminderActionRequest? = null): PendingIntent =
|
||||||
PendingIntent.getActivity(
|
PendingIntent.getActivity(
|
||||||
applicationContext,
|
applicationContext,
|
||||||
action?.let { it.ordinal + 1 } ?: 0,
|
request?.let { it.action.ordinal + 1 } ?: 0,
|
||||||
PeriodNotifier.launchIntent(applicationContext, action),
|
PeriodNotifier.launchIntent(applicationContext, request),
|
||||||
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,
|
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -157,6 +157,32 @@ health app a crash mid-write is adjacent to losing what the user just entered,
|
||||||
and a message they can read beats a process that vanished. The message carries
|
and a message they can read beats a process that vanished. The message carries
|
||||||
the exception *type* and never a record's contents — §45.
|
the exception *type* and never a record's contents — §45.
|
||||||
|
|
||||||
|
### An answer belongs to the day it was asked about
|
||||||
|
|
||||||
|
A notification waits in the shade until somebody deals with it. The handler used
|
||||||
|
`LocalDate.now()`, so a reminder posted on Friday and tapped on Monday recorded
|
||||||
|
Monday — and a period start is the single input the whole prediction engine is
|
||||||
|
built on. Being wrong by a weekend there is worse than never asking.
|
||||||
|
|
||||||
|
The day the question was about now travels in the PendingIntent, written when the
|
||||||
|
notification is built rather than read when it is tapped, and the parked
|
||||||
|
notification action carries it too. `ReminderActionRules` then decides whether the
|
||||||
|
answer is still worth writing: nothing dated in the future, nothing older than a
|
||||||
|
day, nothing already settled by a start she has logged since, and `ENDED` only
|
||||||
|
where something is actually open to close.
|
||||||
|
|
||||||
|
The bias is deliberately towards writing nothing. A stale tap still opens the
|
||||||
|
app, which is where she can see what is recorded and change it — a better outcome
|
||||||
|
than a confident write against the wrong day. Anything unrecognised, including
|
||||||
|
the action strings used before buttons carried their own meaning, is treated the
|
||||||
|
same way: a notification sitting in a shade across an upgrade opens the app and
|
||||||
|
records nothing.
|
||||||
|
|
||||||
|
**The Intent's extras are consumed once.** `MainActivity` removes them after
|
||||||
|
parking, and only parks at all when `savedInstanceState` is null: after process
|
||||||
|
death Android redelivers the original Intent with its extras intact, so a
|
||||||
|
rotation or a restore would otherwise apply a days-old answer a second time.
|
||||||
|
|
||||||
### Scheduling a reminder is not the same as saying when
|
### Scheduling a reminder is not the same as saying when
|
||||||
|
|
||||||
`ExistingPeriodicWorkPolicy.UPDATE` carries the previous request's
|
`ExistingPeriodicWorkPolicy.UPDATE` carries the previous request's
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue