Skip to content

Commit

Permalink
fix(server): don't return archived assets by default (#7278)
Browse files Browse the repository at this point in the history
* don't show archived results by default

* fix e2e

* generate sql

* set default in dto

---------

Co-authored-by: Alex Tran <[email protected]>
  • Loading branch information
mertalev and alextran1502 authored Feb 21, 2024
1 parent bb5236a commit eb73f66
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 51 deletions.
2 changes: 1 addition & 1 deletion mobile/openapi/doc/AssetApi.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mobile/openapi/doc/MetadataSearchDto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mobile/openapi/doc/SmartSearchDto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 4 additions & 14 deletions mobile/openapi/lib/model/metadata_search_dto.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 4 additions & 14 deletions mobile/openapi/lib/model/smart_search_dto.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mobile/openapi/test/metadata_search_dto_test.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mobile/openapi/test/smart_search_dto_test.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions open-api/immich-openapi-specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,7 @@
"required": false,
"in": "query",
"schema": {
"default": false,
"type": "boolean"
}
},
Expand Down Expand Up @@ -8429,6 +8430,7 @@
"type": "string"
},
"withArchived": {
"default": false,
"type": "boolean"
},
"withDeleted": {
Expand Down Expand Up @@ -9500,6 +9502,7 @@
"type": "string"
},
"withArchived": {
"default": false,
"type": "boolean"
},
"withDeleted": {
Expand Down
51 changes: 39 additions & 12 deletions server/e2e/api/specs/asset.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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({
Expand Down Expand Up @@ -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],
}),
},
{
Expand Down Expand Up @@ -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: () => ({
Expand Down Expand Up @@ -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);
});

Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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' },
]),
);
});
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 }),
]),
);
});
});

Expand Down
1 change: 1 addition & 0 deletions server/src/domain/search/dto/search.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class BaseSearchDto {
isArchived?: boolean;

@QueryBoolean({ optional: true })
@ApiProperty({ default: false })
withArchived?: boolean;

@QueryBoolean({ optional: true })
Expand Down
2 changes: 1 addition & 1 deletion server/src/infra/infra.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion server/src/infra/sql/asset.repository.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions server/src/infra/sql/search.repository.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit eb73f66

Please sign in to comment.