From 5c0c98473dc53e2e343a5ecbf475757a852aa106 Mon Sep 17 00:00:00 2001 From: martin <74269598+martabal@users.noreply.github.com> Date: Wed, 21 Feb 2024 23:03:45 +0100 Subject: [PATCH] fix(server, web): people page (#7319) * fix: people page * fix: use locale * fix: e2e * fix: remove useless w-full * fix: don't count people without thumbnail * fix: es6 template string Co-authored-by: Jason Rasmussen --------- Co-authored-by: Jason Rasmussen --- e2e/src/api/specs/person.e2e-spec.ts | 2 ++ mobile/openapi/doc/PeopleResponseDto.md | Bin 501 -> 528 bytes .../lib/model/people_response_dto.dart | Bin 3058 -> 3275 bytes .../test/people_response_dto_test.dart | Bin 706 -> 800 bytes open-api/immich-openapi-specs.json | 4 +++ open-api/typescript-sdk/axios-client/api.ts | 6 ++++ open-api/typescript-sdk/fetch-client.ts | Bin 74480 -> 74500 bytes server/src/domain/person/person.dto.ts | 3 +- .../src/domain/person/person.service.spec.ts | 27 ++---------------- server/src/domain/person/person.service.ts | 9 ++---- .../domain/repositories/person.repository.ts | 7 ++++- .../infra/repositories/person.repository.ts | 22 ++++++++++---- server/src/infra/sql/person.repository.sql | 11 ++++++- .../components/faces-page/show-hide.svelte | 9 ++++-- web/src/routes/(user)/people/+page.svelte | 23 +++++++++++---- 15 files changed, 76 insertions(+), 47 deletions(-) diff --git a/e2e/src/api/specs/person.e2e-spec.ts b/e2e/src/api/specs/person.e2e-spec.ts index d384fde2dc..3f17eac220 100644 --- a/e2e/src/api/specs/person.e2e-spec.ts +++ b/e2e/src/api/specs/person.e2e-spec.ts @@ -56,6 +56,7 @@ describe('/activity', () => { expect(status).toBe(200); expect(body).toEqual({ total: 2, + hidden: 1, people: [ expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ name: 'hidden_person' }), @@ -71,6 +72,7 @@ describe('/activity', () => { expect(status).toBe(200); expect(body).toEqual({ total: 2, + hidden: 1, people: [expect.objectContaining({ name: 'visible_person' })], }); }); diff --git a/mobile/openapi/doc/PeopleResponseDto.md b/mobile/openapi/doc/PeopleResponseDto.md index 2f87f19993a2b2218161069f26624f3ad5672db9..78f9b2207c283aca78e2111e545fcce87028ef89 100644 GIT binary patch delta 35 mcmey$Jb`7x9f^$0l$6vwEiHu_1ud=2yb>@A$k}*Ph7kbajtidv delta 10 RcmbQh@|AhQosHim837v;1iJtL diff --git a/mobile/openapi/lib/model/people_response_dto.dart b/mobile/openapi/lib/model/people_response_dto.dart index 80abedfc720fdfe0896fa2e9546798072e6031bd..02a82cadf10efc6832fe6b44ca9de69ecbb50866 100644 GIT binary patch delta 187 zcmew)ep+%vJELe(YGG++QEG}pNk(R|UPfk0N@||YxV?`6U^t zMNkC_wze>-5T^M;5LJ2^iNzVt`6;QI3fhz3GNlPXgl$!zhAK=BW-b$haMWWJY>{o; jyo-4kBO63+@-9|6E~qk1MV-yrteI>aP`=LObKEKb?sGxe delta 41 zcmV+^0M`G@8S)pfhXIqJ0XmcF0h^Pf0+*9g17MTF19r1r1ginFKL%h1ldTLMa3&BN diff --git a/mobile/openapi/test/people_response_dto_test.dart b/mobile/openapi/test/people_response_dto_test.dart index ad669eeced8d4d465aedb00ffd8ea364e86efddb..94db6eb86bc87bde9906f0aed2720d548ceb1503 100644 GIT binary patch delta 37 ncmX@ax`1uNbw;ktyb^_s%#@VWyvh7bifmA(8I$hh>x{1f`&kWK delta 11 ScmZ3$c8GPub;il2Os@eO+61cr diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 87f0fb4158..cac1d663bd 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -8593,6 +8593,9 @@ }, "PeopleResponseDto": { "properties": { + "hidden": { + "type": "integer" + }, "people": { "items": { "$ref": "#/components/schemas/PersonResponseDto" @@ -8604,6 +8607,7 @@ } }, "required": [ + "hidden", "people", "total" ], diff --git a/open-api/typescript-sdk/axios-client/api.ts b/open-api/typescript-sdk/axios-client/api.ts index bef5ceab1b..c01b200d03 100644 --- a/open-api/typescript-sdk/axios-client/api.ts +++ b/open-api/typescript-sdk/axios-client/api.ts @@ -2801,6 +2801,12 @@ export type PathType = typeof PathType[keyof typeof PathType]; * @interface PeopleResponseDto */ export interface PeopleResponseDto { + /** + * + * @type {number} + * @memberof PeopleResponseDto + */ + 'hidden': number; /** * * @type {Array} diff --git a/open-api/typescript-sdk/fetch-client.ts b/open-api/typescript-sdk/fetch-client.ts index d7ecb906e375b06da8235440a89b0f8ce2fb4777..0ee871ca60c8a76dd7269cb4beff284ad9561818 100644 GIT binary patch delta 30 mcmexxl%?eu%Z8r@{27@kDXDo@3VEfuNvTEFo4*+R`3L~j9S%MK delta 14 WcmZoU#`57P%Z8r@n|Tete*^$HJ_ou0 diff --git a/server/src/domain/person/person.dto.ts b/server/src/domain/person/person.dto.ts index 360a9b2348..b8ad8f0451 100644 --- a/server/src/domain/person/person.dto.ts +++ b/server/src/domain/person/person.dto.ts @@ -127,7 +127,8 @@ export class PersonStatisticsResponseDto { export class PeopleResponseDto { @ApiProperty({ type: 'integer' }) total!: number; - + @ApiProperty({ type: 'integer' }) + hidden!: number; people!: PersonResponseDto[]; } diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index 5da8666016..ffda9034bd 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/server/src/domain/person/person.service.spec.ts @@ -114,35 +114,12 @@ describe(PersonService.name, () => { }); describe('getAll', () => { - it('should get all people with thumbnails', async () => { - personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.noThumbnail]); - personMock.getNumberOfPeople.mockResolvedValue(1); - await expect(sut.getAll(authStub.admin, { withHidden: undefined })).resolves.toEqual({ - total: 1, - people: [responseDto], - }); - expect(personMock.getAllForUser).toHaveBeenCalledWith(authStub.admin.user.id, { - minimumFaceCount: 3, - withHidden: false, - }); - }); - it('should get all visible people with thumbnails', async () => { - personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.hidden]); - personMock.getNumberOfPeople.mockResolvedValue(2); - await expect(sut.getAll(authStub.admin, { withHidden: false })).resolves.toEqual({ - total: 2, - people: [responseDto], - }); - expect(personMock.getAllForUser).toHaveBeenCalledWith(authStub.admin.user.id, { - minimumFaceCount: 3, - withHidden: false, - }); - }); it('should get all hidden and visible people with thumbnails', async () => { personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.hidden]); - personMock.getNumberOfPeople.mockResolvedValue(2); + personMock.getNumberOfPeople.mockResolvedValue({ total: 2, hidden: 1 }); await expect(sut.getAll(authStub.admin, { withHidden: true })).resolves.toEqual({ total: 2, + hidden: 1, people: [ responseDto, { diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index 6fbc409bf8..6300cc743c 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -82,15 +82,12 @@ export class PersonService { minimumFaceCount: machineLearning.facialRecognition.minFaces, withHidden: dto.withHidden || false, }); - const total = await this.repository.getNumberOfPeople(auth.user.id); - const persons: PersonResponseDto[] = people - // with thumbnails - .filter((person) => !!person.thumbnailPath) - .map((person) => mapPerson(person)); + const { total, hidden } = await this.repository.getNumberOfPeople(auth.user.id); return { - people: persons.filter((person) => dto.withHidden || !person.isHidden), + people: people.map((person) => mapPerson(person)), total, + hidden, }; } diff --git a/server/src/domain/repositories/person.repository.ts b/server/src/domain/repositories/person.repository.ts index 80240091a9..85c11fe921 100644 --- a/server/src/domain/repositories/person.repository.ts +++ b/server/src/domain/repositories/person.repository.ts @@ -28,6 +28,11 @@ export interface PersonStatistics { assets: number; } +export interface PeopleStatistics { + total: number; + hidden: number; +} + export interface IPersonRepository { getAll(pagination: PaginationOptions, options?: FindManyOptions): Paginated; getAllForUser(userId: string, options: PersonSearchOptions): Promise; @@ -54,7 +59,7 @@ export interface IPersonRepository { getRandomFace(personId: string): Promise; getStatistics(personId: string): Promise; reassignFace(assetFaceId: string, newPersonId: string): Promise; - getNumberOfPeople(userId: string): Promise; + getNumberOfPeople(userId: string): Promise; reassignFaces(data: UpdateFacesData): Promise; update(entity: Partial): Promise; } diff --git a/server/src/infra/repositories/person.repository.ts b/server/src/infra/repositories/person.repository.ts index 85423b74dd..63b3d570ef 100644 --- a/server/src/infra/repositories/person.repository.ts +++ b/server/src/infra/repositories/person.repository.ts @@ -3,6 +3,7 @@ import { IPersonRepository, Paginated, PaginationOptions, + PeopleStatistics, PersonNameSearchOptions, PersonSearchOptions, PersonStatistics, @@ -69,6 +70,7 @@ export class PersonRepository implements IPersonRepository { .addOrderBy("NULLIF(person.name, '') IS NULL", 'ASC') .addOrderBy('COUNT(face.assetId)', 'DESC') .addOrderBy("NULLIF(person.name, '')", 'ASC', 'NULLS LAST') + .andWhere("person.thumbnailPath != ''") .having("person.name != '' OR COUNT(face.assetId) >= :faces", { faces: options?.minimumFaceCount || 1 }) .groupBy('person.id') .limit(500); @@ -207,15 +209,25 @@ export class PersonRepository implements IPersonRepository { } @GenerateSql({ params: [DummyValue.UUID] }) - async getNumberOfPeople(userId: string): Promise { - return this.personRepository + async getNumberOfPeople(userId: string): Promise { + const items = await this.personRepository .createQueryBuilder('person') .leftJoin('person.faces', 'face') .where('person.ownerId = :userId', { userId }) + .innerJoin('face.asset', 'asset') + .andWhere('asset.isArchived = false') + .andWhere("person.thumbnailPath != ''") + .select('COUNT(DISTINCT(person.id))', 'total') + .addSelect('COUNT(DISTINCT(person.id)) FILTER (WHERE person.isHidden = true)', 'hidden') .having('COUNT(face.assetId) != 0') - .groupBy('person.id') - .withDeleted() - .getCount(); + .getRawOne(); + + const result: PeopleStatistics = { + total: items ? Number.parseInt(items.total) : 0, + hidden: items ? Number.parseInt(items.hidden) : 0, + }; + + return result; } create(entity: Partial): Promise { diff --git a/server/src/infra/sql/person.repository.sql b/server/src/infra/sql/person.repository.sql index bd4a523e86..c2cc45ee88 100644 --- a/server/src/infra/sql/person.repository.sql +++ b/server/src/infra/sql/person.repository.sql @@ -26,6 +26,7 @@ FROM WHERE "person"."ownerId" = $1 AND "asset"."isArchived" = false + AND "person"."thumbnailPath" != '' AND "person"."isHidden" = false GROUP BY "person"."id" @@ -344,12 +345,20 @@ LIMIT -- PersonRepository.getNumberOfPeople SELECT - COUNT(DISTINCT ("person"."id")) AS "cnt" + COUNT(DISTINCT ("person"."id")) AS "total", + COUNT(DISTINCT ("person"."id")) FILTER ( + WHERE + "person"."isHidden" = true + ) AS "hidden" FROM "person" "person" LEFT JOIN "asset_faces" "face" ON "face"."personId" = "person"."id" + INNER JOIN "assets" "asset" ON "asset"."id" = "face"."assetId" + AND ("asset"."deletedAt" IS NULL) WHERE "person"."ownerId" = $1 + AND "asset"."isArchived" = false + AND "person"."thumbnailPath" != '' HAVING COUNT("face"."assetId") != 0 diff --git a/web/src/lib/components/faces-page/show-hide.svelte b/web/src/lib/components/faces-page/show-hide.svelte index e766262280..bee1e98fb7 100644 --- a/web/src/lib/components/faces-page/show-hide.svelte +++ b/web/src/lib/components/faces-page/show-hide.svelte @@ -6,6 +6,7 @@ import { createEventDispatcher } from 'svelte'; import LoadingSpinner from '$lib/components/shared-components/loading-spinner.svelte'; import { mdiClose, mdiEye, mdiEyeOff, mdiRestart } from '@mdi/js'; + import { locale } from '$lib/stores/preferences.store'; const dispatch = createEventDispatcher<{ close: void; @@ -17,6 +18,7 @@ export let showLoadingSpinner: boolean; export let toggleVisibility: boolean; export let screenHeight: number; + export let countTotalPeople: number;
dispatch('close')} /> - +
+

