Skip to content

Commit 41871a5

Browse files
feat(my-festival): migrate to LogEntry check-in model (schema v2) (#463)
* feat(my-festival): migrate to LogEntry check-in model (schema v2) Make the check-in the primary My Festival entity (ADR 0006 Phases 1-2). Storage moves from per-drink UserDrinkState blobs to per-entry LogEntry records plus a per-festival want-to-try set; UserDrinkState becomes a derived per-drink view so no UI consumer changes in this phase. - New LogEntry value object (stable UUID id, when, optional drinkId/title, note, photoIds, rating?, wouldRecommend?). A tasting is drinkId != null; the kind is derived, not stored. Edit/delete key off id. - UserDataStore v2: entry API (readEntries/writeEntry/removeEntry) plus a per-drink want-to-try flag. read/readAll now derive the aggregate (rating/notes/photos = most recent tasting; pours = tasting count) via a single-pass drinkId -> entries index. - One-time, flag-gated, idempotent, crash-safe v1 -> v2 migration (migrateToLogEntries): want-to-try -> plan set; each tasting timestamp -> a LogEntry (when preserved to ms); drink-level rating/notes/photos attach to the most recent tasting, or synthesise one at updatedAt if the drink was rated but never tasted. Deterministic v5 ids make re-runs overwrite rather than duplicate; blobs are deleted only after their entries are written. Rollback fail-safe: a newer-than-build payload is quarantined, never crashes. - Repository mutators re-routed to the entry API, still returning the derived UserDrinkState (#410/#447 shape). Adds a uuid dependency. Golden-tested migration (synthesis, ms carry, idempotency, partial-run recovery, quarantine). Existing model/provider/screen tests stay green. Relates to #315. Fixes #461. * refactor(my-festival): keep drink rating/notes independent of tastings Address review: a rating is personal tracking, not a claim that a drink was drunk — so rating (and notes) must be settable without marking a drink tasted, and clearing the tasting log must never wipe a rating. Store drink-level rating/notes/photos in a per-drink detail record (drink_detail_{festivalId}_{drinkId}) separate from the LogEntry tasting timeline. UserDrinkState derives rating/notes/photos from that record and pours from the tasting entries. This restores exact v1 behaviour: - Rating or noting an untasted drink no longer synthesises a tasting; the drink stays not-tasted. - Toggling tasted off (or deleting all tastings) preserves the rating. - Clearing rating+notes prunes the record to empty, as before. Migration is now lossless and behaviour-preserving: rating/notes/photos move to the detail record, tasting timestamps become bare pour entries, and a rated-but-never-tasted drink keeps isTasted == false. Diverges from ADR 0006's "rating = most recent tasting" derivation per maintainer call; ADR to be updated. Relates to #315. Fixes #461. * docs(adr): amend 0006 — rating/notes are drink-level, not per-tasting Record the sub-decision reversal from #461/#463: a drink's rating and notes are drink-level and independent of the tasting timeline, so a drink can be rated without being marked tasted and clearing tastings never wipes a rating. Supersedes ADR 0006's "rating = most recent tasting" derivation and the rated-but-never-tasted synthesis; the check-in-as-primary-entity decision itself stands. Adds an Amendments section and updates the index. Relates to #461. * fix(my-festival): don't prune want-to-try state on removeTasting Address review (PR #463): removeTasting returned null when a drink had no tastings, even if it was want-to-try or rated — causing the provider to prune still-valid in-memory state. Return the derived state instead, which is null only when the drink has no signal at all. Also harden LogEntry.copyWith to not accept an id: identity is assigned once and storage keys off it, so edits must reuse the same id. Relates to #461. * docs(my-festival): clarify LogEntry doc after rating decoupling Address review (PR #463): the class doc implied a tasting carries the UI-facing rating/notes. After ADR 0006's amendment those are drink-level (a separate detail record); the entry's per-pour rating/note/photo fields are reserved for the #415/#417 capture flow and not populated yet. Relates to #461. * test(my-festival): cover LogEntry.toString and legacy merge-onto-blob Close the patch-coverage gaps on new code: a LogEntry.toString assertion, and a migrateLegacyData test that folds a legacy favourite onto a pre-existing v1 blob and asserts neither the blob's rating nor the folded want-to-try is lost (re-adds coverage for the merge safety path). Relates to #461. * docs(adr): tighten 0006 amendment banner Address review (PR #463): the banner pointed at the already-corrected "Decided" list and lumped wouldRecommend into the change. Point it at the sections that remain superseded (Decision bullet 4, Consequences, Open Question 1) and clarify the amendment covers rating/notes only — wouldRecommend stays a reserved per-pour field. Relates to #461. * refactor(my-festival): drop dead empty-prune branch in _writeV1Blob The private migration helper is only ever called with a non-empty merged state, so the empty-prune guard was unreachable. Removing it simplifies the helper and closes the last patch-coverage gap. Relates to #461. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 98c6178 commit 41871a5

14 files changed

Lines changed: 1481 additions & 413 deletions

docs/adr/0006-check-in-as-primary-my-festival-entity.md

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
11
# ADR 0006: The Check-in as the Primary My Festival Entity
22

3-
**Status**: Accepted
3+
**Status**: Accepted (amended 2026-07-05 — see [Amendments](#amendments))
44

55
**Date**: 2026-07-04
66

77
**Deciders**: Maintainer (richardthe3rd)
88

9+
> **Amendment 2026-07-05 (implemented in #461/#463):** one sub-decision below
10+
> was reversed during implementation. A drink's **rating and notes are
11+
> drink-level and independent of the tasting timeline** — a user can rate or
12+
> note a drink *without* recording that they drank it, and clearing the tasting
13+
> log never wipes a rating. They are **not** derived from "the most recent
14+
> tasting." This supersedes the "Drink-level values are derived, not stored"
15+
> text in [Decision](#decision) bullet 4, the migration's rated-but-never-tasted
16+
> synthesis in [Consequences](#consequences), and Open Question 1 — read the
17+
> [Amendments](#amendments) section for the current model. (This change is about
18+
> **rating and notes** only; `wouldRecommend` remains a reserved **per-pour**
19+
> field, unaffected.) The rest of the ADR — the check-in as the primary diary
20+
> entity, the per-entry timeline, `wantToTry` as the plan axis — stands.
21+
922
**Context**: "My Festival" has two jobs for a user: it is a **plan** (a
1023
forward-looking wishlist of drinks they intend to try) *and* a **diary** (a
1124
backward-looking record of their festival). The current model is
@@ -259,15 +272,62 @@ migration **first and alone**:
259272
## Open Questions
260273

261274
1. **Migration of a rating with no tasting**: synthesise a tasting entry, or keep
262-
a per-drink "overall"? (Recommended: synthesise, to keep one model.)
275+
a per-drink "overall"? — **Resolved** by the 2026-07-05 amendment: keep a
276+
per-drink detail record; do **not** synthesise. See [Amendments](#amendments).
263277
2. **When, if ever, does the wire contract go per-entry?** (Deferred to a future
264278
proto-first ADR; not required for the local diary.)
265279

266280
_Decided:_
267281
- _Kind is **derived** from `drinkId` (present = tasting, absent = freeform
268-
`other`); no stored `kind` field. Two kinds only; rating/recommend on tastings._
269-
- _A drink's displayed/synced rating & recommend = its **most recent tasting's**
270-
value (shown as "your latest"); pours = tasting count._
271-
- _Storage is **per-entry keyed records** + a per-drink `wantToTry` flag; no
272-
entry pruning (explicit delete only)._
282+
`other`); no stored `kind` field. Two kinds only._
283+
- _A drink's rating & notes are **drink-level and independent of tastings**
284+
(amended 2026-07-05 — superseded "most recent tasting's value"); pours =
285+
tasting count. See [Amendments](#amendments)._
286+
- _Storage is **per-entry keyed records** for the timeline, a per-drink
287+
`wantToTry` flag, and a per-drink **detail record** for rating/notes/photos;
288+
no entry pruning (explicit delete only)._
273289
- _Rollout is **migration-first**, one consumer per PR (see Phasing)._
290+
291+
---
292+
293+
## Amendments
294+
295+
### 2026-07-05 — rating & notes are drink-level, independent of tastings
296+
297+
**Implemented in:** #461 / PR #463.
298+
299+
**What changed:** the ADR originally decided that a drink's rating/recommend/notes
300+
**derive from its most recent tasting**, with a rated-but-never-tasted drink
301+
**synthesising** a tasting on migration. Implementation reversed this.
302+
303+
**Why:** rating a drink is *personal tracking, not a claim that the drink was
304+
drunk*. A user must be able to rate or note a drink they have only looked at, and
305+
untoggling "Tasted" (or deleting every tasting) must never wipe a rating. Deriving
306+
rating from a tasting forced a rating to fabricate a tasting — changing `isTasted`
307+
as a side effect and making a note-then-clear leave a phantom pour. That
308+
contradicted the phase's own "interface-preserving, no behaviour change" goal.
309+
310+
**The model now:** three orthogonal per-drink axes.
311+
312+
| Axis | What | Storage |
313+
|---|---|---|
314+
| **Plan** | want-to-try | per-festival `want_to_try_{festivalId}` set |
315+
| **Diary** | timeline of tasting check-ins (pours) | per-entry `log_entry_{festivalId}_{id}` |
316+
| **Detail** | drink-level rating / notes / photos | per-drink `drink_detail_{festivalId}_{drinkId}`, pruned when empty |
317+
318+
`UserDrinkState` derives rating/notes/photos from the **detail** record and pours
319+
from the tasting entries. `isTasted` depends only on tastings, so a rated,
320+
never-tasted drink has `isTasted == false`.
321+
322+
**Migration consequence:** the v1→v2 fold is now lossless and behaviour-preserving
323+
`rating`/`notes`/`photoIds` move to the detail record, tasting timestamps become
324+
**bare pour entries**, and a rated-but-never-tasted drink keeps `isTasted == false`
325+
(no synthesis).
326+
327+
**Still per-pour (unchanged):** `LogEntry` retains optional `rating` /
328+
`wouldRecommend` / `note` fields for **future per-pour capture** (#415/#417). This
329+
amendment only governs the *drink-level* value the current UI reads; if and when a
330+
per-pour vs drink-level rating both exist, reconciling them is a later decision.
331+
332+
**Wire contract (unchanged):** sync still carries the single per-drink aggregate;
333+
the detail record is that drink-level value's home.

docs/adr/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Each ADR follows this structure:
3232
| [0003](0003-parallel-build-strategy.md) | Parallel Build Strategy for Android Releases | Accepted | 2025-12-27 |
3333
| [0004](0004-path-based-url-strategy.md) | Path-Based URL Strategy for Deep Linking | Accepted | 2025-12-21 |
3434
| [0005](0005-e2e-testing-strategy.md) | E2E Testing Strategy (Playwright for URL Smoke Tests) | Accepted | 2025-12-21 |
35-
| [0006](0006-check-in-as-primary-my-festival-entity.md) | The Check-in as the Primary My Festival Entity | Accepted | 2026-07-04 |
35+
| [0006](0006-check-in-as-primary-my-festival-entity.md) | The Check-in as the Primary My Festival Entity | Accepted (amended 2026-07-05) | 2026-07-04 |
3636

3737
## Creating a New ADR
3838

lib/constants/preference_keys.dart

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,36 @@ class PreferenceKeys {
2929

3030
// --- UserDataStore ---
3131

32-
/// Prefix for the unified per-drink personal record (favourite/want-to-try,
33-
/// ratings, tasting events, notes, photos). One structured JSON entry per
34-
/// drink-per-festival, scoped as `$userStatePrefix${festivalId}_$drinkId`.
32+
/// Prefix for the **legacy v1** unified per-drink personal record
33+
/// (favourite/want-to-try, ratings, tasting events, notes, photos). One
34+
/// structured JSON blob per drink-per-festival, scoped as
35+
/// `$userStatePrefix${festivalId}_$drinkId`.
3536
///
36-
/// Unifies the former `favorites`, `ratings`, and `tasting_log_` key schemes
37-
/// (#391). A one-time migration folds any data stored under those legacy keys
38-
/// into this format on first launch (see [legacy keys] below).
37+
/// Unified the former `favorites`, `ratings`, and `tasting_log_` key schemes
38+
/// (#391). Superseded by the v2 [logEntryPrefix] / [wantToTryPrefix] model
39+
/// (ADR 0006): [SharedPreferencesUserDataStore.migrateToLogEntries] reads any
40+
/// blob stored under this prefix, folds it into the v2 model, then deletes it.
41+
/// Read-only from v2 onward; never written again.
3942
static const userStatePrefix = 'user_state_';
4043

44+
/// Prefix for a single **My Festival log entry** (check-in) in the v2 schema
45+
/// (ADR 0006). Scoped as `$logEntryPrefix${festivalId}_$entryId`, where
46+
/// `entryId` is a UUID. One JSON record per entry; edit/delete key off the
47+
/// id. A tasting is an entry whose `drinkId` is non-null.
48+
static const logEntryPrefix = 'log_entry_';
49+
50+
/// Per-festival "want to try" plan set in the v2 schema (ADR 0006). Scoped as
51+
/// `$wantToTryPrefix$festivalId` → a `StringList` of drink IDs. Present only
52+
/// while non-empty (the key is removed when the set empties).
53+
static const wantToTryPrefix = 'want_to_try_';
54+
55+
/// Per-drink **detail** record in the v2 schema: the user's drink-level
56+
/// rating, notes, and photo IDs, stored **independently of the tasting
57+
/// timeline** so a drink can be rated/noted without being marked tasted.
58+
/// Scoped as `$drinkDetailPrefix${festivalId}_$drinkId` → a JSON object.
59+
/// Present only while it carries a signal (pruned when empty).
60+
static const drinkDetailPrefix = 'drink_detail_';
61+
4162
// --- Legacy personal-state keys (read-only; migration only) ---
4263

4364
/// Legacy favourites key: `${favoritesLegacy}_$festivalId``StringList` of
@@ -62,6 +83,15 @@ class PreferenceKeys {
6283
/// collide with a per-drink record key (`user_state_{festivalId}_{drinkId}`).
6384
static const legacyMigrationComplete = 'personal_state_migration_v1';
6485

86+
/// Flag set to `true` after the one-time v1 → v2 migration of per-drink
87+
/// [userStatePrefix] blobs into the v2 LogEntry model
88+
/// ([logEntryPrefix] + [wantToTryPrefix], ADR 0006). When present and true the
89+
/// key-scan is skipped on every launch after the first successful run.
90+
///
91+
/// Like [legacyMigrationComplete], deliberately does NOT share the v2 prefixes
92+
/// so it cannot collide with an entry or want-to-try record.
93+
static const logEntryMigrationComplete = 'my_festival_migration_v2';
94+
6595
// --- FestivalStorageService ---
6696

6797
/// The last selected festival ID. Stored with `setString`.

lib/domain/repositories/api_drink_repository.dart

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import 'dart:async';
22

3+
import 'package:collection/collection.dart';
4+
import 'package:uuid/uuid.dart';
5+
36
import '../../models/models.dart';
47
import '../../services/services.dart';
58
import 'drink_repository.dart';
@@ -15,6 +18,8 @@ class ApiDrinkRepository implements DrinkRepository {
1518
final DrinkCacheService _cacheService;
1619
final AnalyticsService _analyticsService;
1720

21+
static const Uuid _uuid = Uuid();
22+
1823
ApiDrinkRepository({
1924
required BeerApiService apiService,
2025
required UserDataStore userDataStore,
@@ -25,9 +30,14 @@ class ApiDrinkRepository implements DrinkRepository {
2530
_cacheService = cacheService,
2631
_analyticsService = analyticsService;
2732

28-
/// The current record, or a fresh empty one, ready to be mutated and written.
29-
UserDrinkState _mutableState(String festivalId, String drinkId) =>
30-
_userDataStore.read(festivalId, drinkId) ?? UserDrinkState.initial();
33+
/// The drink's tasting entries, oldest first. A tasting is an entry whose
34+
/// `drinkId` matches (ADR 0006).
35+
List<LogEntry> _tastingsFor(String festivalId, String drinkId) =>
36+
_userDataStore
37+
.readEntries(festivalId)
38+
.where((e) => e.drinkId == drinkId)
39+
.toList()
40+
..sort((a, b) => a.when.compareTo(b.when));
3141

3242
@override
3343
Future<List<Drink>> getDrinks(Festival festival) async {
@@ -129,13 +139,9 @@ class ApiDrinkRepository implements DrinkRepository {
129139
String festivalId,
130140
String drinkId,
131141
) async {
132-
final current = _mutableState(festivalId, drinkId);
133-
final persisted = current.copyWith(
134-
wantToTry: !current.wantToTry,
135-
updatedAt: DateTime.now(),
136-
);
137-
await _userDataStore.write(festivalId, drinkId, persisted);
138-
return persisted.isEmpty ? null : persisted;
142+
final want = _userDataStore.readWantToTry(festivalId).contains(drinkId);
143+
await _userDataStore.setWantToTry(festivalId, drinkId, value: !want);
144+
return _userDataStore.read(festivalId, drinkId);
139145
}
140146

141147
@override
@@ -156,25 +162,20 @@ class ApiDrinkRepository implements DrinkRepository {
156162
'Rating must be between 1 and 5 inclusive',
157163
);
158164
}
159-
final current = _mutableState(festivalId, drinkId);
160-
final persisted = current.copyWith(
161-
rating: rating,
162-
updatedAt: DateTime.now(),
163-
);
164-
await _userDataStore.write(festivalId, drinkId, persisted);
165-
return persisted.isEmpty ? null : persisted;
165+
// Rating is a drink-level signal, independent of the tasting timeline — a
166+
// user can rate a drink without recording that they drank it, and removing
167+
// a tasting never touches the rating.
168+
await _userDataStore.setDrinkRating(festivalId, drinkId, rating: rating);
169+
return _userDataStore.read(festivalId, drinkId);
166170
}
167171

168172
@override
169173
Future<UserDrinkState?> removeRating(
170174
String festivalId,
171175
String drinkId,
172176
) async {
173-
final current = _userDataStore.read(festivalId, drinkId);
174-
if (current == null) return null;
175-
final persisted = current.copyWith(rating: null, updatedAt: DateTime.now());
176-
await _userDataStore.write(festivalId, drinkId, persisted);
177-
return persisted.isEmpty ? null : persisted;
177+
await _userDataStore.setDrinkRating(festivalId, drinkId, rating: null);
178+
return _userDataStore.read(festivalId, drinkId);
178179
}
179180

180181
@override
@@ -187,16 +188,21 @@ class ApiDrinkRepository implements DrinkRepository {
187188
String festivalId,
188189
String drinkId,
189190
) async {
190-
final current = _mutableState(festivalId, drinkId);
191-
final now = DateTime.now();
192191
// Binary toggle preserves the prior single-timestamp behaviour: tasting a
193-
// fresh drink records one event; toggling off clears the log. (Recording
194-
// multiple tastings is feature work tracked in #315.)
195-
final next = current.isTasted
196-
? current.copyWith(tastingEvents: const [], updatedAt: now)
197-
: current.copyWith(tastingEvents: [now], updatedAt: now);
198-
await _userDataStore.write(festivalId, drinkId, next);
199-
return next.isEmpty ? null : next;
192+
// fresh drink records one event; toggling off clears the tasting log.
193+
// (Multi-tasting capture is #415, built on addTasting/removeTasting.)
194+
final tastings = _tastingsFor(festivalId, drinkId);
195+
if (tastings.isEmpty) {
196+
await _userDataStore.writeEntry(
197+
festivalId,
198+
LogEntry(id: _uuid.v4(), when: DateTime.now(), drinkId: drinkId),
199+
);
200+
} else {
201+
for (final tasting in tastings) {
202+
await _userDataStore.removeEntry(festivalId, tasting.id);
203+
}
204+
}
205+
return _userDataStore.read(festivalId, drinkId);
200206
}
201207

202208
@override
@@ -205,18 +211,16 @@ class ApiDrinkRepository implements DrinkRepository {
205211
String drinkId, {
206212
DateTime? now,
207213
}) async {
208-
final current = _mutableState(festivalId, drinkId);
209214
final timestamp = now ?? DateTime.now();
210-
// Consecutive rapid taps intentionally append duplicate events rather
211-
// than debouncing: v1 ships a per-timestamp delete UI to recover from
212-
// accidents, and a debounce would add hidden temporal state (#411).
213-
final persisted = current.copyWith(
214-
tastingEvents: [...current.tastingEvents, timestamp],
215-
updatedAt: timestamp,
215+
// Consecutive rapid taps intentionally append duplicate events rather than
216+
// debouncing: v1 ships a per-timestamp delete UI to recover from accidents,
217+
// and a debounce would add hidden temporal state (#411). Each tasting is a
218+
// distinct entry with its own id, so identical timestamps stay distinct.
219+
await _userDataStore.writeEntry(
220+
festivalId,
221+
LogEntry(id: _uuid.v4(), when: timestamp, drinkId: drinkId),
216222
);
217-
await _userDataStore.write(festivalId, drinkId, persisted);
218-
// An append always leaves at least one event, so the record is never empty.
219-
return persisted;
223+
return _userDataStore.read(festivalId, drinkId);
220224
}
221225

222226
@override
@@ -225,18 +229,18 @@ class ApiDrinkRepository implements DrinkRepository {
225229
String drinkId,
226230
DateTime event,
227231
) async {
228-
final current = _userDataStore.read(festivalId, drinkId);
229-
if (current == null) return null;
230-
final updated = [...current.tastingEvents];
231232
// Removes the first matching pour; identical timestamps are distinct pours,
232-
// so only one goes. Returns the record untouched when the event is absent.
233-
if (!updated.remove(event)) return current;
234-
final persisted = current.copyWith(
235-
tastingEvents: updated,
236-
updatedAt: DateTime.now(),
237-
);
238-
await _userDataStore.write(festivalId, drinkId, persisted);
239-
return persisted.isEmpty ? null : persisted;
233+
// so only one goes. Leaves the state untouched when the event is absent.
234+
// Returns the derived state (null only when the drink has no signal at all —
235+
// never prunes a want-to-try / rated drink that simply has no tastings).
236+
final match = _tastingsFor(
237+
festivalId,
238+
drinkId,
239+
).firstWhereOrNull((e) => e.when == event);
240+
if (match != null) {
241+
await _userDataStore.removeEntry(festivalId, match.id);
242+
}
243+
return _userDataStore.read(festivalId, drinkId);
240244
}
241245

242246
@override
@@ -245,16 +249,12 @@ class ApiDrinkRepository implements DrinkRepository {
245249
String drinkId,
246250
String? notes,
247251
) async {
248-
final current = _mutableState(festivalId, drinkId);
249252
// Blank is not a distinct signal from "no note": store it as null so the
250-
// null-means-unset convention holds and the record can prune to empty.
253+
// null-means-unset convention holds. Notes are drink-level, independent of
254+
// the tasting timeline (same as rating).
251255
final normalised = (notes == null || notes.isEmpty) ? null : notes;
252-
final persisted = current.copyWith(
253-
notes: normalised,
254-
updatedAt: DateTime.now(),
255-
);
256-
await _userDataStore.write(festivalId, drinkId, persisted);
257-
return persisted.isEmpty ? null : persisted;
256+
await _userDataStore.setDrinkNotes(festivalId, drinkId, notes: normalised);
257+
return _userDataStore.read(festivalId, drinkId);
258258
}
259259

260260
@override

0 commit comments

Comments
 (0)