mirror of
https://github.com/lichess-org/mobile.git
synced 2026-05-26 13:50:52 +00:00
Link to daily puzzle screen from puzzle widget (#2984)
* Link to puzzle screen in the app from puzzle widget * Extract constants * Fix build problem in widget * Fetch first daily to avoid extra request; add tests --------- Co-authored-by: Vincent Velociter <vincent.velociter@gmail.com>
This commit is contained in:
@@ -63,6 +63,41 @@ flutter test test/model/engine/engine_test.dart
|
||||
flutter test test/model/engine/engine_test.dart --name "test name"
|
||||
```
|
||||
|
||||
### Testing Strategy: Prefer HTTP Mocking Over Provider Overrides
|
||||
|
||||
**Always mock at the HTTP layer** (override `httpClientFactoryProvider`) rather than overriding Riverpod providers directly. Reasons:
|
||||
|
||||
1. **`autoDispose` + `ref.read` interaction**: Many providers are `FutureProvider.autoDispose` and use `ref.withClientCacheFor` which calls `keepAlive()` to prevent premature disposal. Overriding the provider directly bypasses `keepAlive()`, so the provider can be disposed before its future resolves — causing silent test failures where navigation never happens.
|
||||
|
||||
2. **Tests provider logic, not mocks**: Most providers contain real logic (caching, fallback, data transformation) that should be exercised in tests. Replacing a provider with `(_) async => fakeValue` skips all that logic.
|
||||
|
||||
**Pattern to use:**
|
||||
```dart
|
||||
overrides: {
|
||||
httpClientFactoryProvider: httpClientFactoryProvider.overrideWith((ref) {
|
||||
return FakeHttpClientFactory(
|
||||
() => MockClient((request) async {
|
||||
if (request.url.path == '/api/puzzle/daily') {
|
||||
return http.Response(mockDailyPuzzleResponse, 200);
|
||||
}
|
||||
return http.Response('', 404);
|
||||
}),
|
||||
);
|
||||
}),
|
||||
},
|
||||
```
|
||||
|
||||
Direct provider overrides are acceptable for **non-network providers** (repositories backed by mocks, services with no HTTP, etc.) where the provider has no `keepAlive` dependency and the override doesn't skip meaningful logic.
|
||||
|
||||
### Analysis Rules (CRITICAL)
|
||||
|
||||
**Always run `flutter analyze` on every file you edit, including test files, before finishing.**
|
||||
|
||||
Two rules the analyzer enforces that are easy to miss:
|
||||
|
||||
- **`const` constructors**: use `const` (not `final`) when constructing a const-capable class. The analyzer will flag `prefer_const_constructors`. This applies everywhere, including test files.
|
||||
- **No leading underscores for local identifiers**: local variables and functions must not start with `_`. Reserve `_` for library-private top-level or class members.
|
||||
|
||||
### Code Quality Checks
|
||||
```bash
|
||||
# Static analysis
|
||||
|
||||
@@ -16,13 +16,14 @@ struct DailyPuzzleEntry: TimelineEntry {
|
||||
return parts[1] == "w"
|
||||
}
|
||||
|
||||
var puzzleURL: URL? {
|
||||
if let id = puzzleId {
|
||||
return LichessAppGroup.lichessURL(path: "/training/\(id)")
|
||||
} else {
|
||||
return LichessAppGroup.lichessURL(path: "/training/daily")
|
||||
}
|
||||
}
|
||||
/// Custom-scheme deeplink handled by the Flutter app to open the native
|
||||
/// daily-puzzle screen (titled "Daily Puzzle"). See `AppLinksService`.
|
||||
///
|
||||
/// Includes the specific puzzle id when known so the app opens the same
|
||||
/// puzzle the widget is showing (the widget caches the daily puzzle for up
|
||||
/// to 6 hours, so a plain `/training/daily` deeplink could otherwise open
|
||||
/// a different puzzle than the one tapped).
|
||||
var puzzleURL: URL? { dailyPuzzleDeeplink(puzzleId: puzzleId) }
|
||||
|
||||
/// A recognisable position (Italian game after initial moves)
|
||||
/// used as placeholder while the real puzzle loads.
|
||||
|
||||
@@ -47,8 +47,7 @@ struct DailyPuzzleWidgetView: View {
|
||||
)
|
||||
}
|
||||
.padding(.horizontal, DailyPuzzleWidgetLayout.horizontalPadding)
|
||||
.padding(.top, DailyPuzzleWidgetLayout.topPadding)
|
||||
.padding(.bottom, DailyPuzzleWidgetLayout.bottomPadding)
|
||||
.padding(.vertical, DailyPuzzleWidgetLayout.verticalPadding)
|
||||
}
|
||||
|
||||
@ViewBuilder
|
||||
|
||||
@@ -1,12 +1,24 @@
|
||||
import Foundation
|
||||
|
||||
// Note: puzzle widgets use plain HTTPS URLs (e.g. lichess.org/training/{id})
|
||||
// so the app can handle them as universal links and open the native puzzle screen directly.
|
||||
// Note: the daily-puzzle widget uses the `org.lichess.mobile://training/daily`
|
||||
// custom scheme (optionally suffixed with `/{puzzleId}`) so the Flutter app
|
||||
// always opens the native "Daily Puzzle" screen for the tapped puzzle.
|
||||
|
||||
private let kScheme = "org.lichess.mobile"
|
||||
|
||||
/// Builds `org.lichess.mobile://training/daily[/{puzzleId}]` deeplinks handled
|
||||
/// by the Flutter app to open the native daily-puzzle screen. See `AppLinksService`.
|
||||
func dailyPuzzleDeeplink(puzzleId: String?) -> URL? {
|
||||
if let puzzleId {
|
||||
return URL(string: "\(kScheme)://training/daily/\(puzzleId)")
|
||||
}
|
||||
return URL(string: "\(kScheme)://training/daily")
|
||||
}
|
||||
|
||||
extension String {
|
||||
/// Encodes the string into the custom scheme the app listens for to open it in the in-app browser.
|
||||
var lichessWebURL: URL? {
|
||||
guard let encoded = addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) else { return nil }
|
||||
return URL(string: "org.lichess.mobile://open-web?url=\(encoded)")
|
||||
return URL(string: "\(kScheme)://open-web?url=\(encoded)")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,7 +13,10 @@ import 'package:lichess_mobile/src/model/challenge/challenge_repository.dart';
|
||||
import 'package:lichess_mobile/src/model/challenge/challenge_service.dart';
|
||||
import 'package:lichess_mobile/src/model/common/id.dart';
|
||||
import 'package:lichess_mobile/src/model/game/game_repository.dart';
|
||||
import 'package:lichess_mobile/src/model/puzzle/puzzle.dart';
|
||||
import 'package:lichess_mobile/src/model/puzzle/puzzle_angle.dart';
|
||||
import 'package:lichess_mobile/src/model/puzzle/puzzle_providers.dart';
|
||||
import 'package:lichess_mobile/src/model/puzzle/puzzle_theme.dart';
|
||||
import 'package:lichess_mobile/src/model/user/user.dart';
|
||||
import 'package:lichess_mobile/src/tab_scaffold.dart';
|
||||
import 'package:lichess_mobile/src/view/analysis/analysis_screen.dart';
|
||||
@@ -31,6 +34,11 @@ import 'package:url_launcher/url_launcher.dart';
|
||||
|
||||
final _logger = Logger('AppLinks');
|
||||
|
||||
// Deeplink host/path for the iOS daily-puzzle widget tap.
|
||||
// Must stay in sync with Deeplinks.swift in the iOS widget extension.
|
||||
const _kDailyPuzzleDeeplinkHost = 'training';
|
||||
const _kDailyPuzzleDeeplinkPath = 'daily';
|
||||
|
||||
final appLinksServiceProvider = Provider<AppLinksService>((ref) {
|
||||
final service = AppLinksService(ref);
|
||||
ref.onDispose(() => service.dispose());
|
||||
@@ -61,6 +69,14 @@ class AppLinksService {
|
||||
return;
|
||||
}
|
||||
final context = ref.read(currentNavigatorKeyProvider).currentContext;
|
||||
if (uri.scheme == kLichessUriScheme &&
|
||||
uri.host == _kDailyPuzzleDeeplinkHost &&
|
||||
uri.pathSegments.firstOrNull == _kDailyPuzzleDeeplinkPath) {
|
||||
if (context != null && context.mounted) {
|
||||
await handleDailyPuzzleLink(context, uri.pathSegments.elementAtOrNull(1));
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (context != null && context.mounted) {
|
||||
await handleAppLink(context, uri);
|
||||
}
|
||||
@@ -150,6 +166,48 @@ class AppLinksService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Opens the native daily-puzzle screen (same path as tapping the daily-puzzle
|
||||
/// card on the puzzle tab) in response to `org.lichess.mobile://training/daily`
|
||||
/// or `org.lichess.mobile://training/daily/{id}` deeplinks emitted by the iOS
|
||||
/// home-screen widget.
|
||||
///
|
||||
/// Always fetches the current daily puzzle first (cached, so no extra request
|
||||
/// in the common case). When [puzzleId] matches today's daily, it is used
|
||||
/// directly. When it differs (widget cached a stale id), that specific puzzle
|
||||
/// is fetched but NOT flagged as the daily so the user isn't confused when
|
||||
/// navigating back to the puzzle tab.
|
||||
@visibleForTesting
|
||||
Future<void> handleDailyPuzzleLink(BuildContext context, String? puzzleId) async {
|
||||
try {
|
||||
Puzzle puzzle;
|
||||
final dailyPuzzle = await ref.read(dailyPuzzleProvider.future);
|
||||
if (puzzleId == null || dailyPuzzle.puzzle.id == PuzzleId(puzzleId)) {
|
||||
puzzle = dailyPuzzle;
|
||||
} else {
|
||||
// Widget cached a different puzzle than today's daily — fetch it, but
|
||||
// don't mark as daily to avoid confusing the user.
|
||||
try {
|
||||
puzzle = await ref.read(puzzleProvider(PuzzleId(puzzleId)).future);
|
||||
} catch (e, st) {
|
||||
// Fall back to the current daily puzzle rather than leaving the tap
|
||||
// as a no-op when the widget's cached id is stale or unreachable.
|
||||
_logger.info('Failed to load widget puzzle id $puzzleId, falling back: $e', e, st);
|
||||
puzzle = dailyPuzzle;
|
||||
}
|
||||
}
|
||||
if (!context.mounted) return;
|
||||
await Navigator.of(context, rootNavigator: true).push(
|
||||
PuzzleScreen.buildRoute(
|
||||
context,
|
||||
angle: const PuzzleTheme(PuzzleThemeKey.mix),
|
||||
puzzle: puzzle,
|
||||
),
|
||||
);
|
||||
} catch (e, st) {
|
||||
_logger.severe('Failed to open daily puzzle from widget: $e\n$st');
|
||||
}
|
||||
}
|
||||
|
||||
Future<bool> _tryResolveChallengeLink(BuildContext context, Uri appLinkUri) async {
|
||||
try {
|
||||
final challengeId = ChallengeId(appLinkUri.pathSegments[0]);
|
||||
|
||||
@@ -3,6 +3,8 @@ import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:flutter_riverpod/misc.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart';
|
||||
import 'package:lichess_mobile/src/app_links_service.dart';
|
||||
import 'package:lichess_mobile/src/model/challenge/challenge.dart';
|
||||
import 'package:lichess_mobile/src/model/challenge/challenge_repository.dart';
|
||||
@@ -14,6 +16,7 @@ import 'package:lichess_mobile/src/model/game/game_repository.dart';
|
||||
import 'package:lichess_mobile/src/model/game/game_status.dart';
|
||||
import 'package:lichess_mobile/src/model/game/player.dart';
|
||||
import 'package:lichess_mobile/src/model/user/user.dart';
|
||||
import 'package:lichess_mobile/src/network/http.dart';
|
||||
import 'package:lichess_mobile/src/view/analysis/analysis_screen.dart';
|
||||
import 'package:lichess_mobile/src/view/broadcast/broadcast_game_screen.dart';
|
||||
import 'package:lichess_mobile/src/view/broadcast/broadcast_player_results_screen.dart';
|
||||
@@ -26,13 +29,38 @@ import 'package:mocktail/mocktail.dart';
|
||||
|
||||
import 'example_data.dart';
|
||||
import 'model/game/game_socket_example_data.dart';
|
||||
import 'model/puzzle/mock_server_responses.dart';
|
||||
import 'network/fake_http_client_factory.dart';
|
||||
import 'network/fake_websocket_channel.dart';
|
||||
import 'test_provider_scope.dart';
|
||||
|
||||
// Mock response for a cached widget puzzle with a different id than the daily.
|
||||
// Built from the daily mock data to share the same valid game/pgn structure.
|
||||
final _mockStalePuzzleJson = mockDailyPuzzleResponse.trim().replaceFirst(
|
||||
'"id":"0XqV2"',
|
||||
'"id":"stale1"',
|
||||
);
|
||||
|
||||
class MockGameRepository extends Mock implements GameRepository {}
|
||||
|
||||
class MockChallengeRepository extends Mock implements ChallengeRepository {}
|
||||
|
||||
class _DailyPuzzleLinkTestWidget extends ConsumerWidget {
|
||||
const _DailyPuzzleLinkTestWidget({this.puzzleId});
|
||||
|
||||
final String? puzzleId;
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context, WidgetRef ref) {
|
||||
return ElevatedButton(
|
||||
onPressed: () async {
|
||||
await ref.read(appLinksServiceProvider).handleDailyPuzzleLink(context, puzzleId);
|
||||
},
|
||||
child: const Text('test daily link'),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
class _TestWidget extends ConsumerWidget {
|
||||
const _TestWidget({required this.uri});
|
||||
|
||||
@@ -64,7 +92,108 @@ Future<void> triggerAppLink(
|
||||
await tester.tap(find.text('test link'));
|
||||
}
|
||||
|
||||
Future<void> triggerDailyPuzzleLink(
|
||||
WidgetTester tester,
|
||||
String? puzzleId, {
|
||||
Map<ProviderOrFamily, Override>? overrides,
|
||||
}) async {
|
||||
final app = await makeTestProviderScopeApp(
|
||||
tester,
|
||||
overrides: overrides,
|
||||
home: _DailyPuzzleLinkTestWidget(puzzleId: puzzleId),
|
||||
);
|
||||
await tester.pumpWidget(app);
|
||||
await tester.tap(find.text('test daily link'));
|
||||
}
|
||||
|
||||
void main() {
|
||||
group('handleDailyPuzzleLink', () {
|
||||
// Builds an httpClientFactoryProvider override whose mock client routes
|
||||
// puzzle API endpoints to controlled responses.
|
||||
Override puzzleHttpOverride({bool failStaleFetch = false}) {
|
||||
return httpClientFactoryProvider.overrideWith((ref) {
|
||||
return FakeHttpClientFactory(
|
||||
() => MockClient((request) async {
|
||||
if (request.url.path == '/api/puzzle/daily') {
|
||||
return http.Response(mockDailyPuzzleResponse.trim(), 200);
|
||||
}
|
||||
if (request.url.path == '/api/puzzle/stale1') {
|
||||
if (failStaleFetch) return http.Response('Server error', 500);
|
||||
return http.Response(_mockStalePuzzleJson, 200);
|
||||
}
|
||||
return http.Response('', 200);
|
||||
}),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
testWidgets("no puzzle id: opens today's daily puzzle", (tester) async {
|
||||
await triggerDailyPuzzleLink(
|
||||
tester,
|
||||
null,
|
||||
overrides: {httpClientFactoryProvider: puzzleHttpOverride()},
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget(find.byType(PuzzleScreen)),
|
||||
isA<PuzzleScreen>()
|
||||
.having((s) => s.puzzle?.puzzle.id, 'puzzle id', const PuzzleId('0XqV2'))
|
||||
.having((s) => s.puzzle?.isDailyPuzzle, 'is daily', true),
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('puzzle id matches daily: opens daily puzzle without extra fetch', (tester) async {
|
||||
// PuzzleRepository.fetch() does not set isDailyPuzzle, so if puzzleProvider
|
||||
// were called (wrongly) the assertion on isDailyPuzzle: true would fail.
|
||||
await triggerDailyPuzzleLink(
|
||||
tester,
|
||||
'0XqV2', // same id as the daily puzzle
|
||||
overrides: {httpClientFactoryProvider: puzzleHttpOverride()},
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget(find.byType(PuzzleScreen)),
|
||||
isA<PuzzleScreen>()
|
||||
.having((s) => s.puzzle?.puzzle.id, 'puzzle id', const PuzzleId('0XqV2'))
|
||||
.having((s) => s.puzzle?.isDailyPuzzle, 'is daily', true),
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('puzzle id differs from daily: opens specific puzzle not flagged as daily', (
|
||||
tester,
|
||||
) async {
|
||||
await triggerDailyPuzzleLink(
|
||||
tester,
|
||||
'stale1',
|
||||
overrides: {httpClientFactoryProvider: puzzleHttpOverride()},
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget(find.byType(PuzzleScreen)),
|
||||
isA<PuzzleScreen>()
|
||||
.having((s) => s.puzzle?.puzzle.id, 'puzzle id', const PuzzleId('stale1'))
|
||||
.having((s) => s.puzzle?.isDailyPuzzle, 'is daily', isNot(true)),
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('puzzle id differs from daily and fetch fails: falls back to daily puzzle', (
|
||||
tester,
|
||||
) async {
|
||||
await triggerDailyPuzzleLink(
|
||||
tester,
|
||||
'stale1',
|
||||
overrides: {httpClientFactoryProvider: puzzleHttpOverride(failStaleFetch: true)},
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget(find.byType(PuzzleScreen)),
|
||||
isA<PuzzleScreen>()
|
||||
.having((s) => s.puzzle?.puzzle.id, 'puzzle id', const PuzzleId('0XqV2'))
|
||||
.having((s) => s.puzzle?.isDailyPuzzle, 'is daily', true),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('resolveAppLinkUri', () {
|
||||
testWidgets('Nothing happens for an empty path', (WidgetTester tester) async {
|
||||
final uri = Uri.parse('https://lichess.org/');
|
||||
|
||||
Reference in New Issue
Block a user