fix(mobile): newest/oldest album sort (#20743)

* fix(mobile): newest/oldest album sort

* chore: use sqlite to determine album asset timestamps

* Fix missing future

Co-authored-by: Alex <alex.tran1502@gmail.com>

* fix: async handling of sort

* chore: tests

* chore: code review changes

* fix: use created at for newest asset

* fix: use localDateTime for sorting

* chore: cleanup

* chore: use final

* feat: loading indicator

---------

Co-authored-by: Alex <alex.tran1502@gmail.com>

i18n/en.json

@@ -1856,6 +1856,7 @@
   "sort_created": "Date created",
   "sort_items": "Number of items",
   "sort_modified": "Date modified",
+  "sort_newest": "Newest photo",
   "sort_oldest": "Oldest photo",
   "sort_people_by_similarity": "Sort people by similarity",
   "sort_recent": "Most recent photo",

mobile/lib/domain/services/remote_album.service.dart

@@ -1,12 +1,12 @@
 import 'dart:async';
 
+import 'package:collection/collection.dart';
 import 'package:immich_mobile/domain/models/album/album.model.dart';
 import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
 import 'package:immich_mobile/domain/models/user.model.dart';
 import 'package:immich_mobile/infrastructure/repositories/remote_album.repository.dart';
 import 'package:immich_mobile/models/albums/album_search.model.dart';
 import 'package:immich_mobile/repositories/drift_album_api_repository.dart';
-import 'package:immich_mobile/utils/remote_album.utils.dart';
 
 class RemoteAlbumService {
   final DriftRemoteAlbumRepository _repository;
@@ -26,8 +26,21 @@ class RemoteAlbumService {
     return _repository.get(albumId);
   }
 
-  List<RemoteAlbum> sortAlbums(List<RemoteAlbum> albums, RemoteAlbumSortMode sortMode, {bool isReverse = false}) {
+  Future<List<RemoteAlbum>> sortAlbums(
-    return sortMode.sortFn(albums, isReverse);
+    List<RemoteAlbum> albums,
+    RemoteAlbumSortMode sortMode, {
+    bool isReverse = false,
+  }) async {
+    final List<RemoteAlbum> sorted = switch (sortMode) {
+      RemoteAlbumSortMode.created => albums.sortedBy((album) => album.createdAt),
+      RemoteAlbumSortMode.title => albums.sortedBy((album) => album.name),
+      RemoteAlbumSortMode.lastModified => albums.sortedBy((album) => album.updatedAt),
+      RemoteAlbumSortMode.assetCount => albums.sortedBy((album) => album.assetCount),
+      RemoteAlbumSortMode.mostRecent => await _sortByNewestAsset(albums),
+      RemoteAlbumSortMode.mostOldest => await _sortByOldestAsset(albums),
+    };
+
+    return (isReverse ? sorted.reversed : sorted).toList();
   }
 
   List<RemoteAlbum> searchAlbums(
@@ -143,4 +156,60 @@ class RemoteAlbumService {
   Future<int> getCount() {
     return _repository.getCount();
   }
+
+  Future<List<RemoteAlbum>> _sortByNewestAsset(List<RemoteAlbum> albums) async {
+    // map album IDs to their newest asset dates
+    final Map<String, Future<DateTime?>> assetTimestampFutures = {};
+    for (final album in albums) {
+      assetTimestampFutures[album.id] = _repository.getNewestAssetTimestamp(album.id);
+    }
+
+    // await all database queries
+    final entries = await Future.wait(
+      assetTimestampFutures.entries.map((entry) async => MapEntry(entry.key, await entry.value)),
+    );
+    final assetTimestamps = Map.fromEntries(entries);
+
+    final sorted = albums.sorted((a, b) {
+      final aDate = assetTimestamps[a.id] ?? DateTime.fromMillisecondsSinceEpoch(0);
+      final bDate = assetTimestamps[b.id] ?? DateTime.fromMillisecondsSinceEpoch(0);
+      return aDate.compareTo(bDate);
+    });
+
+    return sorted;
+  }
+
+  Future<List<RemoteAlbum>> _sortByOldestAsset(List<RemoteAlbum> albums) async {
+    // map album IDs to their oldest asset dates
+    final Map<String, Future<DateTime?>> assetTimestampFutures = {
+      for (final album in albums) album.id: _repository.getOldestAssetTimestamp(album.id),
+    };
+
+    // await all database queries
+    final entries = await Future.wait(
+      assetTimestampFutures.entries.map((entry) async => MapEntry(entry.key, await entry.value)),
+    );
+    final assetTimestamps = Map.fromEntries(entries);
+
+    final sorted = albums.sorted((a, b) {
+      final aDate = assetTimestamps[a.id] ?? DateTime.fromMillisecondsSinceEpoch(0);
+      final bDate = assetTimestamps[b.id] ?? DateTime.fromMillisecondsSinceEpoch(0);
+      return aDate.compareTo(bDate);
+    });
+
+    return sorted.reversed.toList();
+  }
+}
+
+enum RemoteAlbumSortMode {
+  title("library_page_sort_title"),
+  assetCount("library_page_sort_asset_count"),
+  lastModified("library_page_sort_last_modified"),
+  created("library_page_sort_created"),
+  mostRecent("sort_newest"),
+  mostOldest("sort_oldest");
+
+  final String key;
+
+  const RemoteAlbumSortMode(this.key);
 }

mobile/lib/infrastructure/repositories/remote_album.repository.dart

@@ -265,6 +265,28 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository {
     }).watchSingleOrNull();
   }
 
+  Future<DateTime?> getNewestAssetTimestamp(String albumId) {
+    final query = _db.remoteAlbumAssetEntity.selectOnly()
+      ..where(_db.remoteAlbumAssetEntity.albumId.equals(albumId))
+      ..addColumns([_db.remoteAssetEntity.localDateTime.max()])
+      ..join([
+        innerJoin(_db.remoteAssetEntity, _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId)),
+      ]);
+
+    return query.map((row) => row.read(_db.remoteAssetEntity.localDateTime.max())).getSingleOrNull();
+  }
+
+  Future<DateTime?> getOldestAssetTimestamp(String albumId) {
+    final query = _db.remoteAlbumAssetEntity.selectOnly()
+      ..where(_db.remoteAlbumAssetEntity.albumId.equals(albumId))
+      ..addColumns([_db.remoteAssetEntity.localDateTime.min()])
+      ..join([
+        innerJoin(_db.remoteAssetEntity, _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId)),
+      ]);
+
+    return query.map((row) => row.read(_db.remoteAssetEntity.localDateTime.min())).getSingleOrNull();
+  }
+
   Future<int> getCount() {
     return _db.managers.remoteAlbumEntity.count();
   }

