feat(server): person delete (#19511)

feat(api): person delete

server/src/controllers/person.controller.spec.ts

@@ -60,6 +60,29 @@ describe(PersonController.name, () => {
     });
   });
 
+  describe('DELETE /people', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).delete('/people');
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should require uuids in the body', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .delete('/people')
+        .send({ ids: ['invalid'] });
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest([expect.stringContaining('must be a UUID')]));
+    });
+
+    it('should respond with 204', async () => {
+      const { status } = await request(ctx.getHttpServer())
+        .delete(`/people`)
+        .send({ ids: [factory.uuid()] });
+      expect(status).toBe(204);
+      expect(service.deleteAll).toHaveBeenCalled();
+    });
+  });
+
   describe('GET /people/:id', () => {
     it('should be an authenticated route', async () => {
       await request(ctx.getHttpServer()).get(`/people/${factory.uuid()}`);
@@ -156,6 +179,25 @@ describe(PersonController.name, () => {
     });
   });
 
+  describe('DELETE /people/:id', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).delete(`/people/${factory.uuid()}`);
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should require a valid uuid', async () => {
+      const { status, body } = await request(ctx.getHttpServer()).delete(`/people/invalid`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest([expect.stringContaining('must be a UUID')]));
+    });
+
+    it('should respond with 204', async () => {
+      const { status } = await request(ctx.getHttpServer()).delete(`/people/${factory.uuid()}`);
+      expect(status).toBe(204);
+      expect(service.delete).toHaveBeenCalled();
+    });
+  });
+
   describe('POST /people/:id/merge', () => {
     it('should be an authenticated route', async () => {
       await request(ctx.getHttpServer()).post(`/people/${factory.uuid()}/merge`);

server/src/controllers/person.controller.ts

@@ -1,7 +1,20 @@
-import { Body, Controller, Get, Next, Param, Post, Put, Query, Res } from '@nestjs/common';
+import {
+  Body,
+  Controller,
+  Delete,
+  Get,
+  HttpCode,
+  HttpStatus,
+  Next,
+  Param,
+  Post,
+  Put,
+  Query,
+  Res,
+} from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
 import { NextFunction, Response } from 'express';
-import { BulkIdResponseDto } from 'src/dtos/asset-ids.response.dto';
+import { BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset-ids.response.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
 import {
   AssetFaceUpdateDto,
@@ -49,6 +62,13 @@ export class PersonController {
     return this.service.updateAll(auth, dto);
   }
 
+  @Delete()
+  @HttpCode(HttpStatus.NO_CONTENT)
+  @Authenticated({ permission: Permission.PERSON_DELETE })
+  deletePeople(@Auth() auth: AuthDto, @Body() dto: BulkIdsDto): Promise<void> {
+    return this.service.deleteAll(auth, dto);
+  }
+
   @Get(':id')
   @Authenticated({ permission: Permission.PERSON_READ })
   getPerson(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<PersonResponseDto> {
@@ -65,6 +85,13 @@ export class PersonController {
     return this.service.update(auth, id, dto);
   }
 
+  @Delete(':id')
+  @HttpCode(HttpStatus.NO_CONTENT)
+  @Authenticated({ permission: Permission.PERSON_DELETE })
+  deletePerson(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<void> {
+    return this.service.delete(auth, id);
+  }
+
   @Get(':id/statistics')
   @Authenticated({ permission: Permission.PERSON_STATISTICS })
   getPersonStatistics(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<PersonStatisticsResponseDto> {

server/src/queries/person.repository.sql

@@ -328,3 +328,12 @@ set
   "deletedAt" = $1
 where
   "asset_faces"."id" = $2
+
+-- PersonRepository.getForPeopleDelete
+select
+  "id",
+  "thumbnailPath"
+from
+  "person"
+where
+  "id" in ($1)

server/src/repositories/person.repository.ts

@@ -3,7 +3,7 @@ import { ExpressionBuilder, Insertable, Kysely, Selectable, sql, Updateable } fr
 import { jsonObjectFrom } from 'kysely/helpers/postgres';
 import { InjectKysely } from 'nestjs-kysely';
 import { AssetFaces, DB, FaceSearch, Person } from 'src/db';
-import { ChunkedArray, DummyValue, GenerateSql } from 'src/decorators';
+import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators';
 import { AssetFileType, AssetVisibility, SourceType } from 'src/enum';
 import { removeUndefinedKeys } from 'src/utils/database';
 import { paginationHelper, PaginationOptions } from 'src/utils/pagination';
@@ -102,6 +102,7 @@ export class PersonRepository {
   }
 
   @GenerateSql({ params: [[DummyValue.UUID]] })
+  @Chunked()
   async delete(ids: string[]): Promise<void> {
     if (ids.length === 0) {
       return;
@@ -517,4 +518,13 @@ export class PersonRepository {
       await sql`REINDEX TABLE face_search`.execute(this.db);
     }
   }
+
+  @GenerateSql({ params: [[DummyValue.UUID]] })
+  @Chunked()
+  getForPeopleDelete(ids: string[]) {
+    if (ids.length === 0) {
+      return Promise.resolve([]);
+    }
+    return this.db.selectFrom('person').select(['id', 'thumbnailPath']).where('id', 'in', ids).execute();
+  }
 }

server/src/services/person.service.ts

@@ -4,7 +4,7 @@ import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants';
 import { Person } from 'src/database';
 import { AssetFaces, FaceSearch } from 'src/db';
 import { Chunked, OnJob } from 'src/decorators';
-import { BulkIdErrorReason, BulkIdResponseDto } from 'src/dtos/asset-ids.response.dto';
+import { BulkIdErrorReason, BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset-ids.response.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
 import {
   AssetFaceCreateDto,
@@ -216,6 +216,10 @@ export class PersonService extends BaseService {
     return mapPerson(person);
   }
 
+  delete(auth: AuthDto, id: string): Promise<void> {
+    return this.deleteAll(auth, { ids: [id] });
+  }
+
   async updateAll(auth: AuthDto, dto: PeopleUpdateDto): Promise<BulkIdResponseDto[]> {
     const results: BulkIdResponseDto[] = [];
     for (const person of dto.people) {
@@ -236,8 +240,14 @@ export class PersonService extends BaseService {
     return results;
   }
 
+  async deleteAll(auth: AuthDto, { ids }: BulkIdsDto): Promise<void> {
+    await this.requireAccess({ auth, permission: Permission.PERSON_DELETE, ids });
+    const people = await this.personRepository.getForPeopleDelete(ids);
+    await this.removeAllPeople(people);
+  }
+
   @Chunked()
-  private async delete(people: { id: string; thumbnailPath: string }[]) {
+  private async removeAllPeople(people: { id: string; thumbnailPath: string }[]) {
     await Promise.all(people.map((person) => this.storageRepository.unlink(person.thumbnailPath)));
     await this.personRepository.delete(people.map((person) => person.id));
     this.logger.debug(`Deleted ${people.length} people`);
@@ -246,7 +256,7 @@ export class PersonService extends BaseService {
   @OnJob({ name: JobName.PERSON_CLEANUP, queue: QueueName.BACKGROUND_TASK })
   async handlePersonCleanup(): Promise<JobStatus> {
     const people = await this.personRepository.getAllWithoutFaces();
-    await this.delete(people);
+    await this.removeAllPeople(people);
     return JobStatus.SUCCESS;
   }
 
@@ -589,7 +599,7 @@ export class PersonService extends BaseService {
         this.logger.log(`Merging ${mergeName} into ${primaryName}`);
 
         await this.personRepository.reassignFaces(mergeData);
-        await this.delete([mergePerson]);
+        await this.removeAllPeople([mergePerson]);
 
         this.logger.log(`Merged ${mergeName} into ${primaryName}`);
         results.push({ id: mergeId, success: true });

server/src/utils/access.ts

@@ -256,22 +256,17 @@ const checkOtherAccess = async (access: AccessRepository, request: OtherAccessRe
       return access.memory.checkOwnerAccess(auth.user.id, ids);
     }
 
-    case Permission.PERSON_READ: {
-      return await access.person.checkOwnerAccess(auth.user.id, ids);
-    }
-
-    case Permission.PERSON_UPDATE: {
-      return await access.person.checkOwnerAccess(auth.user.id, ids);
-    }
-
-    case Permission.PERSON_MERGE: {
-      return await access.person.checkOwnerAccess(auth.user.id, ids);
-    }
-
     case Permission.PERSON_CREATE: {
       return access.person.checkFaceOwnerAccess(auth.user.id, ids);
     }
 
+    case Permission.PERSON_READ:
+    case Permission.PERSON_UPDATE:
+    case Permission.PERSON_DELETE:
+    case Permission.PERSON_MERGE: {
+      return await access.person.checkOwnerAccess(auth.user.id, ids);
+    }
+
     case Permission.PERSON_REASSIGN: {
       return access.person.checkFaceOwnerAccess(auth.user.id, ids);
     }