pr feedback

immich-app · May 12, 2024 · ff865e3 · ff865e3
1 parent 7ced61e
commit ff865e3
Show file tree

Hide file tree

Showing 12 changed files with 55 additions and 31 deletions.
diff --git a/e2e/src/api/specs/asset.e2e-spec.ts b/e2e/src/api/specs/asset.e2e-spec.ts
@@ -214,7 +214,7 @@ describe('/asset', () => {
  id: user1Assets[0].id,
  isFavorite: false,
  people: {
- faces: [
+ visiblePeople: [
  {
  birthDate: null,
  id: expect.any(String),
@@ -484,7 +484,7 @@ describe('/asset', () => {
  id: user1Assets[0].id,
  isFavorite: true,
  people: {
- faces: [
+ visiblePeople: [
  {
  birthDate: null,
  id: expect.any(String),

diff --git a/mobile/lib/services/asset.service.dart b/mobile/lib/services/asset.service.dart
@@ -72,7 +72,7 @@ class AssetService {
  final AssetResponseDto? dto =
  await _apiService.assetApi.getAssetInfo(remoteId);
 
- return dto?.people?.faces;
+ return dto?.people?.visiblePeople;
  } catch (error, stack) {
  log.severe(
  'Error while getting remote asset info: ${error.toString()}',

diff --git a/mobile/openapi/doc/PeopleWithFacesResponseDto.md b/mobile/openapi/doc/PeopleWithFacesResponseDto.md
diff --git a/mobile/openapi/lib/model/people_with_faces_response_dto.dart b/mobile/openapi/lib/model/people_with_faces_response_dto.dart
diff --git a/mobile/openapi/test/people_with_faces_response_dto_test.dart b/mobile/openapi/test/people_with_faces_response_dto_test.dart
diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json
@@ -9014,19 +9014,19 @@
  },
  "PeopleWithFacesResponseDto": {
  "properties": {
- "faces": {
+ "numberOfFaces": {
+ "type": "integer"
+ },
+ "visiblePeople": {
  "items": {
  "$ref": "#/components/schemas/PersonWithFacesResponseDto"
  },
  "type": "array"
- },
- "numberOfFaces": {
- "type": "integer"
  }
  },
  "required": [
- "faces",
- "numberOfFaces"
+ "numberOfFaces",
+ "visiblePeople"
  ],
  "type": "object"
  },

diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts
@@ -101,8 +101,8 @@ export type PersonWithFacesResponseDto = {
  thumbnailPath: string;
 };
 export type PeopleWithFacesResponseDto = {
- faces: PersonWithFacesResponseDto[];
  numberOfFaces: number;
+ visiblePeople: PersonWithFacesResponseDto[];
 };
 export type SmartInfoResponseDto = {
  objects?: string[] | null;

diff --git a/server/src/dtos/asset-response.dto.ts b/server/src/dtos/asset-response.dto.ts
@@ -78,7 +78,7 @@ const peopleWithFaces = (faces: AssetFaceEntity[]): PeopleWithFacesResponseDto =
  }
  }
 
- return { faces: result, numberOfFaces: faces.length };
+ return { visiblePeople: result, numberOfFaces: faces.length };
 };
 
 export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): AssetResponseDto {

diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts
@@ -78,7 +78,7 @@ export class PersonWithFacesResponseDto extends PersonResponseDto {
 }
 
 export class PeopleWithFacesResponseDto {
- faces!: PersonWithFacesResponseDto[];
+ visiblePeople!: PersonWithFacesResponseDto[];
  @ApiProperty({ type: 'integer' })
  numberOfFaces!: number;
 }

diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts
@@ -449,6 +449,18 @@ describe(PersonService.name, () => {
  await expect(sut.unassignFace(authStub.admin, faceStub.face1.id)).resolves.toStrictEqual(
  mapFaces(faceStub.unassignedFace, authStub.admin),
  );
+
+ expect(mediaMock.generateThumbnail).not.toHaveBeenCalled();
+ });
+
+ it('should not unassign a face if user has no create access', async () => {
+ personMock.getFaceById.mockResolvedValueOnce(faceStub.face1);
+ accessMock.person.checkOwnerAccess.mockResolvedValue(new Set([personStub.noName.id]));
+ personMock.reassignFace.mockResolvedValue(1);
+ personMock.getRandomFace.mockResolvedValue(null);
+ personMock.getFaceById.mockResolvedValueOnce(faceStub.unassignedFace);
+
+ await expect(sut.unassignFace(authStub.admin, faceStub.face1.id)).rejects.toBeInstanceOf(BadRequestException);
  });
  });
 
@@ -465,6 +477,18 @@ describe(PersonService.name, () => {
  sut.unassignFaces(authStub.admin, { data: [{ assetId: faceStub.face1.id, personId: 'person-1' }] }),
  ).resolves.toStrictEqual([{ id: 'assetFaceId1', success: true }]);
  });
+
+ it('should not unassign a face if the user has no create access', async () => {
+ personMock.getFacesByIds.mockResolvedValueOnce([faceStub.face1]);
+ accessMock.person.checkOwnerAccess.mockResolvedValue(new Set([personStub.noName.id]));
+ personMock.reassignFace.mockResolvedValue(1);
+ personMock.getRandomFace.mockResolvedValue(null);
+ personMock.getFaceById.mockResolvedValueOnce(faceStub.unassignedFace);
+
+ await expect(
+ sut.unassignFaces(authStub.admin, { data: [{ assetId: faceStub.face1.id, personId: 'person-1' }] }),
+ ).rejects.toBeInstanceOf(BadRequestException);
+ });
  });
 
  describe('handlePersonCleanup', () => {

diff --git a/web/src/lib/components/asset-viewer/detail-panel.svelte b/web/src/lib/components/asset-viewer/detail-panel.svelte
@@ -218,7 +218,7 @@
  <div class="flex h-10 w-full items-center justify-between">
  <h2>PEOPLE</h2>
  <div class="flex gap-2 items-center">
- {#if people.faces.some((person) => person.isHidden)}
+ {#if people.visiblePeople.some((person) => person.isHidden)}
  <CircleIconButton
  title="Show hidden people"
  icon={showingHiddenPeople ? mdiEyeOff : mdiEye}
@@ -239,7 +239,7 @@
  </div>
 
  <div class="mt-2 flex flex-wrap gap-2">
- {#each people.faces as person (person.id)}
+ {#each people.visiblePeople as person (person.id)}
  {#if showingHiddenPeople || !person.isHidden}
  <a
  class="w-[90px]"

diff --git a/web/src/lib/utils/thumbnail-util.ts b/web/src/lib/utils/thumbnail-util.ts
@@ -45,7 +45,7 @@ export function getAltText(asset: AssetResponseDto) {
  altText += ` in ${asset.exifInfo.city}, ${asset.exifInfo.country}`;
  }
 
- const names = asset.people?.faces.filter((p) => p.name).map((p) => p.name) ?? [];
+ const names = asset.people?.visiblePeople.filter((p) => p.name).map((p) => p.name) ?? [];
  if (names.length == 1) {
  altText += ` with ${names[0]}`;
  }