Skip to content

Commit 6cb8c4c

Browse files
committed
refactor(puzzle): address review on offline puzzle setting
- Remove now-unused kPuzzleLocalQueueLength constant and its stale comment - Clamp nbOfflinePuzzles on load in PuzzlePreferences.build() so corrupted or manually edited prefs cannot bypass the 50-500 bounds - Add a widget test covering the offline puzzles settings tile and picker
1 parent f7da360 commit 6cb8c4c

3 files changed

Lines changed: 53 additions & 5 deletions

File tree

lib/src/model/puzzle/puzzle_preferences.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ class PuzzlePreferences extends Notifier<PuzzlePrefs> with SessionPreferencesSto
2626

2727
@override
2828
PuzzlePrefs build() {
29-
return fetch();
29+
final prefs = fetch();
30+
// Clamp the offline queue length loaded from storage: corrupted or
31+
// manually edited prefs could otherwise bypass the bounds enforced by
32+
// [setNbOfflinePuzzles] and trigger very large downloads.
33+
final clamped = prefs.nbOfflinePuzzles.clamp(kMinOfflinePuzzles, kMaxOfflinePuzzles);
34+
return clamped == prefs.nbOfflinePuzzles ? prefs : prefs.copyWith(nbOfflinePuzzles: clamped);
3035
}
3136

3237
Future<void> setDifficulty(PuzzleDifficulty difficulty) async {

lib/src/model/puzzle/puzzle_service.dart

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,6 @@ import 'package:result_extensions/result_extensions.dart';
1818

1919
part 'puzzle_service.freezed.dart';
2020

21-
/// Default size of puzzle local cache. Used as a fallback; the effective
22-
/// queue length is user-configurable via [PuzzlePrefs.nbOfflinePuzzles].
23-
const kPuzzleLocalQueueLength = 50;
24-
2521
/// A provider for [PuzzleService].
2622
final puzzleServiceProvider = FutureProvider<PuzzleService>((Ref ref) {
2723
final nbOfflinePuzzles = ref.watch(

test/view/puzzle/puzzle_screen_test.dart

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,53 @@ void main() {
123123
expect(find.text('Your turn'), findsOneWidget);
124124
});
125125

126+
testWidgets(
127+
'Changing the offline puzzles setting updates the displayed value',
128+
variant: kPlatformVariant,
129+
(tester) async {
130+
final app = await makeTestProviderScopeApp(
131+
tester,
132+
home: PuzzleScreen(
133+
angle: const PuzzleTheme(PuzzleThemeKey.mix),
134+
puzzleId: puzzle.puzzle.id,
135+
),
136+
overrides: {
137+
puzzleBatchStorageProvider: puzzleBatchStorageProvider.overrideWith(
138+
(ref) => mockBatchStorage,
139+
),
140+
puzzleStorageProvider: puzzleStorageProvider.overrideWith((ref) => mockHistoryStorage),
141+
},
142+
);
143+
144+
when(
145+
() => mockHistoryStorage.fetch(puzzleId: puzzle.puzzle.id),
146+
).thenAnswer((_) async => puzzle);
147+
148+
await tester.pumpWidget(app);
149+
150+
// wait for the puzzle to load
151+
await tester.pump(const Duration(milliseconds: 200));
152+
153+
// open settings
154+
await tester.tap(find.byIcon(Icons.settings));
155+
await tester.pumpAndSettle();
156+
157+
// the offline puzzles tile shows the default value
158+
final tileFinder = find.widgetWithText(SettingsListTile, 'Offline puzzles');
159+
expect(tileFinder, findsOneWidget);
160+
expect(tester.widget<SettingsListTile>(tileFinder).settingsValue, '50');
161+
162+
// open the choice picker and select a larger value
163+
await tester.tap(tileFinder);
164+
await tester.pumpAndSettle();
165+
await tester.tap(find.text('200').last);
166+
await tester.pumpAndSettle();
167+
168+
// the tile reflects the new value
169+
expect(tester.widget<SettingsListTile>(tileFinder).settingsValue, '200');
170+
},
171+
);
172+
126173
testWidgets('Loads next puzzle when no puzzleId is passed', (tester) async {
127174
final app = await makeTestProviderScopeApp(
128175
tester,

0 commit comments

Comments
 (0)