refactor!: disallow star rating < 1

e2e/src/specs/server/api/asset.e2e-spec.ts

@@ -492,20 +492,6 @@ describe('/asset', () => {
       expect(status).toEqual(200);
     });
 
-    it('should set the negative rating', async () => {
-      const { status, body } = await request(app)
-        .put(`/assets/${user1Assets[0].id}`)
-        .set('Authorization', `Bearer ${user1.accessToken}`)
-        .send({ rating: -1 });
-      expect(body).toMatchObject({
-        id: user1Assets[0].id,
-        exifInfo: expect.objectContaining({
-          rating: -1,
-        }),
-      });
-      expect(status).toEqual(200);
-    });
-
     it('should return tagged people', async () => {
       const { status, body } = await request(app)
         .put(`/assets/${user1Assets[0].id}`)

mobile/lib/infrastructure/repositories/remote_asset.repository.dart

@@ -267,7 +267,7 @@ class RemoteAssetRepository extends DriftDatabaseRepository {
     );
   }
 
-  Future<void> updateRating(String assetId, int rating) async {
+  Future<void> updateRating(String assetId, int? rating) async {
     await (_db.remoteExifEntity.update()..where((row) => row.assetId.equals(assetId))).write(
       RemoteExifEntityCompanion(rating: Value(rating)),
     );

mobile/lib/infrastructure/repositories/search_api.repository.dart

@@ -1,6 +1,7 @@
 import 'package:immich_mobile/domain/models/asset/base_asset.model.dart' hide AssetVisibility;
 import 'package:immich_mobile/infrastructure/repositories/api.repository.dart';
 import 'package:immich_mobile/models/search/search_filter.model.dart';
+import 'package:immich_mobile/utils/option.dart';
 import 'package:openapi/api.dart';
 
 class SearchApiRepository extends ApiRepository {
@@ -37,7 +38,7 @@ class SearchApiRepository extends ApiRepository {
               ? const Optional.absent()
               : Optional.present(filter.date.takenBefore!),
           visibility: Optional.present(filter.display.isArchive ? AssetVisibility.archive : AssetVisibility.timeline),
-          rating: filter.rating.rating == null ? const Optional.absent() : Optional.present(filter.rating.rating!),
+          rating: filter.rating.rating.toOptional(),
           isFavorite: filter.display.isFavorite ? const Optional.present(true) : const Optional.absent(),
           isNotInAlbum: filter.display.isNotInAlbum ? const Optional.present(true) : const Optional.absent(),
           personIds: Optional.present(filter.people.map((e) => e.id).toList()),
@@ -70,7 +71,7 @@ class SearchApiRepository extends ApiRepository {
             ? const Optional.absent()
             : Optional.present(filter.date.takenBefore!),
         visibility: Optional.present(filter.display.isArchive ? AssetVisibility.archive : AssetVisibility.timeline),
-        rating: filter.rating.rating == null ? const Optional.absent() : Optional.present(filter.rating.rating!),
+        rating: filter.rating.rating.toOptional(),
         isFavorite: filter.display.isFavorite ? const Optional.present(true) : const Optional.absent(),
         isNotInAlbum: filter.display.isNotInAlbum ? const Optional.present(true) : const Optional.absent(),
         personIds: Optional.present(filter.people.map((e) => e.id).toList()),

mobile/lib/models/search/search_filter.model.dart

@@ -3,6 +3,7 @@ import 'dart:convert';
 
 import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
 import 'package:immich_mobile/domain/models/person.model.dart';
+import 'package:immich_mobile/utils/option.dart';
 
 class SearchLocationFilter {
   String? country;
@@ -133,19 +134,26 @@ class SearchDateFilter {
 }
 
 class SearchRatingFilter {
-  int? rating;
-  SearchRatingFilter({this.rating});
+  /// none = no filter; some(null) = filter for unrated; some(1-5) = filter for that rating
+  Option<int?> rating;
+  SearchRatingFilter({this.rating = const Option.none()});
 
-  SearchRatingFilter copyWith({int? rating}) {
+  SearchRatingFilter copyWith({Option<int?>? rating}) {
     return SearchRatingFilter(rating: rating ?? this.rating);
   }
 
   Map<String, dynamic> toMap() {
-    return <String, dynamic>{'rating': rating};
+    if (rating.isNone) {
+      return <String, dynamic>{'active': false};
+    }
+    return <String, dynamic>{'active': true, 'value': rating.unwrapOrNull};
   }
 
   factory SearchRatingFilter.fromMap(Map<String, dynamic> map) {
-    return SearchRatingFilter(rating: map['rating'] != null ? map['rating'] as int : null);
+    if (!(map['active'] as bool? ?? false)) {
+      return SearchRatingFilter();
+    }
+    return SearchRatingFilter(rating: Option.some(map['value'] as int?));
   }
 
   String toJson() => json.encode(toMap());
@@ -270,7 +278,7 @@ class SearchFilter {
         display.isNotInAlbum == false &&
         display.isArchive == false &&
         display.isFavorite == false &&
-        rating.rating == null &&
+        rating.rating.isNone &&
         mediaType == AssetType.other;
   }
 

mobile/lib/presentation/pages/search/drift_search.page.dart

@@ -404,12 +404,15 @@ class DriftSearchPage extends HookConsumerWidget {
 
       handleClear() {
         ratingCurrentFilterWidget.value = null;
-        search(filter.value.copyWith(rating: SearchRatingFilter(rating: null)));
+        search(filter.value.copyWith(rating: SearchRatingFilter()));
       }
 
       handleApply() {
-        ratingCurrentFilterWidget.value = rating.rating != null
-            ? Text('rating_count'.t(args: {'count': rating.rating!}), style: context.textTheme.labelLarge)
+        ratingCurrentFilterWidget.value = rating.rating.isSome
+            ? Text(
+                'rating_count'.t(args: {'count': rating.rating.unwrapOrNull ?? 0}),
+                style: context.textTheme.labelLarge,
+              )
             : null;
         search(filter.value.copyWith(rating: rating));
       }

mobile/lib/presentation/widgets/asset_viewer/asset_details/rating_details.widget.dart

@@ -44,7 +44,7 @@ class RatingDetails extends ConsumerWidget {
               await ref.read(actionProvider.notifier).updateRating(ActionSource.viewer, rating.round());
             },
             onClearRating: () async {
-              await ref.read(actionProvider.notifier).updateRating(ActionSource.viewer, 0);
+              await ref.read(actionProvider.notifier).updateRating(ActionSource.viewer, null);
             },
           ),
         ],

mobile/lib/presentation/widgets/asset_viewer/rating_bar.widget.dart

@@ -77,7 +77,11 @@ class _RatingBarState extends State<RatingBar> {
       setState(() {
         _currentRating = newRating;
       });
-      widget.onRatingUpdate?.call(newRating.round());
+      if (newRating == 0) {
+        widget.onClearRating?.call();
+      } else {
+        widget.onRatingUpdate?.call(newRating.round());
+      }
     }
   }
 

mobile/lib/providers/infrastructure/action.provider.dart

@@ -466,7 +466,7 @@ class ActionNotifier extends Notifier<void> {
     }
   }
 
-  Future<ActionResult> updateRating(ActionSource source, int rating) async {
+  Future<ActionResult> updateRating(ActionSource source, int? rating) async {
     final ids = _getRemoteIdsForSource(source);
     if (ids.length != 1) {
       _logger.warning('updateRating called with multiple assets, expected single asset');

mobile/lib/repositories/asset_api.repository.dart

@@ -97,7 +97,7 @@ class AssetApiRepository extends ApiRepository {
     return _api.updateAsset(assetId, UpdateAssetDto(description: Optional.present(description)));
   }
 
-  Future<void> updateRating(String assetId, int rating) {
+  Future<void> updateRating(String assetId, int? rating) {
     return _api.updateAsset(assetId, UpdateAssetDto(rating: Optional.present(rating)));
   }
 

mobile/lib/services/action.service.dart

@@ -231,7 +231,7 @@ class ActionService {
     return true;
   }
 
-  Future<bool> updateRating(String assetId, int rating) async {
+  Future<bool> updateRating(String assetId, int? rating) async {
     // update remote first, then local to ensure consistency
     await _assetApiRepository.updateRating(assetId, rating);
     await _remoteAssetRepository.updateRating(assetId, rating);

mobile/lib/utils/option.dart

@@ -1,3 +1,5 @@
+import 'package:openapi/api.dart' show Optional;
+
 sealed class Option<T> {
   const Option();
 
@@ -56,3 +58,10 @@ final class None<T> extends Option<T> {
 extension ObjectOptionExtension<T> on T? {
   Option<T> toOption() => Option.fromNullable(this);
 }
+
+extension OptionToOptional<T> on Option<T> {
+  Optional<T> toOptional() => switch (this) {
+    None() => const Optional.absent(),
+    Some(:final value) => Optional.present(value),
+  };
+}

mobile/lib/widgets/search/search_filter/star_rating_picker.dart

@@ -2,6 +2,7 @@ import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:immich_mobile/extensions/translate_extensions.dart';
 import 'package:immich_mobile/models/search/search_filter.model.dart';
+import 'package:immich_mobile/utils/option.dart';
 
 class StarRatingPicker extends HookWidget {
   const StarRatingPicker({super.key, required this.onSelect, this.filter});
@@ -13,12 +14,12 @@ class StarRatingPicker extends HookWidget {
     final selectedRating = useState(filter);
 
     return RadioGroup(
-      groupValue: selectedRating.value?.rating,
+      groupValue: selectedRating.value?.rating.fold((v) => v ?? 0, () => null),
       onChanged: (int? newValue) {
         if (newValue == null) {
           return;
         }
-        final newFilter = SearchRatingFilter(rating: newValue);
+        final newFilter = SearchRatingFilter(rating: Option.some(newValue == 0 ? null : newValue));
         selectedRating.value = newFilter;
         onSelect(newFilter);
       },

mobile/test/services/action.service_test.dart

@@ -73,6 +73,32 @@ void main() {
     await Store.clear();
   });
 
+  group('ActionService.updateRating', () {
+    const assetId = 'asset_id_1';
+
+    test('calls both repositories with the given rating', () async {
+      when(() => assetApiRepository.updateRating(assetId, 3)).thenAnswer((_) async {});
+      when(() => remoteAssetRepository.updateRating(assetId, 3)).thenAnswer((_) async {});
+
+      final result = await sut.updateRating(assetId, 3);
+
+      expect(result, isTrue);
+      verify(() => assetApiRepository.updateRating(assetId, 3)).called(1);
+      verify(() => remoteAssetRepository.updateRating(assetId, 3)).called(1);
+    });
+
+    test('calls both repositories with null to clear rating', () async {
+      when(() => assetApiRepository.updateRating(assetId, null)).thenAnswer((_) async {});
+      when(() => remoteAssetRepository.updateRating(assetId, null)).thenAnswer((_) async {});
+
+      final result = await sut.updateRating(assetId, null);
+
+      expect(result, isTrue);
+      verify(() => assetApiRepository.updateRating(assetId, null)).called(1);
+      verify(() => remoteAssetRepository.updateRating(assetId, null)).called(1);
+    });
+  });
+
   group('ActionService.deleteLocal', () {
     test('routes deleted ids to trashed repository when Android trash handling is enabled', () async {
       await Store.put(StoreKey.manageLocalMediaAndroid, true);