mobile/lib/presentation/widgets/album/album_selector.widget.dart

@@ -7,6 +7,7 @@ import 'package:flutter/material.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/domain/models/album/album.model.dart';
 import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
+import 'package:immich_mobile/domain/services/remote_album.service.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/extensions/theme_extensions.dart';
 import 'package:immich_mobile/extensions/translate_extensions.dart';
@@ -18,7 +19,6 @@ import 'package:immich_mobile/providers/infrastructure/current_album.provider.da
 import 'package:immich_mobile/providers/timeline/multiselect.provider.dart';
 import 'package:immich_mobile/providers/user.provider.dart';
 import 'package:immich_mobile/routing/router.dart';
-import 'package:immich_mobile/utils/remote_album.utils.dart';
 import 'package:immich_mobile/widgets/common/confirm_dialog.dart';
 import 'package:immich_mobile/widgets/common/immich_toast.dart';
 import 'package:immich_mobile/widgets/common/search_field.dart';
@@ -138,21 +138,28 @@ class _SortButton extends ConsumerStatefulWidget {
 class _SortButtonState extends ConsumerState<_SortButton> {
   RemoteAlbumSortMode albumSortOption = RemoteAlbumSortMode.lastModified;
   bool albumSortIsReverse = true;
+  bool isSorting = false;
 
-  void onMenuTapped(RemoteAlbumSortMode sortMode) {
+  Future<void> onMenuTapped(RemoteAlbumSortMode sortMode) async {
     final selected = albumSortOption == sortMode;
     // Switch direction
     if (selected) {
       setState(() {
         albumSortIsReverse = !albumSortIsReverse;
+        isSorting = true;
       });
-      ref.read(remoteAlbumProvider.notifier).sortFilteredAlbums(sortMode, isReverse: albumSortIsReverse);
+      await ref.read(remoteAlbumProvider.notifier).sortFilteredAlbums(sortMode, isReverse: albumSortIsReverse);
     } else {
       setState(() {
         albumSortOption = sortMode;
+        isSorting = true;
       });
-      ref.read(remoteAlbumProvider.notifier).sortFilteredAlbums(sortMode, isReverse: albumSortIsReverse);
+      await ref.read(remoteAlbumProvider.notifier).sortFilteredAlbums(sortMode, isReverse: albumSortIsReverse);
     }
+
+    setState(() {
+      isSorting = false;
+    });
   }
 
   @override
@@ -230,6 +237,16 @@ class _SortButtonState extends ConsumerState<_SortButton> {
                   color: context.colorScheme.onSurface.withAlpha(225),
                 ),
               ),
+              isSorting
+                  ? SizedBox(
+                      width: 22,
+                      height: 22,
+                      child: CircularProgressIndicator(
+                        strokeWidth: 2,
+                        color: context.colorScheme.onSurface.withAlpha(225),
+                      ),
+                    )
+                  : const SizedBox.shrink(),
             ],
           ),
         );

