diff --git a/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md new file mode 100644 index 0000000000..68f6f4d0ba --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md @@ -0,0 +1,2031 @@ +# Stored Trip Day Weather Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fetch weather for a trip day at most once, store it as synced trip data, and read it from the database on every later view instead of re-hitting Open-Meteo. + +**Architecture:** A new synced table `trip_day_weather` holds one row per (trip, date), written by a backfill pass that runs when the trip story is viewed and reads by a Riverpod provider subscribed to the table's change tick. The day header stops fetching entirely and becomes a pure widget that renders whatever weather it is handed, with dive-logged weather still taking precedence over a fetched summary. + +**Tech Stack:** Flutter, Drift (SQLite), Riverpod 3, Equatable, Open-Meteo archive API via the existing `WeatherService`. + +**Spec:** `docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md` + +## Global Constraints + +- **Worktree:** all work happens in `/Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather` on branch `worktree-trip-day-weather`. The Bash working directory does NOT reliably persist across turns: put `cd &&` in the same compound command as the work and echo `pwd`, or use absolute paths. A relative-path write from the wrong cwd silently edits the main checkout. +- **Schema version is 171.** Renumbered from 168: that number was already claimed and pushed by PR #1237 (issue #638), but the claim was local and unpushed when this branch picked its number, so an open-PR scan could not see it. 165-170 are claimed by #1290, #1300, #1276, #1237, the gear-twin branch, and #1322. Re-verify with BOTH scans (open-PR diffs AND every worktree's working-tree scalar) immediately before pushing, not just when picking the number. Do NOT raise `minimumCompatibleSchemaVersion` (stays 160): a new table is additive. +- **No em-dashes** (`—`, U+2014) in any output: code, comments, docs, commit messages. En-dashes and " - " as prose punctuation are equally forbidden. A hyphen inside a compound word or CLI flag is fine. +- **No emojis** in code, comments, or documentation. +- **Timestamps are epoch MILLISECONDS** in these tables (`DateTime.millisecondsSinceEpoch`), matching `ItineraryDayRepository`. The `// Unix timestamp` comment on `trip_itinerary_days.date` is misleading; the repository writes milliseconds. +- **Immutability:** never mutate objects or lists. All domain entities get `copyWith`. +- **TDD:** write the failing test first, watch it fail, then implement. +- **Formatting:** run `dart format .` before every commit. +- **Commit messages:** no `Co-Authored-By` trailer, no Claude Code attribution line, no session URL. +- **Units:** anything displaying units must respect the active diver's unit settings. Weather is stored in metric (celsius, m/s, bar) and converted at display time by `UnitFormatter`. + +--- + +### Task 1: Schema, entity, and the v171 migration + +**Files:** +- Create: `lib/features/trips/domain/entities/trip_day_weather.dart` +- Modify: `lib/core/database/database.dart` (table class near `TripItineraryDays` at line 117; `@DriftDatabase(tables: [...])` list; `currentSchemaVersion` at line 3183; `migrationVersions` list; a new `_assertTripDayWeatherSchema()` helper next to `_assertQualityFindingsSchema()` at line 3932; the `onUpgrade` ladder tail at line 8614; the `beforeOpen` backstop block around line 8823) +- Test: `test/core/database/migration_v171_trip_day_weather_test.dart` + +**Interfaces:** +- Consumes: nothing (first task). +- Produces: + - Drift table `TripDayWeather` -> generated row class `TripDayWeatherData`, accessor `_db.tripDayWeather`, companion `TripDayWeatherCompanion`. + - Domain entity `TripDayWeather` (in the `features/trips` namespace; import it `as domain` wherever the Drift row class is also in scope, per the project's import-alias convention) with fields `id`, `tripId`, `date`, `latitude`, `longitude`, `airTemp`, `cloudCover`, `precipitation`, `windSpeed`, `windDirection`, `humidity`, `surfacePressure`, `weatherCode`, `weatherSource`, `fetchedAt`, `createdAt`, `updatedAt`; `copyWith`; and `TripStoryDayWeather toStoryWeather()`. + - `AppDatabase.currentSchemaVersion == 171`. + +- [ ] **Step 1: Re-verify the schema version claim** + +Run from the worktree: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion && \ + for n in $(gh pr list --state open --json number --jq '.[].number'); do \ + v=$(gh pr diff $n 2>/dev/null | grep -E '^\+\s*static const int currentSchemaVersion' | head -1); \ + [ -n "$v" ] && echo "PR $n: $v"; \ + done +``` + +Expected: claims at 165 (#1290), 166 (#1300), 167 (#1276), 168 (#1237), 169 (the dive-computer gear-twin branch, local only), 170 (#1322), plus a stale claim from #603 that is far below main and does not count. NOTE: the #1237 claim read as stale v161 when this plan was written and was in fact a live v168, because its renumber was resolved locally and not yet pushed. An open-PR scan cannot see an unpushed claim; also scan every worktree's working-tree scalar. The next free rung is **171**. + +- [ ] **Step 2: Write the failing migration test** + +Create `test/core/database/migration_v171_trip_day_weather_test.dart`. Model it on the other `migration_v*_test.dart` files in that directory: read one first to copy the exact in-memory database setup helper they use. + +```dart +import 'package:drift/drift.dart'; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/database/database.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('v171 trip_day_weather', () { + test('the ladder claims 171', () { + expect( + AppDatabase.currentSchemaVersion, + greaterThanOrEqualTo(171), + ); + expect(AppDatabase.migrationVersions, contains(171)); + }); + + test('a new database has the trip_day_weather table with every column', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final rows = await db + .customSelect('PRAGMA table_info(trip_day_weather)') + .get(); + final names = rows.map((r) => r.data['name'] as String).toSet(); + + expect(names, containsAll({ + 'id', + 'trip_id', + 'date', + 'latitude', + 'longitude', + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + 'weather_source', + 'fetched_at', + 'created_at', + 'updated_at', + 'hlc', + })); + }); + + test('one row per trip and date', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + await db.customStatement('PRAGMA foreign_keys = OFF'); + + Future insert(String id) => db.customStatement( + 'INSERT INTO trip_day_weather ' + '(id, trip_id, date, latitude, longitude, weather_source, ' + 'fetched_at, created_at, updated_at) ' + "VALUES ('$id', 'trip-1', 1000, 1.0, 2.0, 'openMeteo', 1, 1, 1)", + ); + + await insert('a'); + await expectLater(insert('b'), throwsA(isA())); + }); + + test('the migration adds the table to a stranded v164 database', () async { + // A database stamped at the previous rung must gain the table on open. + final executor = NativeDatabase.memory(); + final raw = AppDatabase(executor); + await raw.customStatement('PRAGMA user_version = 164'); + await raw.customStatement('DROP TABLE IF EXISTS trip_day_weather'); + await raw.close(); + + final db = AppDatabase(executor); + addTearDown(db.close); + final rows = await db + .customSelect('PRAGMA table_info(trip_day_weather)') + .get(); + + expect(rows, isNotEmpty); + }); + }); +} +``` + +Note on the last test: check how the sibling `migration_v*_test.dart` files build a stranded database. If they use a shared helper (a temp file database re-opened at a pinned `user_version`), use that helper verbatim instead of the sketch above, because an in-memory executor cannot always be reopened. + +- [ ] **Step 3: Run the test and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/database/migration_v171_trip_day_weather_test.dart +``` + +Expected: FAIL. `migrationVersions` does not contain 171, and `PRAGMA table_info(trip_day_weather)` returns no rows. + +- [ ] **Step 4: Add the Drift table** + +In `lib/core/database/database.dart`, directly after the `TripItineraryDays` class (which ends at line 136), add: + +```dart +/// Fetched historical weather for one trip day, for days whose dives supply +/// no weather of their own (surface days and dive-free itinerary days). +/// +/// A separate table rather than columns on `trips` or `trip_itinerary_days` +/// on purpose: HLC conflicts resolve per row, so parking an automatic, +/// derived write on a row the diver also edits by hand lets a weather write +/// race a rename or a note edit and lose it. Weather owns its own row and its +/// own clock. +/// +/// Metric storage throughout (celsius, m/s, bar); conversion to the diver's +/// units happens at display time. +class TripDayWeather extends Table { + TextColumn get id => text()(); + TextColumn get tripId => text().references(Trips, #id)(); + + /// Local midnight for the day, as epoch milliseconds (the same convention + /// TripItineraryDays.date is written with). + IntColumn get date => integer()(); + + /// The coordinates the lookup actually used, so a row records what it was + /// fetched for even if the trip's sites later move. + RealColumn get latitude => real()(); + RealColumn get longitude => real()(); + + RealColumn get airTemp => real().nullable()(); // celsius + TextColumn get cloudCover => text().nullable()(); // enum: CloudCover.name + TextColumn get precipitation => + text().nullable()(); // enum: Precipitation.name + RealColumn get windSpeed => real().nullable()(); // m/s + TextColumn get windDirection => + text().nullable()(); // enum: CurrentDirection.name + RealColumn get humidity => real().nullable()(); // 0-100 + RealColumn get surfacePressure => real().nullable()(); // bar + + /// Raw WMO weather code, kept so prose renders in the diver's locale at + /// display time rather than being frozen as English at fetch time. + IntColumn get weatherCode => integer().nullable()(); + + TextColumn get weatherSource => + text().withDefault(const Constant('openMeteo'))(); + IntColumn get fetchedAt => integer()(); + IntColumn get createdAt => integer()(); + IntColumn get updatedAt => integer()(); + + /// Hybrid Logical Clock for cross-device conflict resolution + /// (nullable: rows written before HLC rollout fall back to updatedAt). + TextColumn get hlc => text().nullable()(); + + @override + Set get primaryKey => {id}; +} +``` + +Then add `TripDayWeather,` to the `tables: [...]` list in the `@DriftDatabase` annotation, next to `TripItineraryDays`. + +- [ ] **Step 5: Add the schema-assert helper** + +Next to `_assertQualityFindingsSchema()` (line 3932), which is the pattern to copy, add: + +```dart + /// v171: fetched per-day trip weather. Idempotent, so it doubles as the + /// beforeOpen backstop for a database that took the rung before the unique + /// index existed. + Future _assertTripDayWeatherSchema() async { + await customStatement(''' + CREATE TABLE IF NOT EXISTS trip_day_weather ( + id TEXT NOT NULL PRIMARY KEY, + trip_id TEXT NOT NULL REFERENCES trips (id), + date INTEGER NOT NULL, + latitude REAL NOT NULL, + longitude REAL NOT NULL, + air_temp REAL, + cloud_cover TEXT, + precipitation TEXT, + wind_speed REAL, + wind_direction TEXT, + humidity REAL, + surface_pressure REAL, + weather_code INTEGER, + weather_source TEXT NOT NULL DEFAULT 'openMeteo', + fetched_at INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + hlc TEXT + ) + '''); + await customStatement( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_trip_day_weather_trip_date ' + 'ON trip_day_weather (trip_id, date)', + ); + } +``` + +- [ ] **Step 6: Claim the rung in all six places** + +1. `static const int currentSchemaVersion = 171;` (was 164, line 3183). +2. Append `171,` to the end of the `migrationVersions` list. The ladder is non-contiguous by design (162 is permanently skipped and reserved rungs may be missing); do not "fix" the gaps. +3. The helper docstring above already names v171. +4. In `onUpgrade`, after the `if (from < 164) await reportProgress();` pair at line 8617, add both halves: + +```dart + // v171: trip_day_weather, fetched per-day weather for trip days whose + // dives supply none. + if (from < 171) { + await _assertTripDayWeatherSchema(); + } + if (from < 171) await reportProgress(); +``` + +5. In `beforeOpen`, alongside the other backstops (around line 8823), add: + +```dart + // v171 backstop: re-assert the trip day weather table (the helper is + // CREATE TABLE IF NOT EXISTS, so it is safe on every open). + await _assertTripDayWeatherSchema(); +``` + +6. The test file created in Step 2 already carries the version in its filename and assertions. + +Leave `minimumCompatibleSchemaVersion` at 160. + +- [ ] **Step 7: Run codegen** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + dart run build_runner build --delete-conflicting-outputs +``` + +Expected: succeeds, and `lib/core/database/database.g.dart` now defines `TripDayWeatherData` and `$TripDayWeatherTable`. + +- [ ] **Step 8: Write the domain entity** + +Create `lib/features/trips/domain/entities/trip_day_weather.dart`: + +```dart +import 'package:equatable/equatable.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// Stored historical weather for one trip day. +/// +/// Written only for days whose dives supply no weather of their own; a day +/// with dive-logged weather always renders that instead. +class TripDayWeather extends Equatable { + final String id; + final String tripId; + + /// Local midnight for the day this describes. + final DateTime date; + + /// The coordinates the lookup used. + final double latitude; + final double longitude; + + final double? airTemp; // celsius + final CloudCover? cloudCover; + final Precipitation? precipitation; + final double? windSpeed; // m/s + final CurrentDirection? windDirection; + final double? humidity; // 0-100 + final double? surfacePressure; // bar + final int? weatherCode; + final WeatherSource weatherSource; + final DateTime fetchedAt; + final DateTime createdAt; + final DateTime updatedAt; + + const TripDayWeather({ + required this.id, + required this.tripId, + required this.date, + required this.latitude, + required this.longitude, + this.airTemp, + this.cloudCover, + this.precipitation, + this.windSpeed, + this.windDirection, + this.humidity, + this.surfacePressure, + this.weatherCode, + this.weatherSource = WeatherSource.openMeteo, + required this.fetchedAt, + required this.createdAt, + required this.updatedAt, + }); + + /// True when at least one field the day header can render is present. + /// A result with nothing renderable is not worth a row. + bool get hasRenderableWeather => + airTemp != null || cloudCover != null || precipitation != null; + + /// The compact view model the day header consumes. + TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); + + TripDayWeather copyWith({ + String? id, + String? tripId, + DateTime? date, + double? latitude, + double? longitude, + Object? airTemp = _undefined, + Object? cloudCover = _undefined, + Object? precipitation = _undefined, + Object? windSpeed = _undefined, + Object? windDirection = _undefined, + Object? humidity = _undefined, + Object? surfacePressure = _undefined, + Object? weatherCode = _undefined, + WeatherSource? weatherSource, + DateTime? fetchedAt, + DateTime? createdAt, + DateTime? updatedAt, + }) { + return TripDayWeather( + id: id ?? this.id, + tripId: tripId ?? this.tripId, + date: date ?? this.date, + latitude: latitude ?? this.latitude, + longitude: longitude ?? this.longitude, + airTemp: airTemp == _undefined ? this.airTemp : airTemp as double?, + cloudCover: cloudCover == _undefined + ? this.cloudCover + : cloudCover as CloudCover?, + precipitation: precipitation == _undefined + ? this.precipitation + : precipitation as Precipitation?, + windSpeed: windSpeed == _undefined + ? this.windSpeed + : windSpeed as double?, + windDirection: windDirection == _undefined + ? this.windDirection + : windDirection as CurrentDirection?, + humidity: humidity == _undefined ? this.humidity : humidity as double?, + surfacePressure: surfacePressure == _undefined + ? this.surfacePressure + : surfacePressure as double?, + weatherCode: weatherCode == _undefined + ? this.weatherCode + : weatherCode as int?, + weatherSource: weatherSource ?? this.weatherSource, + fetchedAt: fetchedAt ?? this.fetchedAt, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } + + @override + List get props => [ + id, + tripId, + date, + latitude, + longitude, + airTemp, + cloudCover, + precipitation, + windSpeed, + windDirection, + humidity, + surfacePressure, + weatherCode, + weatherSource, + fetchedAt, + createdAt, + updatedAt, + ]; +} + +// Sentinel value for distinguishing null from undefined in copyWith +const _undefined = Object(); +``` + +Before writing this, confirm `CurrentDirection` and `WeatherSource` exist in `lib/core/constants/enums.dart` (they do: `WeatherSource` is at line 567, and `windDirection` on the dives table is documented as `CurrentDirection.name` at line 733). + +- [ ] **Step 9: Run the migration test and the database suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/database/ +``` + +Expected: PASS, including the pre-existing ladder audit tests. Roughly 465 tests, about 15 seconds. If a ladder audit fails, re-read the six places above; a missing `migrationVersions` entry or a missing `reportProgress()` twin is the usual cause. + +- [ ] **Step 10: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(db): add trip_day_weather table at schema v171 + +One row per trip day, holding fetched historical weather for days whose +dives supply none. A separate table rather than columns on an existing +row so an automatic derived write never conflicts with hand-entered data +under row-level HLC resolution." +``` + +--- + +### Task 2: Repository + +**Files:** +- Create: `lib/features/trips/data/repositories/trip_day_weather_repository.dart` +- Modify: `lib/features/trips/data/repositories/trip_repository.dart:263-267` (the `deleteTrip` transaction) +- Test: `test/features/trips/data/repositories/trip_day_weather_repository_test.dart` + +**Interfaces:** +- Consumes: `TripDayWeather` domain entity and the `_db.tripDayWeather` Drift table from Task 1. +- Produces: `TripDayWeatherRepository` with + - `Stream watchWeatherChanges()` + - `Future> getForTrip(String tripId)` keyed by `date.millisecondsSinceEpoch` + - `Future upsert(TripDayWeather weather)` + - `Future deleteByTripId(String tripId)` + +- [ ] **Step 1: Write the failing repository test** + +Create `test/features/trips/data/repositories/trip_day_weather_repository_test.dart`. Read `test/features/trips/data/repositories/itinerary_day_repository_test.dart` first (if it exists) or another repository test in `test/features/trips/data/repositories/` to copy the exact `DatabaseService` test harness those tests use, since these repositories reach `DatabaseService.instance.database` rather than taking a database argument. + +```dart +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late TripDayWeatherRepository repository; + + // Use the same in-memory DatabaseService setup the sibling trip repository + // tests use, and insert a parent trip row named 'trip-1' before each test + // (trip_id is a non-nullable FK and beforeOpen enables foreign keys). + + TripDayWeather sample({ + String id = 'w1', + String tripId = 'trip-1', + DateTime? date, + double? airTemp = 21.5, + }) { + final day = date ?? DateTime(2026, 3, 8); + return TripDayWeather( + id: id, + tripId: tripId, + date: day, + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: CloudCover.clear, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ); + } + + test('upsert then read back, keyed by date millis', () async { + await repository.upsert(sample()); + + final stored = await repository.getForTrip('trip-1'); + + expect(stored, hasLength(1)); + final row = stored[DateTime(2026, 3, 8).millisecondsSinceEpoch]!; + expect(row.airTemp, 21.5); + expect(row.cloudCover, CloudCover.clear); + expect(row.weatherSource, WeatherSource.openMeteo); + }); + + test('upserting the same day twice keeps one row', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', airTemp: 25)); + + final stored = await repository.getForTrip('trip-1'); + + expect(stored, hasLength(1)); + expect( + stored[DateTime(2026, 3, 8).millisecondsSinceEpoch]!.airTemp, + 25, + ); + }); + + test('getForTrip is scoped to one trip', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: 'trip-2')); + + expect(await repository.getForTrip('trip-1'), hasLength(1)); + }); + + test('deleteByTripId removes only that trip rows', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: 'trip-2')); + + await repository.deleteByTripId('trip-1'); + + expect(await repository.getForTrip('trip-1'), isEmpty); + expect(await repository.getForTrip('trip-2'), hasLength(1)); + }); +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/data/repositories/trip_day_weather_repository_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_repository.dart`. + +- [ ] **Step 3: Write the repository** + +Create `lib/features/trips/data/repositories/trip_day_weather_repository.dart`. `ItineraryDayRepository` is the shape to copy exactly: the `DatabaseService.instance.database` getter, the `SyncRepository()` field, the logger, `markRecordPending` after every write, `logDeletion` for every delete, and `SyncEventBus.notifyLocalChange()` at the end of each mutating method. + +```dart +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + as domain; + +/// Reads and writes stored per-day trip weather. +/// +/// The unique index on (trip_id, date) is what makes an upsert idempotent: +/// two devices that both fetch the same day converge on one row rather than +/// accumulating duplicates. +class TripDayWeatherRepository { + AppDatabase get _db => DatabaseService.instance.database; + final SyncRepository _syncRepository = SyncRepository(); + final _log = LoggerService.forClass(TripDayWeatherRepository); + + /// Emits whenever `trip_day_weather` changes, so the display provider + /// refreshes after a backfill write or a sync import. + Stream watchWeatherChanges() => + _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); + + /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + Future> getForTrip(String tripId) async { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + return {for (final row in rows) row.date: _mapRow(row)}; + } + + /// Insert or replace one day's weather. + Future upsert(domain.TripDayWeather weather) async { + try { + final now = DateTime.now().millisecondsSinceEpoch; + + // Replace any existing row for this day rather than relying on the id: + // the day is the identity, and a peer may have written its own id for + // the same day. + final existing = + await (_db.select(_db.tripDayWeather)..where( + (t) => + t.tripId.equals(weather.tripId) & + t.date.equals(weather.date.millisecondsSinceEpoch), + )) + .getSingleOrNull(); + final id = existing?.id ?? weather.id; + + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherCompanion( + id: Value(id), + tripId: Value(weather.tripId), + date: Value(weather.date.millisecondsSinceEpoch), + latitude: Value(weather.latitude), + longitude: Value(weather.longitude), + airTemp: Value(weather.airTemp), + cloudCover: Value(weather.cloudCover?.name), + precipitation: Value(weather.precipitation?.name), + windSpeed: Value(weather.windSpeed), + windDirection: Value(weather.windDirection?.name), + humidity: Value(weather.humidity), + surfacePressure: Value(weather.surfacePressure), + weatherCode: Value(weather.weatherCode), + weatherSource: Value(weather.weatherSource.name), + fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), + createdAt: Value( + existing?.createdAt ?? weather.createdAt.millisecondsSinceEpoch, + ), + updatedAt: Value(now), + ), + ); + + await _syncRepository.markRecordPending( + entityType: 'tripDayWeather', + recordId: id, + localUpdatedAt: now, + ); + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to store weather for trip ${weather.tripId}', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Delete every stored day for a trip, logging each id for sync. + Future deleteByTripId(String tripId) async { + try { + final existing = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + if (existing.isEmpty) return; + + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).go(); + + for (final row in existing) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: row.id, + ); + } + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to delete weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + domain.TripDayWeather _mapRow(TripDayWeatherData row) { + return domain.TripDayWeather( + id: row.id, + tripId: row.tripId, + date: DateTime.fromMillisecondsSinceEpoch(row.date), + latitude: row.latitude, + longitude: row.longitude, + airTemp: row.airTemp, + cloudCover: row.cloudCover == null + ? null + : CloudCover.values.byName(row.cloudCover!), + precipitation: row.precipitation == null + ? null + : Precipitation.values.byName(row.precipitation!), + windSpeed: row.windSpeed, + windDirection: row.windDirection == null + ? null + : CurrentDirection.values.byName(row.windDirection!), + humidity: row.humidity, + surfacePressure: row.surfacePressure, + weatherCode: row.weatherCode, + weatherSource: WeatherSource.values.byName(row.weatherSource), + fetchedAt: DateTime.fromMillisecondsSinceEpoch(row.fetchedAt), + createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), + updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), + ); + } +} +``` + +If the generated row class is not named `TripDayWeatherData`, check `database.g.dart` for the actual name and use it. + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/data/repositories/trip_day_weather_repository_test.dart +``` + +Expected: PASS. + +- [ ] **Step 5: Wire the trip-delete cascade** + +In `lib/features/trips/data/repositories/trip_repository.dart`, inside the `deleteTrip` transaction (line 263), add the new repository next to the others: + +```dart + await LiveaboardDetailsRepository().deleteByTripId(id); + await ItineraryDayRepository().deleteByTripId(id); + await TripChecklistRepository().deleteByTripId(id); + await TripDayWeatherRepository().deleteByTripId(id); +``` + +Add the import for `trip_day_weather_repository.dart` in the same file's local import group. + +- [ ] **Step 6: Add a deletion test and run the trip repository suite** + +Append to the repository test file: + +```dart + test('deleting a trip takes its weather rows with it', () async { + await repository.upsert(sample()); + + await TripRepository().deleteTrip('trip-1'); + + expect(await repository.getForTrip('trip-1'), isEmpty); + }); +``` + +Import `TripRepository` at the top of the test file. Then: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/ +``` + +Expected: PASS, with no regressions in the existing trip tests. + +- [ ] **Step 7: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): add TripDayWeatherRepository + +Upsert is keyed by (trip, date) rather than by id so two devices that +both fetch the same day converge on one row. Deleting a trip takes its +weather rows with it, logged for sync like every other child record." +``` + +--- + +### Task 3: Sync registration + +**Files:** +- Modify: `lib/core/data/repositories/sync_repository.dart:49` (the `hlcTargets` map) +- Modify: `lib/core/services/sync/sync_data_serializer.dart` (13 sites, listed below) +- Modify: `lib/core/services/sync/sync_service.dart:1209`, `:1959`, `:2120` +- Test: `test/core/services/sync/trip_day_weather_sync_test.dart` + +**Interfaces:** +- Consumes: the `trip_day_weather` table from Task 1. +- Produces: entity type key `'tripDayWeather'` registered end to end, and `SyncData.tripDayWeather` as a `List>` field. + +Every site below is a copy of what `'itineraryDays'` does. Three structural tests already in the suite will fail if any site is missed: `sync_hlc_target_registration_test` (asserts every table with an `hlc` column is in `hlcTargets`), the `entityHasUpdatedAt covers exactly the SyncData entities` test, and the merge-order parity test. + +- [ ] **Step 1: Write the failing round-trip test** + +Create `test/core/services/sync/trip_day_weather_sync_test.dart`, modeled directly on `test/core/services/sync/site_features_sync_test.dart` (the closest analogue: a synced child record with a non-nullable FK to its parent). + +```dart +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/sync/sync_data_serializer.dart'; +import 'package:submersion/core/services/sync/sync_service.dart'; + +import '../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late SyncDataSerializer serializer; + + setUp(() async { + db = await setUpTestDatabase(); + serializer = SyncDataSerializer(); + await db + .into(db.trips) + .insert( + TripsCompanion.insert( + id: 'trip-1', + name: 'Bonaire', + createdAt: 1, + updatedAt: 1, + ), + ); + await db + .into(db.tripDayWeather) + .insert( + TripDayWeatherCompanion.insert( + id: 'w-1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8).millisecondsSinceEpoch, + latitude: 12.16, + longitude: -68.28, + airTemp: const Value(24.0), + cloudCover: const Value('clear'), + fetchedAt: 1, + createdAt: 1, + updatedAt: 1, + ), + ); + }); + + tearDown(tearDownTestDatabase); + + test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { + final record = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(record, isNotNull); + expect(record!['airTemp'], 24.0); + expect(record['cloudCover'], 'clear'); + + // A remote edit merges over the local row (LWW payload apply). + await serializer.upsertRecord('tripDayWeather', { + ...record, + 'airTemp': 26.0, + 'updatedAt': 2, + }); + final merged = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(merged!['airTemp'], 26.0); + + expect(await serializer.recordIdsFor('tripDayWeather'), contains('w-1')); + + await serializer.deleteRecord('tripDayWeather', 'w-1'); + expect(await serializer.fetchRecord('tripDayWeather', 'w-1'), isNull); + }); + + test('the delta export filters on the row own hlc', () async { + await (db.update(db.tripDayWeather)..where((t) => t.id.equals('w-1'))) + .write( + const TripDayWeatherCompanion( + hlc: Value('2026-08-16T00:00:00.000-0000'), + ), + ); + + Future changesetCount(String? watermark) async { + final payload = await serializer.exportChangeset( + deviceId: 'device-1', + hlcWatermark: watermark, + deletions: const [], + ); + return payload.data.tripDayWeather.length; + } + + // A base carries the row; a watermark newer than it excludes it; an + // older watermark includes it. + expect(await changesetCount(null), 1); + expect(await changesetCount('2026-08-17T00:00:00.000-0000'), 0); + expect(await changesetCount('2026-08-15T00:00:00.000-0000'), 1); + }); + + test('tripDayWeather is registered as an hlc target', () { + expect(SyncRepository.hlcTargets.containsKey('tripDayWeather'), isTrue); + expect( + SyncRepository.hlcTargets['tripDayWeather']!.table, + 'trip_day_weather', + ); + }); + + test('tripDayWeather carries an updatedAt flag', () { + expect(SyncService.entityHasUpdatedAt['tripDayWeather'], isTrue); + }); +} +``` + +Two things to check against generated code rather than assume. First, the keys in the `fetchRecord` map come from Drift's generated `toJson()`, which uses Dart field names (`airTemp`, `cloudCover`), not SQL column names; if an assertion fails on a key, confirm the real name in `database.g.dart`. Second, `TripsCompanion.insert` may require more than `id`/`name`/`createdAt`/`updatedAt`; read the `Trips` table definition and supply whatever else is non-nullable and undefaulted. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/services/sync/trip_day_weather_sync_test.dart +``` + +Expected: FAIL, because `hlcTargets` has no `tripDayWeather` key and `SyncData` has no such field. + +- [ ] **Step 3: Register the HLC target** + +In `lib/core/data/repositories/sync_repository.dart`, after the `'itineraryDays'` entry (line 49): + +```dart + 'tripDayWeather': (table: 'trip_day_weather', pk: 'id'), +``` + +- [ ] **Step 4: Add all 13 serializer sites** + +In `lib/core/services/sync/sync_data_serializer.dart`, add a `tripDayWeather` line immediately after each `itineraryDays` line at these locations: + +1. Line 249, the `SyncData` field list: `final List> tripDayWeather;` +2. Line 324, the constructor: `this.tripDayWeather = const [],` +3. Line 400, `toJson`: `'tripDayWeather': tripDayWeather,` +4. Line 477, `fromJson`: `tripDayWeather: _parseList(json['tripDayWeather']),` +5. Line 762, the table tuple list: + +```dart + ( + key: 'tripDayWeather', + table: _db.tripDayWeather, + blob: false, + full: null, + ), +``` + +6. Line 1268, the export block: + +```dart + tripDayWeather: await _safeExport( + 'tripDayWeather', + () => _exportTripDayWeather(hlcSince), + ), +``` + +7. Line 1707, single-record fetch: + +```dart + case 'tripDayWeather': + final row = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).getSingleOrNull(); + return row?.toJson(); +``` + +8. Line 2032, batch fetch: + +```dart + case 'tripDayWeather': + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.isIn(idList))).get(); + return {for (final r in rows) r.id: r.toJson()}; +``` + +9. Line 2592, single upsert: + +```dart + case 'tripDayWeather': + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherData.fromJson(data).toCompanion(false), + ); + return; +``` + +10. Line 3196, batch upsert: + +```dart + case 'tripDayWeather': + await _db.batch( + (b) => b.insertAllOnConflictUpdate( + _db.tripDayWeather, + records + .map((r) => TripDayWeatherData.fromJson(r).toCompanion(false)) + .toList(), + ), + ); + return; +``` + +11. Line 3681, the plain id projection: `case 'tripDayWeather': return plain(_db.tripDayWeather, _db.tripDayWeather.id);` +12. Line 3914, table lookup: `case 'tripDayWeather': return _db.tripDayWeather;` +13. Line 4222, delete by id: + +```dart + case 'tripDayWeather': + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).go(); + return; +``` + +Then add the export helper next to `_exportItineraryDays` (line 4892), copying its body exactly: + +```dart + Future>> _exportTripDayWeather( + String? hlcSince, + ) async { + final query = _db.select(_db.tripDayWeather); + if (hlcSince != null) { + query.where((t) => t.hlc.isBiggerThanValue(hlcSince)); + } + final rows = await query.get(); + return rows.map((r) => r.toJson()).toList(); + } +``` + +Read `_exportItineraryDays` in full before copying: if it does anything else (such as a diver scope filter), mirror that too. + +Use the generated row class name from `database.g.dart` in sites 9 and 10; the plan assumes `TripDayWeatherData`. + +- [ ] **Step 5: Add the three sync_service sites** + +In `lib/core/services/sync/sync_service.dart`: + +1. After the `itineraryDays` merge-order record at line 1209: + +```dart + ( + type: 'tripDayWeather', + records: data.tripDayWeather, + hasUpdatedAt: true, + ), +``` + +2. In `entityHasUpdatedAt` after line 1959: `'tripDayWeather': true,` +3. In the FK parent map after line 2120: + +```dart + 'tripDayWeather': [(field: 'tripId', parent: 'trips', nullable: false)], +``` + +- [ ] **Step 6: Run the sync suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/services/sync/ +``` + +Expected: PASS, including the new round-trip test and the three structural tests. A failure naming an entity coverage mismatch means one of the 17 sites is missing. + +- [ ] **Step 7: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(sync): replicate trip day weather + +Registers tripDayWeather end to end: hlc target, serializer payload and +switch arms, merge order, updatedAt flag, and the trips FK parent so a +changeset never imports weather ahead of its trip." +``` + +--- + +### Task 4: Backfill rules (pure domain logic) + +**Files:** +- Create: `lib/features/trips/domain/services/trip_day_weather_backfill.dart` +- Test: `test/features/trips/domain/services/trip_day_weather_backfill_test.dart` + +**Interfaces:** +- Consumes: `TripStory`, `TripStoryDay`, `TripStoryMapGeometry` (all in `features/trips/domain/entities/`), and the `Map` shape `TripDayWeatherRepository.getForTrip` returns. +- Produces: + +```dart +class TripDayWeatherTarget extends Equatable { + final DateTime date; // local midnight + final double latitude; + final double longitude; + DateTime get localNoon; // date at 12:00, the sample hour to request +} + +class TripDayWeatherBackfill { + static List targetsFor({ + required TripStory story, + required Map stored, + }); +} +``` + +This task is pure: no database, no network, no Riverpod. That is the point. All four skip rules are testable as a plain function. + +- [ ] **Step 1: Write the failing test** + +Create `test/features/trips/domain/services/trip_day_weather_backfill_test.dart`: + +```dart +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; + +void main() { + // Build the smallest story that exercises one rule at a time. Copy the + // Trip/Dive fixture helpers from + // test/features/trips/domain/entities/trip_story_day_test.dart rather than + // hand-rolling new ones. + + Trip trip() => Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 14), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ); + + TripStory storyWith( + List days, { + List points = const [], + }) { + return TripStory( + trip: trip(), + days: days, + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: TripStoryMapGeometry(points: points), + ); + } + + Dive diveWith({double? airTemp}) => + Dive(id: 'd1', dateTime: DateTime(2026, 3, 8, 9), airTemp: airTemp); + + TripStoryDay day({ + required int index, + TripStoryDayKind kind = TripStoryDayKind.past, + List dives = const [], + }) { + return TripStoryDay( + date: DateTime(2026, 3, 8 + index), + dayNumber: index + 1, + kind: kind, + dives: dives, + ); + } + + TripStoryMapPoint pointFor(int dayIndex) => TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: dayIndex, + label: 'Site', + ); + + test('a past day with no dives and a nearby point is a target', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 8)); + expect(targets.single.latitude, 12.16); + expect(targets.single.localNoon, DateTime(2026, 3, 8, 12)); + }); + + test('a day whose dives carry weather is skipped', () { + // Build a dive with airTemp set so TripStoryDay.weather is non-null. + final story = storyWith( + [day(index: 0, dives: [diveWith(airTemp: 26)])], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a future day is skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.future)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day with a stored row is skipped', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final stored = { + DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + ); + }); + + test('a day with no map point anywhere in the story is skipped', () { + final story = storyWith([day(index: 0)]); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day borrows the nearest day point when it has none of its own', () { + // nearestPointForDay already walks outward, so day 1 with a point only on + // day 0 is still a target, at day 0 coordinates. + final story = storyWith( + [day(index: 0, dives: [diveWith(airTemp: 26)]), day(index: 1)], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 9)); + expect(targets.single.latitude, 12.16); + }); +} +``` + +Add the imports these fixtures need: `Trip` from `features/trips/domain/entities/trip.dart` and `Dive` from `features/dive_log/domain/entities/dive.dart`. If `Trip`'s constructor has gained a required parameter, read the entity and supply it. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/domain/services/trip_day_weather_backfill_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_backfill.dart`. + +- [ ] **Step 3: Write the service** + +Create `lib/features/trips/domain/services/trip_day_weather_backfill.dart`: + +```dart +import 'package:equatable/equatable.dart'; + +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// One day that needs a weather lookup, with the coordinates to look it up at. +class TripDayWeatherTarget extends Equatable { + /// Local midnight for the day. + final DateTime date; + final double latitude; + final double longitude; + + const TripDayWeatherTarget({ + required this.date, + required this.latitude, + required this.longitude, + }); + + /// The hour to sample. Noon local reads as "the day's weather" far better + /// than the API's default midnight boundary. + DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); + + @override + List get props => [date, latitude, longitude]; +} + +/// Decides which trip days need a weather lookup. +/// +/// Pure by design: no database, no network. Every skip rule is a plain +/// condition over the built story and the rows already stored. +class TripDayWeatherBackfill { + const TripDayWeatherBackfill._(); + + static List targetsFor({ + required TripStory story, + required Map stored, + }) { + final targets = []; + + for (var index = 0; index < story.days.length; index++) { + final day = story.days[index]; + + // A dive that logged weather is the better source; never override it. + if (day.weather != null) continue; + + // A historical archive has nothing for a day that has not happened. + if (day.kind == TripStoryDayKind.future) continue; + + final date = DateTime(day.date.year, day.date.month, day.date.day); + if (stored.containsKey(date.millisecondsSinceEpoch)) continue; + + // nearestPointForDay walks outward from the day, so a dive-free day + // between two dived days borrows the closer one's coordinates. + final point = story.mapGeometry.nearestPointForDay(index); + if (point == null) continue; + + targets.add( + TripDayWeatherTarget( + date: date, + latitude: point.latitude, + longitude: point.longitude, + ), + ); + } + + return targets; + } +} +``` + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/domain/services/trip_day_weather_backfill_test.dart +``` + +Expected: PASS, all six tests. + +- [ ] **Step 5: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): decide which trip days need a weather lookup + +Pure rules over the built story and the rows already stored: dive-logged +weather wins, future days have no archive, stored days are done, and a +day with no mappable point anywhere in the trip has nowhere to ask." +``` + +--- + +### Task 5: Providers and the fetch loop + +**Files:** +- Create: `lib/features/trips/presentation/providers/trip_day_weather_providers.dart` +- Test: `test/features/trips/presentation/providers/trip_day_weather_providers_test.dart` + +**Interfaces:** +- Consumes: `TripDayWeatherRepository` (Task 2), `TripDayWeatherBackfill.targetsFor` (Task 4), the existing `weatherServiceProvider` from `lib/features/weather/presentation/providers/weather_providers.dart`, and `tripStoryProvider(tripId)` from `trip_story_providers.dart`. +- Produces: + - `tripDayWeatherRepositoryProvider` -> `Provider` + - `tripDayWeatherProvider` -> `FutureProvider.family, String>` keyed by trip id, values keyed by `date.millisecondsSinceEpoch` + - `tripDayWeatherBackfillProvider` -> `FutureProvider.family` keyed by trip id + +**Why the backfill provider does not watch the weather table:** the display provider subscribes to the table tick, but the backfill must not, or every row it writes would invalidate it and start another pass. It reads stored rows directly from the repository instead. Its only reactive dependency is the story itself, so adding dives to a trip re-evaluates what still needs fetching. + +- [ ] **Step 1: Write the failing provider test** + +Create `test/features/trips/presentation/providers/trip_day_weather_providers_test.dart`. The existing `surface_day_weather_provider_test.dart` (deleted in Task 6) is where the `MockClient` weather-response fixture comes from; lift it rather than inventing one, because it encodes the exact Open-Meteo response shape `WeatherMapper` expects. + +```dart +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +/// A full Open-Meteo hourly payload for one day. Copied from the deleted +/// surface_day_weather_provider_test. +http.Response weatherResponse({double noonTemp = 29.0, int cloud = 95}) => + http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T09:00', '2026-03-08T12:00'], + 'temperature_2m': [24.0, noonTemp], + 'relative_humidity_2m': [80.0, 70.0], + 'precipitation': [0.0, 0.0], + 'cloud_cover': [10.0, cloud], + 'wind_speed_10m': [8.0, 12.0], + 'wind_direction_10m': [30.0, 45.0], + 'surface_pressure': [1012.0, 1011.0], + 'weathercode': [0, 0], + }, + }), + 200, + ); + +/// Records upserts instead of touching a database. +class FakeTripDayWeatherRepository implements TripDayWeatherRepository { + FakeTripDayWeatherRepository({this.stored = const {}}); + + final Map stored; + final List upserts = []; + + @override + Future> getForTrip(String tripId) async => stored; + + @override + Future upsert(TripDayWeather weather) async => upserts.add(weather); + + @override + Future deleteByTripId(String tripId) async {} + + @override + Stream watchWeatherChanges() => const Stream.empty(); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + /// Two past days with no dives, and one map point on day 0 that day 1 + /// borrows through nearestPointForDay. + TripStory twoDayStory() => TripStory( + trip: Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ), + days: [ + TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + TripStoryDay( + date: DateTime(2026, 3, 9), + dayNumber: 2, + kind: TripStoryDayKind.past, + ), + ], + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: const TripStoryMapGeometry( + points: [ + TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: 0, + label: 'Site', + ), + ], + ), + ); + + ProviderContainer containerWith({ + required http.Client client, + required FakeTripDayWeatherRepository repository, + }) { + final container = ProviderContainer( + overrides: [ + weatherHttpClientProvider.overrideWithValue(client), + tripDayWeatherRepositoryProvider.overrideWithValue(repository), + tripStoryProvider('trip-1').overrideWith((ref) async => twoDayStory()), + ], + ); + addTearDown(container.dispose); + return container; + } + + test('fetches each target once and stores the result', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((request) async { + calls++; + expect(request.url.queryParameters['timezone'], 'auto'); + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + expect(repository.upserts, hasLength(2)); + expect(repository.upserts.first.airTemp, 29.0); + expect(repository.upserts.first.latitude, 12.16); + expect(repository.upserts.first.tripId, 'trip-1'); + }); + + test('a failed fetch writes no row', () async { + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => http.Response('', 500)), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a result with nothing renderable writes no row', () async { + // Humidity and pressure only: the day header could render none of it, and + // storing it would suppress the retry once the archive catches up. + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient( + (_) async => http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T12:00'], + 'temperature_2m': [null], + 'relative_humidity_2m': [70.0], + 'precipitation': [null], + 'cloud_cover': [null], + 'wind_speed_10m': [null], + 'wind_direction_10m': [null], + 'surface_pressure': [1011.0], + 'weathercode': [null], + }, + }), + 200, + ), + ), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a day already stored is not fetched', () async { + final repository = FakeTripDayWeatherRepository( + stored: { + DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }, + ); + var calls = 0; + final container = containerWith( + client: MockClient((_) async { + calls++; + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 1); + expect(repository.upserts, hasLength(1)); + expect(repository.upserts.single.date, DateTime(2026, 3, 9)); + }); + + test('fetches run one at a time', () async { + final gate = Completer(); + var started = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + started++; + if (started == 1) await gate.future; + return weatherResponse(); + }), + repository: repository, + ); + + final pending = container.read( + tripDayWeatherBackfillProvider('trip-1').future, + ); + await Future.delayed(Duration.zero); + + // The second day must not be in flight while the first is pending. + expect(started, 1); + + gate.complete(); + await pending; + expect(started, 2); + }); +} +``` + +Add the `Trip` import (`features/trips/domain/entities/trip.dart`) for the fixture. Note that `tripStoryProvider` is a `family`, so the override must name the same key the backfill reads: `tripStoryProvider('trip-1')`. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_providers.dart`. + +- [ ] **Step 3: Write the providers** + +Create `lib/features/trips/presentation/providers/trip_day_weather_providers.dart`: + +```dart +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/providers/provider.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +final tripDayWeatherRepositoryProvider = Provider( + (ref) => TripDayWeatherRepository(), +); + +/// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. +/// +/// Subscribes to the table tick, so a row written by the backfill or arriving +/// through sync re-renders the day headers without the widget knowing a fetch +/// ever happened. +final tripDayWeatherProvider = + FutureProvider.family, String>(( + ref, + tripId, + ) async { + final repository = ref.watch(tripDayWeatherRepositoryProvider); + ref.invalidateSelfWhen(repository.watchWeatherChanges()); + return repository.getForTrip(tripId); + }); + +/// Fills the gaps: fetches historical weather for trip days that have none +/// stored and no dive to supply it, then writes what it finds. +/// +/// Deliberately does NOT watch [tripDayWeatherProvider]. Watching the rows it +/// writes would invalidate this provider on every write and start another +/// pass; it reads the stored rows straight from the repository instead. The +/// story is its only reactive input, so assigning dives to a trip re-evaluates +/// what is still missing. +/// +/// Not auto-disposed: one pass per trip per provider container lifetime. +final tripDayWeatherBackfillProvider = FutureProvider.family(( + ref, + tripId, +) async { + final story = await ref.watch(tripStoryProvider(tripId).future); + final repository = ref.watch(tripDayWeatherRepositoryProvider); + final service = ref.watch(weatherServiceProvider); + + final stored = await repository.getForTrip(tripId); + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: stored, + ); + if (targets.isEmpty) return; + + const uuid = Uuid(); + + // Sequential on purpose: a two-week trip would otherwise open with a burst + // of parallel requests, and rows landing one at a time let headers fill in + // progressively. + for (final target in targets) { + final weather = await service.fetchWeather( + latitude: target.latitude, + longitude: target.longitude, + date: target.date, + entryTime: target.localNoon, + useLocationTimezone: true, + ); + if (weather == null) continue; + + final now = DateTime.now(); + final row = TripDayWeather( + id: uuid.v4(), + tripId: tripId, + date: target.date, + latitude: target.latitude, + longitude: target.longitude, + airTemp: weather.airTemp, + cloudCover: weather.cloudCover, + precipitation: weather.precipitation, + windSpeed: weather.windSpeed, + windDirection: weather.windDirection, + humidity: weather.humidity, + surfacePressure: weather.surfacePressure, + weatherCode: weather.weatherCode, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + + // A row the header could render nothing from is worse than no row: it + // would suppress the retry that a later archive update would satisfy. + if (!row.hasRenderableWeather) continue; + + await repository.upsert(row); + } +}); +``` + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +``` + +Expected: PASS, all five tests. + +- [ ] **Step 5: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): fetch and store trip day weather once + +The display provider rides the table tick; the backfill deliberately does +not, so the rows it writes cannot invalidate it into another pass. A miss +or an unrenderable result writes nothing and is retried on a later view." +``` + +--- + +### Task 6: Read path, and delete the per-view fetch + +**Files:** +- Modify: `lib/features/trips/presentation/widgets/story/trip_story_day_header.dart:34-70` (drop the provider watch, take stored weather as a parameter) +- Modify: `lib/features/trips/presentation/widgets/story/trip_story_view.dart:10` (import), `:236-243` (build stored weather instead of a fetch request), and the `TripStoryDayHeader` construction in `_daySliver` +- Delete: `lib/features/trips/presentation/providers/surface_day_weather_provider.dart` +- Delete: `test/features/trips/presentation/providers/surface_day_weather_provider_test.dart` +- Modify: `test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart` (the four weather tests at lines 267-330) +- Modify: `test/features/trips/presentation/widgets/story/trip_story_view_test.dart` (add provider overrides) + +**Interfaces:** +- Consumes: `tripDayWeatherProvider` and `tripDayWeatherBackfillProvider` (Task 5). +- Produces: `TripStoryDayHeader({required TripStoryDay day, TripStoryDayWeather? storedWeather})`. The `surfaceWeatherRequest` parameter and the `SurfaceDayWeatherRequest` type are gone. + +**Note for whoever implements this:** adding a provider dependency to a widget breaks every existing test that pumps it without an override. `trip_story_view_test.dart` pumps the whole view and will need both new providers overridden. Run that file's tests before assuming the change is complete. + +- [ ] **Step 1: Update the header tests first** + +In `test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart`, replace the four tests that override `surfaceDayWeatherProvider` (lines 267-330) with tests that pass the weather in directly. The header no longer fetches, so there is no loading state to test and the "pending future" test at line 305 has no meaning; replace it with the absence case. + +```dart + testWidgets('shows stored weather in the existing badge', (tester) async { + await pumpHeader( + tester, + surfaceDay(), + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), + ); + + expect(find.byIcon(Icons.wb_sunny_outlined), findsOneWidget); + expect(find.text('22°C'), findsOneWidget); + }); + + testWidgets('stored temperature respects Fahrenheit', (tester) async { + final settings = MockSettingsNotifier(); + await settings.setTemperatureUnit(TemperatureUnit.fahrenheit); + await pumpHeader( + tester, + surfaceDay(), + settingsNotifier: settings, + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('71.6°F'), findsOneWidget); + }); + + testWidgets('without stored weather stays badge-free', (tester) async { + await pumpHeader(tester, surfaceDay()); + + expect(find.textContaining('°'), findsNothing); + expect(find.byType(CircularProgressIndicator), findsNothing); + }); + + testWidgets('dive-logged weather wins over stored weather', (tester) async { + // A day whose dive logged 26C must render 26, not the stored 22. + await pumpHeader( + tester, + dayWithDive(airTemp: 26), + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('26°C'), findsOneWidget); + expect(find.text('22°C'), findsNothing); + }); +``` + +Update the `pumpHeader` helper in that file: replace its `surfaceWeatherRequest` parameter with `storedWeather`, and drop the `extra` overrides those tests passed. Use the file's existing fixture for a day with a dive in the last test. + +- [ ] **Step 2: Run the header tests and watch them fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +``` + +Expected: FAIL to compile, because `TripStoryDayHeader` has no `storedWeather` parameter. + +- [ ] **Step 3: Simplify the header** + +In `trip_story_day_header.dart`: + +- Replace the `surfaceWeatherRequest` field and constructor parameter with `final TripStoryDayWeather? storedWeather;` / `this.storedWeather`. +- Delete the `surface_day_weather_provider.dart` import. +- Replace the three lines at 66-69: + +```dart + final request = day.isSurface ? surfaceWeatherRequest : null; + final fetchedWeather = request == null + ? null + : ref.watch(surfaceDayWeatherProvider(request)).asData?.value; + final weather = day.weather ?? fetchedWeather; +``` + +with: + +```dart + // Dive-logged weather always wins: it is what the diver recorded, and a + // fetched day summary is only ever a stand-in for days that logged none. + final weather = day.weather ?? storedWeather; +``` + +Update the class docstring: the header no longer fetches anything, and the note about which days get a badge should say "days with logged or stored weather". + +The widget still needs `ref` for `settingsProvider`, so it stays a `ConsumerWidget`. + +- [ ] **Step 4: Wire the view** + +In `trip_story_view.dart`: + +- Replace the `surface_day_weather_provider.dart` import with `trip_day_weather_providers.dart`. +- In the widget's `build`, watch both providers once for the whole trip: + +```dart + // One read for the whole story; the backfill is a fire-and-forget pass + // whose writes come back through the provider above. + final storedWeather = + ref.watch(tripDayWeatherProvider(widget.story.trip.id)).asData?.value ?? + const {}; + ref.watch(tripDayWeatherBackfillProvider(widget.story.trip.id)); +``` + +Pass `storedWeather` down to `_daySliver`, since that method builds the header. + +- Replace the `weatherPoint` / `surfaceWeatherRequest` block at lines 236-243 with a map lookup: + +```dart + final dayDate = DateTime(day.date.year, day.date.month, day.date.day); + final stored = storedWeather[dayDate.millisecondsSinceEpoch]; +``` + +- Construct the header with `storedWeather: stored?.toStoryWeather()`. + +Check whether `_daySliver` is called from a place that must now thread the map through; if the method is on the state class, reading the value in `build` and passing it as a parameter keeps the data flow explicit. + +- [ ] **Step 5: Delete the replaced provider** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + git rm lib/features/trips/presentation/providers/surface_day_weather_provider.dart \ + test/features/trips/presentation/providers/surface_day_weather_provider_test.dart +``` + +Then confirm nothing still references it: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + grep -rn "surfaceDayWeather\|SurfaceDayWeatherRequest" lib test +``` + +Expected: no output. + +- [ ] **Step 6: Override the new providers in the view test** + +In `test/features/trips/presentation/widgets/story/trip_story_view_test.dart`, add to the overrides that file already builds: + +```dart + tripDayWeatherProvider( + tripId, + ).overrideWith((ref) async => const {}), + tripDayWeatherBackfillProvider(tripId).overrideWith((ref) async {}), +``` + +Without these, the view reaches a real repository and a real HTTP client under `flutter test`. + +- [ ] **Step 7: Run the trips suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/ +``` + +Expected: PASS. A hang here usually means a widget test hit a real database or HTTP client; check the overrides. + +- [ ] **Step 8: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): read trip day weather from the database + +The day header no longer fetches: it renders whatever weather it is +handed, with dive-logged weather still winning. surfaceDayWeatherProvider +and its per-view Open-Meteo call are deleted." +``` + +--- + +### Task 7: Whole-project verification + +**Files:** none created; this task proves the branch is shippable. + +- [ ] **Step 1: Format the whole project** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + dart format . +``` + +- [ ] **Step 2: Analyze the whole project** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + flutter analyze +``` + +Expected: "No issues found!". CI treats infos as fatal, so an info-level finding must be fixed too. Do not pipe this through `grep`: a pipe masks the exit code and hides failures. The "Analyzing ..." line is the receipt that it ran in the right tree. + +- [ ] **Step 3: Run the full test suite once** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test 2>&1 | tail -40 +``` + +Expected: all tests pass. Notes: +- One full run is sufficient before opening a PR; do not run the suite repeatedly. +- Do not overlap this with another local test run, including in another worktree: concurrent runs produce spurious lone failures. +- If the output ends with no summary line and exit code 0, the run was killed by a sibling session rather than passing. Re-run it. +- If a single file fails here but passes when run alone, it is a known cross-test interaction, not a regression in this branch. Confirm by running that file alone before investigating. + +- [ ] **Step 4: Re-check the schema rung** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + grep -n "currentSchemaVersion = " lib/core/database/database.dart | head -1 +``` + +Expected: 171, and still above every claim found by the Task 1 scan. Re-run BOTH scans immediately before pushing, not just when picking the number: a claim can land in between. If another branch has taken 171 since, renumber: the six places from Task 1 Step 6 plus the test filename and this plan, then re-run `flutter test test/core/database/`. + +- [ ] **Step 5: Commit anything the format pass touched** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + git status --short && \ + git add -A && \ + git commit -m "chore: format" || echo "nothing to commit" +``` + +--- + +## What this plan does not do + +- **Range batching.** The Open-Meteo archive endpoint accepts `start_date`/`end_date`, so days sharing a coordinate could collapse into one request. Every day is fetched exactly once ever, so this is an optimization, not a fix. Noted in the spec as a follow-up. +- **A manual refresh action.** Nothing in the UI re-fetches a stored day. Deleting the row is the only way to force a refetch, and no UI exposes that. +- **Weather on dive days.** Dives already store their own, and `TripStoryDay.weather` composes it. diff --git a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md new file mode 100644 index 0000000000..7feed8bc67 --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md @@ -0,0 +1,236 @@ +# Stored trip day weather + +## Problem + +A trip story day that logs no dives shows a weather badge only if the app +fetches one from the network. `surfaceDayWeatherProvider` does that fetch on +every view: it is a `FutureProvider.family` that is deliberately not +auto-disposed, so it caches for the lifetime of the provider container, but +that cache dies with the process. Every cold open of a trip re-hits the +Open-Meteo archive for weather that cannot change, because it is historical. + +Two further gaps follow from the same design: + +- Only surface days fetch at all. `TripStoryDay.isSurface` is false whenever + the day has an itinerary row, so travel days, port days, and sea days show no + weather even though the trip records where the diver was. +- Nothing about the result is durable. It never reaches a backup, another + device, or an export. + +## Goal + +Weather for a trip day is fetched at most once, stored as trip data, and read +from the database on every later view. + +## Scope + +In scope: every trip day whose dives supply no weather, which includes surface +days and dive-free itinerary days. Days in the future are excluded, because a +historical archive has nothing for them. + +Out of scope: dive days. Dives already persist their own weather +(`cloudCover`, `precipitation`, `weatherCode`, `weatherSource`, +`weatherFetchedAt` on the dive row), and `TripStoryDay.weather` already +composes a day summary from them. That remains the higher-precedence source. + +## Storage decision + +The weather lives in a new synced table in the main database rather than in +`local_cache_database.dart` (where `BathymetryCache`, `ReefDataCache`, and +`NoaaTideStations` keep comparable third-party lookups), because it is trip +data: it belongs in backups, in sync, and in exports. + +It gets its own table rather than columns on `trips` or on +`trip_itinerary_days`. Two reasons: + +1. **Row-level conflict resolution.** HLC conflicts are resolved per row. A + background weather write onto the `trips` row or an itinerary row would put + a derived, automatic write in conflict with hand-entered data on the same + row; a weather write racing a trip rename or an itinerary note edit from + another device can lose that edit. Issue #1187 is the standing example of + partial-entity writes wiping fields. +2. **Surface days have no itinerary row.** Materializing one to hold weather + would make the day stop being a surface day, since + `TripStoryDay.hasContent` counts any itinerary row as content, and would + surface a weather-only row in the itinerary tab with a forced `dayType`. + +`LiveaboardDetailRecords` is the existing template for a synced child record +of a trip and is the shape to copy. + +## Data model + +New table `TripDayWeather` in `lib/core/database/database.dart`: + +| Column | Type | Notes | +| --- | --- | --- | +| `id` | text, pk | deterministic UUIDv5 over (`tripId`, day), via `tripDayWeatherRowId` | +| `tripId` | text | references `Trips(#id)` | +| `date` | int | epoch **milliseconds** at **UTC** midnight for the calendar day, via `tripDayMillis`. Milliseconds because that is what `ItineraryDayRepository` writes for `trip_itinerary_days.date`, despite that column comment saying "Unix timestamp". UTC rather than local midnight because the value is part of the row identity: a local midnight epoch differs in every timezone, so two devices would key the same trip day differently and never converge | +| `latitude` | real | the coordinate the lookup used | +| `longitude` | real | the coordinate the lookup used | +| `airTemp` | real, nullable | celsius | +| `cloudCover` | text, nullable | `CloudCover.name` | +| `precipitation` | text, nullable | `Precipitation.name` | +| `windSpeed` | real, nullable | m/s | +| `windDirection` | text, nullable | `CurrentDirection.name` | +| `humidity` | real, nullable | 0-100 | +| `surfacePressure` | real, nullable | bar | +| `weatherCode` | int, nullable | raw WMO code, so prose renders in the diver's locale at display time | +| `weatherSource` | text | defaults to `openMeteo` | +| `fetchedAt` | int | epoch milliseconds | +| `createdAt` | int | epoch milliseconds | +| `updatedAt` | int | epoch milliseconds | +| `hlc` | text, nullable | matches every other synced table | + +Unique index on (`tripId`, `date`). + +The id is **not** a v4 uuid. It is derived from the day it describes, +`UUIDv5(namespace, "$tripId|$dayMillis")`, following the same convention as +`importedDiveComputerId` and `qualityFindingId`. A per-device v4 would let two +devices store the same day under different primary keys; the serializer upserts +by primary key, so the peer's row would miss the `ON CONFLICT` target and hit +the unique index instead, throwing inside the merge transaction and aborting +the whole sync pull. The repository derives the id itself and ignores whatever +a caller passes. + +The stored field set is the full `WeatherData` payload, not just the three +fields the day header renders. The API returns them in one response at no +extra cost, dive rows already store exactly this set, and a later migration to +widen the table is a six-place change on a collision-prone version ladder. +Storing them now costs nothing and renders nothing new. + +Domain entity `TripDayWeather` in +`lib/features/trips/domain/entities/trip_day_weather.dart`, with `copyWith` +per the project convention, plus a mapping to the existing +`TripStoryDayWeather` view model that the day header already consumes. + +## Schema version + +**v171.** Renumbered from 168, which PR #1237 held. Derived by scanning open PR diffs for the scalar, not by grepping +main: main is at v164, and v165, v166, v167 are claimed by PRs #1300, #1290, +and #1276 respectively. Re-verify at implementation time, and re-grep the +scalar after any merge from main, because two branches writing the same number +auto-merge with no conflict marker. + +The claim touches the six places the ladder requires: the +`currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the +`_assertTripDayWeatherSchema()` helper docstring, the `if (from < 171)` +onUpgrade guard and its `reportProgress()` twin, the `beforeOpen` backstop +comment, and the `migration_v171_trip_day_weather_test.dart` filename with its +version assertions. The ladder is non-contiguous by design (v162 is +permanently skipped, and reserved rungs may be missing), so the migration +audit asserts monotonic, unique, and scalar equals max, never contiguous. + +`minimumCompatibleSchemaVersion` does not move: a new table is additive, and +an older build simply ignores it. + +## Fetch and write + +A new `TripDayWeatherRepository` in +`lib/features/trips/data/repositories/trip_day_weather_repository.dart`: + +- `watchWeatherChanges()` emits on every table change, so the display + provider refreshes after a backfill write or a sync import. +- `getForTrip(String tripId)` reads a trip's rows, keyed by + `date.millisecondsSinceEpoch`. +- `upsert(TripDayWeather)` writes one row, keyed on (trip, date) rather than + on id so two devices that both fetch the same day converge on one row. +- `deleteByTripId(String tripId)` removes them, called from the trip-delete + path alongside itinerary days. + +The method names follow `ItineraryDayRepository`, which is the sibling to +copy. + +A backfill service in +`lib/features/trips/domain/services/trip_day_weather_backfill.dart` decides +what to fetch. Given a built `TripStory` and the rows already stored, a day is +fetched only when all of these hold: + +1. `day.weather == null`, so no dive on that day supplies weather. +2. `day.kind != TripStoryDayKind.future`. +3. No stored row exists for (`tripId`, `date`). +4. `story.mapGeometry.nearestPointForDay(index)` yields a coordinate. + +Each qualifying day is fetched through the existing +`WeatherService.fetchWeather` at local noon with `useLocationTimezone: true`, +matching what `surfaceDayWeatherProvider` does today. Requests run +sequentially rather than all at once, since widening from surface days to all +dive-free days raises the first-view request count. Sequential also means rows +land progressively, so headers fill in as results arrive. + +**A fetch that returns null, or returns a `WeatherData` with no usable field, +writes no row and is retried on the next view.** This mirrors the rule already +stated on `ReefDataCache` and `BathymetryCache`: transient failures write no +row. It is also what makes the design correct against the Open-Meteo archive's +few-day lag, since a day fetched too early simply has no row yet and is +retried later. The cost of that policy is that a location with genuinely no +archive data is re-requested once per trip view; that is the same behavior as +today and is bounded by how often a trip is opened. + +Range batching is deliberately not implemented. The archive endpoint accepts a +`start_date`/`end_date` range, so days sharing a coordinate could collapse +into one request (a fixed-base resort trip would go from seven requests to +one), but every day is fetched exactly once ever and then never again. Noted +as a possible follow-up, not built here. + +## Read path + +`trip_story_view.dart` watches `tripDayWeatherProvider(tripId)` once for the +whole trip and passes each day's stored weather into `TripStoryDayHeader`, +replacing the per-day `SurfaceDayWeatherRequest` it builds today. Precedence +in the header is unchanged in spirit: `day.weather ?? storedWeather`, so +dive-logged weather always wins over a fetched summary. + +The view also watches a backfill provider whose job is the side effect of +filling gaps. Because the display provider subscribes to the table's change +tick, a row written by the backfill re-renders the header without the widget +knowing a fetch occurred. + +`lib/features/trips/presentation/providers/surface_day_weather_provider.dart` +and its test are deleted. This design replaces them. + +## Sync + +The serializer works on whole rows (`row.toJson()` on export, +`Entity.fromJson(data).toCompanion(false)` on import), so registering a new +table is mechanical but touches several sites: + +- `sync_repository.dart`'s table map gains + `'tripDayWeather': (table: 'trip_day_weather', pk: 'id')`. +- `sync_data_serializer.dart` gains the payload field, its `toJson`/`fromJson` + entries, its export entry, and the roughly ten `switch` arms that every + synced collection has. +- Tombstones and the trip-deletion FK path follow whatever `itineraryDays` + does, so deleting a trip takes its weather rows with it on every device. + +## Testing + +Tests come first, per the project's TDD rule. + +- **Repository:** upsert, read back, per-trip scoping, and that deleting a + trip removes its weather rows. +- **Backfill:** one test per skip condition (dive-sourced weather present, + future day, row already stored, no coordinate available), plus the two + negative-result cases that must write no row (service returns null; service + returns an all-null `WeatherData`). +- **Migration:** `migration_v171_trip_day_weather_test.dart`, including a + stranded-database fixture at the previous `PRAGMA user_version`, plus the + existing ladder audit in `test/core/database/`. +- **Sync:** a round trip proving a row exports and re-imports intact. +- **Header widget:** a stored row renders the badge; a day whose dives carry + weather still prefers the dive-sourced summary. +- The deleted `surface_day_weather_provider_test.dart` is replaced by the + backfill and header tests above. + +## Risks + +- **Version collision.** The ladder scalar auto-merges with no conflict marker + when two branches write the same number. Re-grep after every merge from + main and re-run `test/core/database/`. +- **Request volume on first view.** Widening from surface days to all + dive-free days increases first-view requests for a trip. Bounded by + sequential fetching, and one-time per day per trip. +- **Coordinate drift.** Stored rows record the coordinate used. If a site + later moves, the stored weather is not re-fetched. Accepted: the row records + what it was fetched for, and a day's weather is not materially sensitive to + a site correction of a few kilometers. diff --git a/lib/core/data/repositories/sync_repository.dart b/lib/core/data/repositories/sync_repository.dart index c7f928a507..2ff9115bdc 100644 --- a/lib/core/data/repositories/sync_repository.dart +++ b/lib/core/data/repositories/sync_repository.dart @@ -47,6 +47,7 @@ class SyncRepository { 'trips': (table: 'trips', pk: 'id'), 'liveaboardDetails': (table: 'liveaboard_detail_records', pk: 'id'), 'itineraryDays': (table: 'trip_itinerary_days', pk: 'id'), + 'tripDayWeather': (table: 'trip_day_weather', pk: 'id'), 'checklistTemplates': (table: 'checklist_templates', pk: 'id'), 'checklistTemplateItems': (table: 'checklist_template_items', pk: 'id'), 'tripChecklistItems': (table: 'trip_checklist_items', pk: 'id'), diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index c530d08c70..ea102b5a04 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -136,6 +136,65 @@ class TripItineraryDays extends Table { Set get primaryKey => {id}; } +/// Fetched historical weather for one trip day, stored for days whose dives +/// supply no weather of their own (surface days and dive-free itinerary days). +/// +/// A separate table rather than columns on `trips` or `trip_itinerary_days` on +/// purpose: HLC conflicts resolve per row, so parking an automatic, derived +/// write on a row the diver also edits by hand would let a weather write race +/// a trip rename or an itinerary note edit and lose it. Weather owns its own +/// row and its own clock. +/// +/// Metric storage throughout (celsius, m/s, bar); conversion to the diver's +/// units happens at display time. +class TripDayWeather extends Table { + // coverage:ignore-start + TextColumn get id => text()(); + TextColumn get tripId => text().references(Trips, #id)(); + + /// UTC midnight for the day, as epoch milliseconds (milliseconds being the + /// convention TripItineraryDays.date is written with). + /// + /// UTC rather than local: this column is half the row identity, and it + /// feeds the derived id. A local midnight epoch differs in every timezone, + /// so two devices would key the same trip day differently and never + /// converge. Write it through tripDayMillis. + IntColumn get date => integer()(); + + /// The coordinates the lookup actually used, so a row records what it was + /// fetched for even if the trip's sites later move. + RealColumn get latitude => real()(); + RealColumn get longitude => real()(); + + RealColumn get airTemp => real().nullable()(); // celsius + TextColumn get cloudCover => text().nullable()(); // enum: CloudCover.name + TextColumn get precipitation => + text().nullable()(); // enum: Precipitation.name + RealColumn get windSpeed => real().nullable()(); // m/s + TextColumn get windDirection => + text().nullable()(); // enum: CurrentDirection.name + RealColumn get humidity => real().nullable()(); // 0-100 + RealColumn get surfacePressure => real().nullable()(); // bar + + /// Raw WMO weather code, kept so the description renders in the diver's + /// locale at display time rather than frozen as English prose at fetch time. + IntColumn get weatherCode => integer().nullable()(); + + TextColumn get weatherSource => + text().withDefault(const Constant('openMeteo'))(); + IntColumn get fetchedAt => integer()(); + IntColumn get createdAt => integer()(); + IntColumn get updatedAt => integer()(); + + /// Hybrid Logical Clock for cross-device conflict resolution + /// (nullable: rows written before HLC rollout fall back to updatedAt). + TextColumn get hlc => text().nullable()(); + // coverage:ignore-end + + @override + Set get primaryKey => {id}; +} + /// Reusable checklist templates for trip planning (issue #164) class ChecklistTemplates extends Table { // coverage:ignore-start @@ -3150,6 +3209,7 @@ String legacyDataSourceId(String diveId) => '$kLegacyDataSourceIdPrefix$diveId'; // Liveaboard tracking (v2.0) LiveaboardDetailRecords, TripItineraryDays, + TripDayWeather, ChecklistTemplates, ChecklistTemplateItems, TripChecklistItems, @@ -3189,7 +3249,7 @@ class AppDatabase extends _$AppDatabase { /// The current schema version as a static constant so that pre-open checks /// (e.g. version-mismatch guard) can reference it without an instance. - static const int currentSchemaVersion = 170; + static const int currentSchemaVersion = 171; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3514,6 +3574,17 @@ class AppDatabase extends _$AppDatabase { // landed 168 past it, so PR #1276 moved its rung up), and 169 belongs to // PR #1320 (dive-computer gear twins). 170, + // v171: trip_day_weather, fetched historical weather for trip days whose + // dives supply none. Renumbered from 168, which PR #1237 (issue #638, + // buddies.is_favorite) had already claimed and pushed; that claim was + // local and unpushed when this branch picked its number, so an open-PR + // scan could not see it. + // 165, 167 and 169 are deliberately absent, not missing: 165 is claimed by + // PR #1290, while 167 and 169 are permanently skipped. main landed past + // both while their branches were open, so PR #1276 moved to 173 and + // PR #1320 to 175. This ladder is non-contiguous by design; the audit + // asserts monotonic, unique, and scalar == max, never contiguous. + 171, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -3967,6 +4038,41 @@ class AppDatabase extends _$AppDatabase { /// v129: quality_findings table for the Data Quality Assistant. /// Idempotent so it is safe to call from both onUpgrade and the /// beforeOpen backstop. + /// v171: fetched per-day trip weather. + /// + /// Idempotent, so it doubles as the beforeOpen backstop for a database + /// stranded at 171 by a parallel branch that never created the table. + Future _assertTripDayWeatherSchema() async { + await customStatement(''' + CREATE TABLE IF NOT EXISTS trip_day_weather ( + id TEXT NOT NULL PRIMARY KEY, + trip_id TEXT NOT NULL REFERENCES trips (id), + date INTEGER NOT NULL, + latitude REAL NOT NULL, + longitude REAL NOT NULL, + air_temp REAL, + cloud_cover TEXT, + precipitation TEXT, + wind_speed REAL, + wind_direction TEXT, + humidity REAL, + surface_pressure REAL, + weather_code INTEGER, + weather_source TEXT NOT NULL DEFAULT 'openMeteo', + fetched_at INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + hlc TEXT + ) + '''); + // The day is the identity: two devices that both fetch it must converge + // on one row rather than accumulating duplicates. + await customStatement( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_trip_day_weather_trip_date ' + 'ON trip_day_weather (trip_id, date)', + ); + } + Future _assertQualityFindingsSchema() async { await customStatement(''' CREATE TABLE IF NOT EXISTS quality_findings ( @@ -8770,6 +8876,12 @@ class AppDatabase extends _$AppDatabase { await _rewriteLegacySacRateLayouts(); } if (from < 170) await reportProgress(); + // v171: trip_day_weather, fetched per-day weather for trip days whose + // dives supply none. + if (from < 171) { + await _assertTripDayWeatherSchema(); + } + if (from < 171) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8982,6 +9094,11 @@ class AppDatabase extends _$AppDatabase { // value map (discussions #354, #803; same restore / sync-adopt // self-heal as v160). The layout rewrite deliberately stays rung-only. await _assertGasConsumptionDisplayColumn(); + // v171 backstop: re-assert trip_day_weather (same parallel-branch + // version-collision self-heal). The helper is CREATE TABLE IF NOT + // EXISTS plus CREATE UNIQUE INDEX IF NOT EXISTS, so it is a no-op on + // every open after the first. + await _assertTripDayWeatherSchema(); // v145 backstop: re-assert the gps_tracks provenance and trim columns. await _assertGpsTrackColumns(); diff --git a/lib/core/services/sync/sync_data_serializer.dart b/lib/core/services/sync/sync_data_serializer.dart index 83b3e40c97..90276fff3c 100644 --- a/lib/core/services/sync/sync_data_serializer.dart +++ b/lib/core/services/sync/sync_data_serializer.dart @@ -248,6 +248,7 @@ class SyncData { final List> trips; final List> liveaboardDetails; final List> itineraryDays; + final List> tripDayWeather; final List> checklistTemplates; final List> checklistTemplateItems; final List> tripChecklistItems; @@ -323,6 +324,7 @@ class SyncData { this.trips = const [], this.liveaboardDetails = const [], this.itineraryDays = const [], + this.tripDayWeather = const [], this.checklistTemplates = const [], this.checklistTemplateItems = const [], this.tripChecklistItems = const [], @@ -399,6 +401,7 @@ class SyncData { 'trips': trips, 'liveaboardDetails': liveaboardDetails, 'itineraryDays': itineraryDays, + 'tripDayWeather': tripDayWeather, 'checklistTemplates': checklistTemplates, 'checklistTemplateItems': checklistTemplateItems, 'tripChecklistItems': tripChecklistItems, @@ -476,6 +479,7 @@ class SyncData { trips: _parseList(json['trips']), liveaboardDetails: _parseList(json['liveaboardDetails']), itineraryDays: _parseList(json['itineraryDays']), + tripDayWeather: _parseList(json['tripDayWeather']), checklistTemplates: _parseList(json['checklistTemplates']), checklistTemplateItems: _parseList(json['checklistTemplateItems']), tripChecklistItems: _parseList(json['tripChecklistItems']), @@ -765,6 +769,7 @@ class SyncDataSerializer { blob: false, full: null, ), + (key: 'tripDayWeather', table: _db.tripDayWeather, blob: false, full: null), ( key: 'checklistTemplates', table: _db.checklistTemplates, @@ -1275,6 +1280,10 @@ class SyncDataSerializer { 'itineraryDays', () => _exportItineraryDays(hlcSince), ), + tripDayWeather: await _safeExport( + 'tripDayWeather', + () => _exportTripDayWeather(hlcSince), + ), checklistTemplates: await _safeExport( 'checklistTemplates', () => _exportChecklistTemplates(hlcSince), @@ -1715,6 +1724,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.equals(recordId))).getSingleOrNull(); return row?.toJson(); + case 'tripDayWeather': + final row = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).getSingleOrNull(); + return row?.toJson(); case 'checklistTemplates': final row = await (_db.select( _db.checklistTemplates, @@ -2040,6 +2054,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.isIn(idList))).get(); return {for (final r in rows) r.id: r.toJson()}; + case 'tripDayWeather': + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.isIn(idList))).get(); + return {for (final r in rows) r.id: r.toJson()}; case 'checklistTemplates': final rows = await (_db.select( _db.checklistTemplates, @@ -2602,6 +2621,13 @@ class SyncDataSerializer { TripItineraryDay.fromJson(data).toCompanion(false), ); return; + case 'tripDayWeather': + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherData.fromJson(data).toCompanion(false), + ); + return; case 'checklistTemplates': await _db .into(_db.checklistTemplates) @@ -3209,6 +3235,16 @@ class SyncDataSerializer { ), ); return; + case 'tripDayWeather': + await _db.batch( + (b) => b.insertAllOnConflictUpdate( + _db.tripDayWeather, + records + .map((r) => TripDayWeatherData.fromJson(r).toCompanion(false)) + .toList(), + ), + ); + return; case 'checklistTemplates': await _db.batch( (b) => b.insertAllOnConflictUpdate( @@ -3686,6 +3722,8 @@ class SyncDataSerializer { ); case 'itineraryDays': return plain(_db.tripItineraryDays, _db.tripItineraryDays.id); + case 'tripDayWeather': + return plain(_db.tripDayWeather, _db.tripDayWeather.id); case 'checklistTemplates': return plain(_db.checklistTemplates, _db.checklistTemplates.id); case 'checklistTemplateItems': @@ -3919,6 +3957,8 @@ class SyncDataSerializer { return _db.liveaboardDetailRecords; case 'itineraryDays': return _db.tripItineraryDays; + case 'tripDayWeather': + return _db.tripDayWeather; case 'checklistTemplates': return _db.checklistTemplates; case 'checklistTemplateItems': @@ -4230,6 +4270,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.equals(recordId))).go(); return; + case 'tripDayWeather': + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).go(); + return; case 'checklistTemplates': await (_db.delete( _db.checklistTemplates, @@ -4906,6 +4951,17 @@ class SyncDataSerializer { return rows.map((r) => r.toJson()).toList(); } + Future>> _exportTripDayWeather( + String? hlcSince, + ) async { + final query = _db.select(_db.tripDayWeather); + if (hlcSince != null) { + query.where((t) => t.hlc.isBiggerThanValue(hlcSince)); + } + final rows = await query.get(); + return rows.map((r) => r.toJson()).toList(); + } + Future>> _exportChecklistTemplates( String? hlcSince, ) async { diff --git a/lib/core/services/sync/sync_service.dart b/lib/core/services/sync/sync_service.dart index 5fd1999a7f..89ea5b0d73 100644 --- a/lib/core/services/sync/sync_service.dart +++ b/lib/core/services/sync/sync_service.dart @@ -1210,6 +1210,11 @@ class SyncService { records: data.itineraryDays, hasUpdatedAt: true, ), + ( + type: 'tripDayWeather', + records: data.tripDayWeather, + hasUpdatedAt: true, + ), ( type: 'checklistTemplates', records: data.checklistTemplates, @@ -1957,6 +1962,7 @@ class SyncService { 'trips': true, 'liveaboardDetails': true, 'itineraryDays': true, + 'tripDayWeather': true, 'checklistTemplates': true, 'checklistTemplateItems': true, 'tripChecklistItems': true, @@ -2118,6 +2124,7 @@ class SyncService { 'siteFeatures': [(field: 'siteId', parent: 'diveSites', nullable: false)], 'liveaboardDetails': [(field: 'tripId', parent: 'trips', nullable: false)], 'itineraryDays': [(field: 'tripId', parent: 'trips', nullable: false)], + 'tripDayWeather': [(field: 'tripId', parent: 'trips', nullable: false)], 'checklistTemplateItems': [ (field: 'templateId', parent: 'checklistTemplates', nullable: false), ], diff --git a/lib/features/trips/data/repositories/trip_day_weather_repository.dart b/lib/features/trips/data/repositories/trip_day_weather_repository.dart new file mode 100644 index 0000000000..30f2bda321 --- /dev/null +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -0,0 +1,280 @@ +import 'package:collection/collection.dart'; +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + as domain; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + show tripDayDate, tripDayMillis, tripDayWeatherRowId; + +/// Reads and writes stored per-day trip weather. +/// +/// The day is the identity, not the row id: `upsert` replaces any existing +/// row for the same (trip, date), so two devices that both fetch the same day +/// converge on one row rather than accumulating duplicates. +class TripDayWeatherRepository { + AppDatabase get _db => DatabaseService.instance.database; + final SyncRepository _syncRepository = SyncRepository(); + final _log = LoggerService.forClass(TripDayWeatherRepository); + + /// The UTC-midnight day key for [date], as epoch milliseconds. + /// + /// The day is the identity, so normalizing here is what actually enforces + /// the (trip, date) uniqueness intent. A caller that passes a DateTime with + /// a time component would otherwise store a second row for the same + /// calendar day, invisible to every midnight-keyed lookup and refetched on + /// every view. Reads normalize too, because a row can also arrive through + /// sync from a peer, bypassing this class entirely. + static int _dayKey(DateTime date) => tripDayMillis(date); + + /// Emits whenever `trip_day_weather` changes, so the display provider + /// refreshes after a backfill write or a sync import. + Stream watchWeatherChanges() => + _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); + + /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + /// + /// One entry per calendar day. Where more than one row lands on the same + /// day, [_preferred] picks which one shows, and explains how a second row + /// gets there in the first place. + Future> getForTrip(String tripId) async { + try { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + + final winners = {}; + for (final row in rows) { + final day = _dayKey(tripDayDate(row.date)); + final held = winners[day]; + winners[day] = held == null + ? row + : _preferred(held, row, tripId: tripId, dayMillis: day); + } + + return winners.map((day, row) => MapEntry(day, _mapRow(row))); + } catch (e, stackTrace) { + _log.error( + 'Failed to read weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Insert or replace one day's weather. + Future upsert(domain.TripDayWeather weather) async { + try { + final now = DateTime.now().millisecondsSinceEpoch; + final dateMillis = _dayKey(weather.date); + + // The id is derived from (trip, day), never taken from the caller, so + // every device writing this day produces the same primary key and sync + // merges by id instead of colliding on the unique index. + final id = tripDayWeatherRowId( + tripId: weather.tripId, + dayMillis: dateMillis, + ); + + final sameDay = await _rowsForDay( + tripId: weather.tripId, + dayMillis: dateMillis, + ); + // Only to preserve createdAt across an update; insertOnConflictUpdate + // would otherwise overwrite it with this write's timestamp. A stray is + // this day under an old id rather than a different record, so the day + // keeps the age it already had when one is absorbed. + final createdAt = sameDay.map((r) => r.createdAt).minOrNull; + final strays = sameDay.where((r) => r.id != id).map((r) => r.id).toList(); + + // Atomic, following the pattern the #553 review established in + // BuddyRepository.deleteBuddy: the row change and its sync bookkeeping + // commit together, and only the observable event is deferred to after + // the commit. A tombstone written outside the transaction could be lost + // while its delete stood, and nothing would ever repair that: the + // backfill skips any day that already has a stored row, so once the + // canonical row exists this day is never upserted again and a + // resurrected stray would sit in the table for good. + await _db.transaction(() async { + // Strays go before the insert, not after. insertOnConflictUpdate + // targets the primary key, so a stray sitting on this same + // (trip_id, date) is not a conflict it can absorb: the insert misses + // the ON CONFLICT target and hits the unique index instead, which + // throws and fails the whole write. + if (strays.isNotEmpty) { + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.isIn(strays))).go(); + } + + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherCompanion( + id: Value(id), + tripId: Value(weather.tripId), + date: Value(dateMillis), + latitude: Value(weather.latitude), + longitude: Value(weather.longitude), + airTemp: Value(weather.airTemp), + cloudCover: Value(weather.cloudCover?.name), + precipitation: Value(weather.precipitation?.name), + windSpeed: Value(weather.windSpeed), + windDirection: Value(weather.windDirection?.name), + humidity: Value(weather.humidity), + surfacePressure: Value(weather.surfacePressure), + weatherCode: Value(weather.weatherCode), + weatherSource: Value(weather.weatherSource.name), + fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), + createdAt: Value( + createdAt ?? weather.createdAt.millisecondsSinceEpoch, + ), + updatedAt: Value(now), + ), + ); + + // A stray is a synced record: dropping it without a tombstone lets + // the peer that sent it hand it straight back on the next pull. + for (final stray in strays) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: stray, + ); + } + + await _syncRepository.markRecordPending( + entityType: 'tripDayWeather', + recordId: id, + localUpdatedAt: now, + ); + }); + + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to store weather for trip: ${weather.tripId}', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Delete every stored day for a trip, logging each id for sync. + Future deleteByTripId(String tripId) async { + try { + final existing = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + if (existing.isEmpty) return; + + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).go(); + + for (final row in existing) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: row.id, + ); + } + SyncEventBus.notifyLocalChange(); + + _log.info('Deleted ${existing.length} weather days for trip: $tripId'); + } catch (e, stackTrace) { + _log.error( + 'Failed to delete weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Every stored row for this trip that falls on [dayMillis]'s calendar day. + /// + /// Filtered in Dart rather than SQL. It could be pushed down now that the + /// key is UTC, since the day is plain integer arithmetic on the stored + /// millis with no zone or DST to consult, but there is nothing to gain: a + /// trip holds one row per day, so the scan is a few dozen rows, and keeping + /// the rule in one Dart function is what stops it drifting from + /// [tripDayMillis]. + Future> _rowsForDay({ + required String tripId, + required int dayMillis, + }) async { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + return rows + .where((r) => _dayKey(tripDayDate(r.date)) == dayMillis) + .toList(); + } + + /// Which of two rows for the same calendar day to show. + /// + /// Two rows reach one day only when [upsert] did not write one of them: a + /// peer on a build that predates the derived id, or a database written + /// before this class normalized. [upsert] clears them out, but a read can + /// land between a sync import and the next write, and it cannot tidy up + /// itself: a delete here would fire the table tick that the display + /// provider subscribes to and invalidate the read in flight. So it chooses. + /// + /// The canonical row wins, so what shows now is what the next upsert keeps. + /// Failing that the most recently updated wins, and an exact tie falls back + /// to the id, so the answer never depends on the order SQLite returned the + /// rows in. + TripDayWeatherData _preferred( + TripDayWeatherData a, + TripDayWeatherData b, { + required String tripId, + required int dayMillis, + }) { + final canonicalId = tripDayWeatherRowId( + tripId: tripId, + dayMillis: dayMillis, + ); + if (a.id == canonicalId) return a; + if (b.id == canonicalId) return b; + if (a.updatedAt != b.updatedAt) return a.updatedAt > b.updatedAt ? a : b; + return a.id.compareTo(b.id) <= 0 ? a : b; + } + + domain.TripDayWeather _mapRow(TripDayWeatherData row) { + return domain.TripDayWeather( + id: row.id, + tripId: row.tripId, + // Normalized, matching the map key getForTrip returns it under: a row + // written by an older build or an out-of-date peer can still carry a + // time component, and handing that back would put time-bearing dates + // into downstream logic. + date: tripDayDate(_dayKey(tripDayDate(row.date))), + latitude: row.latitude, + longitude: row.longitude, + airTemp: row.airTemp, + cloudCover: row.cloudCover == null + ? null + : CloudCover.values.byName(row.cloudCover!), + precipitation: row.precipitation == null + ? null + : Precipitation.values.byName(row.precipitation!), + windSpeed: row.windSpeed, + windDirection: row.windDirection == null + ? null + : CurrentDirection.values.byName(row.windDirection!), + humidity: row.humidity, + surfacePressure: row.surfacePressure, + weatherCode: row.weatherCode, + weatherSource: WeatherSource.values.byName(row.weatherSource), + fetchedAt: DateTime.fromMillisecondsSinceEpoch(row.fetchedAt), + createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), + updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), + ); + } +} diff --git a/lib/features/trips/data/repositories/trip_repository.dart b/lib/features/trips/data/repositories/trip_repository.dart index b1d3478910..0f97eb5cca 100644 --- a/lib/features/trips/data/repositories/trip_repository.dart +++ b/lib/features/trips/data/repositories/trip_repository.dart @@ -12,6 +12,7 @@ import 'package:submersion/features/checklists/data/repositories/trip_checklist_ import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; import 'package:submersion/features/trips/data/repositories/itinerary_day_repository.dart'; import 'package:submersion/features/trips/data/repositories/liveaboard_details_repository.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; import 'package:submersion/features/trips/domain/entities/dive_candidate.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart' as domain; @@ -265,6 +266,7 @@ class TripRepository { await LiveaboardDetailsRepository().deleteByTripId(id); await ItineraryDayRepository().deleteByTripId(id); await TripChecklistRepository().deleteByTripId(id); + await TripDayWeatherRepository().deleteByTripId(id); // Remove trip association from dives (nullable FK) await _db.customUpdate( diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart new file mode 100644 index 0000000000..c8fb0108c1 --- /dev/null +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -0,0 +1,216 @@ +import 'package:equatable/equatable.dart'; +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// Fixed namespace for deterministic trip-day-weather ids (UUIDv5). +/// Never change: the ids already stored depend on it. +const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; + +/// The calendar day of [date] as epoch milliseconds at UTC midnight. +/// +/// The day is the identity of a weather row, so this is what turns a +/// DateTime into one. Shared rather than reimplemented per caller: the row +/// id, the repository's storage key, and the map key reads come back under +/// all have to agree, and three copies of the same two lines would drift. +/// +/// UTC, not local midnight, and the distinction is the whole point. A local +/// `DateTime(y, m, d)` has a different epoch value in every timezone, so two +/// devices looking at the same trip day would derive different keys, and +/// therefore different row ids, and never converge: each would store and +/// refetch its own copy of the day. Divers cross timezones by definition, and +/// one diver flying home is enough to trigger it. +/// +/// The calendar fields are taken as given rather than converted. `toUtc()` +/// would shift a late evening onto the following day; the trip story already +/// hands over a date whose y/m/d is the day it means, matching the app's +/// wall-clock-as-UTC convention for dive timestamps. +int tripDayMillis(DateTime date) => + DateTime.utc(date.year, date.month, date.day).millisecondsSinceEpoch; + +/// The instant [dayMillis] denotes, read in UTC. +/// +/// The only correct way to read a stored day back. `fromMillisecondsSinceEpoch` +/// without `isUtc` returns a LOCAL DateTime, so re-extracting y/m/d from it +/// reads the calendar fields in the device's frame: on any negative UTC +/// offset, UTC midnight is the previous evening locally, and the day walks +/// backwards on every round trip. +/// +/// It reads the value it is given and normalizes nothing, so it is the +/// inverse of [tripDayMillis] only for a value [tripDayMillis] produced. A +/// stored `date` is not guaranteed to be one: rows written before this branch +/// derived ids can carry a time component, and the repository deliberately +/// calls this on those raw values. Run the result back through +/// [tripDayMillis] whenever you need the normalized day rather than the +/// instant as stored. +DateTime tripDayDate(int dayMillis) => + DateTime.fromMillisecondsSinceEpoch(dayMillis, isUtc: true); + +/// Deterministic row id for one trip day. +/// +/// The day is the identity, so the id must be derived from it rather than +/// minted per device. Two devices that both fetch the same day would +/// otherwise insert two rows, and the unique (trip_id, date) index turns the +/// second one into an inbound-sync failure rather than a merge: the +/// serializer upserts by primary key, so a differing id misses the conflict +/// target entirely and hits the index instead. That throws inside the merge +/// transaction and aborts the whole sync pull. +/// +/// [dayMillis] must already be the normalized UTC-midnight day key; pass it +/// through [tripDayMillis]. Local midnight would defeat the purpose: its epoch +/// value differs in every timezone, so the derived id would too and the two +/// devices this exists to converge would not. +String tripDayWeatherRowId({required String tripId, required int dayMillis}) => + const Uuid().v5(kTripDayWeatherNamespace, '$tripId|$dayMillis'); + +/// Stored historical weather for one trip day. +/// +/// Written only for days whose dives supply no weather of their own; a day +/// with dive-logged weather renders that instead, because it is what the +/// diver actually recorded. +/// +/// Metric throughout (celsius, m/s, bar). Conversion to the diver's units +/// happens at display time. +class TripDayWeather extends Equatable { + final String id; + final String tripId; + + /// Local midnight for the day this describes. + final DateTime date; + + /// The coordinates the lookup used. + final double latitude; + final double longitude; + + final double? airTemp; // celsius + final CloudCover? cloudCover; + final Precipitation? precipitation; + final double? windSpeed; // m/s + final CurrentDirection? windDirection; + final double? humidity; // 0-100 + final double? surfacePressure; // bar + + /// Raw WMO weather code (0 clear, 61 rain, 95 thunderstorm, ...), kept so + /// the description can be rendered in the diver's locale at display time. + final int? weatherCode; + + final WeatherSource weatherSource; + final DateTime fetchedAt; + final DateTime createdAt; + final DateTime updatedAt; + + const TripDayWeather({ + required this.id, + required this.tripId, + required this.date, + required this.latitude, + required this.longitude, + this.airTemp, + this.cloudCover, + this.precipitation, + this.windSpeed, + this.windDirection, + this.humidity, + this.surfacePressure, + this.weatherCode, + this.weatherSource = WeatherSource.openMeteo, + required this.fetchedAt, + required this.createdAt, + required this.updatedAt, + }); + + /// True when the day header's badge would actually show something. + /// + /// Wind, humidity, and pressure are stored but never drawn, so a row + /// carrying only those renders as nothing and would suppress the retry that + /// a later archive update would satisfy. + /// + /// Delegates to [TripStoryDayWeather.isRenderable] so the rule that decides + /// what is worth storing is the same one that decides what the header can + /// draw, and the same one the backfill uses to judge a day's dive weather. + bool get hasRenderableWeather => toStoryWeather().isRenderable; + + /// The compact view model the day header consumes. + TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); + + TripDayWeather copyWith({ + String? id, + String? tripId, + DateTime? date, + double? latitude, + double? longitude, + Object? airTemp = _undefined, + Object? cloudCover = _undefined, + Object? precipitation = _undefined, + Object? windSpeed = _undefined, + Object? windDirection = _undefined, + Object? humidity = _undefined, + Object? surfacePressure = _undefined, + Object? weatherCode = _undefined, + WeatherSource? weatherSource, + DateTime? fetchedAt, + DateTime? createdAt, + DateTime? updatedAt, + }) { + return TripDayWeather( + id: id ?? this.id, + tripId: tripId ?? this.tripId, + date: date ?? this.date, + latitude: latitude ?? this.latitude, + longitude: longitude ?? this.longitude, + airTemp: airTemp == _undefined ? this.airTemp : airTemp as double?, + cloudCover: cloudCover == _undefined + ? this.cloudCover + : cloudCover as CloudCover?, + precipitation: precipitation == _undefined + ? this.precipitation + : precipitation as Precipitation?, + windSpeed: windSpeed == _undefined + ? this.windSpeed + : windSpeed as double?, + windDirection: windDirection == _undefined + ? this.windDirection + : windDirection as CurrentDirection?, + humidity: humidity == _undefined ? this.humidity : humidity as double?, + surfacePressure: surfacePressure == _undefined + ? this.surfacePressure + : surfacePressure as double?, + weatherCode: weatherCode == _undefined + ? this.weatherCode + : weatherCode as int?, + weatherSource: weatherSource ?? this.weatherSource, + fetchedAt: fetchedAt ?? this.fetchedAt, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } + + @override + List get props => [ + id, + tripId, + date, + latitude, + longitude, + airTemp, + cloudCover, + precipitation, + windSpeed, + windDirection, + humidity, + surfacePressure, + weatherCode, + weatherSource, + fetchedAt, + createdAt, + updatedAt, + ]; +} + +// Sentinel value for distinguishing null from undefined in copyWith +const _undefined = Object(); diff --git a/lib/features/trips/domain/entities/trip_story_day.dart b/lib/features/trips/domain/entities/trip_story_day.dart index 000e73b5da..70d272153a 100644 --- a/lib/features/trips/domain/entities/trip_story_day.dart +++ b/lib/features/trips/domain/entities/trip_story_day.dart @@ -133,6 +133,19 @@ class TripStoryDayWeather extends Equatable { this.precipitation, }); + /// True when the day header's badge would actually draw something. + /// + /// [Precipitation.none] does not count, and that is the whole point of this + /// getter. `WeatherMapper.mapPrecipitation` never returns null: a missing + /// reading becomes `none`, so a dive whose weather lookup resolved nothing + /// still stores `none`. `weatherIconFor` gives `none` no glyph of its own, + /// so such a day renders as blank. Treating it as "this day has weather" + /// would leave the day badge-free forever. + bool get isRenderable => + airTemp != null || + cloudCover != null || + (precipitation != null && precipitation != Precipitation.none); + @override List get props => [airTemp, cloudCover, precipitation]; } diff --git a/lib/features/trips/domain/services/trip_day_weather_backfill.dart b/lib/features/trips/domain/services/trip_day_weather_backfill.dart new file mode 100644 index 0000000000..5cd9a66bca --- /dev/null +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -0,0 +1,85 @@ +import 'package:equatable/equatable.dart'; + +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// One trip day that needs a weather lookup, with the coordinates to look it +/// up at. +class TripDayWeatherTarget extends Equatable { + /// Local midnight for the day. + final DateTime date; + final double latitude; + final double longitude; + + const TripDayWeatherTarget({ + required this.date, + required this.latitude, + required this.longitude, + }); + + /// The hour to sample. Noon local reads as "the day's weather" far better + /// than the archive's default midnight boundary, which straddles two days. + DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); + + @override + List get props => [date, latitude, longitude]; +} + +/// Decides which trip days need a weather lookup. +/// +/// Pure by design: no database, no network, no clock. Every skip rule is a +/// plain condition over the built story and the rows already stored, which is +/// what makes each one directly testable. +class TripDayWeatherBackfill { + const TripDayWeatherBackfill._(); + + static List targetsFor({ + required TripStory story, + required Map stored, + }) { + final targets = []; + + for (var index = 0; index < story.days.length; index++) { + final day = story.days[index]; + + // A dive that logged weather is the better source: it is what the diver + // recorded. Never override it with a fetched summary. + // + // Renderability, not mere presence, is the test. A dive whose weather + // lookup resolved nothing still stores Precipitation.none, because + // WeatherMapper never returns null precipitation, and that renders as a + // blank badge. Skipping on presence alone would leave such a day + // permanently badge-free. + if (day.weather?.isRenderable ?? false) continue; + + // A historical archive has nothing for a day that has not happened. + if (day.kind == TripStoryDayKind.future) continue; + + // Ask with the key the rows are actually stored under. getForTrip keys + // by tripDayMillis, which is UTC midnight so two devices converge on one + // row; a local DateTime's epoch agrees with that only at UTC+0. Building + // the lookup from the local value instead made every stored day read as + // missing on any other device and refetch on every view, which is the + // thing this whole feature exists to stop. + final date = DateTime(day.date.year, day.date.month, day.date.day); + if (stored.containsKey(tripDayMillis(date))) continue; + + // nearestPointForDay walks outward from the day, so a dive-free day + // between two dived days borrows the closer one's coordinates. A trip + // with no mappable point anywhere has nowhere to ask. + final point = story.mapGeometry.nearestPointForDay(index); + if (point == null) continue; + + targets.add( + TripDayWeatherTarget( + date: date, + latitude: point.latitude, + longitude: point.longitude, + ), + ); + } + + return targets; + } +} diff --git a/lib/features/trips/presentation/providers/surface_day_weather_provider.dart b/lib/features/trips/presentation/providers/surface_day_weather_provider.dart deleted file mode 100644 index daedc7a4bb..0000000000 --- a/lib/features/trips/presentation/providers/surface_day_weather_provider.dart +++ /dev/null @@ -1,58 +0,0 @@ -import 'package:equatable/equatable.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; - -import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; - -/// One historical-weather lookup for a contentless trip day. -/// -/// Value equality makes this a stable Riverpod family key, so scrolling a day -/// header out of the sliver tree and back in reuses the same in-memory result. -class SurfaceDayWeatherRequest extends Equatable { - final DateTime date; - final double latitude; - final double longitude; - - const SurfaceDayWeatherRequest({ - required this.date, - required this.latitude, - required this.longitude, - }); - - DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); - - @override - List get props => [date, latitude, longitude]; -} - -/// Best-effort historical weather for a trip surface day. -/// -/// Intentionally not auto-disposed: a trip story can repeatedly mount and -/// unmount day slivers while scrolling, and each request should be fetched at -/// most once during the provider container's lifetime. -final surfaceDayWeatherProvider = - FutureProvider.family(( - ref, - request, - ) async { - final weather = await ref - .watch(weatherServiceProvider) - .fetchWeather( - latitude: request.latitude, - longitude: request.longitude, - date: DateTime( - request.date.year, - request.date.month, - request.date.day, - ), - entryTime: request.localNoon, - useLocationTimezone: true, - ); - if (weather == null) return null; - - return TripStoryDayWeather( - airTemp: weather.airTemp, - cloudCover: weather.cloudCover, - precipitation: weather.precipitation, - ); - }); diff --git a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart new file mode 100644 index 0000000000..14bfaa6e11 --- /dev/null +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -0,0 +1,104 @@ +import 'package:submersion/core/providers/provider.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +final tripDayWeatherRepositoryProvider = Provider( + (ref) => TripDayWeatherRepository(), +); + +/// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. +/// +/// Subscribes to the table tick, so a row written by the backfill or arriving +/// through sync re-renders the day headers without the widget knowing a fetch +/// ever happened. +final tripDayWeatherProvider = + FutureProvider.family, String>(( + ref, + tripId, + ) async { + final repository = ref.watch(tripDayWeatherRepositoryProvider); + ref.invalidateSelfWhen(repository.watchWeatherChanges()); + return repository.getForTrip(tripId); + }); + +/// Fills the gaps: fetches historical weather for trip days that have none +/// stored and no dive to supply it, then writes what it finds. +/// +/// The story is its only reactive input, so assigning dives to a trip +/// re-evaluates what is still missing. +/// +/// autoDispose is what makes the retry policy above true. A miss writes no +/// row precisely so the day is tried again later; a provider that outlived +/// the view would serve its completed state on every subsequent navigation +/// and a transient failure would stand until the app restarted. Disposing +/// with the view means returning to the trip runs the pass again, which skips +/// the days that succeeded and retries only the ones still missing. While the +/// view stays mounted the provider stays alive, so scrolling does not refetch. +// no-tick: a side-effecting pass, not a cached query. It renders nothing (the +// value is void), and subscribing to the weather tick would make every row it +// writes invalidate the pass that wrote it. Its rows reach the UI through +// [tripDayWeatherProvider], which does subscribe. A stale read costs nothing: +// a row it misses is simply not refetched, and a stored day is skipped anyway. +final tripDayWeatherBackfillProvider = FutureProvider.autoDispose + .family((ref, tripId) async { + final story = await ref.watch(tripStoryProvider(tripId).future); + final repository = ref.watch(tripDayWeatherRepositoryProvider); + final service = ref.watch(weatherServiceProvider); + + final stored = await repository.getForTrip(tripId); + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: stored, + ); + if (targets.isEmpty) return; + + // Sequential on purpose: a two-week trip would otherwise open with a burst + // of parallel requests, and rows landing one at a time let the day headers + // fill in progressively. + for (final target in targets) { + final weather = await service.fetchWeather( + latitude: target.latitude, + longitude: target.longitude, + date: target.date, + entryTime: target.localNoon, + useLocationTimezone: true, + ); + // A miss writes nothing and is retried on the next view, which is what + // makes this correct against the archive's few-day publication lag. + if (weather == null) continue; + + final now = DateTime.now(); + final dayMillis = tripDayMillis(target.date); + final row = TripDayWeather( + // The repository derives this same id from (trip, day) and never takes + // the caller's, so every device converges on one row. Derived here too + // rather than left as a placeholder: an entity carrying an id that is + // not its own reaches logs and any future validation as a lie. + id: tripDayWeatherRowId(tripId: tripId, dayMillis: dayMillis), + tripId: tripId, + date: target.date, + latitude: target.latitude, + longitude: target.longitude, + airTemp: weather.airTemp, + cloudCover: weather.cloudCover, + precipitation: weather.precipitation, + windSpeed: weather.windSpeed, + windDirection: weather.windDirection, + humidity: weather.humidity, + surfacePressure: weather.surfacePressure, + weatherCode: weather.weatherCode, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + + // A row the header could render nothing from is worse than no row: it + // would suppress the retry that a later archive update would satisfy. + if (!row.hasRenderableWeather) continue; + + await repository.upsert(row); + } + }); diff --git a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart index fc5f3d30b1..cf55cc3427 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart @@ -9,7 +9,6 @@ import 'package:submersion/features/settings/presentation/providers/settings_pro import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; import 'package:submersion/features/trips/presentation/helpers/day_type_l10n.dart'; import 'package:submersion/features/trips/presentation/helpers/weather_icon.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; import 'package:submersion/l10n/arb/app_localizations.dart'; import 'package:submersion/l10n/l10n_extension.dart'; @@ -18,8 +17,9 @@ import 'package:submersion/l10n/l10n_extension.dart'; /// (surfaceContainer, one step above the page surface) so the sticky headers /// read as chapter anchors and day cards scroll underneath cleanly. The badge /// echoes the map's primary-colored day pins, tying header and pin together. -/// Days with logged weather get a trailing badge: conditions icon plus air -/// temperature in the diver's temperature unit. +/// Days with logged or stored weather get a trailing badge: conditions icon +/// plus air temperature in the diver's temperature unit. The header never +/// fetches: it renders whatever weather it is handed. /// /// Every day of the trip gets one of these, including surface days (which /// carry no card body at all and are nothing but this header). Presenting a @@ -36,13 +36,12 @@ class TripStoryDayHeader extends ConsumerWidget { static const double minHeight = 52; final TripStoryDay day; - final SurfaceDayWeatherRequest? surfaceWeatherRequest; - const TripStoryDayHeader({ - super.key, - required this.day, - this.surfaceWeatherRequest, - }); + /// Weather stored for this day, passed down by the story view from one + /// per-trip read. Null when nothing is stored yet. + final TripStoryDayWeather? storedWeather; + + const TripStoryDayHeader({super.key, required this.day, this.storedWeather}); @override Widget build(BuildContext context, WidgetRef ref) { @@ -62,11 +61,15 @@ class TripStoryDayHeader extends ConsumerWidget { ...day.siteNames, ].map((part) => part.trim()).where((part) => part.isNotEmpty).toList(); - final request = day.isSurface ? surfaceWeatherRequest : null; - final fetchedWeather = request == null - ? null - : ref.watch(surfaceDayWeatherProvider(request)).asData?.value; - final weather = day.weather ?? fetchedWeather; + // Dive-logged weather wins: it is what the diver recorded, and a stored + // day summary is only ever a stand-in for days that logged none. It wins + // only when it can actually draw something, though: a dive whose weather + // lookup resolved nothing still carries Precipitation.none, and letting + // that mask a stored row would render the day blank. + final diveWeather = day.weather; + final weather = (diveWeather != null && diveWeather.isRenderable) + ? diveWeather + : (storedWeather ?? diveWeather); final units = UnitFormatter(ref.watch(settingsProvider)); final weatherBadge = _weatherBadge(context, theme, units, weather); diff --git a/lib/features/trips/presentation/widgets/story/trip_story_view.dart b/lib/features/trips/presentation/widgets/story/trip_story_view.dart index 66676226b4..c687480488 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_view.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_view.dart @@ -7,7 +7,8 @@ import 'package:latlong2/latlong.dart'; import 'package:submersion/features/checklists/presentation/widgets/trip_checklist_section.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart'; import 'package:submersion/features/trips/domain/entities/trip_story.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_flight_countdown_card.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_card.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_header.dart'; @@ -154,6 +155,16 @@ class _TripStoryViewState extends ConsumerState @override Widget build(BuildContext context) { + final tripId = widget.story.trip.id; + // One read for the whole story. Watched here rather than inside the + // LayoutBuilder below, whose builder runs at layout time, not build time. + final storedWeather = + ref.watch(tripDayWeatherProvider(tripId)).asData?.value ?? + const {}; + // Fire and forget: the backfill's writes come back through the provider + // above via the table tick, so a row landing re-renders its day header. + ref.watch(tripDayWeatherBackfillProvider(tripId)); + return LayoutBuilder( builder: (context, constraints) { final wide = constraints.maxWidth >= _wideBreakpoint; @@ -172,7 +183,7 @@ class _TripStoryViewState extends ConsumerState siteCount: _siteCount, ), ), - ..._contentSlivers(), + ..._contentSlivers(storedWeather), ], ), ); @@ -195,7 +206,9 @@ class _TripStoryViewState extends ConsumerState Expanded( child: NotificationListener( onNotification: _onScroll, - child: CustomScrollView(slivers: _contentSlivers()), + child: CustomScrollView( + slivers: _contentSlivers(storedWeather), + ), ), ), ], @@ -229,18 +242,18 @@ class _TripStoryViewState extends ConsumerState /// One day chapter: a SliverMainAxisGroup whose pinned header sticks below /// the map until the next day's group pushes it out. Every day gets the same /// header, surface days included - theirs simply has no body under it. - Widget _daySliver(TripStory story, int index, int? todayIndex) { + Widget _daySliver( + TripStory story, + int index, + int? todayIndex, + Map storedWeather, + ) { final day = story.days[index]; - final weatherPoint = day.isSurface - ? story.mapGeometry.nearestPointForDay(index) - : null; - final surfaceWeatherRequest = weatherPoint == null - ? null - : SurfaceDayWeatherRequest( - date: day.date, - latitude: weatherPoint.latitude, - longitude: weatherPoint.longitude, - ); + // Keyed through the same helper the repository stores under, so the + // lookup cannot drift from the write. Computing the key inline here was + // how the two came apart: it silently found nothing and every badge + // disappeared. + final stored = storedWeather[tripDayMillis(day.date)]; final showTodayDivider = todayIndex != null && index == todayIndex; const divider = SliverPadding( padding: EdgeInsets.symmetric(horizontal: 16), @@ -274,7 +287,7 @@ class _TripStoryViewState extends ConsumerState PinnedHeaderSliver( child: TripStoryDayHeader( day: day, - surfaceWeatherRequest: surfaceWeatherRequest, + storedWeather: stored?.toStoryWeather(), ), ), body, @@ -282,7 +295,7 @@ class _TripStoryViewState extends ConsumerState ); } - List _contentSlivers() { + List _contentSlivers(Map storedWeather) { final story = widget.story; final trip = story.trip; final todayIndex = story.todayIndex; @@ -314,7 +327,7 @@ class _TripStoryViewState extends ConsumerState sliver: SliverToBoxAdapter(child: TripVesselSection(tripId: trip.id)), ), for (var index = 0; index < story.days.length; index++) - _daySliver(story, index, todayIndex), + _daySliver(story, index, todayIndex, storedWeather), if (trip.notes.isNotEmpty) SliverPadding( padding: const EdgeInsets.fromLTRB(16, 0, 16, 16), diff --git a/test/core/database/migration_v171_trip_day_weather_test.dart b/test/core/database/migration_v171_trip_day_weather_test.dart new file mode 100644 index 0000000000..6ce941840d --- /dev/null +++ b/test/core/database/migration_v171_trip_day_weather_test.dart @@ -0,0 +1,140 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v171 adds `trip_day_weather`: fetched historical weather for trip days +/// whose dives supply none of their own (surface days and dive-free itinerary +/// days). Its own table rather than columns on `trips` or `trip_itinerary_days` +/// because HLC conflicts resolve per row, and an automatic derived write must +/// not race the diver's hand edits on the same row. +NativeDatabase _dbAt164() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 164'); + rawDb.execute(''' + CREATE TABLE trips ( + id TEXT NOT NULL PRIMARY KEY, + name TEXT NOT NULL, + start_date INTEGER NOT NULL, + end_date INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute( + "INSERT INTO trips (id, name, start_date, end_date, created_at, " + "updated_at) VALUES ('t1', 'Bonaire', 0, 0, 0, 0)", + ); + }, + ); +} + +Future> _columnsOf(AppDatabase db, String table) async { + final cols = await db.customSelect("PRAGMA table_info('$table')").get(); + return cols.map((c) => c.read('name')).toSet(); +} + +void main() { + test('v171 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(171)); + expect(AppDatabase.migrationVersions, contains(171)); + }); + + test('a fresh database has trip_day_weather with every column', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + expect( + await _columnsOf(db, 'trip_day_weather'), + containsAll({ + 'id', + 'trip_id', + 'date', + 'latitude', + 'longitude', + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + 'weather_source', + 'fetched_at', + 'created_at', + 'updated_at', + 'hlc', + }), + ); + }); + + test('the weather payload columns are all nullable', () async { + // A fetch that resolves only some fields must still store a row; every + // reading is independently optional. + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('trip_day_weather')") + .get(); + for (final name in [ + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + ]) { + final column = cols.firstWhere((c) => c.read('name') == name); + expect(column.read('notnull'), 0, reason: '$name must be nullable'); + } + }); + + test('one row per trip and date', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + await db.customStatement('PRAGMA foreign_keys = OFF'); + + Future insert(String id) => db.customStatement( + 'INSERT INTO trip_day_weather ' + '(id, trip_id, date, latitude, longitude, weather_source, ' + 'fetched_at, created_at, updated_at) ' + "VALUES ('$id', 't1', 1000, 1.0, 2.0, 'openMeteo', 1, 1, 1)", + ); + + await insert('a'); + // Two devices that both fetch the same day must converge on one row + // rather than accumulating duplicates. + await expectLater(insert('b'), throwsA(anything)); + }); + + test('a database at v164 gains the table and keeps its rows', () async { + final db = AppDatabase(_dbAt164()); + addTearDown(db.close); + + expect(await _columnsOf(db, 'trip_day_weather'), isNotEmpty); + final trip = await db + .customSelect("SELECT name FROM trips WHERE id = 't1'") + .getSingle(); + expect(trip.read('name'), 'Bonaire'); + }); + + test('a database stranded at a parallel-branch v171 gains the table via ' + 'beforeOpen', () async { + // Stamped AT 171 but without the table: the onUpgrade block never runs, + // so only the beforeOpen backstop can create it. + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 171'); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + expect(await _columnsOf(db, 'trip_day_weather'), isNotEmpty); + }); +} diff --git a/test/core/services/sync/sync_parent_refs_completeness_test.dart b/test/core/services/sync/sync_parent_refs_completeness_test.dart index 7b97c84141..0626edf0e3 100644 --- a/test/core/services/sync/sync_parent_refs_completeness_test.dart +++ b/test/core/services/sync/sync_parent_refs_completeness_test.dart @@ -24,6 +24,7 @@ void main() { 'trips': 'trips', 'liveaboard_detail_records': 'liveaboardDetails', 'trip_itinerary_days': 'itineraryDays', + 'trip_day_weather': 'tripDayWeather', 'checklist_templates': 'checklistTemplates', 'checklist_template_items': 'checklistTemplateItems', 'trip_checklist_items': 'tripChecklistItems', diff --git a/test/core/services/sync/trip_day_weather_sync_test.dart b/test/core/services/sync/trip_day_weather_sync_test.dart new file mode 100644 index 0000000000..66719a2aee --- /dev/null +++ b/test/core/services/sync/trip_day_weather_sync_test.dart @@ -0,0 +1,160 @@ +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/sync/sync_data_serializer.dart'; +import 'package:submersion/core/services/sync/sync_service.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +import '../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late SyncDataSerializer serializer; + + final dayMillis = DateTime(2026, 3, 8).millisecondsSinceEpoch; + final rowId = tripDayWeatherRowId(tripId: 'trip-1', dayMillis: dayMillis); + + setUp(() async { + db = await setUpTestDatabase(); + serializer = SyncDataSerializer(); + await db + .into(db.trips) + .insert( + TripsCompanion.insert( + id: 'trip-1', + name: 'Bonaire', + startDate: 0, + endDate: 0, + createdAt: 1, + updatedAt: 1, + ), + ); + await db + .into(db.tripDayWeather) + .insert( + TripDayWeatherCompanion.insert( + id: tripDayWeatherRowId( + tripId: 'trip-1', + dayMillis: DateTime(2026, 3, 8).millisecondsSinceEpoch, + ), + tripId: 'trip-1', + date: DateTime(2026, 3, 8).millisecondsSinceEpoch, + latitude: 12.16, + longitude: -68.28, + airTemp: const Value(24.0), + cloudCover: const Value('clear'), + fetchedAt: 1, + createdAt: 1, + updatedAt: 1, + ), + ); + }); + + tearDown(tearDownTestDatabase); + + test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { + final record = await serializer.fetchRecord('tripDayWeather', rowId); + expect(record, isNotNull); + expect(record!['airTemp'], 24.0); + expect(record['cloudCover'], 'clear'); + + // A remote edit merges over the local row (LWW payload apply). + await serializer.upsertRecord('tripDayWeather', { + ...record, + 'airTemp': 26.0, + 'updatedAt': 2, + }); + final merged = await serializer.fetchRecord('tripDayWeather', rowId); + expect(merged!['airTemp'], 26.0); + + expect(await serializer.recordIdsFor('tripDayWeather'), contains(rowId)); + + await serializer.deleteRecord('tripDayWeather', rowId); + expect(await serializer.fetchRecord('tripDayWeather', rowId), isNull); + }); + + test('the delta export filters on the row own hlc', () async { + await (db.update( + db.tripDayWeather, + )..where((t) => t.id.equals(rowId))).write( + const TripDayWeatherCompanion(hlc: Value('2026-08-16T00:00:00.000-0000')), + ); + + Future changesetCount(String? watermark) async { + final payload = await serializer.exportChangeset( + deviceId: 'device-1', + hlcWatermark: watermark, + deletions: const [], + ); + return payload.data.tripDayWeather.length; + } + + // A base carries the row; a watermark newer than it excludes it; an older + // watermark includes it. + expect(await changesetCount(null), 1); + expect(await changesetCount('2026-08-17T00:00:00.000-0000'), 0); + expect(await changesetCount('2026-08-15T00:00:00.000-0000'), 1); + }); + + test('a peer row for the same day merges instead of throwing', () async { + // Two devices that both fetch the same day must converge. A v4 id per + // device would insert a second row and violate the unique (trip_id, date) + // index, and because the merge runs in a transaction that aborts the + // whole sync pull, not just this row. + // The peer derives the same id from the same (trip, day). + await serializer.upsertRecord('tripDayWeather', { + 'id': tripDayWeatherRowId(tripId: 'trip-1', dayMillis: dayMillis), + 'tripId': 'trip-1', + 'date': dayMillis, + 'latitude': 12.16, + 'longitude': -68.28, + 'airTemp': 26.0, + 'weatherSource': 'openMeteo', + 'fetchedAt': 2, + 'createdAt': 2, + 'updatedAt': 2, + }); + + final rows = await db.select(db.tripDayWeather).get(); + expect(rows, hasLength(1)); + expect(rows.single.airTemp, 26.0); + }); + + test('the row id is derived from trip and day, not minted per device', () { + final day = DateTime(2026, 3, 8).millisecondsSinceEpoch; + + expect( + tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day), + tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day), + ); + expect( + tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day), + isNot(tripDayWeatherRowId(tripId: 'trip-2', dayMillis: day)), + ); + expect( + tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day), + isNot( + tripDayWeatherRowId( + tripId: 'trip-1', + dayMillis: DateTime(2026, 3, 9).millisecondsSinceEpoch, + ), + ), + ); + }); + + test('tripDayWeather is registered as an hlc target', () { + // An omission here is silent: _stampHlc no-ops on an unknown entity type, + // the column stays NULL, and the incremental export's hlc > watermark + // filter then excludes the row from every changeset forever. + expect(SyncRepository.hlcTargets.containsKey('tripDayWeather'), isTrue); + expect( + SyncRepository.hlcTargets['tripDayWeather']!.table, + 'trip_day_weather', + ); + }); + + test('tripDayWeather carries an updatedAt flag', () { + expect(SyncService.entityHasUpdatedAt['tripDayWeather'], isTrue); + }); +} diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart new file mode 100644 index 0000000000..c31e36c9ae --- /dev/null +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -0,0 +1,395 @@ +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart' as db; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/data/repositories/trip_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late TripDayWeatherRepository repository; + late TripRepository tripRepository; + late String testTripId; + late String otherTripId; + + final day1 = DateTime(2026, 3, 8); + final day2 = DateTime(2026, 3, 9); + + Trip createTestTrip({String name = 'Test Trip'}) { + final now = DateTime.now(); + return Trip( + id: '', + name: name, + startDate: day1, + endDate: DateTime(2026, 3, 14), + createdAt: now, + updatedAt: now, + ); + } + + TripDayWeather sample({ + String id = 'w1', + String? tripId, + DateTime? date, + double? airTemp = 21.5, + CloudCover? cloudCover = CloudCover.clear, + }) { + final now = DateTime(2026, 3, 15); + return TripDayWeather( + id: id, + tripId: tripId ?? testTripId, + date: date ?? day1, + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: cloudCover, + windSpeed: 6.5, + windDirection: CurrentDirection.north, + humidity: 70, + surfacePressure: 1.011, + weatherCode: 0, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + } + + setUp(() async { + await setUpTestDatabase(); + repository = TripDayWeatherRepository(); + tripRepository = TripRepository(); + + // Two trips, to prove the queries are scoped. trip_id is a non-nullable + // FK and beforeOpen turns foreign keys on. + testTripId = (await tripRepository.createTrip( + createTestTrip(name: 'Weather Test Trip'), + )).id; + otherTripId = (await tripRepository.createTrip( + createTestTrip(name: 'Other Trip'), + )).id; + }); + + tearDown(() async { + await tearDownTestDatabase(); + }); + + group('TripDayWeatherRepository', () { + test('getForTrip is empty before anything is stored', () async { + expect(await repository.getForTrip(testTripId), isEmpty); + }); + + test('upsert then read back, keyed by date millis', () async { + await repository.upsert(sample()); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + final row = stored[tripDayMillis(day1)]!; + expect(row.airTemp, 21.5); + expect(row.cloudCover, CloudCover.clear); + expect(row.windDirection, CurrentDirection.north); + expect(row.weatherCode, 0); + expect(row.weatherSource, WeatherSource.openMeteo); + expect(row.latitude, 12.16); + // A calendar day in a device-independent frame, so UTC. + expect(row.date, DateTime.utc(2026, 3, 8)); + }); + + test('a null payload field round-trips as null', () async { + await repository.upsert(sample(airTemp: null, cloudCover: null)); + + final row = (await repository.getForTrip( + testTripId, + ))[tripDayMillis(day1)]!; + + expect(row.airTemp, isNull); + expect(row.cloudCover, isNull); + expect(row.precipitation, isNull); + }); + + test('upserting the same day twice keeps one row', () async { + await repository.upsert(sample()); + // A different id for the same day: the day is the identity, so this + // must replace rather than accumulate. + await repository.upsert(sample(id: 'w2', airTemp: 25)); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); + }); + + test('a date with a time component is stored under UTC midnight', () { + // The repository owns the (trip, date) uniqueness invariant, so it + // normalizes rather than trusting every caller to. A row keyed on a + // stray time would be invisible to day-keyed lookups and would refetch + // forever. UTC, so the key does not move with the device timezone. + return () async { + await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30))); + + final stored = await repository.getForTrip(testTripId); + + expect(stored.keys.single, tripDayMillis(day1)); + expect(stored[tripDayMillis(day1)]!.date, DateTime.utc(2026, 3, 8)); + }(); + }); + + test('the same day at two times of day stays one row', () async { + await repository.upsert(sample(date: DateTime(2026, 3, 8, 6))); + await repository.upsert( + sample(id: 'w2', date: DateTime(2026, 3, 8, 23), airTemp: 25), + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); + }); + + test('two different days both persist', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', date: day2, airTemp: 19)); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(2)); + expect(stored[tripDayMillis(day2)]!.airTemp, 19); + }); + + test('getForTrip is scoped to one trip', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + expect(await repository.getForTrip(testTripId), hasLength(1)); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('deleteByTripId removes only that trip rows', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + await repository.deleteByTripId(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('deleteByTripId on a trip with no weather is a no-op', () async { + await repository.deleteByTripId(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + }); + + test('deleting a trip takes its weather rows with it', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + await tripRepository.deleteTrip(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('watchWeatherChanges emits after a write', () async { + final emissions = []; + final subscription = repository.watchWeatherChanges().listen( + emissions.add, + ); + addTearDown(subscription.cancel); + + await repository.upsert(sample()); + await Future.delayed(Duration.zero); + + expect(emissions, isNotEmpty); + }); + }); + + group('rows upsert did not write', () { + // Every row in this group goes straight into the table, because that is + // the only way one can carry an id other than the derived one: a peer on + // an older build of this feature, or a database written before the id + // became deterministic. Reconciling rows it did not create is exactly + // what the repository is being asked to do here. + // Built from the day key plus an explicit offset, never from a local + // DateTime's epoch. The day is UTC midnight, so a local value lands on a + // neighbouring UTC day under a large enough offset and stops being a + // stray for the day under test: the fixture would be asserting timezone + // arithmetic instead of reconciliation, which is how these passed at + // UTC+0 and failed at UTC+9. + Future insertRaw({ + required String id, + required DateTime day, + Duration offset = Duration.zero, + double? airTemp, + int updatedAt = 0, + int createdAt = 0, + }) async { + await DatabaseService.instance.database + .into(DatabaseService.instance.database.tripDayWeather) + .insert( + db.TripDayWeatherCompanion( + id: Value(id), + tripId: Value(testTripId), + date: Value(tripDayMillis(day) + offset.inMilliseconds), + latitude: const Value(12.16), + longitude: const Value(-68.28), + airTemp: Value(airTemp), + weatherSource: Value(WeatherSource.openMeteo.name), + fetchedAt: const Value(0), + createdAt: Value(createdAt), + updatedAt: Value(updatedAt), + ), + ); + } + + Future> allRows() => DatabaseService + .instance + .database + .select(DatabaseService.instance.database.tripDayWeather) + .get(); + + test('upsert replaces a same-day row stored under a foreign id', () async { + // The unique index is on (trip_id, date) but insertOnConflictUpdate + // targets the primary key, so a foreign-id row on the same midnight + // makes the canonical insert miss the conflict target and hit the + // index. Without cleanup this throws rather than merging. + await insertRaw(id: 'from-a-peer', day: day1, airTemp: 10); + + await repository.upsert(sample(airTemp: 25)); + + final rows = await allRows(); + expect(rows, hasLength(1)); + expect(rows.single.airTemp, 25); + expect(rows.single.id, isNot('from-a-peer')); + }); + + test('upsert replaces a same-day row stored off midnight', () async { + await insertRaw( + id: 'from-an-older-build', + day: day1, + offset: const Duration(hours: 17, minutes: 30), + airTemp: 10, + ); + + await repository.upsert(sample(airTemp: 25)); + + final rows = await allRows(); + expect(rows, hasLength(1)); + expect(rows.single.date, tripDayMillis(day1)); + expect(rows.single.airTemp, 25); + }); + + test('a row for another day is left alone', () async { + await insertRaw(id: 'other-day', day: day2, airTemp: 10); + + await repository.upsert(sample()); + + final rows = await allRows(); + expect(rows, hasLength(2)); + expect(rows.map((r) => r.id), contains('other-day')); + }); + + test('replacing a stray logs its deletion for sync', () async { + // A stray is a synced record. Dropping it without a tombstone lets the + // peer that sent it hand it straight back on the next pull. + await insertRaw(id: 'from-a-peer', day: day1); + + await repository.upsert(sample()); + + final deletions = await SyncRepository().getAllDeletions(); + expect( + deletions.where( + (d) => + d.entityType == 'tripDayWeather' && d.recordId == 'from-a-peer', + ), + hasLength(1), + ); + }); + + test('upsert keeps the createdAt of the stray it absorbs', () async { + // The stray is this day's row under an old id, not a different record, + // so the day keeps the age it already had. + await insertRaw( + id: 'from-a-peer', + day: day1, + offset: const Duration(hours: 17, minutes: 30), + createdAt: 1000, + ); + + await repository.upsert(sample()); + + expect((await allRows()).single.createdAt, 1000); + }); + + test( + 'getForTrip prefers the canonical row over a same-day stray', + () async { + // Reads land between a sync import and the next upsert, so the choice + // cannot wait for the write side to tidy up, and it cannot depend on + // the order SQLite happens to return rows in. + await repository.upsert(sample(airTemp: 25)); + await insertRaw( + id: 'from-a-peer', + day: day1, + offset: const Duration(hours: 17, minutes: 30), + airTemp: 10, + updatedAt: 9999999, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); + }, + ); + + test('getForTrip falls back to the most recently updated stray', () async { + // Off-midnight strays within the same UTC day. "Same day" is a UTC + // question now: a row stored at 23:00 local on a negative offset falls + // on the following UTC day and is genuinely a different day's row. + await insertRaw( + id: 'peer-a', + day: day1, + offset: const Duration(hours: 6), + airTemp: 10, + updatedAt: 100, + ); + await insertRaw( + id: 'peer-b', + day: day1, + offset: const Duration(hours: 23), + airTemp: 20, + updatedAt: 200, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[tripDayMillis(day1)]!.airTemp, 20); + }); + + test('getForTrip does not write while resolving strays', () async { + // Reads stay pure: a cleanup here would fire the table tick and + // invalidate the provider that just read. + await insertRaw( + id: 'peer-a', + day: day1, + offset: const Duration(hours: 6), + ); + await insertRaw( + id: 'peer-b', + day: day1, + offset: const Duration(hours: 23), + ); + + await repository.getForTrip(testTripId); + + expect(await allRows(), hasLength(2)); + }); + }); +} diff --git a/test/features/trips/domain/entities/trip_day_weather_test.dart b/test/features/trips/domain/entities/trip_day_weather_test.dart new file mode 100644 index 0000000000..c3a8bb8f38 --- /dev/null +++ b/test/features/trips/domain/entities/trip_day_weather_test.dart @@ -0,0 +1,151 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +void main() { + TripDayWeather weather({ + double? airTemp, + CloudCover? cloudCover, + Precipitation? precipitation, + double? windSpeed, + double? humidity, + }) { + final now = DateTime(2026, 3, 9); + return TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + windSpeed: windSpeed, + humidity: humidity, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + } + + group('tripDayMillis', () { + test('is the calendar day in UTC, not the device local midnight', () { + // The key must not depend on where the device is standing. A local + // DateTime(y, m, d) has a different epoch value in every timezone, so + // two devices would derive different row ids for the same calendar day + // and never converge. + expect( + tripDayMillis(DateTime(2026, 3, 8, 17, 30)), + DateTime.utc(2026, 3, 8).millisecondsSinceEpoch, + ); + }); + + test('takes the calendar fields as given, never shifting the day', () { + // Guards the wrong fix: converting with toUtc() would move a late + // evening local time onto the following calendar day. + expect( + tripDayMillis(DateTime(2026, 3, 8, 23, 59)), + tripDayMillis(DateTime.utc(2026, 3, 8, 0, 1)), + ); + }); + + test('distinct days stay distinct', () { + expect( + tripDayMillis(DateTime(2026, 3, 8)), + isNot(tripDayMillis(DateTime(2026, 3, 9))), + ); + }); + }); + + group('tripDayWeatherRowId', () { + test('is stable for one calendar day regardless of the time given', () { + expect( + tripDayWeatherRowId( + tripId: 't1', + dayMillis: tripDayMillis(DateTime(2026, 3, 8, 1)), + ), + tripDayWeatherRowId( + tripId: 't1', + dayMillis: tripDayMillis(DateTime(2026, 3, 8, 22)), + ), + ); + }); + }); + + group('hasRenderableWeather', () { + test('air temperature alone counts', () { + expect(weather(airTemp: 24).hasRenderableWeather, isTrue); + }); + + test('cloud cover alone counts', () { + expect( + weather(cloudCover: CloudCover.overcast).hasRenderableWeather, + isTrue, + ); + }); + + test('active precipitation alone counts', () { + expect( + weather(precipitation: Precipitation.rain).hasRenderableWeather, + isTrue, + ); + }); + + test('precipitation none alone does NOT count', () { + // WeatherMapper.mapPrecipitation never returns null: a missing reading + // becomes Precipitation.none, so `none` is not evidence that the fetch + // resolved anything. weatherIconFor gives it no glyph either. + expect( + weather(precipitation: Precipitation.none).hasRenderableWeather, + isFalse, + ); + }); + + test('wind and humidity alone do NOT count', () { + // Stored, but never drawn in the day header badge. + expect( + weather( + windSpeed: 6.5, + humidity: 70, + precipitation: Precipitation.none, + ).hasRenderableWeather, + isFalse, + ); + }); + + test('an empty result does not count', () { + expect(weather().hasRenderableWeather, isFalse); + }); + }); + + group('toStoryWeather', () { + test('carries only the three fields the header renders', () { + final story = weather( + airTemp: 24, + cloudCover: CloudCover.clear, + precipitation: Precipitation.none, + windSpeed: 6.5, + humidity: 70, + ).toStoryWeather(); + + expect(story.airTemp, 24); + expect(story.cloudCover, CloudCover.clear); + expect(story.precipitation, Precipitation.none); + }); + }); + + group('copyWith', () { + test('clears a nullable field when passed null explicitly', () { + final cleared = weather(airTemp: 24).copyWith(airTemp: null); + + expect(cleared.airTemp, isNull); + }); + + test('leaves an untouched field alone', () { + final same = weather(airTemp: 24).copyWith(latitude: 0); + + expect(same.airTemp, 24); + expect(same.latitude, 0); + }); + }); +} diff --git a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart new file mode 100644 index 0000000000..e9f75dcdb2 --- /dev/null +++ b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart @@ -0,0 +1,305 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; + +void main() { + Trip trip() => Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 14), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ); + + TripStory storyWith( + List days, { + List points = const [], + }) { + return TripStory( + trip: trip(), + days: days, + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: TripStoryMapGeometry(points: points), + ); + } + + Dive diveWith({ + double? airTemp, + CloudCover? cloudCover, + Precipitation? precipitation, + }) => Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); + + TripStoryDay day({ + required int index, + TripStoryDayKind kind = TripStoryDayKind.past, + List dives = const [], + }) { + return TripStoryDay( + date: DateTime(2026, 3, 8 + index), + dayNumber: index + 1, + kind: kind, + dives: dives, + ); + } + + TripStoryMapPoint pointFor(int dayIndex) => TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: dayIndex, + label: 'Site', + ); + + TripDayWeather storedFor(DateTime date) => TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: date, + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ); + + group('TripDayWeatherBackfill.targetsFor', () { + test('a past day with no dives and a nearby point is a target', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 8)); + expect(targets.single.latitude, 12.16); + expect(targets.single.longitude, -68.28); + // Noon local reads as "the day's weather" far better than the API's + // default midnight boundary. + expect(targets.single.localNoon, DateTime(2026, 3, 8, 12)); + }); + + test('a day whose dives carry weather is skipped', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(airTemp: 26)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a dive-free itinerary day is a target, not just a surface day', () { + // The day has dives with no weather at all, so nothing supplies it. + final story = storyWith( + [ + day(index: 0, dives: [diveWith()]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }); + + test('a future day is skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.future)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('today is not skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.today)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }); + + test('a day with a stored row is skipped', () { + // Keyed the way the repository keys it. getForTrip returns a map keyed + // by tripDayMillis, so keying this fixture any other way would assert a + // contract production never offers: it would pass here and still refetch + // every day on a real device. + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final stored = { + tripDayMillis(DateTime(2026, 3, 8)): storedFor(DateTime(2026, 3, 8)), + }; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + ); + }); + + test('the stored lookup is not the device local midnight', () { + // The day key is UTC midnight so two devices converge on one row. A + // lookup built from a local DateTime's epoch agrees with it only at + // UTC+0, so on any other device every stored day reads as missing and + // is refetched on every view, which is the whole thing this feature + // exists to stop. Discriminating only off UTC; run with + // TZ=America/New_York to see it bite. + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final localKey = DateTime(2026, 3, 8).millisecondsSinceEpoch; + final utcKey = tripDayMillis(DateTime(2026, 3, 8)); + + final stored = {utcKey: storedFor(DateTime(2026, 3, 8))}; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + reason: + 'a row stored under the repository key must skip the day ' + '(local key $localKey vs day key $utcKey)', + ); + }); + + test('a day with no map point anywhere in the story is skipped', () { + final story = storyWith([day(index: 0)]); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day borrows the nearest day point when it has none of its own', () { + // nearestPointForDay walks outward, so day 1 with a point only on day 0 + // is still a target, at day 0's coordinates. + final story = storyWith( + [ + day(index: 0, dives: [diveWith(airTemp: 26)]), + day(index: 1), + ], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 9)); + expect(targets.single.latitude, 12.16); + }); + + test('targets come back in day order', () { + final story = storyWith( + [day(index: 0), day(index: 1), day(index: 2)], + points: [pointFor(1)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets.map((t) => t.date).toList(), [ + DateTime(2026, 3, 8), + DateTime(2026, 3, 9), + DateTime(2026, 3, 10), + ]); + }); + + test( + 'a dive carrying only Precipitation.none does NOT count as weather', + () { + // WeatherMapper never returns null precipitation, so a dive whose + // lookup resolved nothing still stores `none`. The header draws no + // glyph for it, so treating it as "this day has weather" would skip the + // backfill and leave the day permanently badge-free. + final story = storyWith( + [ + day(index: 0, dives: [diveWith(precipitation: Precipitation.none)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }, + ); + + test('a dive carrying active precipitation DOES count as weather', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(precipitation: Precipitation.rain)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a dive carrying only cloud cover counts as weather', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(cloudCover: CloudCover.overcast)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day date with a time component is normalized to midnight', () { + // The story day's date should already be date-only, but a stored row is + // keyed on midnight millis, so a stray time would never match and the + // day would refetch forever. + final story = storyWith( + [ + TripStoryDay( + date: DateTime(2026, 3, 8, 17, 30), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + ], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets.single.date, DateTime(2026, 3, 8)); + }); + }); +} diff --git a/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart b/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart deleted file mode 100644 index 1f7eda2944..0000000000 --- a/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart +++ /dev/null @@ -1,98 +0,0 @@ -import 'dart:convert'; - -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; -import 'package:submersion/core/constants/enums.dart'; -import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; -import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; - -http.Response weatherResponse() => http.Response( - jsonEncode({ - 'hourly': { - 'time': ['2024-06-15T09:00', '2024-06-15T12:00'], - 'temperature_2m': [24.0, 29.0], - 'relative_humidity_2m': [80.0, 70.0], - 'precipitation': [0.0, 1.0], - 'cloud_cover': [10.0, 95.0], - 'wind_speed_10m': [8.0, 12.0], - 'wind_direction_10m': [30.0, 45.0], - 'surface_pressure': [1012.0, 1011.0], - 'weathercode': [0, 61], - }, - }), - 200, -); - -void main() { - final request = SurfaceDayWeatherRequest( - date: DateTime(2024, 6, 15), - latitude: 12.1, - longitude: -68.2, - ); - - test('fetches local noon and maps compact trip weather', () async { - final client = MockClient((http.Request httpRequest) async { - expect(httpRequest.url.queryParameters['timezone'], 'auto'); - expect(httpRequest.url.queryParameters['start_date'], '2024-06-15'); - return weatherResponse(); - }); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - final weather = await container.read( - surfaceDayWeatherProvider(request).future, - ); - - expect( - weather, - const TripStoryDayWeather( - airTemp: 29, - cloudCover: CloudCover.overcast, - precipitation: Precipitation.lightRain, - ), - ); - }); - - test('caches one request for an equal family key', () async { - var calls = 0; - final client = MockClient((_) async { - calls++; - return weatherResponse(); - }); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - await container.read(surfaceDayWeatherProvider(request).future); - await container.read( - surfaceDayWeatherProvider( - SurfaceDayWeatherRequest( - date: DateTime(2024, 6, 15), - latitude: 12.1, - longitude: -68.2, - ), - ).future, - ); - - expect(calls, 1); - }); - - test('propagates unavailable weather as null', () async { - final client = MockClient((_) async => http.Response('', 500)); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - expect( - await container.read(surfaceDayWeatherProvider(request).future), - isNull, - ); - }); -} diff --git a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart new file mode 100644 index 0000000000..479fe7c06d --- /dev/null +++ b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart @@ -0,0 +1,300 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +/// A full Open-Meteo hourly payload for one day, in the shape WeatherMapper +/// expects (copied from the deleted surface_day_weather_provider_test). +http.Response weatherResponse({double noonTemp = 29.0, num cloud = 95}) => + http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T09:00', '2026-03-08T12:00'], + 'temperature_2m': [24.0, noonTemp], + 'relative_humidity_2m': [80.0, 70.0], + 'precipitation': [0.0, 0.0], + 'cloud_cover': [10.0, cloud], + 'wind_speed_10m': [8.0, 12.0], + 'wind_direction_10m': [30.0, 45.0], + 'surface_pressure': [1012.0, 1011.0], + 'weathercode': [0, 0], + }, + }), + 200, + ); + +/// Nothing the day header could render: humidity and pressure only. +http.Response unrenderableResponse() => http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T12:00'], + 'temperature_2m': [null], + 'relative_humidity_2m': [70.0], + 'precipitation': [null], + 'cloud_cover': [null], + 'wind_speed_10m': [null], + 'wind_direction_10m': [null], + 'surface_pressure': [1011.0], + 'weathercode': [null], + }, + }), + 200, +); + +/// Records upserts instead of touching a database. +class FakeTripDayWeatherRepository implements TripDayWeatherRepository { + FakeTripDayWeatherRepository({this.stored = const {}}); + + final Map stored; + final List upserts = []; + + @override + Future> getForTrip(String tripId) async => stored; + + @override + Future upsert(TripDayWeather weather) async => upserts.add(weather); + + @override + Future deleteByTripId(String tripId) async {} + + @override + Stream watchWeatherChanges() => const Stream.empty(); +} + +void main() { + /// Two past days with no dives, and one map point on day 0 that day 1 + /// borrows through nearestPointForDay. + TripStory twoDayStory() => TripStory( + trip: Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ), + days: [ + TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + TripStoryDay( + date: DateTime(2026, 3, 9), + dayNumber: 2, + kind: TripStoryDayKind.past, + ), + ], + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: const TripStoryMapGeometry( + points: [ + TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: 0, + label: 'Site', + ), + ], + ), + ); + + ProviderContainer containerWith({ + required http.Client client, + required FakeTripDayWeatherRepository repository, + }) { + final container = ProviderContainer( + overrides: [ + weatherHttpClientProvider.overrideWithValue(client), + tripDayWeatherRepositoryProvider.overrideWithValue(repository), + tripStoryProvider('trip-1').overrideWith((ref) async => twoDayStory()), + ], + ); + addTearDown(container.dispose); + return container; + } + + group('tripDayWeatherBackfillProvider', () { + test('fetches each target once and stores the result', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((request) async { + calls++; + // Local noon at the coordinate, not the archive's GMT midnight. + expect(request.url.queryParameters['timezone'], 'auto'); + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + expect(repository.upserts, hasLength(2)); + final first = repository.upserts.first; + expect(first.tripId, 'trip-1'); + expect(first.date, DateTime(2026, 3, 8)); + expect(first.airTemp, 29.0); + expect(first.cloudCover, CloudCover.overcast); + expect(first.latitude, 12.16); + expect(first.longitude, -68.28); + expect(first.weatherSource, WeatherSource.openMeteo); + // The full payload is stored even though the header renders only part. + expect(first.humidity, 70.0); + expect(first.surfacePressure, isNotNull); + expect(repository.upserts[1].date, DateTime(2026, 3, 9)); + }); + + test('a failed fetch writes no row', () async { + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => http.Response('', 500)), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a result with nothing renderable writes no row', () async { + // Storing it would suppress the retry that a later archive update + // would satisfy. + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => unrenderableResponse()), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a day already stored is not fetched', () async { + final repository = FakeTripDayWeatherRepository( + stored: { + // Keyed as getForTrip keys it. A local DateTime's epoch matches the + // day key only at UTC+0, so keying it that way would assert a + // contract production never offers. + tripDayMillis(DateTime(2026, 3, 8)): TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }, + ); + var calls = 0; + final container = containerWith( + client: MockClient((_) async { + calls++; + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 1); + expect(repository.upserts, hasLength(1)); + expect(repository.upserts.single.date, DateTime(2026, 3, 9)); + }); + + test('a miss is retried when the trip is viewed again', () async { + // The documented policy is that a miss writes nothing and is retried on + // the next view. A provider that stays alive for the container's + // lifetime would serve its completed state instead, so a transient + // failure would not be retried until the app restarted. + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + calls++; + return http.Response('', 500); + }), + repository: repository, + ); + + // First visit: the view mounts, watches, and the pass runs. + final first = container.listen( + tripDayWeatherBackfillProvider('trip-1'), + (_, _) {}, + ); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + expect(calls, 2, reason: 'two days, both missing'); + + // Leaving the trip drops the last listener. + first.close(); + await Future.delayed(Duration.zero); + + // Returning to the trip must run the pass again. + container.listen(tripDayWeatherBackfillProvider('trip-1'), (_, _) {}); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 4); + }); + + test('the pass does not re-run while the view stays mounted', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + calls++; + return http.Response('', 500); + }), + repository: repository, + ); + + container.listen(tripDayWeatherBackfillProvider('trip-1'), (_, _) {}); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + // A rebuild re-watches; that must not start another pass. + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + }); + + test('fetches run one at a time', () async { + final gate = Completer(); + var started = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + started++; + if (started == 1) await gate.future; + return weatherResponse(); + }), + repository: repository, + ); + + final pending = container.read( + tripDayWeatherBackfillProvider('trip-1').future, + ); + await Future.delayed(Duration.zero); + + // A two-week trip must not open with a burst of parallel requests. + expect(started, 1); + + gate.complete(); + await pending; + expect(started, 2); + }); + }); +} diff --git a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart index a7c2789da2..0fde9196fa 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart @@ -1,5 +1,3 @@ -import 'dart:async'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:intl/intl.dart'; @@ -10,7 +8,6 @@ import 'package:submersion/features/dive_log/domain/entities/dive.dart'; import 'package:submersion/features/dive_sites/domain/entities/dive_site.dart'; import 'package:submersion/features/trips/domain/entities/itinerary_day.dart'; import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_header.dart'; import 'package:submersion/l10n/arb/app_localizations.dart'; @@ -33,7 +30,7 @@ Future pumpHeader( TripStoryDay day, { double textScale = 1.0, MockSettingsNotifier? settingsNotifier, - SurfaceDayWeatherRequest? surfaceWeatherRequest, + TripStoryDayWeather? storedWeather, List extra = const [], }) async { // The header dates itself with DateFormat.MMMEd(), which resolves against @@ -65,7 +62,7 @@ Future pumpHeader( ).copyWith(textScaler: TextScaler.linear(textScale)), child: TripStoryDayHeader( day: day, - surfaceWeatherRequest: surfaceWeatherRequest, + storedWeather: storedWeather, ), ), ), @@ -200,12 +197,6 @@ void main() { dayNumber: 2, kind: TripStoryDayKind.past, ); - final request = SurfaceDayWeatherRequest( - date: DateTime(2026, 3, 8), - latitude: 12.1, - longitude: -68.2, - ); - testWidgets('gets the same badge and title line as any other day', ( tester, ) async { @@ -264,73 +255,97 @@ void main() { expect(find.byType(Icon), findsNothing); }); - testWidgets('shows fetched weather in the existing badge', (tester) async { + testWidgets('shows stored weather in the existing badge', (tester) async { await pumpHeader( tester, surfaceDay(), - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider(request).overrideWith( - (ref) async => const TripStoryDayWeather( - airTemp: 22, - cloudCover: CloudCover.clear, - ), - ), - ], + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), ); - await tester.pump(); expect(find.byIcon(Icons.wb_sunny_outlined), findsOneWidget); expect(find.text('22°C'), findsOneWidget); }); - testWidgets('fetched temperature respects Fahrenheit', (tester) async { + testWidgets('stored temperature respects Fahrenheit', (tester) async { final settings = MockSettingsNotifier(); await settings.setTemperatureUnit(TemperatureUnit.fahrenheit); await pumpHeader( tester, surfaceDay(), settingsNotifier: settings, - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider( - request, - ).overrideWith((ref) async => const TripStoryDayWeather(airTemp: 22)), - ], + storedWeather: const TripStoryDayWeather(airTemp: 22), ); - await tester.pump(); expect(find.text('71.6°F'), findsOneWidget); }); - testWidgets('loading and failed weather stay badge-free', (tester) async { - final pending = Completer(); - await pumpHeader( - tester, - surfaceDay(), - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider( - request, - ).overrideWith((ref) => pending.future), - ], - ); + testWidgets('without stored weather stays badge-free', (tester) async { + // The header no longer fetches anything, so there is no loading state + // and no spinner to guard against: absent weather is simply no badge. + await pumpHeader(tester, surfaceDay()); expect(find.textContaining('°'), findsNothing); expect(find.byType(CircularProgressIndicator), findsNothing); + }); - pending.completeError(Exception('weather unavailable')); - await tester.pump(); + testWidgets('stored weather wins when the dive weather renders nothing', ( + tester, + ) async { + // A dive whose lookup resolved nothing still carries + // Precipitation.none, which draws no glyph. The stored row is the only + // thing that can render, so it must not be masked. + final day = TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 2, + kind: TripStoryDayKind.past, + dives: [ + Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + precipitation: Precipitation.none, + ), + ], + ); - expect(find.textContaining('°'), findsNothing); - expect(find.byType(CircularProgressIndicator), findsNothing); + await pumpHeader( + tester, + day, + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), + ); + + expect(find.text('22°C'), findsOneWidget); }); - testWidgets('without a request stays badge-free', (tester) async { - await pumpHeader(tester, surfaceDay()); + testWidgets('dive-logged weather wins over stored weather', (tester) async { + // What the diver recorded outranks a fetched day summary. + final day = TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 2, + kind: TripStoryDayKind.past, + dives: [ + Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + airTemp: 26, + site: const DiveSite(id: 'site-a', name: 'Blue Corner'), + ), + ], + ); - expect(find.textContaining('°'), findsNothing); - expect(find.byType(CircularProgressIndicator), findsNothing); + await pumpHeader( + tester, + day, + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('26°C'), findsOneWidget); + expect(find.text('22°C'), findsNothing); }); }); diff --git a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart index 55df7be3ec..8083c1df94 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart @@ -1,11 +1,10 @@ -import 'dart:convert'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; import 'package:submersion/core/providers/provider.dart'; import 'package:submersion/features/checklists/domain/entities/trip_checklist_item.dart'; import 'package:submersion/features/dive_log/domain/entities/dive.dart'; @@ -78,12 +77,14 @@ Future pumpView( List extra = const [], Size viewSize = const Size(800, 2600), http.Client? weatherHttpClient, + Map? tripDayWeather, }) async { tester.view.physicalSize = viewSize; tester.view.devicePixelRatio = 1.0; addTearDown(tester.view.reset); final overrides = await getBaseOverrides( weatherHttpClient: weatherHttpClient, + tripDayWeather: tripDayWeather, ); final stats = TripWithStats(trip: story.trip, diveCount: 2); final router = GoRouter( @@ -405,9 +406,50 @@ void main() { expect(find.text('Surface day'), findsOneWidget); }); - testWidgets('only the surface day fetches from the nearest trip point', ( + testWidgets('the surface day renders stored weather', (tester) async { + final trip = _trip( + start: DateTime(2026, 3, 25), + end: DateTime(2026, 3, 27), + ); + final story = _story( + trip, + dives: [ + _diveAt('d1', DateTime(2026, 3, 25, 9), 12.10, -68.20), + _diveAt('d3', DateTime(2026, 3, 27, 9), 13.30, -69.40), + ], + today: DateTime(2026, 6, 1), + ); + final surfaceDate = DateTime(2026, 3, 26); + final now = DateTime(2026, 3, 28); + + await pumpView( + tester, + story, + tripDayWeather: { + tripDayMillis(surfaceDate): TripDayWeather( + id: 'w1', + tripId: trip.id, + date: surfaceDate, + latitude: 12.10, + longitude: -68.20, + airTemp: 26, + cloudCover: CloudCover.clear, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ), + }, + ); + await tester.pump(); + + expect(find.text('26°C'), findsOneWidget); + }); + + testWidgets('a day with nothing stored renders no weather badge', ( tester, ) async { + // The view no longer falls back to a network fetch while rendering: an + // unstored day is simply badge-free until the backfill writes its row. final trip = _trip( start: DateTime(2026, 3, 25), end: DateTime(2026, 3, 27), @@ -421,34 +463,15 @@ void main() { today: DateTime(2026, 6, 1), ); var calls = 0; - final client = MockClient((request) async { + final client = MockClient((_) async { calls++; - expect(request.url.queryParameters['latitude'], '12.1'); - expect(request.url.queryParameters['longitude'], '-68.2'); - expect(request.url.queryParameters['start_date'], '2026-03-26'); - expect(request.url.queryParameters['timezone'], 'auto'); - return http.Response( - jsonEncode({ - 'hourly': { - 'time': ['2026-03-26T12:00'], - 'temperature_2m': [26.0], - 'relative_humidity_2m': [70.0], - 'precipitation': [0.0], - 'cloud_cover': [10.0], - 'wind_speed_10m': [8.0], - 'wind_direction_10m': [30.0], - 'surface_pressure': [1012.0], - 'weathercode': [0], - }, - }), - 200, - ); + return http.Response('', 500); }); await pumpView(tester, story, weatherHttpClient: client); await tester.pump(); - expect(calls, 1); - expect(find.text('26°C'), findsOneWidget); + expect(calls, 0); + expect(find.textContaining('°C'), findsNothing); }); } diff --git a/test/helpers/mock_providers.dart b/test/helpers/mock_providers.dart index 69e8446883..d35bf934f7 100644 --- a/test/helpers/mock_providers.dart +++ b/test/helpers/mock_providers.dart @@ -28,6 +28,8 @@ import 'package:submersion/features/pre_dive/domain/entities/pre_dive_session.da import 'package:submersion/features/pre_dive/presentation/providers/pre_dive_providers.dart'; import 'package:submersion/core/utils/coordinates/coordinate_format.dart'; import 'package:submersion/features/settings/presentation/providers/settings_providers.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; typedef Override = riverpod.Override; @@ -541,6 +543,7 @@ Future> getBaseOverrides({ MockSettingsNotifier? settingsNotifier, http.Client? weatherHttpClient, PreDiveSession? linkedPreDiveSession, + Map? tripDayWeather, }) async { SharedPreferences.setMockInitialValues({}); final prefs = await SharedPreferences.getInstance(); @@ -571,5 +574,13 @@ Future> getBaseOverrides({ weatherHttpClientProvider.overrideWithValue( weatherHttpClient ?? MockClient((_) async => http.Response('', 500)), ), + // Stored trip day weather reaches the real repository and a database + // widget tests do not have; the backfill would additionally walk the + // story and fetch. Both default to inert here, so a test that cares about + // the badge overrides tripDayWeatherProvider with its own rows. + tripDayWeatherProvider.overrideWith( + (ref, tripId) async => tripDayWeather ?? const {}, + ), + tripDayWeatherBackfillProvider.overrideWith((ref, tripId) async {}), ]; }