diff --git a/lib/domain/repositories/api_drink_repository.dart b/lib/domain/repositories/api_drink_repository.dart index 5a46c4e6..3c92be43 100644 --- a/lib/domain/repositories/api_drink_repository.dart +++ b/lib/domain/repositories/api_drink_repository.dart @@ -80,7 +80,7 @@ class ApiDrinkRepository implements DrinkRepository { @override Future getFavoriteStatus(String festivalId, String drinkId) { final item = _favoritesService.getFavoriteItem(festivalId, drinkId); - return Future.value(item?.status); + return Future.value(item?.status.value); } @override diff --git a/lib/models/favorite_item.dart b/lib/models/favorite_item.dart index fd2ce957..58867d8f 100644 --- a/lib/models/favorite_item.dart +++ b/lib/models/favorite_item.dart @@ -1,3 +1,25 @@ +/// Status values for favorite items in the festival log. +enum FavoriteStatus { + /// Drink is on the 'want to try' list. + wantToTry('want_to_try'), + + /// Drink has been tasted at least once. + tasted('tasted'); + + const FavoriteStatus(this.value); + + /// The string value used for JSON serialization. + final String value; + + /// Creates a FavoriteStatus from a string value. + static FavoriteStatus fromString(String value) { + return values.firstWhere( + (status) => status.value == value, + orElse: () => FavoriteStatus.wantToTry, + ); + } +} + /// Represents a drink in the user's festival log. /// /// Tracks whether a drink is on the 'want to try' list or has been tasted, @@ -16,8 +38,8 @@ class FavoriteItem { /// Drink ID. final String id; - /// Status: 'want_to_try' or 'tasted'. - final String status; + /// Current status of this drink in the festival log. + final FavoriteStatus status; /// List of tasting timestamps (empty if want_to_try). final List tries; @@ -35,7 +57,9 @@ class FavoriteItem { factory FavoriteItem.fromJson(Map json) { return FavoriteItem( id: json['id'] as String, - status: json['status'] as String? ?? 'want_to_try', + status: FavoriteStatus.fromString( + json['status'] as String? ?? 'want_to_try', + ), tries: (json['tries'] as List?) ?.map((e) => DateTime.parse(e as String)) .toList() ?? @@ -50,7 +74,7 @@ class FavoriteItem { Map toJson() { return { 'id': id, - 'status': status, + 'status': status.value, 'tries': tries.map((t) => t.toIso8601String()).toList(), if (notes != null) 'notes': notes, 'createdAt': createdAt.toIso8601String(), @@ -61,10 +85,10 @@ class FavoriteItem { /// Creates a copy with updated fields. /// /// To explicitly clear notes, pass an empty Optional: `notes: Optional.value(null)`. - /// To keep existing notes, omit the parameter: `copyWith(status: 'tasted')`. + /// To keep existing notes, omit the parameter: `copyWith(status: FavoriteStatus.tasted)`. FavoriteItem copyWith({ String? id, - String? status, + FavoriteStatus? status, List? tries, Optional? notes, DateTime? createdAt, @@ -80,6 +104,13 @@ class FavoriteItem { ); } + /// Equality comparison based on drink ID only. + /// + /// Two FavoriteItems are considered equal if they have the same id, + /// regardless of status, tries, notes, or timestamps. This design + /// allows FavoriteItem to be used in Sets and as Map keys where + /// uniqueness is determined by the drink being tracked, not its + /// specific state. @override bool operator ==(Object other) => identical(this, other) || @@ -92,6 +123,23 @@ class FavoriteItem { } /// Wrapper class for explicitly passing null values in copyWith methods. +/// +/// Used to distinguish between omitting a parameter (keep existing value) +/// and explicitly passing null (clear the value). This is particularly +/// useful for optional fields like notes where both "no change" and +/// "set to null" are valid operations. +/// +/// Example usage: +/// ```dart +/// // Keep existing notes +/// item.copyWith(status: FavoriteStatus.tasted); +/// +/// // Clear notes (set to null) +/// item.copyWith(notes: Optional.value(null)); +/// +/// // Set new notes value +/// item.copyWith(notes: Optional.value('Great beer!')); +/// ``` class Optional { const Optional.value(this.value); diff --git a/lib/services/storage_service.dart b/lib/services/storage_service.dart index aedae6e0..ba89fd12 100644 --- a/lib/services/storage_service.dart +++ b/lib/services/storage_service.dart @@ -51,7 +51,7 @@ class FavoritesService { favorites[drinkId] = FavoriteItem( id: drinkId, - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, @@ -78,7 +78,7 @@ class FavoritesService { final now = DateTime.now(); favorites[drinkId] = FavoriteItem( id: drinkId, - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, @@ -109,7 +109,7 @@ class FavoritesService { // Not in log yet, add as tasted favorites[drinkId] = FavoriteItem( id: drinkId, - status: 'tasted', + status: FavoriteStatus.tasted, tries: [now], createdAt: now, updatedAt: now, @@ -117,7 +117,7 @@ class FavoritesService { } else { // Already in log, add timestamp and update status favorites[drinkId] = existing.copyWith( - status: 'tasted', + status: FavoriteStatus.tasted, tries: [...existing.tries, now], updatedAt: now, ); @@ -136,12 +136,14 @@ class FavoritesService { final existing = favorites[drinkId]; if (existing == null) return; - final updatedTries = existing.tries.where((t) => t != timestamp).toList(); + final updatedTries = existing.tries + .where((t) => t.millisecondsSinceEpoch != timestamp.millisecondsSinceEpoch) + .toList(); if (updatedTries.isEmpty) { // No more tries, revert to 'want to try' favorites[drinkId] = existing.copyWith( - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], updatedAt: DateTime.now(), ); diff --git a/test/models_test.dart b/test/models_test.dart index f76721a1..73cfc076 100644 --- a/test/models_test.dart +++ b/test/models_test.dart @@ -1164,7 +1164,7 @@ void main() { test('creates favorite item with all fields', () { final item = FavoriteItem( id: 'drink-123', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], notes: 'Looks interesting', createdAt: now, @@ -1172,7 +1172,7 @@ void main() { ); expect(item.id, 'drink-123'); - expect(item.status, 'want_to_try'); + expect(item.status, FavoriteStatus.wantToTry); expect(item.tries, isEmpty); expect(item.notes, 'Looks interesting'); expect(item.createdAt, now); @@ -1182,13 +1182,13 @@ void main() { test('creates tasted item with tries', () { final item = FavoriteItem( id: 'drink-456', - status: 'tasted', + status: FavoriteStatus.tasted, tries: [now, later], createdAt: now, updatedAt: later, ); - expect(item.status, 'tasted'); + expect(item.status, FavoriteStatus.tasted); expect(item.tries.length, 2); expect(item.tries, contains(now)); expect(item.tries, contains(later)); @@ -1211,7 +1211,7 @@ void main() { final item = FavoriteItem.fromJson(json); expect(item.id, 'drink-789'); - expect(item.status, 'tasted'); + expect(item.status, FavoriteStatus.tasted); expect(item.tries.length, 2); expect(item.notes, 'Excellent beer'); }); @@ -1226,7 +1226,7 @@ void main() { final item = FavoriteItem.fromJson(json); expect(item.id, 'drink-minimal'); - expect(item.status, 'want_to_try'); // Default status + expect(item.status, FavoriteStatus.wantToTry); // Default status expect(item.tries, isEmpty); expect(item.notes, isNull); }); @@ -1250,7 +1250,7 @@ void main() { test('converts to JSON correctly', () { final item = FavoriteItem( id: 'drink-abc', - status: 'tasted', + status: FavoriteStatus.tasted, tries: [now], notes: 'Great!', createdAt: now, @@ -1271,7 +1271,7 @@ void main() { test('excludes null notes from JSON', () { final item = FavoriteItem( id: 'drink-no-notes', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, @@ -1285,7 +1285,7 @@ void main() { test('roundtrip through JSON maintains data', () { final original = FavoriteItem( id: 'drink-roundtrip', - status: 'tasted', + status: FavoriteStatus.tasted, tries: [now, later], notes: 'Test notes', createdAt: now, @@ -1311,20 +1311,20 @@ void main() { test('creates copy with updated fields', () { final original = FavoriteItem( id: 'drink-copy', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, ); final updated = original.copyWith( - status: 'tasted', + status: FavoriteStatus.tasted, tries: [later], updatedAt: later, ); expect(updated.id, original.id); // Unchanged - expect(updated.status, 'tasted'); // Changed + expect(updated.status, FavoriteStatus.tasted); // Changed expect(updated.tries, [later]); // Changed expect(updated.createdAt, original.createdAt); // Unchanged expect(updated.updatedAt, later); // Changed @@ -1333,7 +1333,7 @@ void main() { test('preserves unchanged fields', () { final original = FavoriteItem( id: 'drink-preserve', - status: 'tasted', + status: FavoriteStatus.tasted, tries: [now], notes: 'Original notes', createdAt: now, @@ -1355,7 +1355,7 @@ void main() { test('equal items have same id', () { final item1 = FavoriteItem( id: 'drink-eq', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, @@ -1363,7 +1363,7 @@ void main() { final item2 = FavoriteItem( id: 'drink-eq', - status: 'tasted', + status: FavoriteStatus.tasted, tries: [later], createdAt: later, updatedAt: later, @@ -1376,7 +1376,7 @@ void main() { test('different items have different ids', () { final item1 = FavoriteItem( id: 'drink-1', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, @@ -1384,7 +1384,7 @@ void main() { final item2 = FavoriteItem( id: 'drink-2', - status: 'want_to_try', + status: FavoriteStatus.wantToTry, tries: [], createdAt: now, updatedAt: now, diff --git a/test/storage_service_test.dart b/test/storage_service_test.dart index 93e445a7..a1f8ece6 100644 --- a/test/storage_service_test.dart +++ b/test/storage_service_test.dart @@ -1,4 +1,5 @@ import 'package:flutter_test/flutter_test.dart'; +import 'package:cambridge_beer_festival/models/models.dart'; import 'package:cambridge_beer_festival/services/storage_service.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -17,7 +18,7 @@ void main() { final favorites = favoritesService.getFavorites('cbf2025'); expect(favorites, isEmpty); - expect(favorites, isA>()); + expect(favorites, isA>()); }); test('addFavorite adds drink to favorites with want_to_try status', () async { @@ -28,7 +29,7 @@ void main() { final favorites = favoritesService.getFavorites('cbf2025'); expect(favorites.containsKey('drink-123'), isTrue); - expect(favorites['drink-123']!.status, 'want_to_try'); + expect(favorites['drink-123']!.status, FavoriteStatus.wantToTry); expect(favorites['drink-123']!.tries, isEmpty); }); @@ -78,7 +79,7 @@ void main() { final item = favoritesService.getFavoriteItem('cbf2025', 'drink-123'); expect(item, isNotNull); - expect(item!.status, 'want_to_try'); + expect(item!.status, FavoriteStatus.wantToTry); }); test('toggleFavorite removes drink when already favorite', () async { @@ -146,7 +147,7 @@ void main() { final item = favoritesService.getFavoriteItem('cbf2025', 'drink-123'); expect(item, isNotNull); - expect(item!.status, 'tasted'); + expect(item!.status, FavoriteStatus.tasted); expect(item.tries.length, 1); }); @@ -159,7 +160,7 @@ void main() { final item = favoritesService.getFavoriteItem('cbf2025', 'drink-123'); expect(item, isNotNull); - expect(item!.status, 'tasted'); + expect(item!.status, FavoriteStatus.tasted); expect(item.tries.length, 1); }); @@ -181,6 +182,7 @@ void main() { favoritesService = FavoritesService(prefs); await favoritesService.markAsTasted('cbf2025', 'drink-123'); + await Future.delayed(const Duration(milliseconds: 10)); // Ensure different timestamps await favoritesService.markAsTasted('cbf2025', 'drink-123'); final item = favoritesService.getFavoriteItem('cbf2025', 'drink-123'); @@ -205,7 +207,7 @@ void main() { await favoritesService.deleteTry('cbf2025', 'drink-123', timestamp); final updated = favoritesService.getFavoriteItem('cbf2025', 'drink-123'); - expect(updated!.status, 'want_to_try'); + expect(updated!.status, FavoriteStatus.wantToTry); expect(updated.tries, isEmpty); }); @@ -221,6 +223,36 @@ void main() { ); }); + test('deleteTry works correctly after JSON serialization', () async { + // This test verifies that DateTime comparison works correctly + // after data is persisted and deserialized from JSON storage. + final prefs = await SharedPreferences.getInstance(); + favoritesService = FavoritesService(prefs); + + // Mark as tasted to create a timestamp + await favoritesService.markAsTasted('cbf2025', 'drink-123'); + await Future.delayed(const Duration(milliseconds: 10)); // Ensure different timestamps + await favoritesService.markAsTasted('cbf2025', 'drink-123'); + + // Create new service instance to force reload from JSON + final newService = FavoritesService(prefs); + + // Get the timestamp from the reloaded data + final item = newService.getFavoriteItem('cbf2025', 'drink-123'); + expect(item, isNotNull); + expect(item!.tries.length, 2); + + final firstTimestamp = item.tries.first; + + // Delete the timestamp using the reloaded service + await newService.deleteTry('cbf2025', 'drink-123', firstTimestamp); + + // Verify the timestamp was deleted + final updated = newService.getFavoriteItem('cbf2025', 'drink-123'); + expect(updated!.tries.length, 1); + expect(updated.tries.contains(firstTimestamp), isFalse); + }); + test('updateNotes sets notes on favorite item', () async { final prefs = await SharedPreferences.getInstance(); favoritesService = FavoritesService(prefs);