mobile/lib/providers/infrastructure/remote_album.provider.dart

@@ -5,7 +5,6 @@ import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
 import 'package:immich_mobile/domain/models/user.model.dart';
 import 'package:immich_mobile/domain/services/remote_album.service.dart';
 import 'package:immich_mobile/models/albums/album_search.model.dart';
-import 'package:immich_mobile/utils/remote_album.utils.dart';
 import 'package:logging/logging.dart';
 import 'package:riverpod_annotation/riverpod_annotation.dart';
 
@@ -71,8 +70,8 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
     state = state.copyWith(filteredAlbums: state.albums);
   }
 
-  void sortFilteredAlbums(RemoteAlbumSortMode sortMode, {bool isReverse = false}) {
+  Future<void> sortFilteredAlbums(RemoteAlbumSortMode sortMode, {bool isReverse = false}) async {
-    final sortedAlbums = _remoteAlbumService.sortAlbums(state.filteredAlbums, sortMode, isReverse: isReverse);
+    final sortedAlbums = await _remoteAlbumService.sortAlbums(state.filteredAlbums, sortMode, isReverse: isReverse);
     state = state.copyWith(filteredAlbums: sortedAlbums);
   }
 

mobile/lib/utils/remote_album.utils.dart

@@ -1,64 +0,0 @@
-import 'package:collection/collection.dart';
-import 'package:immich_mobile/domain/models/album/album.model.dart';
-
-typedef AlbumSortFn = List<RemoteAlbum> Function(List<RemoteAlbum> albums, bool isReverse);
-
-class _RemoteAlbumSortHandlers {
-  const _RemoteAlbumSortHandlers._();
-
-  static const AlbumSortFn created = _sortByCreated;
-  static List<RemoteAlbum> _sortByCreated(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sortedBy((album) => album.createdAt);
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-
-  static const AlbumSortFn title = _sortByTitle;
-  static List<RemoteAlbum> _sortByTitle(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sortedBy((album) => album.name);
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-
-  static const AlbumSortFn lastModified = _sortByLastModified;
-  static List<RemoteAlbum> _sortByLastModified(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sortedBy((album) => album.updatedAt);
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-
-  static const AlbumSortFn assetCount = _sortByAssetCount;
-  static List<RemoteAlbum> _sortByAssetCount(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sorted((a, b) => a.assetCount.compareTo(b.assetCount));
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-
-  static const AlbumSortFn mostRecent = _sortByMostRecent;
-  static List<RemoteAlbum> _sortByMostRecent(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sorted((a, b) {
-      // For most recent, we sort by updatedAt in descending order
-      return b.updatedAt.compareTo(a.updatedAt);
-    });
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-
-  static const AlbumSortFn mostOldest = _sortByMostOldest;
-  static List<RemoteAlbum> _sortByMostOldest(List<RemoteAlbum> albums, bool isReverse) {
-    final sorted = albums.sorted((a, b) {
-      // For oldest, we sort by createdAt in ascending order
-      return a.createdAt.compareTo(b.createdAt);
-    });
-    return (isReverse ? sorted.reversed : sorted).toList();
-  }
-}
-
-enum RemoteAlbumSortMode {
-  title("library_page_sort_title", _RemoteAlbumSortHandlers.title),
-  assetCount("library_page_sort_asset_count", _RemoteAlbumSortHandlers.assetCount),
-  lastModified("library_page_sort_last_modified", _RemoteAlbumSortHandlers.lastModified),
-  created("library_page_sort_created", _RemoteAlbumSortHandlers.created),
-  mostRecent("sort_recent", _RemoteAlbumSortHandlers.mostRecent),
-  mostOldest("sort_oldest", _RemoteAlbumSortHandlers.mostOldest);
-
-  final String key;
-  final AlbumSortFn sortFn;
-
-  const RemoteAlbumSortMode(this.key, this.sortFn);
-}

mobile/test/domain/services/album.service_test.dart

@@ -0,0 +1,116 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:immich_mobile/domain/models/album/album.model.dart';
+import 'package:immich_mobile/domain/services/remote_album.service.dart';
+import 'package:immich_mobile/infrastructure/repositories/remote_album.repository.dart';
+import 'package:immich_mobile/repositories/drift_album_api_repository.dart';
+import 'package:mocktail/mocktail.dart';
+
+import '../../infrastructure/repository.mock.dart';
+
+void main() {
+  late RemoteAlbumService sut;
+  late DriftRemoteAlbumRepository mockRemoteAlbumRepo;
+  late DriftAlbumApiRepository mockAlbumApiRepo;
+
+  setUp(() {
+    mockRemoteAlbumRepo = MockRemoteAlbumRepository();
+    mockAlbumApiRepo = MockDriftAlbumApiRepository();
+    sut = RemoteAlbumService(mockRemoteAlbumRepo, mockAlbumApiRepo);
+
+    when(() => mockRemoteAlbumRepo.getNewestAssetTimestamp(any())).thenAnswer((invocation) {
+      // Simulate a timestamp for the newest asset in the album
+      final albumID = invocation.positionalArguments[0] as String;
+
+      if (albumID == '1') {
+        return Future.value(DateTime(2023, 1, 1));
+      } else if (albumID == '2') {
+        return Future.value(DateTime(2023, 2, 1));
+      }
+
+      return Future.value(DateTime.fromMillisecondsSinceEpoch(0));
+    });
+
+    when(() => mockRemoteAlbumRepo.getOldestAssetTimestamp(any())).thenAnswer((invocation) {
+      // Simulate a timestamp for the oldest asset in the album
+      final albumID = invocation.positionalArguments[0] as String;
+
+      if (albumID == '1') {
+        return Future.value(DateTime(2019, 1, 1));
+      } else if (albumID == '2') {
+        return Future.value(DateTime(2019, 2, 1));
+      }
+
+      return Future.value(DateTime.fromMillisecondsSinceEpoch(0));
+    });
+  });
+
+  final albumA = RemoteAlbum(
+    id: '1',
+    name: 'Album A',
+    description: "",
+    isActivityEnabled: false,
+    order: AlbumAssetOrder.asc,
+    assetCount: 1,
+    createdAt: DateTime(2023, 1, 1),
+    updatedAt: DateTime(2023, 1, 2),
+    ownerId: 'owner1',
+    ownerName: "Test User",
+  );
+
+  final albumB = RemoteAlbum(
+    id: '2',
+    name: 'Album B',
+    description: "",
+    isActivityEnabled: false,
+    order: AlbumAssetOrder.desc,
+    assetCount: 2,
+    createdAt: DateTime(2023, 2, 1),
+    updatedAt: DateTime(2023, 2, 2),
+    ownerId: 'owner2',
+    ownerName: "Test User",
+  );
+
+  group('sortAlbums', () {
+    test('should sort correctly based on name', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.title);
+      expect(result, [albumA, albumB]);
+    });
+
+    test('should sort correctly based on createdAt', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.created);
+      expect(result, [albumA, albumB]);
+    });
+
+    test('should sort correctly based on updatedAt', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.lastModified);
+      expect(result, [albumA, albumB]);
+    });
+
+    test('should sort correctly based on assetCount', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.assetCount);
+      expect(result, [albumA, albumB]);
+    });
+
+    test('should sort correctly based on newestAssetTimestamp', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.mostRecent);
+      expect(result, [albumA, albumB]);
+    });
+
+    test('should sort correctly based on oldestAssetTimestamp', () async {
+      final albums = [albumB, albumA];
+
+      final result = await sut.sortAlbums(albums, RemoteAlbumSortMode.mostOldest);
+      expect(result, [albumB, albumA]);
+    });
+  });
+}

