Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): don't return archived assets by default #7278

Merged
merged 5 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -9497,6 +9499,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