From e2954b64118bb69423bda3826fe0b34d2ce5da62 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Thu, 11 Jun 2026 20:41:02 +0600 Subject: [PATCH] fix(mobile): show albums whose assets are all trashed (#28985) --- .../repositories/remote_album.repository.dart | 23 +++-- .../remote_album_repository_test.dart | 88 +++++++++++++++++++ 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/mobile/lib/infrastructure/repositories/remote_album.repository.dart b/mobile/lib/infrastructure/repositories/remote_album.repository.dart index 71d96bb0cb..92dd9b070a 100644 --- a/mobile/lib/infrastructure/repositories/remote_album.repository.dart +++ b/mobile/lib/infrastructure/repositories/remote_album.repository.dart @@ -20,7 +20,10 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { const DriftRemoteAlbumRepository(this._db) : super(_db); Future> getAll({Set sortBy = const {SortRemoteAlbumsBy.updatedAt}}) { - final assetCount = _db.remoteAlbumAssetEntity.assetId.count(distinct: true); + // Count non-trashed assets via the joined asset table. Filtering trashed assets in the + // join condition (instead of the where clause) keeps albums whose assets are all trashed + // in the result, the same way truly empty albums are kept + final assetCount = _db.remoteAssetEntity.id.count(distinct: true); final query = _db.remoteAlbumEntity.select().join([ leftOuterJoin( @@ -30,7 +33,8 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { ), leftOuterJoin( _db.remoteAssetEntity, - _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId), + _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId) & + _db.remoteAssetEntity.deletedAt.isNull(), useColumns: false, ), leftOuterJoin( @@ -47,7 +51,6 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { ), ]); query - ..where(_db.remoteAssetEntity.deletedAt.isNull()) ..addColumns([assetCount]) ..addColumns([_db.userEntity.name, _db.userEntity.id]) ..addColumns([_db.remoteAlbumUserEntity.userId.count(distinct: true)]) @@ -79,7 +82,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { } Future get(String albumId) { - final assetCount = _db.remoteAlbumAssetEntity.assetId.count(distinct: true); + final assetCount = _db.remoteAssetEntity.id.count(distinct: true); final query = _db.remoteAlbumEntity.select().join([ @@ -90,7 +93,8 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { ), leftOuterJoin( _db.remoteAssetEntity, - _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId), + _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId) & + _db.remoteAssetEntity.deletedAt.isNull(), useColumns: false, ), leftOuterJoin( @@ -106,7 +110,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { useColumns: false, ), ]) - ..where(_db.remoteAlbumEntity.id.equals(albumId) & _db.remoteAssetEntity.deletedAt.isNull()) + ..where(_db.remoteAlbumEntity.id.equals(albumId)) ..addColumns([assetCount]) ..addColumns([_db.userEntity.name, _db.userEntity.id]) ..addColumns([_db.remoteAlbumUserEntity.userId.count(distinct: true)]) @@ -515,7 +519,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { return []; } - final assetCount = _db.remoteAlbumAssetEntity.assetId.count(distinct: true); + final assetCount = _db.remoteAssetEntity.id.count(distinct: true); final query = _db.remoteAlbumEntity.select().join([ leftOuterJoin( @@ -525,7 +529,8 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { ), leftOuterJoin( _db.remoteAssetEntity, - _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId), + _db.remoteAssetEntity.id.equalsExp(_db.remoteAlbumAssetEntity.assetId) & + _db.remoteAssetEntity.deletedAt.isNull(), useColumns: false, ), leftOuterJoin( @@ -541,7 +546,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository { useColumns: false, ), ]) - ..where(_db.remoteAlbumEntity.id.isIn(albumIds) & _db.remoteAssetEntity.deletedAt.isNull()) + ..where(_db.remoteAlbumEntity.id.isIn(albumIds)) ..addColumns([assetCount]) ..addColumns([_db.remoteAlbumUserEntity.userId.count(distinct: true)]) ..addColumns([_db.userEntity.name, _db.userEntity.id]) diff --git a/mobile/test/medium/repositories/remote_album_repository_test.dart b/mobile/test/medium/repositories/remote_album_repository_test.dart index e4d803a51c..5a82ff315f 100644 --- a/mobile/test/medium/repositories/remote_album_repository_test.dart +++ b/mobile/test/medium/repositories/remote_album_repository_test.dart @@ -44,6 +44,94 @@ void main() { }); }); + group('getAll', () { + test('returns album when all of its assets are trashed', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + final asset1 = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + final asset2 = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: asset1.id); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: asset2.id); + + final albums = await sut.getAll(); + + expect(albums, hasLength(1)); + expect(albums.first.id, album.id); + expect(albums.first.assetCount, 0); + }); + + test('excludes trashed assets from assetCount', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + final active1 = await ctx.newRemoteAsset(ownerId: user.id); + final active2 = await ctx.newRemoteAsset(ownerId: user.id); + final trashed = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: active1.id); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: active2.id); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: trashed.id); + + final albums = await sut.getAll(); + + expect(albums, hasLength(1)); + expect(albums.first.assetCount, 2); + }); + + test('returns album without assets', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + + final albums = await sut.getAll(); + + expect(albums, hasLength(1)); + expect(albums.first.id, album.id); + expect(albums.first.assetCount, 0); + }); + }); + + group('get', () { + test('returns the album when all of its assets are trashed', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + final asset = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: asset.id); + + final result = await sut.get(album.id); + + expect(result, isNotNull); + expect(result?.id, album.id); + expect(result?.assetCount, 0); + }); + }); + + group('getAlbumsContainingAsset', () { + test('excludes trashed assets from assetCount', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + final asset = await ctx.newRemoteAsset(ownerId: user.id); + final trashed = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: asset.id); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: trashed.id); + + final albums = await sut.getAlbumsContainingAsset(asset.id); + + expect(albums, hasLength(1)); + expect(albums.first.id, album.id); + expect(albums.first.assetCount, 1); + }); + + test('returns albums for a trashed asset', () async { + final user = await ctx.newUser(); + final album = await ctx.newRemoteAlbum(ownerId: user.id); + final trashed = await ctx.newRemoteAsset(ownerId: user.id, deletedAt: DateTime(2025, 1, 1)); + await ctx.newRemoteAlbumAsset(albumId: album.id, assetId: trashed.id); + + final albums = await sut.getAlbumsContainingAsset(trashed.id); + + expect(albums, hasLength(1)); + expect(albums.first.assetCount, 0); + }); + }); + group('getSortedAlbumIds', () { late String userId;