From e4b550ab527a9e87e49514896826933108d7e3f2 Mon Sep 17 00:00:00 2001 From: Roland Geider Date: Fri, 28 Mar 2025 17:21:31 +0100 Subject: [PATCH 1/4] Refactor app version handling and update authentication flow Previously, this was only triggered when logging in to the application. If a user just opened the app, it would just stop working. We also now always check this min version and have removed the option from the android manifest file since disabling this doesn't make much sense and we have many other platforms as well (iOS, flatpak) --- android/app/src/main/AndroidManifest.xml | 4 - lib/helpers/consts.dart | 3 - lib/main.dart | 33 ++++--- lib/providers/auth.dart | 93 +++++++++++++------ lib/screens/auth_screen.dart | 7 -- lib/screens/home_tabs_screen.dart | 53 ++++++----- lib/screens/update_app_screen.dart | 5 +- test/auth/auth_provider_test.dart | 12 +-- test/auth/auth_screen_test.dart | 2 +- .../nutritional_plan_screen_test.mocks.dart | 27 ++++-- .../nutritional_plans_screen_test.mocks.dart | 27 ++++-- test/utils.dart | 2 +- 12 files changed, 161 insertions(+), 107 deletions(-) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 1e198ad9..4e50bad4 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -29,10 +29,6 @@ android:enableOnBackInvokedCallback="true" android:networkSecurityConfig="@xml/network_security_config"> - - + authResultSnapshot.connectionState == ConnectionState.waiting + ? const SplashScreen() + : const AuthScreen(), + ); + } + } @override Widget build(BuildContext context) { @@ -157,15 +174,7 @@ class MyApp extends StatelessWidget { highContrastTheme: wgerLightThemeHc, highContrastDarkTheme: wgerDarkThemeHc, themeMode: user.themeMode, - home: auth.isAuth - ? HomeTabsScreen() - : FutureBuilder( - future: auth.tryAutoLogin(), - builder: (ctx, authResultSnapshot) => - authResultSnapshot.connectionState == ConnectionState.waiting - ? const SplashScreen() - : const AuthScreen(), - ), + home: _getHomeScreen(auth), routes: { DashboardScreen.routeName: (ctx) => const DashboardScreen(), FormScreen.routeName: (ctx) => const FormScreen(), diff --git a/lib/providers/auth.dart b/lib/providers/auth.dart index f4ca6f39..a419d25d 100644 --- a/lib/providers/auth.dart +++ b/lib/providers/auth.dart @@ -38,6 +38,12 @@ enum LoginActions { proceed, } +enum AuthState { + updateRequired, + loggedIn, + loggedOut, +} + class AuthProvider with ChangeNotifier { final _logger = Logger('AuthProvider'); @@ -46,6 +52,7 @@ class AuthProvider with ChangeNotifier { String? serverVersion; PackageInfo? applicationVersion; Map metadata = {}; + AuthState state = AuthState.loggedOut; static const MIN_APP_VERSION_URL = 'min-app-version'; static const SERVER_VERSION_URL = 'version'; @@ -54,7 +61,7 @@ class AuthProvider with ChangeNotifier { late http.Client client; - AuthProvider([http.Client? client, bool? checkMetadata]) { + AuthProvider([http.Client? client]) { this.client = client ?? http.Client(); } @@ -83,23 +90,16 @@ class AuthProvider with ChangeNotifier { } /// Checking if there is a new version of the application. - Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) async { - metadata ??= this.metadata; - - if (!metadata.containsKey(MANIFEST_KEY_CHECK_UPDATE) || - metadata[MANIFEST_KEY_CHECK_UPDATE] == 'false') { - return false; - } - + Future applicationUpdateRequired([String? version]) async { final applicationCurrentVersion = version ?? applicationVersion!.version; final response = await client.get(makeUri(serverUrl!, MIN_APP_VERSION_URL)); final currentVersion = Version.parse(applicationCurrentVersion); final requiredAppVersion = Version.parse(jsonDecode(response.body)); - return requiredAppVersion > currentVersion; + final result = requiredAppVersion > currentVersion; + _logger.fine('Application update required: $result'); + + return result; } /// Registers a new user @@ -160,15 +160,13 @@ class AuthProvider with ChangeNotifier { await initVersions(serverUrl); // If update is required don't log in user - if (await applicationUpdateRequired( - applicationVersion!.version, - {MANIFEST_KEY_CHECK_UPDATE: 'true'}, - )) { + if (await applicationUpdateRequired()) { return {'action': LoginActions.update}; } // Log user in token = responseData['token']; + state = AuthState.loggedIn; notifyListeners(); // store login data in shared preferences @@ -195,23 +193,58 @@ class AuthProvider with ChangeNotifier { return userData['serverUrl'] as String; } - Future tryAutoLogin() async { + /// Tries to auto-login the user with the stored token + Future tryAutoLogin() async { final prefs = await SharedPreferences.getInstance(); if (!prefs.containsKey(PREFS_USER)) { - _logger.info('autologin failed'); - return false; + _logger.info('autologin failed, no saved user data'); + state = AuthState.loggedOut; + return; } - final extractedUserData = json.decode(prefs.getString(PREFS_USER)!); - token = extractedUserData['token']; - serverUrl = extractedUserData['serverUrl']; + final userData = json.decode(prefs.getString(PREFS_USER)!); + + if (!userData.containsKey('token') || !userData.containsKey('serverUrl')) { + _logger.info('autologin failed, no token or serverUrl'); + state = AuthState.loggedOut; + return; + } + + token = userData['token']; + serverUrl = userData['serverUrl']; + + if (token == null || serverUrl == null) { + _logger.info('autologin failed, token or serverUrl is null'); + state = AuthState.loggedOut; + return; + } + + // // Try to talk to a URL using the token, if this doesn't work, log out + final response = await client.head( + makeUri(serverUrl!, 'routine'), + headers: { + HttpHeaders.contentTypeHeader: 'application/json; charset=UTF-8', + HttpHeaders.userAgentHeader: getAppNameHeader(), + HttpHeaders.authorizationHeader: 'Token $token' + }, + ); + if (response.statusCode != 200) { + _logger.info('autologin failed, statusCode: ${response.statusCode}'); + await logout(); + return; + } + + await initVersions(serverUrl!); + + // If update is required don't log in user + if (await applicationUpdateRequired()) { + state = AuthState.updateRequired; + } else { + state = AuthState.loggedIn; + _logger.info('autologin successful'); + } - _logger.info('autologin successful'); - setApplicationVersion(); - setServerVersion(); notifyListeners(); - //_autoLogout(); - return true; } Future logout({bool shouldNotify = true}) async { @@ -219,6 +252,7 @@ class AuthProvider with ChangeNotifier { token = null; serverUrl = null; dataInit = false; + state = AuthState.loggedOut; if (shouldNotify) { notifyListeners(); @@ -236,7 +270,8 @@ class AuthProvider with ChangeNotifier { if (applicationVersion != null) { out = '/${applicationVersion!.version} ' '(${applicationVersion!.packageName}; ' - 'build: ${applicationVersion!.buildNumber})'; + 'build: ${applicationVersion!.buildNumber})' + ' - https://github.com/wger-project'; } return 'wger App$out'; } diff --git a/lib/screens/auth_screen.dart b/lib/screens/auth_screen.dart index c4c73eca..085f29e2 100644 --- a/lib/screens/auth_screen.dart +++ b/lib/screens/auth_screen.dart @@ -89,12 +89,6 @@ class AuthScreen extends StatelessWidget { ), ), ), - // Positioned( - // top: 0.4 * deviceSize.height, - // left: 15, - // right: 15, - // child: const , - // ), ], ), ); @@ -166,7 +160,6 @@ class _AuthCardState extends State { void _submit(BuildContext context) async { if (!_formKey.currentState!.validate()) { - // Invalid! return; } _formKey.currentState!.save(); diff --git a/lib/screens/home_tabs_screen.dart b/lib/screens/home_tabs_screen.dart index b038a942..22ac70c5 100644 --- a/lib/screens/home_tabs_screen.dart +++ b/lib/screens/home_tabs_screen.dart @@ -146,27 +146,8 @@ class _HomeTabsScreenState extends State with SingleTickerProvid future: _initialData, builder: (context, snapshot) { if (snapshot.connectionState != ConnectionState.done) { - return Scaffold( - body: Center( - child: Column( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - const Center( - child: SizedBox( - height: 70, - child: RiveAnimation.asset( - 'assets/animations/wger_logo.riv', - animations: ['idle_loop2'], - ), - ), - ), - Text( - AppLocalizations.of(context).loadingText, - style: Theme.of(context).textTheme.headlineSmall, - ), - ], - ), - ), + return const Scaffold( + body: LoadingWidget(), ); } else { return Scaffold( @@ -204,3 +185,33 @@ class _HomeTabsScreenState extends State with SingleTickerProvid ); } } + +class LoadingWidget extends StatelessWidget { + const LoadingWidget({ + super.key, + }); + + @override + Widget build(BuildContext context) { + return Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const Center( + child: SizedBox( + height: 70, + child: RiveAnimation.asset( + 'assets/animations/wger_logo.riv', + animations: ['idle_loop2'], + ), + ), + ), + Text( + AppLocalizations.of(context).loadingText, + style: Theme.of(context).textTheme.headlineSmall, + ), + ], + ), + ); + } +} diff --git a/lib/screens/update_app_screen.dart b/lib/screens/update_app_screen.dart index 8b1996c7..2b01eeec 100644 --- a/lib/screens/update_app_screen.dart +++ b/lib/screens/update_app_screen.dart @@ -30,10 +30,7 @@ class UpdateAppScreen extends StatelessWidget { AppLocalizations.of(context).appUpdateTitle, style: Theme.of(context).textTheme.headlineSmall, ), - content: Column( - mainAxisSize: MainAxisSize.min, - children: [Text(AppLocalizations.of(context).appUpdateContent)], - ), + content: Text(AppLocalizations.of(context).appUpdateContent), actions: null, ), ); diff --git a/test/auth/auth_provider_test.dart b/test/auth/auth_provider_test.dart index 8c7d9db4..cea67ef8 100644 --- a/test/auth/auth_provider_test.dart +++ b/test/auth/auth_provider_test.dart @@ -15,11 +15,9 @@ void main() { path: 'api/v2/min-app-version/', ); - final testMetadata = {'wger.check_min_app_version': 'true'}; - setUp(() { mockClient = MockClient(); - authProvider = AuthProvider(mockClient, false); + authProvider = AuthProvider(mockClient); authProvider.serverUrl = 'http://localhost'; }); @@ -27,7 +25,7 @@ void main() { test('app version higher than min version', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.2.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0'); // assert expect(updateNeeded, false); @@ -36,7 +34,7 @@ void main() { test('app version higher than min version - 1', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.1', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.1'); // assert expect(updateNeeded, true); @@ -45,7 +43,7 @@ void main() { test('app version higher than min version - 2', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.1', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.1'); // assert expect(updateNeeded, true); @@ -54,7 +52,7 @@ void main() { test('app version equal as min version', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0'); // assert expect(updateNeeded, false); diff --git a/test/auth/auth_screen_test.dart b/test/auth/auth_screen_test.dart index 953fd535..ece5cfaa 100644 --- a/test/auth/auth_screen_test.dart +++ b/test/auth/auth_screen_test.dart @@ -73,7 +73,7 @@ void main() { setUp(() { mockClient = MockClient(); - authProvider = AuthProvider(mockClient, false); + authProvider = AuthProvider(mockClient); authProvider.serverUrl = 'https://wger.de'; SharedPreferences.setMockInitialValues({}); diff --git a/test/nutrition/nutritional_plan_screen_test.mocks.dart b/test/nutrition/nutritional_plan_screen_test.mocks.dart index 09ca8fcb..694d7827 100644 --- a/test/nutrition/nutritional_plan_screen_test.mocks.dart +++ b/test/nutrition/nutritional_plan_screen_test.mocks.dart @@ -202,6 +202,18 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { returnValueForMissingStub: null, ); + @override + _i2.AuthState get state => (super.noSuchMethod( + Invocation.getter(#state), + returnValue: _i2.AuthState.updateRequired, + ) as _i2.AuthState); + + @override + set state(_i2.AuthState? _state) => super.noSuchMethod( + Invocation.setter(#state, _state), + returnValueForMissingStub: null, + ); + @override _i3.Client get client => (super.noSuchMethod( Invocation.getter(#client), @@ -253,12 +265,8 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { ) as _i5.Future); @override - _i5.Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) => - (super.noSuchMethod( - Invocation.method(#applicationUpdateRequired, [version, metadata]), + _i5.Future applicationUpdateRequired([String? version]) => (super.noSuchMethod( + Invocation.method(#applicationUpdateRequired, [version]), returnValue: _i5.Future.value(false), ) as _i5.Future); @@ -308,10 +316,11 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { ) as _i5.Future); @override - _i5.Future tryAutoLogin() => (super.noSuchMethod( + _i5.Future tryAutoLogin() => (super.noSuchMethod( Invocation.method(#tryAutoLogin, []), - returnValue: _i5.Future.value(false), - ) as _i5.Future); + returnValue: _i5.Future.value(), + returnValueForMissingStub: _i5.Future.value(), + ) as _i5.Future); @override _i5.Future logout({bool? shouldNotify = true}) => (super.noSuchMethod( diff --git a/test/nutrition/nutritional_plans_screen_test.mocks.dart b/test/nutrition/nutritional_plans_screen_test.mocks.dart index dff174c1..a135ff8e 100644 --- a/test/nutrition/nutritional_plans_screen_test.mocks.dart +++ b/test/nutrition/nutritional_plans_screen_test.mocks.dart @@ -94,6 +94,18 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { returnValueForMissingStub: null, ); + @override + _i3.AuthState get state => (super.noSuchMethod( + Invocation.getter(#state), + returnValue: _i3.AuthState.updateRequired, + ) as _i3.AuthState); + + @override + set state(_i3.AuthState? _state) => super.noSuchMethod( + Invocation.setter(#state, _state), + returnValueForMissingStub: null, + ); + @override _i2.Client get client => (super.noSuchMethod( Invocation.getter(#client), @@ -145,12 +157,8 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { ) as _i5.Future); @override - _i5.Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) => - (super.noSuchMethod( - Invocation.method(#applicationUpdateRequired, [version, metadata]), + _i5.Future applicationUpdateRequired([String? version]) => (super.noSuchMethod( + Invocation.method(#applicationUpdateRequired, [version]), returnValue: _i5.Future.value(false), ) as _i5.Future); @@ -200,10 +208,11 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { ) as _i5.Future); @override - _i5.Future tryAutoLogin() => (super.noSuchMethod( + _i5.Future tryAutoLogin() => (super.noSuchMethod( Invocation.method(#tryAutoLogin, []), - returnValue: _i5.Future.value(false), - ) as _i5.Future); + returnValue: _i5.Future.value(), + returnValueForMissingStub: _i5.Future.value(), + ) as _i5.Future); @override _i5.Future logout({bool? shouldNotify = true}) => (super.noSuchMethod( diff --git a/test/utils.dart b/test/utils.dart index 9244b4da..db590b38 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -23,7 +23,7 @@ import 'measurements/measurement_provider_test.mocks.dart'; import 'other/base_provider_test.mocks.dart'; // Test Auth provider -final AuthProvider testAuthProvider = AuthProvider(MockClient(), false) +final AuthProvider testAuthProvider = AuthProvider(MockClient()) ..token = 'FooBar' ..serverUrl = 'https://localhost'; From 6742c8091a3b6fd4f36591e93b827b08e0c64ca5 Mon Sep 17 00:00:00 2001 From: Roland Geider Date: Tue, 1 Apr 2025 21:39:45 +0200 Subject: [PATCH 2/4] Return the actions enum directly, no need for a dict here --- .../exercises/exercise_api.freezed.dart | 1193 ++++++++--------- lib/models/exercises/exercise_api.g.dart | 24 +- .../exercises/ingredient_api.freezed.dart | 694 +++++----- lib/models/exercises/ingredient_api.g.dart | 21 +- lib/providers/auth.dart | 16 +- lib/screens/auth_screen.dart | 14 +- .../nutritional_plan_screen_test.mocks.dart | 16 +- .../nutritional_plans_screen_test.mocks.dart | 16 +- 8 files changed, 955 insertions(+), 1039 deletions(-) diff --git a/lib/models/exercises/exercise_api.freezed.dart b/lib/models/exercises/exercise_api.freezed.dart index 650a6df8..711987ac 100644 --- a/lib/models/exercises/exercise_api.freezed.dart +++ b/lib/models/exercises/exercise_api.freezed.dart @@ -1,3 +1,4 @@ +// dart format width=80 // coverage:ignore-file // GENERATED CODE - DO NOT MODIFY BY HAND // ignore_for_file: type=lint @@ -9,180 +10,101 @@ part of 'exercise_api.dart'; // FreezedGenerator // ************************************************************************** +// dart format off T _$identity(T value) => value; - -final _privateConstructorUsedError = UnsupportedError( - 'It seems like you constructed your class using `MyClass._()`. This constructor is only meant to be used by freezed and you are not supposed to need it nor use it.\nPlease check the documentation here for more information: https://github.com/rrousselGit/freezed#adding-getters-and-methods-to-our-models'); - ExerciseApiData _$ExerciseApiDataFromJson(Map json) { return _ExerciseBaseData.fromJson(json); } /// @nodoc mixin _$ExerciseApiData { - int get id => throw _privateConstructorUsedError; - String get uuid => throw _privateConstructorUsedError; // ignore: invalid_annotation_target + int get id; + String get uuid; // ignore: invalid_annotation_target @JsonKey(name: 'variations') - int? get variationId => throw _privateConstructorUsedError; // ignore: invalid_annotation_target + int? get variationId; // ignore: invalid_annotation_target @JsonKey(name: 'created') - DateTime get created => throw _privateConstructorUsedError; // ignore: invalid_annotation_target + DateTime get created; // ignore: invalid_annotation_target @JsonKey(name: 'last_update') - DateTime get lastUpdate => - throw _privateConstructorUsedError; // ignore: invalid_annotation_target + DateTime get lastUpdate; // ignore: invalid_annotation_target @JsonKey(name: 'last_update_global') - DateTime get lastUpdateGlobal => throw _privateConstructorUsedError; - ExerciseCategory get category => throw _privateConstructorUsedError; - List get muscles => - throw _privateConstructorUsedError; // ignore: invalid_annotation_target + DateTime get lastUpdateGlobal; + ExerciseCategory get category; + List get muscles; // ignore: invalid_annotation_target @JsonKey(name: 'muscles_secondary') - List get musclesSecondary => - throw _privateConstructorUsedError; // ignore: invalid_annotation_target - List get equipment => - throw _privateConstructorUsedError; // ignore: invalid_annotation_target + List get musclesSecondary; // ignore: invalid_annotation_target + List get equipment; // ignore: invalid_annotation_target @JsonKey(name: 'translations', defaultValue: []) - List get translations => throw _privateConstructorUsedError; - List get images => throw _privateConstructorUsedError; - List