mobile/test/infrastructure/repository.mock.dart

@@ -2,12 +2,14 @@ import 'package:immich_mobile/infrastructure/repositories/device_asset.repositor
 import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/log.repository.dart';
+import 'package:immich_mobile/infrastructure/repositories/remote_album.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/storage.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/store.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/sync_api.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/sync_stream.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/user.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/user_api.repository.dart';
+import 'package:immich_mobile/repositories/drift_album_api_repository.dart';
 import 'package:mocktail/mocktail.dart';
 
 class MockStoreRepository extends Mock implements IsarStoreRepository {}
@@ -22,6 +24,8 @@ class MockSyncStreamRepository extends Mock implements SyncStreamRepository {}
 
 class MockLocalAlbumRepository extends Mock implements DriftLocalAlbumRepository {}
 
+class MockRemoteAlbumRepository extends Mock implements DriftRemoteAlbumRepository {}
+
 class MockLocalAssetRepository extends Mock implements DriftLocalAssetRepository {}
 
 class MockStorageRepository extends Mock implements StorageRepository {}
@@ -30,3 +34,5 @@ class MockStorageRepository extends Mock implements StorageRepository {}
 class MockUserApiRepository extends Mock implements UserApiRepository {}
 
 class MockSyncApiRepository extends Mock implements SyncApiRepository {}
+
+class MockDriftAlbumApiRepository extends Mock implements DriftAlbumApiRepository {}