1
0
mirror of https://github.com/immich-app/immich.git synced 2024-12-25 10:43:13 +02:00

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 <jrasm91@gmail.com>

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
This commit is contained in:
martin 2024-02-21 23:03:45 +01:00 committed by GitHub
parent 546edc2e91
commit 5c0c98473d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 76 additions and 47 deletions

View File

@ -56,6 +56,7 @@ describe('/activity', () => {
expect(status).toBe(200); expect(status).toBe(200);
expect(body).toEqual({ expect(body).toEqual({
total: 2, total: 2,
hidden: 1,
people: [ people: [
expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ name: 'visible_person' }),
expect.objectContaining({ name: 'hidden_person' }), expect.objectContaining({ name: 'hidden_person' }),
@ -71,6 +72,7 @@ describe('/activity', () => {
expect(status).toBe(200); expect(status).toBe(200);
expect(body).toEqual({ expect(body).toEqual({
total: 2, total: 2,
hidden: 1,
people: [expect.objectContaining({ name: 'visible_person' })], people: [expect.objectContaining({ name: 'visible_person' })],
}); });
}); });

Binary file not shown.

Binary file not shown.

Binary file not shown.

View File

@ -8593,6 +8593,9 @@
}, },
"PeopleResponseDto": { "PeopleResponseDto": {
"properties": { "properties": {
"hidden": {
"type": "integer"
},
"people": { "people": {
"items": { "items": {
"$ref": "#/components/schemas/PersonResponseDto" "$ref": "#/components/schemas/PersonResponseDto"
@ -8604,6 +8607,7 @@
} }
}, },
"required": [ "required": [
"hidden",
"people", "people",
"total" "total"
], ],

View File

@ -2801,6 +2801,12 @@ export type PathType = typeof PathType[keyof typeof PathType];
* @interface PeopleResponseDto * @interface PeopleResponseDto
*/ */
export interface PeopleResponseDto { export interface PeopleResponseDto {
/**
*
* @type {number}
* @memberof PeopleResponseDto
*/
'hidden': number;
/** /**
* *
* @type {Array<PersonResponseDto>} * @type {Array<PersonResponseDto>}

Binary file not shown.

View File

@ -127,7 +127,8 @@ export class PersonStatisticsResponseDto {
export class PeopleResponseDto { export class PeopleResponseDto {
@ApiProperty({ type: 'integer' }) @ApiProperty({ type: 'integer' })
total!: number; total!: number;
@ApiProperty({ type: 'integer' })
hidden!: number;
people!: PersonResponseDto[]; people!: PersonResponseDto[];
} }

View File

@ -114,35 +114,12 @@ describe(PersonService.name, () => {
}); });
describe('getAll', () => { 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 () => { it('should get all hidden and visible people with thumbnails', async () => {
personMock.getAllForUser.mockResolvedValue([personStub.withName, personStub.hidden]); 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({ await expect(sut.getAll(authStub.admin, { withHidden: true })).resolves.toEqual({
total: 2, total: 2,
hidden: 1,
people: [ people: [
responseDto, responseDto,
{ {

View File

@ -82,15 +82,12 @@ export class PersonService {
minimumFaceCount: machineLearning.facialRecognition.minFaces, minimumFaceCount: machineLearning.facialRecognition.minFaces,
withHidden: dto.withHidden || false, withHidden: dto.withHidden || false,
}); });
const total = await this.repository.getNumberOfPeople(auth.user.id); const { total, hidden } = await this.repository.getNumberOfPeople(auth.user.id);
const persons: PersonResponseDto[] = people
// with thumbnails
.filter((person) => !!person.thumbnailPath)
.map((person) => mapPerson(person));
return { return {
people: persons.filter((person) => dto.withHidden || !person.isHidden), people: people.map((person) => mapPerson(person)),
total, total,
hidden,
}; };
} }

View File

@ -28,6 +28,11 @@ export interface PersonStatistics {
assets: number; assets: number;
} }
export interface PeopleStatistics {
total: number;
hidden: number;
}
export interface IPersonRepository { export interface IPersonRepository {
getAll(pagination: PaginationOptions, options?: FindManyOptions<PersonEntity>): Paginated<PersonEntity>; getAll(pagination: PaginationOptions, options?: FindManyOptions<PersonEntity>): Paginated<PersonEntity>;
getAllForUser(userId: string, options: PersonSearchOptions): Promise<PersonEntity[]>; getAllForUser(userId: string, options: PersonSearchOptions): Promise<PersonEntity[]>;
@ -54,7 +59,7 @@ export interface IPersonRepository {
getRandomFace(personId: string): Promise<AssetFaceEntity | null>; getRandomFace(personId: string): Promise<AssetFaceEntity | null>;
getStatistics(personId: string): Promise<PersonStatistics>; getStatistics(personId: string): Promise<PersonStatistics>;
reassignFace(assetFaceId: string, newPersonId: string): Promise<number>; reassignFace(assetFaceId: string, newPersonId: string): Promise<number>;
getNumberOfPeople(userId: string): Promise<number>; getNumberOfPeople(userId: string): Promise<PeopleStatistics>;
reassignFaces(data: UpdateFacesData): Promise<number>; reassignFaces(data: UpdateFacesData): Promise<number>;
update(entity: Partial<PersonEntity>): Promise<PersonEntity>; update(entity: Partial<PersonEntity>): Promise<PersonEntity>;
} }

View File

@ -3,6 +3,7 @@ import {
IPersonRepository, IPersonRepository,
Paginated, Paginated,
PaginationOptions, PaginationOptions,
PeopleStatistics,
PersonNameSearchOptions, PersonNameSearchOptions,
PersonSearchOptions, PersonSearchOptions,
PersonStatistics, PersonStatistics,
@ -69,6 +70,7 @@ export class PersonRepository implements IPersonRepository {
.addOrderBy("NULLIF(person.name, '') IS NULL", 'ASC') .addOrderBy("NULLIF(person.name, '') IS NULL", 'ASC')
.addOrderBy('COUNT(face.assetId)', 'DESC') .addOrderBy('COUNT(face.assetId)', 'DESC')
.addOrderBy("NULLIF(person.name, '')", 'ASC', 'NULLS LAST') .addOrderBy("NULLIF(person.name, '')", 'ASC', 'NULLS LAST')
.andWhere("person.thumbnailPath != ''")
.having("person.name != '' OR COUNT(face.assetId) >= :faces", { faces: options?.minimumFaceCount || 1 }) .having("person.name != '' OR COUNT(face.assetId) >= :faces", { faces: options?.minimumFaceCount || 1 })
.groupBy('person.id') .groupBy('person.id')
.limit(500); .limit(500);
@ -207,15 +209,25 @@ export class PersonRepository implements IPersonRepository {
} }
@GenerateSql({ params: [DummyValue.UUID] }) @GenerateSql({ params: [DummyValue.UUID] })
async getNumberOfPeople(userId: string): Promise<number> { async getNumberOfPeople(userId: string): Promise<PeopleStatistics> {
return this.personRepository const items = await this.personRepository
.createQueryBuilder('person') .createQueryBuilder('person')
.leftJoin('person.faces', 'face') .leftJoin('person.faces', 'face')
.where('person.ownerId = :userId', { userId }) .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') .having('COUNT(face.assetId) != 0')
.groupBy('person.id') .getRawOne();
.withDeleted()
.getCount(); const result: PeopleStatistics = {
total: items ? Number.parseInt(items.total) : 0,
hidden: items ? Number.parseInt(items.hidden) : 0,
};
return result;
} }
create(entity: Partial<PersonEntity>): Promise<PersonEntity> { create(entity: Partial<PersonEntity>): Promise<PersonEntity> {

View File

@ -26,6 +26,7 @@ FROM
WHERE WHERE
"person"."ownerId" = $1 "person"."ownerId" = $1
AND "asset"."isArchived" = false AND "asset"."isArchived" = false
AND "person"."thumbnailPath" != ''
AND "person"."isHidden" = false AND "person"."isHidden" = false
GROUP BY GROUP BY
"person"."id" "person"."id"
@ -344,12 +345,20 @@ LIMIT
-- PersonRepository.getNumberOfPeople -- PersonRepository.getNumberOfPeople
SELECT 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 FROM
"person" "person" "person" "person"
LEFT JOIN "asset_faces" "face" ON "face"."personId" = "person"."id" 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 WHERE
"person"."ownerId" = $1 "person"."ownerId" = $1
AND "asset"."isArchived" = false
AND "person"."thumbnailPath" != ''
HAVING HAVING
COUNT("face"."assetId") != 0 COUNT("face"."assetId") != 0

View File

@ -6,6 +6,7 @@
import { createEventDispatcher } from 'svelte'; import { createEventDispatcher } from 'svelte';
import LoadingSpinner from '$lib/components/shared-components/loading-spinner.svelte'; import LoadingSpinner from '$lib/components/shared-components/loading-spinner.svelte';
import { mdiClose, mdiEye, mdiEyeOff, mdiRestart } from '@mdi/js'; import { mdiClose, mdiEye, mdiEyeOff, mdiRestart } from '@mdi/js';
import { locale } from '$lib/stores/preferences.store';
const dispatch = createEventDispatcher<{ const dispatch = createEventDispatcher<{
close: void; close: void;
@ -17,6 +18,7 @@
export let showLoadingSpinner: boolean; export let showLoadingSpinner: boolean;
export let toggleVisibility: boolean; export let toggleVisibility: boolean;
export let screenHeight: number; export let screenHeight: number;
export let countTotalPeople: number;
</script> </script>
<section <section
@ -28,7 +30,10 @@
> >
<div class="flex items-center"> <div class="flex items-center">
<CircleIconButton icon={mdiClose} on:click={() => dispatch('close')} /> <CircleIconButton icon={mdiClose} on:click={() => dispatch('close')} />
<p class="ml-4 hidden sm:block">Show & hide people</p> <div class="flex gap-2 items-center">
<p class="ml-2">Show & hide people</p>
<p class="text-sm text-gray-400 dark:text-gray-600">({countTotalPeople.toLocaleString($locale)})</p>
</div>
</div> </div>
<div class="flex items-center justify-end"> <div class="flex items-center justify-end">
<div class="flex items-center md:mr-8"> <div class="flex items-center md:mr-8">
@ -47,7 +52,7 @@
</div> </div>
</div> </div>
<div class="flex w-full flex-wrap gap-1 bg-immich-bg p-2 pb-8 dark:bg-immich-dark-bg md:px-8 mt-16"> <div class="flex flex-wrap gap-1 bg-immich-bg p-2 pb-8 dark:bg-immich-dark-bg md:px-8 mt-16">
<slot /> <slot />
</div> </div>
</section> </section>

View File

@ -40,11 +40,13 @@
import { mdiAccountOff, mdiEyeOutline } from '@mdi/js'; import { mdiAccountOff, mdiEyeOutline } from '@mdi/js';
import { onDestroy, onMount } from 'svelte'; import { onDestroy, onMount } from 'svelte';
import type { PageData } from './$types'; import type { PageData } from './$types';
import { locale } from '$lib/stores/preferences.store';
export let data: PageData; export let data: PageData;
let people = data.people.people; let people = data.people.people;
let countTotalPeople = data.people.total; let countTotalPeople = data.people.total;
let countHiddenPeople = data.people.hidden;
let selectHidden = false; let selectHidden = false;
let initialHiddenValues: Record<string, boolean> = {}; let initialHiddenValues: Record<string, boolean> = {};
@ -75,7 +77,7 @@
$: searchedPeopleLocal = searchName ? searchNameLocal(searchName, searchedPeople, maximumLengthSearchPeople) : []; $: searchedPeopleLocal = searchName ? searchNameLocal(searchName, searchedPeople, maximumLengthSearchPeople) : [];
$: countVisiblePeople = people.filter((person) => !person.isHidden).length; $: countVisiblePeople = countTotalPeople - countHiddenPeople;
const onKeyboardPress = (event: KeyboardEvent) => handleKeyboardPress(event); const onKeyboardPress = (event: KeyboardEvent) => handleKeyboardPress(event);
@ -152,6 +154,11 @@
for (const person of people) { for (const person of people) {
if (person.isHidden !== initialHiddenValues[person.id]) { if (person.isHidden !== initialHiddenValues[person.id]) {
changed.push({ id: person.id, isHidden: person.isHidden }); changed.push({ id: person.id, isHidden: person.isHidden });
if (person.isHidden) {
countHiddenPeople++;
} else {
countHiddenPeople--;
}
// Update the initial hidden values // Update the initial hidden values
initialHiddenValues[person.id] = person.isHidden; initialHiddenValues[person.id] = person.isHidden;
@ -203,10 +210,10 @@
const mergedPerson = await getPerson({ id: personToBeMergedIn.id }); const mergedPerson = await getPerson({ id: personToBeMergedIn.id });
countVisiblePeople--;
people = people.filter((person: PersonResponseDto) => person.id !== personToMerge.id); people = people.filter((person: PersonResponseDto) => person.id !== personToMerge.id);
people = people.map((person: PersonResponseDto) => (person.id === personToBeMergedIn.id ? mergedPerson : person)); people = people.map((person: PersonResponseDto) => (person.id === personToBeMergedIn.id ? mergedPerson : person));
countHiddenPeople--;
countTotalPeople--;
notificationController.show({ notificationController.show({
message: 'Merge people successfully', message: 'Merge people successfully',
type: NotificationType.Info, type: NotificationType.Info,
@ -274,7 +281,7 @@
} }
showChangeNameModal = false; showChangeNameModal = false;
countHiddenPeople++;
notificationController.show({ notificationController.show({
message: 'Changed visibility successfully', message: 'Changed visibility successfully',
type: NotificationType.Info, type: NotificationType.Info,
@ -423,7 +430,10 @@
</FullScreenModal> </FullScreenModal>
{/if} {/if}
<UserPageLayout title="People" description={countTotalPeople === 0 ? undefined : `(${countTotalPeople.toString()})`}> <UserPageLayout
title="People"
description={countVisiblePeople === 0 ? undefined : `(${countVisiblePeople.toLocaleString($locale)})`}
>
<svelte:fragment slot="buttons"> <svelte:fragment slot="buttons">
{#if countTotalPeople > 0} {#if countTotalPeople > 0}
<div class="flex gap-2 items-center justify-center"> <div class="flex gap-2 items-center justify-center">
@ -522,9 +532,10 @@
on:change={handleToggleVisibility} on:change={handleToggleVisibility}
bind:showLoadingSpinner bind:showLoadingSpinner
bind:toggleVisibility bind:toggleVisibility
{countTotalPeople}
screenHeight={innerHeight} screenHeight={innerHeight}
> >
<div class="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-5 xl:grid-cols-7 2xl:grid-cols-9 gap-1"> <div class="w-full grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-5 xl:grid-cols-7 2xl:grid-cols-9 gap-1">
{#each people as person, index (person.id)} {#each people as person, index (person.id)}
<button <button
class="relative" class="relative"