Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion LESSONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ An app that promises "export all your data" and "delete all your data" has hidde
`TrackingCategory.isNumeric` was `categoryType != "default"`, which was correct while every non-default type happened to store numbers. Adding the label-valued "yes_no" and "time" types would have silently routed "Yes"/"HH:mm" strings into numeric chart math (`toFloatOrNull()` returning null everywhere) with no compile error, because a negated predicate auto-includes every future variant. When a derived property gates behaviour, define membership positively (enumerate the types that ARE numeric); then a new variant defaults to the safe side and the property's KDoc records why. Grep for `!=` against discriminator fields whenever adding a variant to a string-keyed or enum type.

**A batch save surface must re-derive per-entry rules from the single-entry screen it replaces — "block the save" becomes "skip the entry", and "untouched" must be distinguished from "empty"**
A screen that saves one entry can block its Save button on invalid input (empty numeric field, zero count). A unified surface that saves many entries at once cannot block the whole save on one bad entry — each single-entry blocking rule must be translated to "skip this entry, leave any stored log untouched". The batch surface also introduces a state the single screen never had: an entry the user never interacted with. Saving those with their displayed defaults fabricates logs for every category on every save; track a per-entry `touched` flag and only persist entries that are touched or already stored. Exception: preserve any existing always-save semantics verbatim (GoFlo's pinned-category period fan-out deliberately saves untouched pinned entries), or the two surfaces silently produce different data for the same user action.
A screen that saves one entry can block its Save button on invalid input (empty numeric field, zero count). A unified surface that saves many entries at once cannot block the whole save on one bad entry — each single-entry blocking rule must be translated to "skip this entry, leave any stored log untouched". The batch surface also introduces a state the single screen never had: an entry the user never interacted with. Saving those with their displayed defaults fabricates logs for every category on every save; track a per-entry `touched` flag and only persist entries that are touched or already stored. Be suspicious of any inherited always-save rule when porting: GoFlo carried the period screen's pinned-category fan-out (save every pinned category on every period save, untouched or not) into the unified screen for parity, and the result was logs the user could not delete, because "Delete entry" then "Save" wrote the default straight back. A rule that fabricates a value the user never entered is a bug the moment the same screen also offers delete.

**Parallel write paths must each respect every category setting**
When two code paths write to the same store (e.g. `LogPeriodViewModel.syncSymptomsToTrackingLog` and `LogCategoryViewModel.save` both writing to `tracking_logs`), each path must independently read and apply every relevant category flag. If a new flag is added (like `trackAgainstTime`) and only one path is updated, the other silently ignores the setting. When adding a per-category behaviour flag, grep for all call sites of the underlying `saveLog` / `updateLogInPlace` and confirm they all handle the new flag.
Expand Down Expand Up @@ -112,6 +112,9 @@ When a form has section labels ("Flow", "Symptoms") and entered values ("Medium"
**Room migrations can be tested on the JVM: real SQLite via sqlite-jdbc + a reflection proxy for `SupportSQLiteDatabase`**
Room's `MigrationTestHelper` needs instrumented tests *and* exported schema JSON (`exportSchema = true`); a project with neither can still test migrations properly. Build the pre-migration schema by hand in an in-memory database (`org.xerial:sqlite-jdbc`, test-only dependency), seed representative data, then run the actual `Migration` object through a `java.lang.reflect.Proxy` implementing `SupportSQLiteDatabase` that routes `execSQL` to JDBC and throws for anything else — migrations that only `execSQL` need nothing more, and the proxy compiles regardless of the interface's exact member list (hand-implementing the ~35-member interface risks a CI-only compile break). Assert the post-migration schema with `PRAGMA table_info` against the exact shape Room generates for the entity — including `DEFAULT` clauses, which must match the entity's `@ColumnInfo(defaultValue=…)` annotations or Room throws `IllegalStateException` at first open on device. This exercises the real migration SQL on a real SQLite engine in a plain unit test.

**A save that composes "absorb, then edit boundaries" must feed the edit the post-absorb boundary, not the one loaded before**
The day screen saved a period day as `logPeriodDay(day)` (which re-derives episodes, so a day just before an episode moves its start back) followed by `updateEpisode(id, start = loadedStart, …)` (which trims day rows outside `start..end`). For any day before the loaded start, the second call silently deleted the row the first had just added: the UI said "continues the period", the save reported success, and nothing changed. When one step can move a boundary and a later step re-asserts a boundary captured before that step, derive the re-asserted value from both (`minOf(loadedStart, day)`) and show that derived boundary in the UI before saving, so the screen never promises a shape the save will undo.

**Gate prediction display on window end, not window start**
A prediction window (e.g. a 5-day expected period) should remain visible as long as any part of the window is current — gate on `windowEnd >= today`, not `windowStart >= today`. Gating on the start collapses the display to zero the moment the window begins, which is precisely when it matters most. Apply the same principle to any "active range" feature: fertility windows, ovulation windows, reminders that span multiple days.

Expand Down
Loading
Loading