Show & hide people

+

({countTotalPeople.toLocaleString($locale)})

+
@@ -47,7 +52,7 @@
-
+
diff --git a/web/src/routes/(user)/people/+page.svelte b/web/src/routes/(user)/people/+page.svelte index c28f4d1f6c..eba6ed2764 100644 --- a/web/src/routes/(user)/people/+page.svelte +++ b/web/src/routes/(user)/people/+page.svelte @@ -40,11 +40,13 @@ import { mdiAccountOff, mdiEyeOutline } from '@mdi/js'; import { onDestroy, onMount } from 'svelte'; import type { PageData } from './$types'; + import { locale } from '$lib/stores/preferences.store'; export let data: PageData; let people = data.people.people; let countTotalPeople = data.people.total; + let countHiddenPeople = data.people.hidden; let selectHidden = false; let initialHiddenValues: Record = {}; @@ -75,7 +77,7 @@ $: searchedPeopleLocal = searchName ? searchNameLocal(searchName, searchedPeople, maximumLengthSearchPeople) : []; - $: countVisiblePeople = people.filter((person) => !person.isHidden).length; + $: countVisiblePeople = countTotalPeople - countHiddenPeople; const onKeyboardPress = (event: KeyboardEvent) => handleKeyboardPress(event); @@ -152,6 +154,11 @@ for (const person of people) { if (person.isHidden !== initialHiddenValues[person.id]) { changed.push({ id: person.id, isHidden: person.isHidden }); + if (person.isHidden) { + countHiddenPeople++; + } else { + countHiddenPeople--; + } // Update the initial hidden values initialHiddenValues[person.id] = person.isHidden; @@ -203,10 +210,10 @@ const mergedPerson = await getPerson({ id: personToBeMergedIn.id }); - countVisiblePeople--; people = people.filter((person: PersonResponseDto) => person.id !== personToMerge.id); people = people.map((person: PersonResponseDto) => (person.id === personToBeMergedIn.id ? mergedPerson : person)); - + countHiddenPeople--; + countTotalPeople--; notificationController.show({ message: 'Merge people successfully', type: NotificationType.Info, @@ -274,7 +281,7 @@ } showChangeNameModal = false; - + countHiddenPeople++; notificationController.show({ message: 'Changed visibility successfully', type: NotificationType.Info, @@ -423,7 +430,10 @@ {/if} - + {#if countTotalPeople > 0}
@@ -522,9 +532,10 @@ on:change={handleToggleVisibility} bind:showLoadingSpinner bind:toggleVisibility + {countTotalPeople} screenHeight={innerHeight} > -
+
{#each people as person, index (person.id)}