Skip to content

Commit db8a05b

Browse files
richardthe3rdrichardthe3rd
authored
Merge pull request #201 from richardthe3rd/copilot/sub-pr-200
Refactor favorite status to type-safe enum and fix DateTime comparison
2 parents 5c8624a + 91ba27f commit db8a05b

5 files changed

Lines changed: 118 additions & 36 deletions

File tree

‎lib/domain/repositories/api_drink_repository.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class ApiDrinkRepository implements DrinkRepository {
8080
@override
8181
Future<String?> getFavoriteStatus(String festivalId, String drinkId) {
8282
final item = _favoritesService.getFavoriteItem(festivalId, drinkId);
83-
return Future.value(item?.status);
83+
return Future.value(item?.status.value);
8484
}
8585

8686
@override

‎lib/models/favorite_item.dart‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
/// Status values for favorite items in the festival log.
2+
enum FavoriteStatus {
3+
/// Drink is on the 'want to try' list.
4+
wantToTry('want_to_try'),
5+
6+
/// Drink has been tasted at least once.
7+
tasted('tasted');
8+
9+
const FavoriteStatus(this.value);
10+
11+
/// The string value used for JSON serialization.
12+
final String value;
13+
14+
/// Creates a FavoriteStatus from a string value.
15+
static FavoriteStatus fromString(String value) {
16+
return values.firstWhere(
17+
(status) => status.value == value,
18+
orElse: () => FavoriteStatus.wantToTry,
19+
);
20+
}
21+
}
22+
123
/// Represents a drink in the user's festival log.
224
///
325
/// Tracks whether a drink is on the 'want to try' list or has been tasted,
@@ -16,8 +38,8 @@ class FavoriteItem {
1638
/// Drink ID.
1739
final String id;
1840

19-
/// Status: 'want_to_try' or 'tasted'.
20-
final String status;
41+
/// Current status of this drink in the festival log.
42+
final FavoriteStatus status;
2143

2244
/// List of tasting timestamps (empty if want_to_try).
2345
final List<DateTime> tries;
@@ -35,7 +57,9 @@ class FavoriteItem {
3557
factory FavoriteItem.fromJson(Map<String, dynamic> json) {
3658
return FavoriteItem(
3759
id: json['id'] as String,
38-
status: json['status'] as String? ?? 'want_to_try',
60+
status: FavoriteStatus.fromString(
61+
json['status'] as String? ?? 'want_to_try',
62+
),
3963
tries: (json['tries'] as List?)
4064
?.map((e) => DateTime.parse(e as String))
4165
.toList() ??
@@ -50,7 +74,7 @@ class FavoriteItem {
5074
Map<String, dynamic> toJson() {
5175
return {
5276
'id': id,
53-
'status': status,
77+
'status': status.value,
5478
'tries': tries.map((t) => t.toIso8601String()).toList(),
5579
if (notes != null) 'notes': notes,
5680
'createdAt': createdAt.toIso8601String(),
@@ -61,10 +85,10 @@ class FavoriteItem {
6185
/// Creates a copy with updated fields.
6286
///
6387
/// To explicitly clear notes, pass an empty Optional: `notes: Optional.value(null)`.
64-
/// To keep existing notes, omit the parameter: `copyWith(status: 'tasted')`.
88+
/// To keep existing notes, omit the parameter: `copyWith(status: FavoriteStatus.tasted)`.
6589
FavoriteItem copyWith({
6690
String? id,
67-
String? status,
91+
FavoriteStatus? status,
6892
List<DateTime>? tries,
6993
Optional<String?>? notes,
7094
DateTime? createdAt,
@@ -80,6 +104,13 @@ class FavoriteItem {
80104
);
81105
}
82106

107+
/// Equality comparison based on drink ID only.
108+
///
109+
/// Two FavoriteItems are considered equal if they have the same id,
110+
/// regardless of status, tries, notes, or timestamps. This design
111+
/// allows FavoriteItem to be used in Sets and as Map keys where
112+
/// uniqueness is determined by the drink being tracked, not its
113+
/// specific state.
83114
@override
84115
bool operator ==(Object other) =>
85116
identical(this, other) ||
@@ -92,6 +123,23 @@ class FavoriteItem {
92123
}
93124

94125
/// Wrapper class for explicitly passing null values in copyWith methods.
126+
///
127+
/// Used to distinguish between omitting a parameter (keep existing value)
128+
/// and explicitly passing null (clear the value). This is particularly
129+
/// useful for optional fields like notes where both "no change" and
130+
/// "set to null" are valid operations.
131+
///
132+
/// Example usage:
133+
/// ```dart
134+
/// // Keep existing notes
135+
/// item.copyWith(status: FavoriteStatus.tasted);
136+
///
137+
/// // Clear notes (set to null)
138+
/// item.copyWith(notes: Optional.value(null));
139+
///
140+
/// // Set new notes value
141+
/// item.copyWith(notes: Optional.value('Great beer!'));
142+
/// ```
95143
class Optional<T> {
96144
const Optional.value(this.value);
97145

‎lib/services/storage_service.dart‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class FavoritesService {
5151

5252
favorites[drinkId] = FavoriteItem(
5353
id: drinkId,
54-
status: 'want_to_try',
54+
status: FavoriteStatus.wantToTry,
5555
tries: [],
5656
createdAt: now,
5757
updatedAt: now,
@@ -78,7 +78,7 @@ class FavoritesService {
7878
final now = DateTime.now();
7979
favorites[drinkId] = FavoriteItem(
8080
id: drinkId,
81-
status: 'want_to_try',
81+
status: FavoriteStatus.wantToTry,
8282
tries: [],
8383
createdAt: now,
8484
updatedAt: now,
@@ -109,15 +109,15 @@ class FavoritesService {
109109
// Not in log yet, add as tasted
110110
favorites[drinkId] = FavoriteItem(
111111
id: drinkId,
112-
status: 'tasted',
112+
status: FavoriteStatus.tasted,
113113
tries: [now],
114114
createdAt: now,
115115
updatedAt: now,
116116
);
117117
} else {
118118
// Already in log, add timestamp and update status
119119
favorites[drinkId] = existing.copyWith(
120-
status: 'tasted',
120+
status: FavoriteStatus.tasted,
121121
tries: [...existing.tries, now],
122122
updatedAt: now,
123123
);
@@ -136,12 +136,14 @@ class FavoritesService {
136136
final existing = favorites[drinkId];
137137
if (existing == null) return;
138138

139-
final updatedTries = existing.tries.where((t) => t != timestamp).toList();
139+
final updatedTries = existing.tries
140+
.where((t) => t.millisecondsSinceEpoch != timestamp.millisecondsSinceEpoch)
141+
.toList();
140142

141143
if (updatedTries.isEmpty) {
142144
// No more tries, revert to 'want to try'
143145
favorites[drinkId] = existing.copyWith(
144-
status: 'want_to_try',
146+
status: FavoriteStatus.wantToTry,
145147
tries: [],
146148
updatedAt: DateTime.now(),
147149
);

‎test/models_test.dart‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,15 +1164,15 @@ void main() {
11641164
test('creates favorite item with all fields', () {
11651165
final item = FavoriteItem(
11661166
id: 'drink-123',
1167-
status: 'want_to_try',
1167+
status: FavoriteStatus.wantToTry,
11681168
tries: [],
11691169
notes: 'Looks interesting',
11701170
createdAt: now,
11711171
updatedAt: now,
11721172
);
11731173

11741174
expect(item.id, 'drink-123');
1175-
expect(item.status, 'want_to_try');
1175+
expect(item.status, FavoriteStatus.wantToTry);
11761176
expect(item.tries, isEmpty);
11771177
expect(item.notes, 'Looks interesting');
11781178
expect(item.createdAt, now);
@@ -1182,13 +1182,13 @@ void main() {
11821182
test('creates tasted item with tries', () {
11831183
final item = FavoriteItem(
11841184
id: 'drink-456',
1185-
status: 'tasted',
1185+
status: FavoriteStatus.tasted,
11861186
tries: [now, later],
11871187
createdAt: now,
11881188
updatedAt: later,
11891189
);
11901190

1191-
expect(item.status, 'tasted');
1191+
expect(item.status, FavoriteStatus.tasted);
11921192
expect(item.tries.length, 2);
11931193
expect(item.tries, contains(now));
11941194
expect(item.tries, contains(later));
@@ -1211,7 +1211,7 @@ void main() {
12111211
final item = FavoriteItem.fromJson(json);
12121212

12131213
expect(item.id, 'drink-789');
1214-
expect(item.status, 'tasted');
1214+
expect(item.status, FavoriteStatus.tasted);
12151215
expect(item.tries.length, 2);
12161216
expect(item.notes, 'Excellent beer');
12171217
});
@@ -1226,7 +1226,7 @@ void main() {
12261226
final item = FavoriteItem.fromJson(json);
12271227

12281228
expect(item.id, 'drink-minimal');
1229-
expect(item.status, 'want_to_try'); // Default status
1229+
expect(item.status, FavoriteStatus.wantToTry); // Default status
12301230
expect(item.tries, isEmpty);
12311231
expect(item.notes, isNull);
12321232
});
@@ -1250,7 +1250,7 @@ void main() {
12501250
test('converts to JSON correctly', () {
12511251
final item = FavoriteItem(
12521252
id: 'drink-abc',
1253-
status: 'tasted',
1253+
status: FavoriteStatus.tasted,
12541254
tries: [now],
12551255
notes: 'Great!',
12561256
createdAt: now,
@@ -1271,7 +1271,7 @@ void main() {
12711271
test('excludes null notes from JSON', () {
12721272
final item = FavoriteItem(
12731273
id: 'drink-no-notes',
1274-
status: 'want_to_try',
1274+
status: FavoriteStatus.wantToTry,
12751275
tries: [],
12761276
createdAt: now,
12771277
updatedAt: now,
@@ -1285,7 +1285,7 @@ void main() {
12851285
test('roundtrip through JSON maintains data', () {
12861286
final original = FavoriteItem(
12871287
id: 'drink-roundtrip',
1288-
status: 'tasted',
1288+
status: FavoriteStatus.tasted,
12891289
tries: [now, later],
12901290
notes: 'Test notes',
12911291
createdAt: now,
@@ -1311,20 +1311,20 @@ void main() {
13111311
test('creates copy with updated fields', () {
13121312
final original = FavoriteItem(
13131313
id: 'drink-copy',
1314-
status: 'want_to_try',
1314+
status: FavoriteStatus.wantToTry,
13151315
tries: [],
13161316
createdAt: now,
13171317
updatedAt: now,
13181318
);
13191319

13201320
final updated = original.copyWith(
1321-
status: 'tasted',
1321+
status: FavoriteStatus.tasted,
13221322
tries: [later],
13231323
updatedAt: later,
13241324
);
13251325

13261326
expect(updated.id, original.id); // Unchanged
1327-
expect(updated.status, 'tasted'); // Changed
1327+
expect(updated.status, FavoriteStatus.tasted); // Changed
13281328
expect(updated.tries, [later]); // Changed
13291329
expect(updated.createdAt, original.createdAt); // Unchanged
13301330
expect(updated.updatedAt, later); // Changed
@@ -1333,7 +1333,7 @@ void main() {
13331333
test('preserves unchanged fields', () {
13341334
final original = FavoriteItem(
13351335
id: 'drink-preserve',
1336-
status: 'tasted',
1336+
status: FavoriteStatus.tasted,
13371337
tries: [now],
13381338
notes: 'Original notes',
13391339
createdAt: now,
@@ -1355,15 +1355,15 @@ void main() {
13551355
test('equal items have same id', () {
13561356
final item1 = FavoriteItem(
13571357
id: 'drink-eq',
1358-
status: 'want_to_try',
1358+
status: FavoriteStatus.wantToTry,
13591359
tries: [],
13601360
createdAt: now,
13611361
updatedAt: now,
13621362
);
13631363

13641364
final item2 = FavoriteItem(
13651365
id: 'drink-eq',
1366-
status: 'tasted',
1366+
status: FavoriteStatus.tasted,
13671367
tries: [later],
13681368
createdAt: later,
13691369
updatedAt: later,
@@ -1376,15 +1376,15 @@ void main() {
13761376
test('different items have different ids', () {
13771377
final item1 = FavoriteItem(
13781378
id: 'drink-1',
1379-
status: 'want_to_try',
1379+
status: FavoriteStatus.wantToTry,
13801380
tries: [],
13811381
createdAt: now,
13821382
updatedAt: now,
13831383
);
13841384

13851385
final item2 = FavoriteItem(
13861386
id: 'drink-2',
1387-
status: 'want_to_try',
1387+
status: FavoriteStatus.wantToTry,
13881388
tries: [],
13891389
createdAt: now,
13901390
updatedAt: now,

0 commit comments

Comments
 (0)