Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1809,9 +1809,8 @@
"rate_asset": "Rate Asset",
"rating": "Star rating",
"rating_clear": "Clear rating",
"rating_count": "{count, plural, one {# star} other {# stars}}",
"rating_count": "{count, plural, =-1 {Rejected} =0 {Unrated} one {# star} other {# stars}}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "Rejected" even mean in this context? Usually it's rather bad to use a magic number like -1 to report such a state

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was introduced as being a valid exif rating in #15699. I can imagine using workflows or scripts you would use it to auto-archive assets, or when using an external tool to browse your library, etc. Unless you search for a rating and then explicitly change the url to use rating=-1, you'll never see that string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really see that use case. Could you update the PR to only support 1-5, and everything else becomes invalid (unrated)

"rating_description": "Display the EXIF rating in the info panel",
"rating_set": "Rating set to {rating, plural, one {# star} other {# stars}}",
"reaction_options": "Reaction options",
"read_changelog": "Read Changelog",
"readonly_mode_disabled": "Read-only mode disabled",
Expand Down
4 changes: 2 additions & 2 deletions mobile/openapi/lib/api/search_api.dart

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

10 changes: 3 additions & 7 deletions mobile/openapi/lib/model/asset_bulk_update_dto.dart

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

12 changes: 4 additions & 8 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.

12 changes: 4 additions & 8 deletions mobile/openapi/lib/model/random_search_dto.dart

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

12 changes: 4 additions & 8 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.

12 changes: 4 additions & 8 deletions mobile/openapi/lib/model/statistics_search_dto.dart

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

10 changes: 3 additions & 7 deletions mobile/openapi/lib/model/update_asset_dto.dart

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

