From 88020b362ae3e7817c241e3f44123db71e767ffc Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 11 Aug 2026 20:14:49 +0600 Subject: [PATCH] trim the datetime helpers and comments from review --- .../repositories/remote_album.repository.dart | 2 +- .../repositories/remote_asset.repository.dart | 6 ++--- .../repositories/sync_stream.repository.dart | 10 +++----- mobile/lib/utils/datetime_helpers.dart | 25 +++---------------- mobile/lib/utils/migration.dart | 2 -- mobile/test/drift/main/migration_test.dart | 5 ---- .../repositories/merged_asset_drift_test.dart | 13 +++------- 7 files changed, 14 insertions(+), 49 deletions(-) diff --git a/mobile/lib/infrastructure/repositories/remote_album.repository.dart b/mobile/lib/infrastructure/repositories/remote_album.repository.dart index 5d98b2afbc..ae197a7aa1 100644 --- a/mobile/lib/infrastructure/repositories/remote_album.repository.dart +++ b/mobile/lib/infrastructure/repositories/remote_album.repository.dart @@ -328,7 +328,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { isFavorite: Value(source.isFavorite), visibility: const Value(AssetVisibility.timeline), isEdited: Value(source.isEdited), - groupDate: Value(remoteGroupDate(null, source.createdAt)), + groupDate: Value(timelineGroupDate(source.createdAt.toLocal())), ), mode: InsertMode.insertOrIgnore, ); diff --git a/mobile/lib/infrastructure/repositories/remote_asset.repository.dart b/mobile/lib/infrastructure/repositories/remote_asset.repository.dart index 34dfbaac8a..3ac4d908bd 100644 --- a/mobile/lib/infrastructure/repositories/remote_asset.repository.dart +++ b/mobile/lib/infrastructure/repositories/remote_asset.repository.dart @@ -12,6 +12,7 @@ import 'package:immich_mobile/infrastructure/entities/stack.entity.drift.dart'; import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; import 'package:immich_mobile/utils/datetime_helpers.dart'; import 'package:immich_mobile/utils/option.dart'; +import 'package:immich_mobile/utils/timezone.dart'; import 'package:maplibre_gl/maplibre_gl.dart'; class RemoteAssetRepository extends DriftDatabaseRepository { @@ -293,9 +294,8 @@ class RemoteAssetRepository extends DriftDatabaseRepository { } Future updateDateTime(List ids, DateTime dateTime, {String? timeZone}) { - // the picker value arrives as UTC; its wall day lives in the offset - final offset = tryParseUtcOffset(timeZone); - final groupDate = timelineGroupDate(offset != null ? dateTime.toUtc().add(offset) : dateTime); + final (adjusted, _) = applyTimezoneOffset(dateTime: dateTime, timeZone: timeZone); + final groupDate = timelineGroupDate(adjusted); return _db.batch((batch) async { for (final id in ids) { batch.update( diff --git a/mobile/lib/infrastructure/repositories/sync_stream.repository.dart b/mobile/lib/infrastructure/repositories/sync_stream.repository.dart index ce71dddfb6..82d002c335 100644 --- a/mobile/lib/infrastructure/repositories/sync_stream.repository.dart +++ b/mobile/lib/infrastructure/repositories/sync_stream.repository.dart @@ -200,6 +200,7 @@ class SyncStreamRepository extends DriftDatabaseRepository { try { await _db.batch((batch) { for (final asset in data) { + final groupDate = asset.localDateTime ?? asset.fileCreatedAt?.toLocal(); final companion = RemoteAssetEntityCompanion( name: Value(asset.originalFileName), type: Value(asset.type.toAssetType()), @@ -211,9 +212,7 @@ class SyncStreamRepository extends DriftDatabaseRepository { isFavorite: Value(asset.isFavorite), ownerId: Value(asset.ownerId), localDateTime: Value(asset.localDateTime), - groupDate: asset.localDateTime == null && asset.fileCreatedAt == null - ? const Value.absent() - : Value(remoteGroupDate(asset.localDateTime, asset.fileCreatedAt!)), + groupDate: groupDate == null ? const Value.absent() : Value(timelineGroupDate(groupDate)), thumbHash: Value(asset.thumbhash), deletedAt: Value(asset.deletedAt), visibility: Value(asset.visibility.toAssetVisibility()), @@ -242,6 +241,7 @@ class SyncStreamRepository extends DriftDatabaseRepository { try { await _db.batch((batch) { for (final asset in data) { + final groupDate = asset.localDateTime ?? asset.fileCreatedAt?.toLocal(); final companion = RemoteAssetEntityCompanion( name: Value(asset.originalFileName), type: Value(asset.type.toAssetType()), @@ -253,9 +253,7 @@ class SyncStreamRepository extends DriftDatabaseRepository { isFavorite: Value(asset.isFavorite), ownerId: Value(asset.ownerId), localDateTime: Value(asset.localDateTime), - groupDate: asset.localDateTime == null && asset.fileCreatedAt == null - ? const Value.absent() - : Value(remoteGroupDate(asset.localDateTime, asset.fileCreatedAt!)), + groupDate: groupDate == null ? const Value.absent() : Value(timelineGroupDate(groupDate)), thumbHash: Value(asset.thumbhash), deletedAt: Value(asset.deletedAt), visibility: Value(asset.visibility.toAssetVisibility()), diff --git a/mobile/lib/utils/datetime_helpers.dart b/mobile/lib/utils/datetime_helpers.dart index bcad1fdf44..6c0778f180 100644 --- a/mobile/lib/utils/datetime_helpers.dart +++ b/mobile/lib/utils/datetime_helpers.dart @@ -18,25 +18,6 @@ DateTime? tryFromSecondsSinceEpoch(int? secondsSinceEpoch, {bool isUtc = false}) } } -// Uses the held components, so convert with toLocal() first for instants. -// Dates sqlite cannot read (year outside 1..9999) fall to null instead of a mangled day -String? timelineGroupDate(DateTime value) { - if (value.year < 1 || value.year > 9999) { - return null; - } - return value.toIso8601String().split('T').first; -} - -// 'UTC+14:00' style, from the date picker path -Duration? tryParseUtcOffset(String? value) { - final match = value == null ? null : RegExp(r'^UTC([+-])(\d{2}):(\d{2})$').firstMatch(value); - if (match == null) { - return null; - } - final minutes = int.parse(match[2]!) * 60 + int.parse(match[3]!); - return Duration(minutes: match[1] == '-' ? -minutes : minutes); -} - -// group_date for remote rows: wall day when known, else the local day of createdAt -String? remoteGroupDate(DateTime? localDateTime, DateTime createdAt) => - (localDateTime != null ? timelineGroupDate(localDateTime) : null) ?? timelineGroupDate(createdAt.toLocal()); +// no DateFormat: it tracks Intl.defaultLocale and needs locale init per isolate +String timelineGroupDate(DateTime value) => + '${value.year.toString().padLeft(4, '0')}-${value.month.toString().padLeft(2, '0')}-${value.day.toString().padLeft(2, '0')}'; diff --git a/mobile/lib/utils/migration.dart b/mobile/lib/utils/migration.dart index 04077dae32..3347456e89 100644 --- a/mobile/lib/utils/migration.dart +++ b/mobile/lib/utils/migration.dart @@ -48,8 +48,6 @@ Future migrateDatabaseIfNeeded(Drift drift) async { Future _migrateTo28(Drift drift) => backfillAssetGroupDates(drift); -// Store-level on purpose: runs after the date heals in this chain, so group_date is -// computed from the corrected values. STRFTIME drops dates sqlite cannot read. Future backfillAssetGroupDates(Drift drift) async { await drift.customStatement( "UPDATE remote_asset_entity SET group_date = COALESCE(STRFTIME('%Y-%m-%d', local_date_time), STRFTIME('%Y-%m-%d', created_at, 'localtime'))", diff --git a/mobile/test/drift/main/migration_test.dart b/mobile/test/drift/main/migration_test.dart index 28636ed62c..93f7c5b5e4 100644 --- a/mobile/test/drift/main/migration_test.dart +++ b/mobile/test/drift/main/migration_test.dart @@ -41,8 +41,6 @@ void main() { }); group('v32 group_date backfill', () { - // 26479 class: platform handed us a date sqlite cannot read. the bucket stream - // must not die, and the broken row must vanish from buckets and assets alike. test( 'garbage created_at survives migration and is filtered from the timeline', () async { @@ -80,8 +78,6 @@ void main() { }, ); - // The drift migration only adds the column; the backfill is a store-level step so a - // created_at heal (29193) that runs before it actually lands in group_date. test('a created_at heal before the backfill lands in the header day', () async { await initializeDateFormatting(); final schema = await verifier.schemaAt(31); @@ -98,7 +94,6 @@ void main() { final db = Drift(schema.newConnection()); await verifier.migrateAndValidate(db, 32); - // 29193's store-level heal await db.customStatement( "UPDATE local_asset_entity SET created_at = updated_at WHERE julianday(created_at) > julianday(updated_at)", ); diff --git a/mobile/test/infrastructure/repositories/merged_asset_drift_test.dart b/mobile/test/infrastructure/repositories/merged_asset_drift_test.dart index a37dde3733..a44ed131b2 100644 --- a/mobile/test/infrastructure/repositories/merged_asset_drift_test.dart +++ b/mobile/test/infrastructure/repositories/merged_asset_drift_test.dart @@ -49,7 +49,7 @@ void main() { updatedAt: Value(createdAt), uploadedAt: Value(createdAt), localDateTime: Value(localDateTime), - groupDate: Value(remoteGroupDate(localDateTime, createdAt)), + groupDate: Value(timelineGroupDate(localDateTime ?? createdAt.toLocal())), ), ); @@ -72,8 +72,7 @@ void main() { .insert(LocalAlbumAssetEntityCompanion.insert(assetId: id, albumId: _albumId)); } - // Mirrors how the timeline pairs headers with tiles: buckets only carry a count, the assets - // come from one flat list that is addressed by the running offset of the previous buckets. + // the timeline pairs headers to assets by running offset into one flat list Future> headerForEachAsset(GroupAssetsBy groupBy) async { final buckets = await db.mergedAssetDrift.mergedBucket(groupBy: groupBy.index, userIds: [_userId]).get(); final assets = await db.mergedAssetDrift.mergedAsset(userIds: [_userId], limit: (_) => Limit(1000, 0)).get(); @@ -89,8 +88,7 @@ void main() { return pairs; } - // Regression for #29864: buckets group by localDateTime but assets were ordered by createdAt, - // so a header minted from one asset's localDateTime was rendered above a different asset. + // #29864: buckets grouped by localDateTime while assets sorted by createdAt group('mergedAsset ordering matches mergedBucket grouping', () { Future seedGhost() async { await insertRemote('asset-a', createdAt: DateTime(2026, 4, 26, 10)); @@ -122,9 +120,6 @@ void main() { expect(await headerForEachAsset(GroupAssetsBy.day), [('asset-b', '2026-04-26'), ('asset-a', '2026-04-26')]); }); - // Web-consistency: the server groups on the date only and sorts within a day by - // createdAt, so two assets whose wall clock and createdAt disagree within the same - // day must come out in createdAt order. test('same-day assets order by createdAt like the web timeline', () async { await insertRemote( 'shot-late-upload-early', @@ -163,8 +158,6 @@ void main() { expect(await headerForEachAsset(GroupAssetsBy.month), [('may', '2024-05'), ('april', '2024-04')]); }); - // Store-migration order: the 29193 heal runs first, the group_date backfill after it, - // so the corrected date is what lands under the header. test('a created_at heal before the backfill lands in the header day', () async { await db .into(db.localAssetEntity)