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

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', () => {

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()

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()

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,
       };

server/src/schema/migrations/1771535611395-ConvertRating0ToNull.ts

@@ -0,0 +1,9 @@
+import { Kysely, sql } from 'kysely';
+
+export async function up(db: Kysely<any>): Promise<void> {
+  await sql`UPDATE "asset_exif" SET "rating" = NULL WHERE "rating" = 0;`.execute(db);
+}
+
+export async function down(): Promise<void> {
+  // not supported
+}

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(

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', () => {

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,
       },