From e454c3566b162951d158a5a4383e079a63d43c61 Mon Sep 17 00:00:00 2001 From: Mees Frensel <33722705+meesfrensel@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:54:20 +0100 Subject: [PATCH] refactor: star rating (#26357) * refactor: star rating * transform rating 0 to null in controller dto * migrate rating 0 to null * deprecate rating -1 * rating type annotation * update Rating type --- i18n/en.json | 3 +- mobile/openapi/lib/api/search_api.dart | 4 +- .../lib/model/asset_bulk_update_dto.dart | 12 +- .../lib/model/metadata_search_dto.dart | 12 +- .../openapi/lib/model/random_search_dto.dart | 12 +- .../openapi/lib/model/smart_search_dto.dart | 12 +- .../lib/model/statistics_search_dto.dart | 12 +- .../openapi/lib/model/update_asset_dto.dart | 12 +- open-api/immich-openapi-specs.json | 145 ++++++++++++++++-- open-api/typescript-sdk/src/fetch-client.ts | 26 ++-- .../src/controllers/asset.controller.spec.ts | 18 ++- server/src/dtos/asset.dto.ts | 16 +- server/src/dtos/search.dto.ts | 17 +- server/src/repositories/media.repository.ts | 2 +- .../1771535611395-ConvertRating0ToNull.ts | 9 ++ server/src/services/asset.service.ts | 2 +- server/src/services/metadata.service.spec.ts | 36 +++++ server/src/services/metadata.service.ts | 4 +- .../asset-viewer/actions/rating-action.svelte | 3 +- .../detail-panel-star-rating.svelte | 8 +- .../search-bar/search-ratings-section.svelte | 6 +- web/src/lib/elements/StarRating.svelte | 46 +++--- web/src/lib/modals/SearchFilterModal.svelte | 2 +- .../[[assetId=id]]/+page.svelte | 2 + 24 files changed, 294 insertions(+), 127 deletions(-) create mode 100644 server/src/schema/migrations/1771535611395-ConvertRating0ToNull.ts diff --git a/i18n/en.json b/i18n/en.json index b99dac5609..808fbeb695 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1810,9 +1810,8 @@ "rate_asset": "Rate Asset", "rating": "Star rating", "rating_clear": "Clear rating", - "rating_count": "{count, plural, one {# star} other {# stars}}", + "rating_count": "{count, plural, =0 {Unrated} one {# star} other {# stars}}", "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", diff --git a/mobile/openapi/lib/api/search_api.dart b/mobile/openapi/lib/api/search_api.dart index 1b8ed3d9e4..085958de66 100644 --- a/mobile/openapi/lib/api/search_api.dart +++ b/mobile/openapi/lib/api/search_api.dart @@ -410,7 +410,7 @@ class SearchApi { /// Filter by person IDs /// /// * [num] rating: - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// * [num] size: /// Number of results to return @@ -633,7 +633,7 @@ class SearchApi { /// Filter by person IDs /// /// * [num] rating: - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// * [num] size: /// Number of results to return diff --git a/mobile/openapi/lib/model/asset_bulk_update_dto.dart b/mobile/openapi/lib/model/asset_bulk_update_dto.dart index a373743852..99bac7abfa 100644 --- a/mobile/openapi/lib/model/asset_bulk_update_dto.dart +++ b/mobile/openapi/lib/model/asset_bulk_update_dto.dart @@ -86,16 +86,10 @@ class AssetBulkUpdateDto { /// num? longitude; - /// Rating + /// Rating in range [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Time zone (IANA timezone) @@ -223,7 +217,9 @@ class AssetBulkUpdateDto { isFavorite: mapValueOfType(json, r'isFavorite'), latitude: num.parse('${json[r'latitude']}'), longitude: num.parse('${json[r'longitude']}'), - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), timeZone: mapValueOfType(json, r'timeZone'), visibility: AssetVisibility.fromJson(json[r'visibility']), ); diff --git a/mobile/openapi/lib/model/metadata_search_dto.dart b/mobile/openapi/lib/model/metadata_search_dto.dart index 4a7ca403ab..81f8d41527 100644 --- a/mobile/openapi/lib/model/metadata_search_dto.dart +++ b/mobile/openapi/lib/model/metadata_search_dto.dart @@ -256,16 +256,10 @@ class MetadataSearchDto { /// String? previewPath; - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Number of results to return @@ -754,7 +748,9 @@ class MetadataSearchDto { ? (json[r'personIds'] as Iterable).cast().toList(growable: false) : const [], previewPath: mapValueOfType(json, r'previewPath'), - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), size: num.parse('${json[r'size']}'), state: mapValueOfType(json, r'state'), tagIds: json[r'tagIds'] is Iterable diff --git a/mobile/openapi/lib/model/random_search_dto.dart b/mobile/openapi/lib/model/random_search_dto.dart index 7e0fb0c5c2..4166fc9f3c 100644 --- a/mobile/openapi/lib/model/random_search_dto.dart +++ b/mobile/openapi/lib/model/random_search_dto.dart @@ -159,16 +159,10 @@ class RandomSearchDto { /// Filter by person IDs List personIds; - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Number of results to return @@ -565,7 +559,9 @@ class RandomSearchDto { personIds: json[r'personIds'] is Iterable ? (json[r'personIds'] as Iterable).cast().toList(growable: false) : const [], - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), size: num.parse('${json[r'size']}'), state: mapValueOfType(json, r'state'), tagIds: json[r'tagIds'] is Iterable diff --git a/mobile/openapi/lib/model/smart_search_dto.dart b/mobile/openapi/lib/model/smart_search_dto.dart index 7d43cea872..5f8214467f 100644 --- a/mobile/openapi/lib/model/smart_search_dto.dart +++ b/mobile/openapi/lib/model/smart_search_dto.dart @@ -199,16 +199,10 @@ class SmartSearchDto { /// String? queryAssetId; - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Number of results to return @@ -605,7 +599,9 @@ class SmartSearchDto { : const [], query: mapValueOfType(json, r'query'), queryAssetId: mapValueOfType(json, r'queryAssetId'), - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), size: num.parse('${json[r'size']}'), state: mapValueOfType(json, r'state'), tagIds: json[r'tagIds'] is Iterable diff --git a/mobile/openapi/lib/model/statistics_search_dto.dart b/mobile/openapi/lib/model/statistics_search_dto.dart index fce2feb421..d5bbf448a3 100644 --- a/mobile/openapi/lib/model/statistics_search_dto.dart +++ b/mobile/openapi/lib/model/statistics_search_dto.dart @@ -164,16 +164,10 @@ class StatisticsSearchDto { /// Filter by person IDs List personIds; - /// Filter by rating + /// Filter by rating [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Filter by state/province name @@ -495,7 +489,9 @@ class StatisticsSearchDto { personIds: json[r'personIds'] is Iterable ? (json[r'personIds'] as Iterable).cast().toList(growable: false) : const [], - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), state: mapValueOfType(json, r'state'), tagIds: json[r'tagIds'] is Iterable ? (json[r'tagIds'] as Iterable).cast().toList(growable: false) diff --git a/mobile/openapi/lib/model/update_asset_dto.dart b/mobile/openapi/lib/model/update_asset_dto.dart index 42e8ec387f..8526995934 100644 --- a/mobile/openapi/lib/model/update_asset_dto.dart +++ b/mobile/openapi/lib/model/update_asset_dto.dart @@ -71,16 +71,10 @@ class UpdateAssetDto { /// num? longitude; - /// Rating + /// Rating in range [1-5], or null for unrated /// /// Minimum value: -1 /// Maximum value: 5 - /// - /// 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. - /// num? rating; /// Asset visibility @@ -178,7 +172,9 @@ class UpdateAssetDto { latitude: num.parse('${json[r'latitude']}'), livePhotoVideoId: mapValueOfType(json, r'livePhotoVideoId'), longitude: num.parse('${json[r'longitude']}'), - rating: num.parse('${json[r'rating']}'), + rating: json[r'rating'] == null + ? null + : num.parse('${json[r'rating']}'), visibility: AssetVisibility.fromJson(json[r'visibility']), ); } diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 85ea126a6d..fecd8933a8 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -9407,10 +9407,27 @@ "name": "rating", "required": false, "in": "query", - "description": "Filter by rating", + "description": "Filter by rating [1-5], or null for unrated", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable", "schema": { "minimum": -1, "maximum": 5, + "nullable": true, "type": "number" } }, @@ -15872,10 +15889,27 @@ "type": "number" }, "rating": { - "description": "Rating", + "description": "Rating in range [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "timeZone": { "description": "Time zone (IANA timezone)", @@ -18988,10 +19022,27 @@ "type": "string" }, "rating": { - "description": "Filter by rating", + "description": "Filter by rating [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "size": { "description": "Number of results to return", @@ -20714,10 +20765,27 @@ "type": "array" }, "rating": { - "description": "Filter by rating", + "description": "Filter by rating [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "size": { "description": "Number of results to return", @@ -22088,10 +22156,27 @@ "type": "string" }, "rating": { - "description": "Filter by rating", + "description": "Filter by rating [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "size": { "description": "Number of results to return", @@ -22322,10 +22407,27 @@ "type": "array" }, "rating": { - "description": "Filter by rating", + "description": "Filter by rating [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "state": { "description": "Filter by state/province name", @@ -25209,10 +25311,27 @@ "type": "number" }, "rating": { - "description": "Rating", + "description": "Rating in range [1-5], or null for unrated", "maximum": 5, "minimum": -1, - "type": "number" + "nullable": true, + "type": "number", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Updated", + "description": "Using -1 as a rating is deprecated and will be removed in the next major version." + } + ], + "x-immich-state": "Stable" }, "visibility": { "allOf": [ diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 9dae33541e..fd07ce01a7 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -834,8 +834,8 @@ export type AssetBulkUpdateDto = { latitude?: number; /** Longitude coordinate */ longitude?: number; - /** Rating */ - rating?: number; + /** Rating in range [1-5], or null for unrated */ + rating?: number | null; /** Time zone (IANA timezone) */ timeZone?: string; /** Asset visibility */ @@ -944,8 +944,8 @@ export type UpdateAssetDto = { livePhotoVideoId?: string | null; /** Longitude coordinate */ longitude?: number; - /** Rating */ - rating?: number; + /** Rating in range [1-5], or null for unrated */ + rating?: number | null; /** Asset visibility */ visibility?: AssetVisibility; }; @@ -1711,8 +1711,8 @@ export type MetadataSearchDto = { personIds?: string[]; /** Filter by preview file path */ previewPath?: string; - /** Filter by rating */ - rating?: number; + /** Filter by rating [1-5], or null for unrated */ + rating?: number | null; /** Number of results to return */ size?: number; /** Filter by state/province name */ @@ -1827,8 +1827,8 @@ export type RandomSearchDto = { ocr?: string; /** Filter by person IDs */ personIds?: string[]; - /** Filter by rating */ - rating?: number; + /** Filter by rating [1-5], or null for unrated */ + rating?: number | null; /** Number of results to return */ size?: number; /** Filter by state/province name */ @@ -1903,8 +1903,8 @@ export type SmartSearchDto = { query?: string; /** Asset ID to use as search reference */ queryAssetId?: string; - /** Filter by rating */ - rating?: number; + /** Filter by rating [1-5], or null for unrated */ + rating?: number | null; /** Number of results to return */ size?: number; /** Filter by state/province name */ @@ -1969,8 +1969,8 @@ export type StatisticsSearchDto = { ocr?: string; /** Filter by person IDs */ personIds?: string[]; - /** Filter by rating */ - rating?: number; + /** Filter by rating [1-5], or null for unrated */ + rating?: number | null; /** Filter by state/province name */ state?: string | null; /** Filter by tag IDs */ @@ -5454,7 +5454,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; diff --git a/server/src/controllers/asset.controller.spec.ts b/server/src/controllers/asset.controller.spec.ts index 2893a27539..69bf1f6443 100644 --- a/server/src/controllers/asset.controller.spec.ts +++ b/server/src/controllers/asset.controller.spec.ts @@ -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', () => { diff --git a/server/src/dtos/asset.dto.ts b/server/src/dtos/asset.dto.ts index 00ea46f789..b7bd7a18e8 100644 --- a/server/src/dtos/asset.dto.ts +++ b/server/src/dtos/asset.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; -import { Type } from 'class-transformer'; +import { Transform, Type } from 'class-transformer'; import { IsArray, IsDateString, @@ -16,6 +16,7 @@ import { ValidateIf, ValidateNested, } from 'class-validator'; +import { HistoryBuilder, Property } from 'src/decorators'; import { BulkIdsDto } from 'src/dtos/asset-ids.response.dto'; import { AssetType, AssetVisibility } from 'src/enum'; import { AssetStats } from 'src/repositories/asset.repository'; @@ -56,12 +57,19 @@ export class UpdateAssetBase { @IsNotEmpty() longitude?: number; - @ApiProperty({ description: 'Rating' }) - @Optional() + @Property({ + description: 'Rating in range [1-5], or null for unrated', + history: new HistoryBuilder() + .added('v1') + .stable('v2') + .updated('v2.6.0', 'Using -1 as a rating is deprecated and will be removed in the next major version.'), + }) + @Optional({ nullable: true }) @IsInt() @Max(5) @Min(-1) - rating?: number; + @Transform(({ value }) => (value === 0 ? null : value)) + rating?: number | null; @ApiProperty({ description: 'Asset description' }) @Optional() diff --git a/server/src/dtos/search.dto.ts b/server/src/dtos/search.dto.ts index 47a1889e47..f72ecdf8b6 100644 --- a/server/src/dtos/search.dto.ts +++ b/server/src/dtos/search.dto.ts @@ -2,7 +2,7 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { Type } from 'class-transformer'; import { IsInt, IsNotEmpty, IsString, Max, Min } from 'class-validator'; import { Place } from 'src/database'; -import { HistoryBuilder } from 'src/decorators'; +import { HistoryBuilder, Property } from 'src/decorators'; import { AlbumResponseDto } from 'src/dtos/album.dto'; import { AssetResponseDto } from 'src/dtos/asset-response.dto'; import { AssetOrder, AssetType, AssetVisibility } from 'src/enum'; @@ -103,12 +103,21 @@ class BaseSearchDto { @ValidateUUID({ each: true, optional: true, description: 'Filter by album IDs' }) albumIds?: string[]; - @ApiPropertyOptional({ type: 'number', description: 'Filter by rating', minimum: -1, maximum: 5 }) - @Optional() + @Property({ + type: 'number', + description: 'Filter by rating [1-5], or null for unrated', + minimum: -1, + maximum: 5, + history: new HistoryBuilder() + .added('v1') + .stable('v2') + .updated('v2.6.0', 'Using -1 as a rating is deprecated and will be removed in the next major version.'), + }) + @Optional({ nullable: true }) @IsInt() @Max(5) @Min(-1) - rating?: number; + rating?: number | null; @ApiPropertyOptional({ description: 'Filter by OCR text content' }) @IsString() diff --git a/server/src/repositories/media.repository.ts b/server/src/repositories/media.repository.ts index 7b0b30583d..58e006171a 100644 --- a/server/src/repositories/media.repository.ts +++ b/server/src/repositories/media.repository.ts @@ -107,7 +107,7 @@ export class MediaRepository { ExposureTime: tags.exposureTime, ProfileDescription: tags.profileDescription, ColorSpace: tags.colorspace, - Rating: tags.rating, + Rating: tags.rating === null ? 0 : tags.rating, // specially convert Orientation to numeric Orientation# for exiftool 'Orientation#': tags.orientation ? Number(tags.orientation) : undefined, }; diff --git a/server/src/schema/migrations/1771535611395-ConvertRating0ToNull.ts b/server/src/schema/migrations/1771535611395-ConvertRating0ToNull.ts new file mode 100644 index 0000000000..8faebb250e --- /dev/null +++ b/server/src/schema/migrations/1771535611395-ConvertRating0ToNull.ts @@ -0,0 +1,9 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`UPDATE "asset_exif" SET "rating" = NULL WHERE "rating" = 0;`.execute(db); +} + +export async function down(): Promise { + // not supported +} diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index f41004dd1c..387b700f01 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -516,7 +516,7 @@ export class AssetService extends BaseService { dateTimeOriginal?: string; latitude?: number; longitude?: number; - rating?: number; + rating?: number | null; }) { const { id, description, dateTimeOriginal, latitude, longitude, rating } = dto; const writes = _.omitBy( diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index feaba36b1d..92ec13bea5 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -1423,6 +1423,20 @@ describe(MetadataService.name, () => { ); }); + it('should handle 0 as unrated -> null', async () => { + const asset = AssetFactory.create(); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(asset); + mockReadTags({ Rating: 0 }); + + await sut.handleMetadataExtraction({ id: asset.id }); + expect(mocks.asset.upsertExif).toHaveBeenCalledWith( + expect.objectContaining({ + rating: null, + }), + { lockedPropertiesBehavior: 'skip' }, + ); + }); + it('should handle valid negative rating value', async () => { const asset = AssetFactory.create(); mocks.assetJob.getForMetadataExtraction.mockResolvedValue(asset); @@ -1780,6 +1794,28 @@ describe(MetadataService.name, () => { 'timeZone', ]); }); + + it('should write rating', async () => { + const asset = factory.jobAssets.sidecarWrite(); + asset.exifInfo.rating = 4; + + mocks.assetJob.getLockedPropertiesForMetadataExtraction.mockResolvedValue(['rating']); + mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset); + await expect(sut.handleSidecarWrite({ id: asset.id })).resolves.toBe(JobStatus.Success); + expect(mocks.metadata.writeTags).toHaveBeenCalledWith(asset.files[0].path, { Rating: 4 }); + expect(mocks.asset.unlockProperties).toHaveBeenCalledWith(asset.id, ['rating']); + }); + + it('should write null rating as 0', async () => { + const asset = factory.jobAssets.sidecarWrite(); + asset.exifInfo.rating = null; + + mocks.assetJob.getLockedPropertiesForMetadataExtraction.mockResolvedValue(['rating']); + mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset); + await expect(sut.handleSidecarWrite({ id: asset.id })).resolves.toBe(JobStatus.Success); + expect(mocks.metadata.writeTags).toHaveBeenCalledWith(asset.files[0].path, { Rating: 0 }); + expect(mocks.asset.unlockProperties).toHaveBeenCalledWith(asset.id, ['rating']); + }); }); describe('firstDateTime', () => { diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index c5d7278d56..f22d4682fa 100644 --- a/server/src/services/metadata.service.ts +++ b/server/src/services/metadata.service.ts @@ -301,7 +301,7 @@ export class MetadataService extends BaseService { // comments description: String(exifTags.ImageDescription || exifTags.Description || '').trim(), profileDescription: exifTags.ProfileDescription || null, - rating: validateRange(exifTags.Rating, -1, 5), + rating: exifTags.Rating === 0 ? null : validateRange(exifTags.Rating, -1, 5), // grouping livePhotoCID: (exifTags.ContentIdentifier || exifTags.MediaGroupUUID) ?? null, @@ -451,7 +451,7 @@ export class MetadataService extends BaseService { dateTimeOriginal: asset.exifInfo.dateTimeOriginal as string | null, latitude: asset.exifInfo.latitude, longitude: asset.exifInfo.longitude, - rating: asset.exifInfo.rating, + rating: asset.exifInfo.rating ?? 0, tags: asset.exifInfo.tags, timeZone: asset.exifInfo.timeZone, }, diff --git a/web/src/lib/components/asset-viewer/actions/rating-action.svelte b/web/src/lib/components/asset-viewer/actions/rating-action.svelte index 3791fccf23..c5b0197121 100644 --- a/web/src/lib/components/asset-viewer/actions/rating-action.svelte +++ b/web/src/lib/components/asset-viewer/actions/rating-action.svelte @@ -17,10 +17,9 @@ const rateAsset = async (rating: number | null) => { try { - const updateAssetDto = rating === null ? {} : { rating }; await updateAsset({ id: asset.id, - updateAssetDto, + updateAssetDto: { rating }, }); asset = { diff --git a/web/src/lib/components/asset-viewer/detail-panel-star-rating.svelte b/web/src/lib/components/asset-viewer/detail-panel-star-rating.svelte index 15bf1f21d6..81e7d4e1fb 100644 --- a/web/src/lib/components/asset-viewer/detail-panel-star-rating.svelte +++ b/web/src/lib/components/asset-viewer/detail-panel-star-rating.svelte @@ -1,5 +1,5 @@ {#if !authManager.isSharedLink && $preferences?.ratings.enabled} -
+
handlePromiseError(handleChangeRating(rating))} />
{/if} diff --git a/web/src/lib/components/shared-components/search-bar/search-ratings-section.svelte b/web/src/lib/components/shared-components/search-bar/search-ratings-section.svelte index 4f01848a6c..7b9fa7414f 100644 --- a/web/src/lib/components/shared-components/search-bar/search-ratings-section.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-ratings-section.svelte @@ -4,13 +4,13 @@ import Combobox from '../combobox.svelte'; interface Props { - rating?: number; + rating?: number | null; } let { rating = $bindable() }: Props = $props(); const options = [ - { value: '0', label: $t('rating_count', { values: { count: 0 } }) }, + { value: 'null', label: $t('rating_count', { values: { count: 0 } }) }, { value: '1', label: $t('rating_count', { values: { count: 1 } }) }, { value: '2', label: $t('rating_count', { values: { count: 2 } }) }, { value: '3', label: $t('rating_count', { values: { count: 3 } }) }, @@ -26,7 +26,7 @@ placeholder={$t('search_rating')} hideLabel {options} - selectedOption={rating === undefined ? undefined : options[rating]} + selectedOption={rating === undefined ? undefined : options[rating === null ? 0 : rating]} onSelect={(r) => (rating = r === undefined ? undefined : Number.parseInt(r.value))} /> diff --git a/web/src/lib/elements/StarRating.svelte b/web/src/lib/elements/StarRating.svelte index f345dc86b7..803e408ec1 100644 --- a/web/src/lib/elements/StarRating.svelte +++ b/web/src/lib/elements/StarRating.svelte @@ -3,27 +3,28 @@ import { shortcuts } from '$lib/actions/shortcut'; import { generateId } from '$lib/utils/generate-id'; import { Icon } from '@immich/ui'; + import { mdiStar, mdiStarOutline } from '@mdi/js'; import { t } from 'svelte-i18n'; + export type Rating = 1 | 2 | 3 | 4 | 5 | null; + interface Props { count?: number; - rating: number; + rating: Rating; readOnly?: boolean; - onRating: (rating: number) => void | undefined; + onRating: (rating: Rating) => void | undefined; } let { count = 5, rating, readOnly = false, onRating }: Props = $props(); let ratingSelection = $derived(rating); - let hoverRating = $state(0); - let focusRating = $state(0); + let hoverRating: Rating = $state(null); + let focusRating: Rating = $state(null); let timeoutId: ReturnType | undefined; - const starIcon = - 'M10.788 3.21c.448-1.077 1.976-1.077 2.424 0l2.082 5.007 5.404.433c1.164.093 1.636 1.545.749 2.305l-4.117 3.527 1.257 5.273c.271 1.136-.964 2.033-1.96 1.425L12 18.354 7.373 21.18c-.996.608-2.231-.29-1.96-1.425l1.257-5.273-4.117-3.527c-.887-.76-.415-2.212.749-2.305l5.404-.433 2.082-5.006z'; const id = generateId(); - const handleSelect = (newRating: number) => { + const handleSelect = (newRating: Rating) => { if (readOnly) { return; } @@ -35,7 +36,7 @@ onRating(newRating); }; - const setHoverRating = (value: number) => { + const setHoverRating = (value: Rating) => { if (readOnly) { return; } @@ -43,11 +44,11 @@ }; const reset = () => { - setHoverRating(0); - focusRating = 0; + setHoverRating(null); + focusRating = null; }; - const handleSelectDebounced = (value: number) => { + const handleSelectDebounced = (value: Rating) => { clearTimeout(timeoutId); timeoutId = setTimeout(() => { handleSelect(value); @@ -58,7 +59,7 @@
setHoverRating(0)} + onmouseleave={() => setHoverRating(null)} use:focusOutside={{ onFocusOut: reset }} use:shortcuts={[ { shortcut: { key: 'ArrowLeft' }, preventDefault: false, onShortcut: (event) => event.stopPropagation() }, @@ -69,7 +70,7 @@
{#each { length: count } as _, index (index)} {@const value = index + 1} - {@const filled = hoverRating >= value || (hoverRating === 0 && ratingSelection >= value)} + {@const filled = hoverRating === null ? (ratingSelection || 0) >= value : hoverRating >= value} {@const starId = `${id}-${value}`} @@ -77,19 +78,12 @@ for={starId} class:cursor-pointer={!readOnly} class:ring-2={focusRating === value} - onmouseover={() => setHoverRating(value)} + onmouseover={() => setHoverRating(value as Rating)} tabindex={-1} data-testid="star" > {$t('rating_count', { values: { count: value } })} - + { - focusRating = value; + focusRating = value as Rating; }} - onchange={() => handleSelectDebounced(value)} + onchange={() => handleSelectDebounced(value as Rating)} class="sr-only" /> {/each}
-{#if ratingSelection > 0 && !readOnly} +{#if ratingSelection !== null && !readOnly}