From c330b147aaa945be636f73cf42f53eb9bb5cef02 Mon Sep 17 00:00:00 2001 From: Eli <88557639+lishaduck@users.noreply.github.com> Date: Tue, 2 Jan 2024 17:17:27 -0600 Subject: [PATCH 1/2] refactor: login page "pops" --- lib/app/app_router.dart | 31 +++++++++++++------ .../presentation/auth_page/auth_page.dart | 17 ++++++++-- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/lib/app/app_router.dart b/lib/app/app_router.dart index 2d0b7595..064c8504 100644 --- a/lib/app/app_router.dart +++ b/lib/app/app_router.dart @@ -16,7 +16,7 @@ part "app_router.gr.dart"; /// The router for the application. @AutoRouterConfig(replaceInRouteName: "Page,Route") -class AppRouter extends _$AppRouter { +class AppRouter extends _$AppRouter implements AutoRouteGuard { /// Create a new instance of [AppRouter]. AppRouter({required this.ref}); @@ -28,20 +28,31 @@ class AppRouter extends _$AppRouter { transitionsBuilder: TransitionsBuilders.slideLeftWithFade, ); + @override + Future onNavigation( + NavigationResolver resolver, + StackRouter router, + ) async { + final authState = ref.read(userProvider).valueOrNull; + + if (authState != null || resolver.route.name == AuthRoute.name) { + resolver.next(); // continue navigation + } else { + // else we navigate to the Login page so we get authenticated + + // tip: use resolver.redirect to have the redirected route + // automatically removed from the stack when the resolver is completed + await resolver.redirect(const AuthRoute()).then( + (didLogin) => resolver.next((didLogin ?? false) as bool), + ); + } + } + @override List get routes => [ AutoRoute( page: WrapperRoute.page, path: "/", - guards: [ - AutoRouteGuard.redirect( - (resolver) { - final authState = ref.read(userProvider).valueOrNull; - - return (authState != null) ? null : const AuthRoute(); - }, - ), - ], children: [ AutoRoute( page: PirateCoinsRoute.page, diff --git a/lib/features/auth/presentation/auth_page/auth_page.dart b/lib/features/auth/presentation/auth_page/auth_page.dart index c8ca28c3..b62cbf10 100644 --- a/lib/features/auth/presentation/auth_page/auth_page.dart +++ b/lib/features/auth/presentation/auth_page/auth_page.dart @@ -11,7 +11,7 @@ import "../../../../l10n/l10n.dart"; import "../../application/auth_service.dart"; /// The page located at `/login/`. -@RoutePage() +@RoutePage() class AuthPage extends ConsumerWidget { /// Create a new instance of [AuthPage]. const AuthPage({super.key}); @@ -38,7 +38,7 @@ class AuthPage extends ConsumerWidget { .authenticate(); if (context.mounted) { - await context.router.push(const DashboardRoute()); + await context.go(); } }, icon: const Icon(Icons.g_mobiledata), @@ -52,7 +52,7 @@ class AuthPage extends ConsumerWidget { .anonymous(); if (context.mounted) { - await context.router.push(const DashboardRoute()); + await context.go(); } }, icon: const Icon(Icons.person), @@ -67,3 +67,14 @@ class AuthPage extends ConsumerWidget { ); } } + +extension _Go on BuildContext { + Future go() async { + // if we were redirected here, go back to the right page. + await router.pop(true); + if (mounted) { + // otherwise, go to the dashboard. + await router.push(const DashboardRoute()); + } + } +} From b65112867b7ecc14a4aff3f47ceed6848e85d62f Mon Sep 17 00:00:00 2001 From: Eli <88557639+lishaduck@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:58:15 -0500 Subject: [PATCH 2/2] wip: --- lib/app/app_router.dart | 4 +- .../auth/application/auth_service.dart | 29 +++- lib/features/auth/data/auth_repository.dart | 137 +++++++++--------- .../auth/domain/pirate_auth_model.dart | 2 +- .../auth/domain/pirate_user_entity.dart | 3 + test/app/app_test.dart | 1 - .../wrapper_page/wrapper_page_test.dart | 1 - 7 files changed, 100 insertions(+), 77 deletions(-) diff --git a/lib/app/app_router.dart b/lib/app/app_router.dart index 064c8504..9b0847a1 100644 --- a/lib/app/app_router.dart +++ b/lib/app/app_router.dart @@ -33,9 +33,9 @@ class AppRouter extends _$AppRouter implements AutoRouteGuard { NavigationResolver resolver, StackRouter router, ) async { - final authState = ref.read(userProvider).valueOrNull; + final authState = ref.read(userProvider).valueOrNull?.isLoggedIn; - if (authState != null || resolver.route.name == AuthRoute.name) { + if ((authState ?? false) || (resolver.route.name == AuthRoute.name)) { resolver.next(); // continue navigation } else { // else we navigate to the Login page so we get authenticated diff --git a/lib/features/auth/application/auth_service.dart b/lib/features/auth/application/auth_service.dart index 6f8355d3..b45fa2ee 100644 --- a/lib/features/auth/application/auth_service.dart +++ b/lib/features/auth/application/auth_service.dart @@ -17,7 +17,7 @@ part "auth_service.g.dart"; base class PirateAuthService extends _$PirateAuthService { @override FutureOr build() async { - return _createSession(anonymous: true); + return _fetchSession(); } /// Authenticate the current user. @@ -27,13 +27,25 @@ base class PirateAuthService extends _$PirateAuthService { Future _createSession({bool anonymous = false}) async { final auth = ref.read(authProvider); - final account = await auth.authenticate(anonymous: anonymous); + await auth.authenticate(anonymous: anonymous); + final account = await auth.getData(); return PirateAuthModel( user: account, ); } + Future _fetchSession() async { + final auth = ref.read(authProvider); + try { + final account = await auth.getData(); + + return PirateAuthModel(user: account); + } catch (e) { + return const PirateAuthModel(user: null); + } + } + /// Create a new anonymous session for the user. Future anonymous() async { state = await AsyncValue.guard(() => _createSession(anonymous: true)); @@ -45,9 +57,20 @@ base class PirateAuthService extends _$PirateAuthService { /// Use [pirateAuthServiceProvider] for more granular output. @Riverpod(keepAlive: true) Future user(UserRef ref) async => await ref.watch( - pirateAuthServiceProvider.selectAsync((value) => value.user), + pirateAuthServiceProvider.selectAsync((value) { + return value.user ?? fakeUser; + }), ); +/// A fake user, for use when all else fails. +final fakeUser = PirateUserEntity( + name: redactedName, + email: redactedEmail, + accountType: AccountType.student, + avatar: redactedAvatar, + isLoggedIn: false, +); + /// Get the current user's name. /// /// Named as such to prevent a naming conflict with riverpod. diff --git a/lib/features/auth/data/auth_repository.dart b/lib/features/auth/data/auth_repository.dart index 1565e2be..47f07142 100644 --- a/lib/features/auth/data/auth_repository.dart +++ b/lib/features/auth/data/auth_repository.dart @@ -20,86 +20,93 @@ part "auth_repository.g.dart"; /// A repository for authentication. abstract interface class AuthRepository { /// Authenticate the user. - Future authenticate({required bool anonymous}); + Future authenticate({required bool anonymous}); + + /// Get data about the current user. + Future getData(); } /// The default implementation of [AuthRepository]. base class _AppwriteAuthRepository implements AuthRepository { /// Create a new instance of [_AppwriteAuthRepository]. - const _AppwriteAuthRepository( - Account account, - Device platform, - AvatarRepository avatar, - ) : _account = account, - _platform = platform, - _avatarRepo = avatar; + const _AppwriteAuthRepository(this.account, this.platform, this.avatarRepo); /// The Appwrite [Account]. - final Account _account; + final Account account; /// The [currentPlatform]. - final Device _platform; + final Device platform; /// The current user's avatar. - final AvatarRepository _avatarRepo; + final AvatarRepository avatarRepo; + + /// Get a user from Appwrite. + Future getUser() => account.get(); @override - Future authenticate({bool anonymous = false}) async { - User account; + Future getData() async { + final user = await getUser(); + + return getUserData(user); + } + + Future getUserData(User user) async { + final accountType = AccountType.fromEmail(user.email); + final avatar = await avatarRepo.getAvatar(); + + return PirateUserEntity( + name: user.name, + email: user.email, + accountType: accountType, + avatar: avatar, + isLoggedIn: true, + ); + } + + @override + Future authenticate({bool anonymous = false}) async { try { - account = await _account.get(); + await getUser(); } catch (e, s) { log.warning("Failed to fetch session.", e, s); - if (anonymous) { - try { - await _account.createAnonymousSession(); - } catch (e, s) { - log.warning("Failed to create anonymous session.", e, s); - } - } else { - try { - // Go to the Google account login page. - switch (_platform) { - // Both Android and iOS need the same behavior, so it reuses it. - case Device.android || Device.ios: - await _account.createOAuth2Session( - provider: "google", - ); - - // TODO(lishaduck): The web needs different behavior than that of linux/mac/windows/fuchsia. - case Device.web || - Device.linux || - Device.macos || - Device.windows || - Device.other: - await _account.createOAuth2Session( - provider: "google", - success: "${Uri.base.origin}/auth.html", - failure: "${Uri.base}", - ); - } - } catch (e, s) { - log.warning("Failed to create OAuth2 session.", e, s); - } - } + await _createAccount(anonymous); - account = await _account.get(); + await getUser(); } + } - try { - final accountType = AccountType.fromEmail(account.email); - final avatar = await _avatarRepo.getAvatar(); - - return PirateUserEntity( - name: account.name, - email: account.email, - accountType: accountType, - avatar: avatar, - ); - } catch (e) { - log.warning("Failed to fetch user data.", e); - - return fakeUser; + Future _createAccount(bool anonymous) async { + if (anonymous) { + try { + await account.createAnonymousSession(); + } catch (e, s) { + log.warning("Failed to create anonymous session.", e, s); + } + } else { + try { + // Go to the Google account login page. + switch (platform) { + // Both Android and iOS need the same behavior, so it reuses it. + case Device.android || Device.ios: + await account.createOAuth2Session( + provider: "google", + ); + + // TODO(lishaduck): The web needs different behavior than that of linux/mac/windows/fuchsia. + case Device.web || + Device.linux || + Device.macos || + Device.windows || + Device.other: + await account.createOAuth2Session( + provider: "google", + success: "${Uri.base.origin}/auth.html", + failure: "${Uri.base}", + ); + } + } catch (e, s) { + log.warning("Failed to create OAuth2 session.", e, s); + } } } } @@ -113,14 +120,6 @@ const redactedName = "Anonymous"; /// The avatar used in case things go wrong. final redactedAvatar = Uint8List(1); -/// A fake user, for use when all else fails. -final fakeUser = PirateUserEntity( - name: redactedName, - email: redactedEmail, - accountType: AccountType.student, - avatar: redactedAvatar, -); - /// Get the authentication data provider. @Riverpod(keepAlive: true) AuthRepository auth(AuthRef ref) { diff --git a/lib/features/auth/domain/pirate_auth_model.dart b/lib/features/auth/domain/pirate_auth_model.dart index 6c620949..fa0eccd3 100644 --- a/lib/features/auth/domain/pirate_auth_model.dart +++ b/lib/features/auth/domain/pirate_auth_model.dart @@ -17,7 +17,7 @@ sealed class PirateAuthModel with _$PirateAuthModel implements Model { /// Create a new, immutable instance of [PirateAuthModel]. const factory PirateAuthModel({ /// The user. - required PirateUserEntity user, + required PirateUserEntity? user, }) = _PirateAuthModel; /// Convert a JSON [Map] into a new, immutable instance of [PirateAuthModel]. diff --git a/lib/features/auth/domain/pirate_user_entity.dart b/lib/features/auth/domain/pirate_user_entity.dart index 8a1b9749..a8365ee3 100644 --- a/lib/features/auth/domain/pirate_user_entity.dart +++ b/lib/features/auth/domain/pirate_user_entity.dart @@ -25,6 +25,9 @@ sealed class PirateUserEntity with _$PirateUserEntity implements Model { /// The user's avatar. @Uint8ListConverter() required Uint8List avatar, + + /// If the user has logged in through Appwrite. + required bool isLoggedIn, }) = _PirateUser; const PirateUserEntity._(); diff --git a/test/app/app_test.dart b/test/app/app_test.dart index 481f5c4c..4490d35e 100644 --- a/test/app/app_test.dart +++ b/test/app/app_test.dart @@ -1,4 +1,3 @@ -// ignore_for_file: scoped_providers_should_specify_dependencies, prefer_const_constructors, prefer_const_literals_to_create_immutables import "package:appwrite/appwrite.dart"; import "package:flutter/material.dart"; import "package:flutter_test/flutter_test.dart"; diff --git a/test/features/dashboard/presentation/wrapper_page/wrapper_page_test.dart b/test/features/dashboard/presentation/wrapper_page/wrapper_page_test.dart index f720f68d..8bcc500a 100644 --- a/test/features/dashboard/presentation/wrapper_page/wrapper_page_test.dart +++ b/test/features/dashboard/presentation/wrapper_page/wrapper_page_test.dart @@ -5,7 +5,6 @@ import "package:hooks_riverpod/hooks_riverpod.dart"; import "package:pirate_code/app/app.dart"; import "package:pirate_code/app/app_router.dart"; import "package:pirate_code/features/auth/application/auth_service.dart"; -import "package:pirate_code/features/auth/data/auth_repository.dart"; import "package:pirate_code/features/dashboard/presentation/wrapper_page/wrapper_page.dart"; import "package:pirate_code/l10n/l10n.dart"; import "package:pirate_code/utils/design.dart";