diff --git a/lib/features/trips/data/repositories/trip_day_weather_repository.dart b/lib/features/trips/data/repositories/trip_day_weather_repository.dart index 399645d5d8..046e2030c7 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:drift/drift.dart'; import 'package:submersion/core/constants/enums.dart'; @@ -8,6 +9,8 @@ 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 tripDayMillis, tripDayWeatherRowId; /// Reads and writes stored per-day trip weather. /// @@ -19,18 +22,42 @@ class TripDayWeatherRepository { final SyncRepository _syncRepository = SyncRepository(); final _log = LoggerService.forClass(TripDayWeatherRepository); + /// Local midnight 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(); - return {for (final row in rows) row.date: _mapRow(row)}; + + final winners = {}; + for (final row in rows) { + final day = _dayKey(DateTime.fromMillisecondsSinceEpoch(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', @@ -45,44 +72,77 @@ class TripDayWeatherRepository { Future upsert(domain.TripDayWeather weather) async { try { final now = DateTime.now().millisecondsSinceEpoch; - final dateMillis = weather.date.millisecondsSinceEpoch; - - // Reuse the stored row's id when the day already has one: a peer may - // have written its own uuid for this day, and replacing it under a new - // id would violate the unique index and orphan the peer's sync record. - final existing = - await (_db.select(_db.tripDayWeather)..where( - (t) => - t.tripId.equals(weather.tripId) & t.date.equals(dateMillis), - )) - .getSingleOrNull(); - final id = existing?.id ?? weather.id; - - 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( - existing?.createdAt ?? weather.createdAt.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(); + + 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. One transaction, so a day is + // never left with its old row deleted and no new one in its place. + 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), ), - updatedAt: Value(now), - ), - ); + ); + }); + + // After the row is in place, so a failed write leaves no tombstone for + // a day that still has its original row. A stray is a synced record: + // dropping it without one lets the peer that sent it hand it back on + // the next pull. + for (final stray in strays) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: stray, + ); + } await _syncRepository.markRecordPending( entityType: 'tripDayWeather', @@ -131,11 +191,67 @@ class TripDayWeatherRepository { } } + /// Every stored row for this trip that falls on [dayMillis]'s calendar day. + /// + /// Filtered in Dart rather than SQL: local midnight is not something SQLite + /// can derive from the stored epoch millis without knowing the zone and its + /// DST history. A trip holds one row per day, so the scan is a few dozen + /// rows at most. + 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(DateTime.fromMillisecondsSinceEpoch(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, - date: DateTime.fromMillisecondsSinceEpoch(row.date), + // 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: DateTime.fromMillisecondsSinceEpoch( + _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)), + ), latitude: row.latitude, longitude: row.longitude, airTemp: row.airTemp, diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index a9ae4be434..227b404b6b 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -1,8 +1,37 @@ 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'; + +/// Local midnight for [date], as epoch milliseconds. +/// +/// 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. +int tripDayMillis(DateTime date) => + DateTime(date.year, date.month, date.day).millisecondsSinceEpoch; + +/// 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 normalized to local midnight; pass it through +/// [tripDayMillis]. +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 @@ -65,15 +94,10 @@ class TripDayWeather extends Equatable { /// carrying only those renders as nothing and would suppress the retry that /// a later archive update would satisfy. /// - /// [Precipitation.none] does not count. `WeatherMapper.mapPrecipitation` - /// returns non-null always, defaulting a missing reading to `none`, so - /// `none` cannot be read as evidence that the fetch resolved anything. It - /// also earns no glyph of its own in `weatherIconFor`, which falls through - /// to cloud cover. - bool get hasRenderableWeather => - airTemp != null || - cloudCover != null || - (precipitation != null && precipitation != Precipitation.none); + /// 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( 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 index 05222ec407..0842cfa18f 100644 --- a/lib/features/trips/domain/services/trip_day_weather_backfill.dart +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -45,7 +45,13 @@ class TripDayWeatherBackfill { // A dive that logged weather is the better source: it is what the diver // recorded. Never override it with a fetched summary. - if (day.weather != null) continue; + // + // 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; diff --git a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart index f337c35611..2591b4ff15 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -1,5 +1,3 @@ -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'; @@ -52,8 +50,6 @@ final tripDayWeatherBackfillProvider = FutureProvider.family(( ); 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 the day headers // fill in progressively. @@ -70,8 +66,13 @@ final tripDayWeatherBackfillProvider = FutureProvider.family(( if (weather == null) continue; final now = DateTime.now(); + final dayMillis = tripDayMillis(target.date); final row = TripDayWeather( - id: uuid.v4(), + // 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, 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 bea6ef77ee..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 @@ -61,9 +61,15 @@ class TripStoryDayHeader extends ConsumerWidget { ...day.siteNames, ].map((part) => part.trim()).where((part) => part.isNotEmpty).toList(); - // Dive-logged weather always wins: it is what the diver recorded, and a - // stored day summary is only ever a stand-in for days that logged none. - final weather = day.weather ?? storedWeather; + // 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/test/core/services/sync/trip_day_weather_sync_test.dart b/test/core/services/sync/trip_day_weather_sync_test.dart index 781fb8f41c..66719a2aee 100644 --- a/test/core/services/sync/trip_day_weather_sync_test.dart +++ b/test/core/services/sync/trip_day_weather_sync_test.dart @@ -4,6 +4,7 @@ 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'; @@ -11,6 +12,9 @@ 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(); @@ -30,7 +34,10 @@ void main() { .into(db.tripDayWeather) .insert( TripDayWeatherCompanion.insert( - id: 'w-1', + id: tripDayWeatherRowId( + tripId: 'trip-1', + dayMillis: DateTime(2026, 3, 8).millisecondsSinceEpoch, + ), tripId: 'trip-1', date: DateTime(2026, 3, 8).millisecondsSinceEpoch, latitude: 12.16, @@ -47,7 +54,7 @@ void main() { tearDown(tearDownTestDatabase); test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { - final record = await serializer.fetchRecord('tripDayWeather', 'w-1'); + final record = await serializer.fetchRecord('tripDayWeather', rowId); expect(record, isNotNull); expect(record!['airTemp'], 24.0); expect(record['cloudCover'], 'clear'); @@ -58,19 +65,19 @@ void main() { 'airTemp': 26.0, 'updatedAt': 2, }); - final merged = await serializer.fetchRecord('tripDayWeather', 'w-1'); + final merged = await serializer.fetchRecord('tripDayWeather', rowId); expect(merged!['airTemp'], 26.0); - expect(await serializer.recordIdsFor('tripDayWeather'), contains('w-1')); + expect(await serializer.recordIdsFor('tripDayWeather'), contains(rowId)); - await serializer.deleteRecord('tripDayWeather', 'w-1'); - expect(await serializer.fetchRecord('tripDayWeather', 'w-1'), isNull); + 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('w-1'))).write( + )..where((t) => t.id.equals(rowId))).write( const TripDayWeatherCompanion(hlc: Value('2026-08-16T00:00:00.000-0000')), ); @@ -90,6 +97,52 @@ void main() { 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 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 index 3686260aa5..4379f53a3d 100644 --- a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -1,5 +1,9 @@ +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'; @@ -119,6 +123,33 @@ void main() { expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); }); + test('a date with a time component is stored under local 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 midnight-keyed lookups and would + // refetch forever. + return () async { + await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30))); + + final stored = await repository.getForTrip(testTripId); + + expect(stored.keys.single, day1.millisecondsSinceEpoch); + expect(stored[day1.millisecondsSinceEpoch]!.date, day1); + }(); + }); + + 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[day1.millisecondsSinceEpoch]!.airTemp, 25); + }); + test('two different days both persist', () async { await repository.upsert(sample()); await repository.upsert(sample(id: 'w2', date: day2, airTemp: 19)); @@ -176,4 +207,165 @@ void main() { 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. + Future insertRaw({ + required String id, + required DateTime date, + 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(date.millisecondsSinceEpoch), + 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', date: 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', + date: DateTime(2026, 3, 8, 17, 30), + airTemp: 10, + ); + + await repository.upsert(sample(airTemp: 25)); + + final rows = await allRows(); + expect(rows, hasLength(1)); + expect(rows.single.date, day1.millisecondsSinceEpoch); + expect(rows.single.airTemp, 25); + }); + + test('a row for another day is left alone', () async { + await insertRaw(id: 'other-day', date: 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', date: 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', + date: DateTime(2026, 3, 8, 17, 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', + date: DateTime(2026, 3, 8, 17, 30), + airTemp: 10, + updatedAt: 9999999, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + }, + ); + + test('getForTrip falls back to the most recently updated stray', () async { + await insertRaw( + id: 'peer-a', + date: DateTime(2026, 3, 8, 6), + airTemp: 10, + updatedAt: 100, + ); + await insertRaw( + id: 'peer-b', + date: DateTime(2026, 3, 8, 23), + airTemp: 20, + updatedAt: 200, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.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', date: DateTime(2026, 3, 8, 6)); + await insertRaw(id: 'peer-b', date: DateTime(2026, 3, 8, 23)); + + await repository.getForTrip(testTripId); + + expect(await allRows(), hasLength(2)); + }); + }); } 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 index 70e6563e72..44e1152522 100644 --- a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart +++ b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart @@ -1,4 +1,5 @@ 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'; @@ -28,8 +29,17 @@ void main() { ); } - Dive diveWith({double? airTemp}) => - Dive(id: 'd1', dateTime: DateTime(2026, 3, 8, 9), airTemp: airTemp); + 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, @@ -196,6 +206,55 @@ void main() { ]); }); + 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 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 a6c4adc02e..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 @@ -291,6 +291,37 @@ void main() { expect(find.byType(CircularProgressIndicator), findsNothing); }); + 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, + ), + ], + ); + + await pumpHeader( + tester, + day, + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), + ); + + expect(find.text('22°C'), findsOneWidget); + }); + testWidgets('dive-logged weather wins over stored weather', (tester) async { // What the diver recorded outranks a fetched day summary. final day = TripStoryDay(