From eb73f6605bc6260c358bf1a988538fc96ebbd73e Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Tue, 20 Feb 2024 22:59:26 -0500 Subject: [PATCH] fix(server): don't return archived assets by default (#7278) * don't show archived results by default * fix e2e * generate sql * set default in dto --------- Co-authored-by: Alex Tran --- mobile/openapi/doc/AssetApi.md | 2 +- mobile/openapi/doc/MetadataSearchDto.md | 2 +- mobile/openapi/doc/SmartSearchDto.md | 2 +- .../lib/model/metadata_search_dto.dart | 18 ++----- .../openapi/lib/model/smart_search_dto.dart | 18 ++----- .../test/metadata_search_dto_test.dart | 2 +- .../openapi/test/smart_search_dto_test.dart | 2 +- open-api/immich-openapi-specs.json | 3 ++ server/e2e/api/specs/asset.e2e-spec.ts | 51 ++++++++++++++----- server/src/domain/search/dto/search.dto.ts | 1 + server/src/infra/infra.utils.ts | 2 +- server/src/infra/sql/asset.repository.sql | 2 +- server/src/infra/sql/search.repository.sql | 14 +++-- 13 files changed, 68 insertions(+), 51 deletions(-) diff --git a/mobile/openapi/doc/AssetApi.md b/mobile/openapi/doc/AssetApi.md index 5691965bc7..93b758a595 100644 --- a/mobile/openapi/doc/AssetApi.md +++ b/mobile/openapi/doc/AssetApi.md @@ -1153,7 +1153,7 @@ Name | Type | Description | Notes **updatedAfter** | **DateTime**| | [optional] **updatedBefore** | **DateTime**| | [optional] **webpPath** | **String**| | [optional] - **withArchived** | **bool**| | [optional] + **withArchived** | **bool**| | [optional] [default to false] **withDeleted** | **bool**| | [optional] **withExif** | **bool**| | [optional] **withPeople** | **bool**| | [optional] diff --git a/mobile/openapi/doc/MetadataSearchDto.md b/mobile/openapi/doc/MetadataSearchDto.md index bfbf81749e..d1d098fb0e 100644 --- a/mobile/openapi/doc/MetadataSearchDto.md +++ b/mobile/openapi/doc/MetadataSearchDto.md @@ -46,7 +46,7 @@ Name | Type | Description | Notes **updatedAfter** | [**DateTime**](DateTime.md) | | [optional] **updatedBefore** | [**DateTime**](DateTime.md) | | [optional] **webpPath** | **String** | | [optional] -**withArchived** | **bool** | | [optional] +**withArchived** | **bool** | | [optional] [default to false] **withDeleted** | **bool** | | [optional] **withExif** | **bool** | | [optional] **withPeople** | **bool** | | [optional] diff --git a/mobile/openapi/doc/SmartSearchDto.md b/mobile/openapi/doc/SmartSearchDto.md index fd9bc35490..d4ec1a70f6 100644 --- a/mobile/openapi/doc/SmartSearchDto.md +++ b/mobile/openapi/doc/SmartSearchDto.md @@ -37,7 +37,7 @@ Name | Type | Description | Notes **type** | [**AssetTypeEnum**](AssetTypeEnum.md) | | [optional] **updatedAfter** | [**DateTime**](DateTime.md) | | [optional] **updatedBefore** | [**DateTime**](DateTime.md) | | [optional] -**withArchived** | **bool** | | [optional] +**withArchived** | **bool** | | [optional] [default to false] **withDeleted** | **bool** | | [optional] **withExif** | **bool** | | [optional] diff --git a/mobile/openapi/lib/model/metadata_search_dto.dart b/mobile/openapi/lib/model/metadata_search_dto.dart index 47756cd527..86a2856e66 100644 --- a/mobile/openapi/lib/model/metadata_search_dto.dart +++ b/mobile/openapi/lib/model/metadata_search_dto.dart @@ -51,7 +51,7 @@ class MetadataSearchDto { this.updatedAfter, this.updatedBefore, this.webpPath, - this.withArchived, + this.withArchived = false, this.withDeleted, this.withExif, this.withPeople, @@ -356,13 +356,7 @@ class MetadataSearchDto { /// String? webpPath; - /// - /// Please note: This property should have been non-nullable! Since the specification file - /// does not include a default value (using the "default:" property), however, the generated - /// source code must fall back to having a nullable type. - /// Consider adding a "default:" property in the specification file to hide this note. - /// - bool? withArchived; + bool withArchived; /// /// Please note: This property should have been non-nullable! Since the specification file @@ -483,7 +477,7 @@ class MetadataSearchDto { (updatedAfter == null ? 0 : updatedAfter!.hashCode) + (updatedBefore == null ? 0 : updatedBefore!.hashCode) + (webpPath == null ? 0 : webpPath!.hashCode) + - (withArchived == null ? 0 : withArchived!.hashCode) + + (withArchived.hashCode) + (withDeleted == null ? 0 : withDeleted!.hashCode) + (withExif == null ? 0 : withExif!.hashCode) + (withPeople == null ? 0 : withPeople!.hashCode) + @@ -680,11 +674,7 @@ class MetadataSearchDto { } else { // json[r'webpPath'] = null; } - if (this.withArchived != null) { json[r'withArchived'] = this.withArchived; - } else { - // json[r'withArchived'] = null; - } if (this.withDeleted != null) { json[r'withDeleted'] = this.withDeleted; } else { @@ -756,7 +746,7 @@ class MetadataSearchDto { updatedAfter: mapDateTime(json, r'updatedAfter', r''), updatedBefore: mapDateTime(json, r'updatedBefore', r''), webpPath: mapValueOfType(json, r'webpPath'), - withArchived: mapValueOfType(json, r'withArchived'), + withArchived: mapValueOfType(json, r'withArchived') ?? false, withDeleted: mapValueOfType(json, r'withDeleted'), withExif: mapValueOfType(json, r'withExif'), withPeople: mapValueOfType(json, r'withPeople'), diff --git a/mobile/openapi/lib/model/smart_search_dto.dart b/mobile/openapi/lib/model/smart_search_dto.dart index 269a071020..664850db82 100644 --- a/mobile/openapi/lib/model/smart_search_dto.dart +++ b/mobile/openapi/lib/model/smart_search_dto.dart @@ -42,7 +42,7 @@ class SmartSearchDto { this.type, this.updatedAfter, this.updatedBefore, - this.withArchived, + this.withArchived = false, this.withDeleted, this.withExif, }); @@ -273,13 +273,7 @@ class SmartSearchDto { /// DateTime? updatedBefore; - /// - /// Please note: This property should have been non-nullable! Since the specification file - /// does not include a default value (using the "default:" property), however, the generated - /// source code must fall back to having a nullable type. - /// Consider adding a "default:" property in the specification file to hide this note. - /// - bool? withArchived; + bool withArchived; /// /// Please note: This property should have been non-nullable! Since the specification file @@ -364,7 +358,7 @@ class SmartSearchDto { (type == null ? 0 : type!.hashCode) + (updatedAfter == null ? 0 : updatedAfter!.hashCode) + (updatedBefore == null ? 0 : updatedBefore!.hashCode) + - (withArchived == null ? 0 : withArchived!.hashCode) + + (withArchived.hashCode) + (withDeleted == null ? 0 : withDeleted!.hashCode) + (withExif == null ? 0 : withExif!.hashCode); @@ -514,11 +508,7 @@ class SmartSearchDto { } else { // json[r'updatedBefore'] = null; } - if (this.withArchived != null) { json[r'withArchived'] = this.withArchived; - } else { - // json[r'withArchived'] = null; - } if (this.withDeleted != null) { json[r'withDeleted'] = this.withDeleted; } else { @@ -569,7 +559,7 @@ class SmartSearchDto { type: AssetTypeEnum.fromJson(json[r'type']), updatedAfter: mapDateTime(json, r'updatedAfter', r''), updatedBefore: mapDateTime(json, r'updatedBefore', r''), - withArchived: mapValueOfType(json, r'withArchived'), + withArchived: mapValueOfType(json, r'withArchived') ?? false, withDeleted: mapValueOfType(json, r'withDeleted'), withExif: mapValueOfType(json, r'withExif'), ); diff --git a/mobile/openapi/test/metadata_search_dto_test.dart b/mobile/openapi/test/metadata_search_dto_test.dart index f1635de4e0..f817b7da74 100644 --- a/mobile/openapi/test/metadata_search_dto_test.dart +++ b/mobile/openapi/test/metadata_search_dto_test.dart @@ -206,7 +206,7 @@ void main() { // TODO }); - // bool withArchived + // bool withArchived (default value: false) test('to test the property `withArchived`', () async { // TODO }); diff --git a/mobile/openapi/test/smart_search_dto_test.dart b/mobile/openapi/test/smart_search_dto_test.dart index 84a85cf208..4db3ac0808 100644 --- a/mobile/openapi/test/smart_search_dto_test.dart +++ b/mobile/openapi/test/smart_search_dto_test.dart @@ -161,7 +161,7 @@ void main() { // TODO }); - // bool withArchived + // bool withArchived (default value: false) test('to test the property `withArchived`', () async { // TODO }); diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 790fe8e8ec..d32d5b820d 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -2463,6 +2463,7 @@ "required": false, "in": "query", "schema": { + "default": false, "type": "boolean" } }, @@ -8429,6 +8430,7 @@ "type": "string" }, "withArchived": { + "default": false, "type": "boolean" }, "withDeleted": { @@ -9500,6 +9502,7 @@ "type": "string" }, "withArchived": { + "default": false, "type": "boolean" }, "withDeleted": { diff --git a/server/e2e/api/specs/asset.e2e-spec.ts b/server/e2e/api/specs/asset.e2e-spec.ts index 5993a70400..7789881206 100644 --- a/server/e2e/api/specs/asset.e2e-spec.ts +++ b/server/e2e/api/specs/asset.e2e-spec.ts @@ -50,6 +50,7 @@ describe(`${AssetController.name} (e2e)`, () => { let asset3: AssetResponseDto; let asset4: AssetResponseDto; let asset5: AssetResponseDto; + let asset6: AssetResponseDto; const createAsset = async ( loginResponse: LoginResponseDto, @@ -96,12 +97,11 @@ describe(`${AssetController.name} (e2e)`, () => { beforeEach(async () => { await testApp.reset({ entities: [AssetEntity, AssetStackEntity] }); - [asset1, asset2, asset3, asset4, asset5] = await Promise.all([ + [asset1, asset2, asset3, asset4, asset5, asset6] = await Promise.all([ createAsset(user1, new Date('1970-01-01')), createAsset(user1, new Date('1970-02-10')), createAsset(user1, new Date('1970-02-11'), { isFavorite: true, - isArchived: true, isExternal: true, isReadOnly: true, type: AssetType.VIDEO, @@ -118,6 +118,9 @@ describe(`${AssetController.name} (e2e)`, () => { createAsset(user1, new Date('1970-01-01'), { deletedAt: yesterday.toJSDate(), }), + createAsset(user1, new Date('1970-02-11'), { + isArchived: true, + }), ]); await assetRepository.upsertExif({ @@ -275,14 +278,14 @@ describe(`${AssetController.name} (e2e)`, () => { should: 'should search by isArchived (true)', deferred: () => ({ query: { isArchived: true }, - assets: [asset3], + assets: [asset6], }), }, { should: 'should search by isArchived (false)', deferred: () => ({ query: { isArchived: false }, - assets: [asset2, asset1], + assets: [asset3, asset2, asset1], }), }, { @@ -313,6 +316,20 @@ describe(`${AssetController.name} (e2e)`, () => { assets: [asset3], }), }, + { + should: 'should search by withArchived (true)', + deferred: () => ({ + query: { withArchived: true }, + assets: [asset3, asset6, asset2, asset1], + }), + }, + { + should: 'should search by withArchived (false)', + deferred: () => ({ + query: { withArchived: false }, + assets: [asset3, asset2, asset1], + }), + }, { should: 'should search by createdBefore', deferred: () => ({ @@ -902,7 +919,7 @@ describe(`${AssetController.name} (e2e)`, () => { .get('/asset/statistics') .set('Authorization', `Bearer ${user1.accessToken}`); - expect(body).toEqual({ images: 5, videos: 1, total: 6 }); + expect(body).toEqual({ images: 6, videos: 1, total: 7 }); expect(status).toBe(200); }); @@ -923,7 +940,7 @@ describe(`${AssetController.name} (e2e)`, () => { .query({ isArchived: true }); expect(status).toBe(200); - expect(body).toEqual({ images: 2, videos: 1, total: 3 }); + expect(body).toEqual({ images: 3, videos: 0, total: 3 }); }); it('should return stats of all favored and archived assets', async () => { @@ -933,7 +950,7 @@ describe(`${AssetController.name} (e2e)`, () => { .query({ isFavorite: true, isArchived: true }); expect(status).toBe(200); - expect(body).toEqual({ images: 1, videos: 1, total: 2 }); + expect(body).toEqual({ images: 1, videos: 0, total: 1 }); }); it('should return stats of all assets neither favored nor archived', async () => { @@ -1041,7 +1058,7 @@ describe(`${AssetController.name} (e2e)`, () => { expect.arrayContaining([ { count: 1, timeBucket: '2023-11-01T00:00:00.000Z' }, { count: 1, timeBucket: '1970-01-01T00:00:00.000Z' }, - { count: 1, timeBucket: '1970-02-01T00:00:00.000Z' }, + { count: 2, timeBucket: '1970-02-01T00:00:00.000Z' }, ]), ); }); @@ -1198,8 +1215,13 @@ describe(`${AssetController.name} (e2e)`, () => { .set('Authorization', `Bearer ${user1.accessToken}`); expect(status).toBe(200); - expect(body).toHaveLength(1); - expect(body).toEqual(expect.arrayContaining([expect.objectContaining({ id: asset2.id })])); + expect(body).toHaveLength(2); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: asset2.id }), + expect.objectContaining({ id: asset3.id }), + ]), + ); }); it('should get all map markers', async () => { @@ -1209,8 +1231,13 @@ describe(`${AssetController.name} (e2e)`, () => { .query({ isArchived: false }); expect(status).toBe(200); - expect(body).toHaveLength(1); - expect(body).toEqual([expect.objectContaining({ id: asset2.id })]); + expect(body).toHaveLength(2); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: asset2.id }), + expect.objectContaining({ id: asset3.id }), + ]), + ); }); }); diff --git a/server/src/domain/search/dto/search.dto.ts b/server/src/domain/search/dto/search.dto.ts index 519e39fd2e..4f2aa18199 100644 --- a/server/src/domain/search/dto/search.dto.ts +++ b/server/src/domain/search/dto/search.dto.ts @@ -23,6 +23,7 @@ class BaseSearchDto { isArchived?: boolean; @QueryBoolean({ optional: true }) + @ApiProperty({ default: false }) withArchived?: boolean; @QueryBoolean({ optional: true }) diff --git a/server/src/infra/infra.utils.ts b/server/src/infra/infra.utils.ts index ab7d744317..1538958e0f 100644 --- a/server/src/infra/infra.utils.ts +++ b/server/src/infra/infra.utils.ts @@ -183,7 +183,7 @@ export function searchAssetBuilder( _.omitBy( { ...status, - isArchived: isArchived ?? withArchived, + isArchived: isArchived ?? (withArchived ? undefined : false), encodedVideoPath: isEncoded ? Not(IsNull()) : undefined, livePhotoVideoId: isMotion ? Not(IsNull()) : undefined, }, diff --git a/server/src/infra/sql/asset.repository.sql b/server/src/infra/sql/asset.repository.sql index d971129e75..e5cf6771fd 100644 --- a/server/src/infra/sql/asset.repository.sql +++ b/server/src/infra/sql/asset.repository.sql @@ -434,7 +434,7 @@ WHERE AND 1 = 1 AND "asset"."ownerId" IN ($2) AND 1 = 1 - AND 1 = 1 + AND "asset"."isArchived" = $3 ) AND ("asset"."deletedAt" IS NULL) ORDER BY diff --git a/server/src/infra/sql/search.repository.sql b/server/src/infra/sql/search.repository.sql index ebae46f65b..a21697c268 100644 --- a/server/src/infra/sql/search.repository.sql +++ b/server/src/infra/sql/search.repository.sql @@ -79,7 +79,10 @@ FROM AND "exifInfo"."lensModel" = $2 AND 1 = 1 AND 1 = 1 - AND "asset"."isFavorite" = $3 + AND ( + "asset"."isFavorite" = $3 + AND "asset"."isArchived" = $4 + ) AND ( "stack"."primaryAssetId" = "asset"."id" OR "asset"."stackId" IS NULL @@ -177,16 +180,19 @@ WHERE AND "exifInfo"."lensModel" = $2 AND 1 = 1 AND 1 = 1 - AND "asset"."isFavorite" = $3 + AND ( + "asset"."isFavorite" = $3 + AND "asset"."isArchived" = $4 + ) AND ( "stack"."primaryAssetId" = "asset"."id" OR "asset"."stackId" IS NULL ) - AND "asset"."ownerId" IN ($4) + AND "asset"."ownerId" IN ($5) ) AND ("asset"."deletedAt" IS NULL) ORDER BY - "search"."embedding" <= > $5 ASC + "search"."embedding" <= > $6 ASC LIMIT 101 COMMIT