17 changes: 12 additions & 5 deletions open-api/immich-openapi-specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -9407,10 +9407,11 @@
"name": "rating",
"required": false,
"in": "query",
"description": "Filter by rating",
"description": "Filter by rating. -1 means rejected, null unrated, 1-5 the star rating",
"schema": {
"minimum": -1,
"maximum": 5,
"nullable": true,
"type": "number"
}
},
Expand Down Expand Up @@ -15875,6 +15876,7 @@
"description": "Rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"timeZone": {
Expand Down Expand Up @@ -18986,9 +18988,10 @@
"type": "string"
},
"rating": {
"description": "Filter by rating",
"description": "Filter by rating. -1 means rejected, null unrated, 1-5 the star rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"size": {
Expand Down Expand Up @@ -20712,9 +20715,10 @@
"type": "array"
},
"rating": {
"description": "Filter by rating",
"description": "Filter by rating. -1 means rejected, null unrated, 1-5 the star rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"size": {
Expand Down Expand Up @@ -22086,9 +22090,10 @@
"type": "string"
},
"rating": {
"description": "Filter by rating",
"description": "Filter by rating. -1 means rejected, null unrated, 1-5 the star rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"size": {
Expand Down Expand Up @@ -22320,9 +22325,10 @@
"type": "array"
},
"rating": {
"description": "Filter by rating",
"description": "Filter by rating. -1 means rejected, null unrated, 1-5 the star rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"state": {
Expand Down Expand Up @@ -25099,6 +25105,7 @@
"description": "Rating",
"maximum": 5,
"minimum": -1,
"nullable": true,
"type": "number"
},
"visibility": {
Expand Down
22 changes: 11 additions & 11 deletions open-api/typescript-sdk/src/fetch-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ export type AssetBulkUpdateDto = {
/** Longitude coordinate */
longitude?: number;
/** Rating */
rating?: number;
rating?: number | null;
/** Time zone (IANA timezone) */
timeZone?: string;
/** Asset visibility */
Expand Down Expand Up @@ -945,7 +945,7 @@ export type UpdateAssetDto = {
/** Longitude coordinate */
longitude?: number;
/** Rating */
rating?: number;
rating?: number | null;
/** Asset visibility */
visibility?: AssetVisibility;
};
Expand Down Expand Up @@ -1709,8 +1709,8 @@ export type MetadataSearchDto = {
personIds?: string[];
/** Filter by preview file path */
previewPath?: string;
/** Filter by rating */
rating?: number;
/** Filter by rating. -1 means rejected, null unrated, 1-5 the star rating */
rating?: number | null;
/** Number of results to return */
size?: number;
/** Filter by state/province name */
Expand Down Expand Up @@ -1825,8 +1825,8 @@ export type RandomSearchDto = {
ocr?: string;
/** Filter by person IDs */
personIds?: string[];
/** Filter by rating */
rating?: number;
/** Filter by rating. -1 means rejected, null unrated, 1-5 the star rating */
rating?: number | null;
/** Number of results to return */
size?: number;
/** Filter by state/province name */
Expand Down Expand Up @@ -1901,8 +1901,8 @@ export type SmartSearchDto = {
query?: string;
/** Asset ID to use as search reference */
queryAssetId?: string;
/** Filter by rating */
rating?: number;
/** Filter by rating. -1 means rejected, null unrated, 1-5 the star rating */
rating?: number | null;
/** Number of results to return */
size?: number;
/** Filter by state/province name */
Expand Down Expand Up @@ -1967,8 +1967,8 @@ export type StatisticsSearchDto = {
ocr?: string;
/** Filter by person IDs */
personIds?: string[];
/** Filter by rating */
rating?: number;
/** Filter by rating. -1 means rejected, null unrated, 1-5 the star rating */
rating?: number | null;
/** Filter by state/province name */
state?: string | null;
/** Filter by tag IDs */
Expand Down Expand Up @@ -5422,7 +5422,7 @@ export function searchLargeAssets({ albumIds, city, country, createdAfter, creat
model?: string | null;
ocr?: string;
personIds?: string[];
rating?: number;
rating?: number | null;
size?: number;
state?: string | null;
tagIds?: string[] | null;
Expand Down
18 changes: 17 additions & 1 deletion server/src/controllers/asset.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,28 @@ describe(AssetController.name, () => {
});

it('should reject invalid rating', async () => {
for (const test of [{ rating: 7 }, { rating: 3.5 }, { rating: null }]) {
for (const test of [{ rating: 7 }, { rating: 3.5 }, { rating: -2 }]) {
const { status, body } = await request(ctx.getHttpServer()).put(`/assets/${factory.uuid()}`).send(test);
expect(status).toBe(400);
expect(body).toEqual(factory.responses.badRequest());
}
});

it('should convert rating 0 to null', async () => {
const assetId = factory.uuid();
const { status } = await request(ctx.getHttpServer()).put(`/assets/${assetId}`).send({ rating: 0 });
expect(service.update).toHaveBeenCalledWith(undefined, assetId, { rating: null });
expect(status).toBe(200);
});

it('should leave correct ratings as-is', async () => {
const assetId = factory.uuid();
for (const test of [{ rating: -1 }, { rating: 1 }, { rating: 5 }]) {
const { status } = await request(ctx.getHttpServer()).put(`/assets/${assetId}`).send(test);
expect(service.update).toHaveBeenCalledWith(undefined, assetId, test);
expect(status).toBe(200);
}
});
});

describe('GET /assets/statistics', () => {
Expand Down
7 changes: 4 additions & 3 deletions server/src/dtos/asset.dto.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ApiProperty } from '@nestjs/swagger';
import { Type } from 'class-transformer';
import { Transform, Type } from 'class-transformer';
import {
IsArray,
IsDateString,
Expand Down Expand Up @@ -57,11 +57,12 @@ export class UpdateAssetBase {
longitude?: number;

@ApiProperty({ description: 'Rating' })
@Optional()
@Optional({ nullable: true })
@IsInt()
@Max(5)
@Min(-1)
rating?: number;
@Transform(({ value }) => (value === 0 ? null : value))
rating?: number | null;

@ApiProperty({ description: 'Asset description' })
@Optional()
Expand Down
Loading
Loading