From ae4ff582ead9717c5a78688dea60179415ee3c10 Mon Sep 17 00:00:00 2001 From: CalvinKern Date: Wed, 22 Apr 2020 13:40:42 -0600 Subject: [PATCH] [Flutter-Parent][RC-3.2.0] Fix migration for users not on refresh tokens (#753) Also some misc null fixes --- .../parentapp/plugins/OldAppMigrations.kt | 4 ++++ .../lib/network/utils/api_prefs.dart | 4 +++- .../manage_students_screen.dart | 3 ++- .../qr_login/qr_login_tutorial_screen.dart | 6 +++-- .../flutter_parent/lib/utils/crash_utils.dart | 4 ++-- apps/flutter_parent/pubspec.yaml | 2 +- .../test/network/api_prefs_test.dart | 22 +++++++++++++++++++ .../utils/widgets/masquerade_ui_test.dart | 1 + 8 files changed, 39 insertions(+), 7 deletions(-) diff --git a/apps/flutter_parent/android/app/src/main/kotlin/com/instructure/parentapp/plugins/OldAppMigrations.kt b/apps/flutter_parent/android/app/src/main/kotlin/com/instructure/parentapp/plugins/OldAppMigrations.kt index 782d5800ed..d3970b188e 100644 --- a/apps/flutter_parent/android/app/src/main/kotlin/com/instructure/parentapp/plugins/OldAppMigrations.kt +++ b/apps/flutter_parent/android/app/src/main/kotlin/com/instructure/parentapp/plugins/OldAppMigrations.kt @@ -65,6 +65,10 @@ object OldAppMigrations { it.put("domain", "${it.optString("protocol") ?: "https"}://${it.optString("domain")}") it.remove("protocol") + // Some users may not be on refresh tokens, so pull their old token as the 'accessToken' + val token = it.optString("token") + if (token != null && token.isNotEmpty()) it.put("accessToken", token) + // Add client id/secret if this is the current user if (refreshToken == it.optString("refreshToken")) { it.put("clientId", prefs.getString("client_id", null)) diff --git a/apps/flutter_parent/lib/network/utils/api_prefs.dart b/apps/flutter_parent/lib/network/utils/api_prefs.dart index 8516e076bb..94b4b805e4 100644 --- a/apps/flutter_parent/lib/network/utils/api_prefs.dart +++ b/apps/flutter_parent/lib/network/utils/api_prefs.dart @@ -90,7 +90,9 @@ class ApiPrefs { static bool isLoggedIn() { _checkInit(); - return getAuthToken() != null && getDomain() != null; + final token = getAuthToken() ?? ''; + final domain = getDomain() ?? ''; + return token.isNotEmpty && domain.isNotEmpty; } static Login getCurrentLogin() { diff --git a/apps/flutter_parent/lib/screens/manage_students/manage_students_screen.dart b/apps/flutter_parent/lib/screens/manage_students/manage_students_screen.dart index 98e4e16b84..233ddcbbb7 100644 --- a/apps/flutter_parent/lib/screens/manage_students/manage_students_screen.dart +++ b/apps/flutter_parent/lib/screens/manage_students/manage_students_screen.dart @@ -232,7 +232,8 @@ class _ManageStudentsState extends State { void _showAddStudentDialog() async { locator().logEvent(AnalyticsEventConstants.ADD_STUDENT_MANAGE_STUDENTS); bool studentPaired = await _addStudentDialog(context); - if (studentPaired) { + // Can be null if popped by dismissing + if (studentPaired == true) { _refreshKey.currentState.show(); } } diff --git a/apps/flutter_parent/lib/screens/qr_login/qr_login_tutorial_screen.dart b/apps/flutter_parent/lib/screens/qr_login/qr_login_tutorial_screen.dart index 477deb3908..afa62173eb 100644 --- a/apps/flutter_parent/lib/screens/qr_login/qr_login_tutorial_screen.dart +++ b/apps/flutter_parent/lib/screens/qr_login/qr_login_tutorial_screen.dart @@ -16,6 +16,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_parent/l10n/app_localizations.dart'; +import 'package:flutter_parent/network/utils/analytics.dart'; import 'package:flutter_parent/router/panda_router.dart'; import 'package:flutter_parent/screens/qr_login/qr_login_tutorial_screen_interactor.dart'; import 'package:flutter_parent/utils/design/parent_theme.dart'; @@ -54,12 +55,13 @@ class _QRLoginTutorialScreenState extends State { shape: CircleBorder(side: BorderSide(color: Colors.transparent)), onPressed: () async { var barcodeResult = await locator().scan(); - if (barcodeResult.isSuccess) { + if (barcodeResult?.isSuccess == true) { locator().pushRoute(context, PandaRouter.qrLogin(barcodeResult.result)); } else { + locator().logMessage(barcodeResult?.errorType?.toString() ?? 'No barcode result'); _showSnackBarError( context, - barcodeResult.errorType == QRError.invalidQR + barcodeResult?.errorType == QRError.invalidQR ? L10n(context).invalidQRCodeError : L10n(context).qrCodeNoCameraError); } diff --git a/apps/flutter_parent/lib/utils/crash_utils.dart b/apps/flutter_parent/lib/utils/crash_utils.dart index d7771e367e..14fe5574c2 100644 --- a/apps/flutter_parent/lib/utils/crash_utils.dart +++ b/apps/flutter_parent/lib/utils/crash_utils.dart @@ -54,8 +54,8 @@ class CrashUtils { // Set any user info that will help to debug the issue await Future.wait([ - FlutterCrashlytics().setInfo('domain', ApiPrefs.getDomain()), - FlutterCrashlytics().setInfo('user_id', ApiPrefs.getUser()?.id), + FlutterCrashlytics().setInfo('domain', ApiPrefs.getDomain() ?? 'null'), + FlutterCrashlytics().setInfo('user_id', ApiPrefs.getUser()?.id ?? 'null'), ]); await FlutterCrashlytics().reportCrash(exception, stacktrace, forceCrash: false); diff --git a/apps/flutter_parent/pubspec.yaml b/apps/flutter_parent/pubspec.yaml index f49a694c02..0e7b2c6933 100644 --- a/apps/flutter_parent/pubspec.yaml +++ b/apps/flutter_parent/pubspec.yaml @@ -25,7 +25,7 @@ description: Canvas Parent # In iOS, build-name is used as CFBundleShortVersionString while build-number used as CFBundleVersion. # Read more about iOS versioning at # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html -version: 3.2.0+29 +version: 3.2.0+30 module: androidX: true diff --git a/apps/flutter_parent/test/network/api_prefs_test.dart b/apps/flutter_parent/test/network/api_prefs_test.dart index bbeb0e57be..cd63d23244 100644 --- a/apps/flutter_parent/test/network/api_prefs_test.dart +++ b/apps/flutter_parent/test/network/api_prefs_test.dart @@ -84,6 +84,28 @@ void main() { expect(ApiPrefs.isLoggedIn(), true); }); + test('is logged in returns false with an empty token', () async { + var login = Login((b) => b + ..domain = 'domain' + ..accessToken = '' + ..user = CanvasModelTestUtils.mockUser().toBuilder()); + await setupPlatformChannels(config: PlatformConfig(mockApiPrefs: {ApiPrefs.KEY_CURRENT_LOGIN_UUID: login.uuid})); + await ApiPrefs.addLogin(login); + + expect(ApiPrefs.isLoggedIn(), false); + }); + + test('is logged in returns false with an empty domain', () async { + var login = Login((b) => b + ..domain = '' + ..accessToken = 'token' + ..user = CanvasModelTestUtils.mockUser().toBuilder()); + await setupPlatformChannels(config: PlatformConfig(mockApiPrefs: {ApiPrefs.KEY_CURRENT_LOGIN_UUID: login.uuid})); + await ApiPrefs.addLogin(login); + + expect(ApiPrefs.isLoggedIn(), false); + }); + test('getApiUrl returns the domain with the api path added', () async { var login = Login((b) => b ..domain = 'domain' diff --git a/apps/flutter_parent/test/utils/widgets/masquerade_ui_test.dart b/apps/flutter_parent/test/utils/widgets/masquerade_ui_test.dart index e9da676039..7be74bd948 100644 --- a/apps/flutter_parent/test/utils/widgets/masquerade_ui_test.dart +++ b/apps/flutter_parent/test/utils/widgets/masquerade_ui_test.dart @@ -38,6 +38,7 @@ void main() { Login normalLogin = Login((b) => b ..domain = 'domain' + ..accessToken = 'token' ..user = CanvasModelTestUtils.mockUser().toBuilder()); Login masqueradeLogin = normalLogin.rebuild((b) => b