fix(mobile): deduplicate people in asset details panel (#28972)

This commit is contained in:
Santo Shakil
2026-06-10 20:37:45 +06:00
committed by GitHub
parent 92a75b0cd3
commit 07813135b5
2 changed files with 91 additions and 13 deletions
@@ -16,20 +16,21 @@ class DriftPeopleRepository extends DriftDatabaseRepository {
}
Future<List<DriftPerson>> getAssetPeople(String assetId) async {
final query =
_db.select(_db.assetFaceEntity).join([
innerJoin(_db.personEntity, _db.personEntity.id.equalsExp(_db.assetFaceEntity.personId)),
])..where(
_db.assetFaceEntity.assetId.equals(assetId) &
_db.assetFaceEntity.isVisible.equals(true) &
_db.assetFaceEntity.deletedAt.isNull() &
_db.personEntity.isHidden.equals(false),
);
// An asset can have multiple face records for the same person (e.g., metadata
// imports alongside ML detections). Use a subquery instead of a join so each
// person is returned once, regardless of how many of their faces are on the asset
final faceQuery = _db.assetFaceEntity.selectOnly()
..addColumns([_db.assetFaceEntity.personId])
..where(
_db.assetFaceEntity.assetId.equals(assetId) &
_db.assetFaceEntity.isVisible.equals(true) &
_db.assetFaceEntity.deletedAt.isNull(),
);
return query.map((row) {
final person = row.readTable(_db.personEntity);
return person.toDto();
}).get();
final query = _db.select(_db.personEntity)
..where((row) => row.id.isInQuery(faceQuery) & row.isHidden.equals(false));
return query.map((row) => row.toDto()).get();
}
Future<List<DriftPerson>> getAllPeople({int minFaces = 3}) async {
@@ -0,0 +1,77 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/infrastructure/repositories/people.repository.dart';
import '../repository_context.dart';
void main() {
late MediumRepositoryContext ctx;
late DriftPeopleRepository sut;
setUp(() {
ctx = MediumRepositoryContext();
sut = DriftPeopleRepository(ctx.db);
});
tearDown(() async {
await ctx.dispose();
});
group('getAssetPeople', () {
test('does not duplicate a person with multiple face records on the same asset', () async {
// Regression check for #20585: a join on asset_face_entity returned one row
// per face, so a person appeared twice in the asset details panel when the
// same face was on the asset more than once (e.g., metadata import + ML)
final user = await ctx.newUser();
final asset = await ctx.newRemoteAsset(ownerId: user.id);
final person = await ctx.newPerson(ownerId: user.id);
await ctx.newFace(assetId: asset.id, personId: person.id);
await ctx.newFace(assetId: asset.id, personId: person.id);
final people = await sut.getAssetPeople(asset.id);
expect(people, hasLength(1));
expect(people.single.id, person.id);
});
test('returns all distinct people of an asset', () async {
final user = await ctx.newUser();
final asset = await ctx.newRemoteAsset(ownerId: user.id);
final person1 = await ctx.newPerson(ownerId: user.id);
final person2 = await ctx.newPerson(ownerId: user.id);
await ctx.newFace(assetId: asset.id, personId: person1.id);
await ctx.newFace(assetId: asset.id, personId: person2.id);
final people = await sut.getAssetPeople(asset.id);
expect(people, hasLength(2));
expect(people.map((person) => person.id), containsAll([person1.id, person2.id]));
});
test('does not return hidden people', () async {
final user = await ctx.newUser();
final asset = await ctx.newRemoteAsset(ownerId: user.id);
final hidden = await ctx.newPerson(ownerId: user.id, isHidden: true);
await ctx.newFace(assetId: asset.id, personId: hidden.id);
final people = await sut.getAssetPeople(asset.id);
expect(people, isEmpty);
});
test('does not return people from other assets', () async {
final user = await ctx.newUser();
final asset = await ctx.newRemoteAsset(ownerId: user.id);
final otherAsset = await ctx.newRemoteAsset(ownerId: user.id);
final person = await ctx.newPerson(ownerId: user.id);
await ctx.newFace(assetId: otherAsset.id, personId: person.id);
final people = await sut.getAssetPeople(asset.id);
expect(people, isEmpty);
});
});
}