sevenhill/Immich
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mobile): order album/place/person timelines by local date (#29338)

Browse Source 
* fix(mobile): order album/place/person timelines by local date

* fold the timeline date ordering into one helper and tighten the tests

* drop the redundant none guard from the place asset query

* format
This commit is contained in:
Santo Shakil
2026-08-20 13:52:33 +06:00
committed by GitHub
parent 47dccf7283
commit a066b5f301
3 changed files with 112 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mobile/lib/infrastructure/repositories/timeline.repository.dart
+31 -12
View File
@@ -185,7 +185,8 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
TimelineQuery remoteAlbum(String albumId, GroupAssetsBy groupBy) => (
bucketSource: () => _watchRemoteAlbumBucket(albumId, groupBy: groupBy),
assetSource: (offset, count) => _getRemoteAlbumBucketAssets(albumId, offset: offset, count: count),
assetSource: (offset, count) =>
_getRemoteAlbumBucketAssets(albumId, groupBy: groupBy, offset: offset, count: count),
origin: TimelineOrigin.remoteAlbum,
);
@@ -239,7 +240,12 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
.handleError((error) => const <Bucket>[]);
}
Future<List<BaseAsset>> _getRemoteAlbumBucketAssets(String albumId, {required int offset, required int count}) async {
Future<List<BaseAsset>> _getRemoteAlbumBucketAssets(
String albumId, {
required int offset,
required int count,
GroupAssetsBy groupBy = GroupAssetsBy.day,
}) async {
final albumData = await (_db.remoteAlbumEntity.select()..where((row) => row.id.equals(albumId))).getSingleOrNull();
// If album doesn't exist (was deleted), return empty list
@@ -266,11 +272,9 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
),
])..where(_db.remoteAssetEntity.deletedAt.isNull() & _db.remoteAlbumAssetEntity.albumId.equals(albumId));
if (isAscending) {
query.orderBy([OrderingTerm.asc(_db.remoteAssetEntity.createdAt)]);
} else {
query.orderBy([OrderingTerm.desc(_db.remoteAssetEntity.createdAt)]);
}
query.orderBy(
_assetDateOrder(groupBy, ascending: isAscending).map((order) => order(_db.remoteAssetEntity)).toList(),
);
query.limit(count, offset: offset);
@@ -377,13 +381,14 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
TimelineQuery place(String place, GroupAssetsBy groupBy) => (
bucketSource: () => _watchPlaceBucket(place, groupBy: groupBy),
assetSource: (offset, count) => _getPlaceBucketAssets(place, offset: offset, count: count),
assetSource: (offset, count) => _getPlaceBucketAssets(place, groupBy: groupBy, offset: offset, count: count),
origin: TimelineOrigin.place,
);
TimelineQuery person(String userId, String personId, GroupAssetsBy groupBy) => (
bucketSource: () => _watchPersonBucket(userId, personId, groupBy: groupBy),
assetSource: (offset, count) => _getPersonBucketAssets(userId, personId, offset: offset, count: count),
assetSource: (offset, count) =>
_getPersonBucketAssets(userId, personId, groupBy: groupBy, offset: offset, count: count),
origin: TimelineOrigin.person,
);
@@ -420,7 +425,12 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
}).watch();
}
Future<List<BaseAsset>> _getPlaceBucketAssets(String place, {required int offset, required int count}) {
Future<List<BaseAsset>> _getPlaceBucketAssets(
String place, {
required int offset,
required int count,
GroupAssetsBy groupBy = GroupAssetsBy.day,
}) {
final query =
_db.remoteAssetEntity.select().join([
innerJoin(
@@ -434,7 +444,7 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
_db.remoteAssetEntity.visibility.equalsValue(AssetVisibility.timeline) &
_db.remoteExifEntity.city.equals(place),
)
..orderBy([OrderingTerm.desc(_db.remoteAssetEntity.createdAt)])
..orderBy(_assetDateOrder(groupBy).map((order) => order(_db.remoteAssetEntity)).toList())
..limit(count, offset: offset);
return query.map((row) => row.readTable(_db.remoteAssetEntity).toDto()).get();
}
@@ -490,6 +500,7 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
String personId, {
required int offset,
required int count,
GroupAssetsBy groupBy = GroupAssetsBy.day,
}) {
final idQuery = _db.assetFaceEntity.selectOnly()
..addColumns([_db.assetFaceEntity.assetId])
@@ -507,7 +518,7 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
row.ownerId.equals(userId) &
row.visibility.equalsValue(AssetVisibility.timeline),
)
..orderBy([(row) => OrderingTerm.desc(row.createdAt)])
..orderBy(_assetDateOrder(groupBy))
..limit(count, offset: offset);
return query.map((row) => row.toDto()).get();
@@ -717,6 +728,14 @@ class TimelineRepository extends DatabaseAccessor<Drift> with $TimelineRepositor
List<Bucket> _generateBuckets(int count) => count == 0 ? const [] : [Bucket(assetCount: count)];
List<OrderingTerm Function($RemoteAssetEntityTable)> _assetDateOrder(GroupAssetsBy groupBy, {bool ascending = false}) {
OrderingTerm order(Expression<Object> exp) => ascending ? OrderingTerm.asc(exp) : OrderingTerm.desc(exp);
return [
if (groupBy != GroupAssetsBy.none) (row) => order(row.effectiveCreatedAt(groupBy)),
(row) => order(row.createdAt),
];
}
extension on Expression<DateTime> {
Expression<String> dateFmt(GroupAssetsBy groupBy, {bool toLocal = false}) {
// DateTimes are stored in UTC, so we need to convert them to local time inside the query before formatting
mobile/test/medium/repositories/timeline_repository_test.dart
+79
View File
@@ -46,6 +46,60 @@ void main() {
expect((assets.first as RemoteAsset).id, remoteAsset.id);
expect([localAsset1.id, localAsset2.id], contains((assets.first as RemoteAsset).localId));
});
test('orders shifted album assets in both directions and keeps normal asset order (#28852)', () async {
final user = await ctx.newUser();
final descendingAlbum = await ctx.newRemoteAlbum(ownerId: user.id, order: .desc);
final ascendingAlbum = await ctx.newRemoteAlbum(ownerId: user.id, order: .asc);
final shiftedLater = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 2, 12),
localDateTime: DateTime.utc(2024, 9, 3, 12),
);
final shiftedEarlier = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 3, 12),
localDateTime: DateTime.utc(2024, 9, 2, 12),
);
final normalLater = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 4, 14),
localDateTime: DateTime.utc(2024, 9, 4, 14),
);
final normalEarlier = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 4, 12),
localDateTime: DateTime.utc(2024, 9, 4, 12),
);
final seeded = [shiftedLater, shiftedEarlier, normalLater, normalEarlier];
for (final asset in seeded) {
await ctx.newRemoteAlbumAsset(albumId: descendingAlbum.id, assetId: asset.id);
await ctx.newRemoteAlbumAsset(albumId: ascendingAlbum.id, assetId: asset.id);
}
final descending = sut.remoteAlbum(descendingAlbum.id, .day);
final ascending = sut.remoteAlbum(ascendingAlbum.id, .day);
final buckets = await descending.bucketSource().first;
expect(buckets, hasLength(3));
expect(buckets.map((bucket) => bucket.assetCount), [2, 1, 1]);
final descendingAssets = await descending.assetSource(0, 10);
expect(descendingAssets.map((asset) => (asset as RemoteAsset).id), [
normalLater.id,
normalEarlier.id,
shiftedLater.id,
shiftedEarlier.id,
]);
final ascendingAssets = await ascending.assetSource(0, 10);
expect(ascendingAssets.map((asset) => (asset as RemoteAsset).id), [
shiftedEarlier.id,
shiftedLater.id,
normalEarlier.id,
normalLater.id,
]);
});
});
group('person assets', () {
@@ -69,6 +123,31 @@ void main() {
expect(assets, hasLength(1));
expect((assets.first as RemoteAsset).id, asset.id);
});
test('orders shifted person assets by effective date (#28852)', () async {
final user = await ctx.newUser();
final person = await ctx.newPerson(ownerId: user.id);
final shiftedLater = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 2, 12),
localDateTime: DateTime.utc(2024, 9, 3, 12),
);
final shiftedEarlier = await ctx.newRemoteAsset(
ownerId: user.id,
createdAt: DateTime.utc(2024, 9, 3, 12),
localDateTime: DateTime.utc(2024, 9, 2, 12),
);
await ctx.newFace(assetId: shiftedLater.id, personId: person.id);
await ctx.newFace(assetId: shiftedEarlier.id, personId: person.id);
final query = sut.person(user.id, person.id, .day);
final buckets = await query.bucketSource().first;
expect(buckets, hasLength(2));
final assets = await query.assetSource(0, 10);
expect(assets.map((asset) => (asset as RemoteAsset).id), [shiftedLater.id, shiftedEarlier.id]);
});
});
group('live photos', () {
mobile/test/medium/repository_context.dart
+2 -1
View File
@@ -121,6 +121,7 @@ class MediumRepositoryContext {
String? stackId,
String? thumbHash,
String? libraryId,
DateTime? localDateTime,
}) async {
id ??= TestUtils.uuid();
createdAt ??= TestUtils.date();
@@ -144,7 +145,7 @@ class MediumRepositoryContext {
isEdited: .new(isEdited ?? false),
livePhotoVideoId: .new(livePhotoVideoId),
stackId: .new(stackId),
localDateTime: .new(createdAt.toLocal()),
localDateTime: .new(localDateTime ?? createdAt.toLocal()),
thumbHash: .new(TestUtils.uuid(thumbHash)),
libraryId: .new(TestUtils.uuid(libraryId)),
),