From ca2660f0c2ea3c754d2fc88ac68fa516f3d99bdd Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 7 Sep 2022 05:45:31 -0700 Subject: [PATCH] Fix #4366, #3611, #4323: Add app data reset flow to facilitate being able to reset the admin pin (#4418) ## Explanation Fixes #4366 Fixes #3611 Fixes #4323 This PR introduces a data reset flow for cases when the user fails to remember their administrator pin. Since there's no remote profile support currently, the only option is to clear all profiles on the device. This is done using a two-step dialog since the action is permanent (at least for now--in the future we could consider making a local backup that could be restored, but that could become pretty complicated). While this isn't a great solution, it's better than not having an option at all since users can currently get permanently stuck (see #3611). While this doesn't directly fix #3611, it mitigates it by providing a proper in-app means of resetting app data. #4323 seems unrelated, but resetting the app required routing back to SplashActivity which means this PR needed to introduce a robustness mechanism to not set the app locale unless it wasn't already initialized (which bypasses the bug mentioned by that issue). Mechanically, the deletion process involves: - Deleting all profiles on the device - Updating the in-memory cache (which causes a few UI glitches) - "Restarting" the app by ending the activity and routing back to SplashActivity This replaces the existing "forgot PIN" flow for admins, and it technically does not reset non-profile data (such as the device installation ID, onboarding status, analytics/crash events, and some device-wide preferences). ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only This PR is using the app's existing alert dialog theme, so it's not particularly interesting to show landscape/tablet screenshots as the UI isn't fundamentally different. The same is the case for special internationalization or accessibility concerns. In terms of a demonstration of the new flow, see the following video: https://user-images.githubusercontent.com/12983742/186572278-d0238e62-d013-419a-9c49-afe97cd10a5d.mp4 #4567 is tracking introducing tests, and the PR addressing it for these changes will include demonstrations of changed Espresso tests. Commits: * Add app data reset flow for resetting admin pin. * Fix broken test. * Update text & fix data reset issue. --- .../app/profile/PinPasswordActivity.kt | 2 +- .../profile/PinPasswordActivityPresenter.kt | 73 +++++++++++++------ .../app/profile/ProfileChooserViewModel.kt | 2 +- .../app/splash/SplashActivityPresenter.kt | 7 +- app/src/main/res/values/strings.xml | 10 ++- .../app/profile/PinPasswordActivityTest.kt | 2 +- .../profile/ProfileManagementController.kt | 30 +++++++- 7 files changed, 91 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivity.kt b/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivity.kt index 54a57da71c7..2930f74c828 100644 --- a/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivity.kt +++ b/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivity.kt @@ -44,6 +44,6 @@ class PinPasswordActivity : InjectableAppCompatActivity(), ProfileRouteDialogInt override fun onDestroy() { super.onDestroy() - pinPasswordActivityPresenter.dismissAlertDialog() + pinPasswordActivityPresenter.handleOnDestroy() } } diff --git a/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivityPresenter.kt index 1a7ba8e23dc..b9592c2a5c3 100644 --- a/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/profile/PinPasswordActivityPresenter.kt @@ -1,8 +1,5 @@ package org.oppia.android.app.profile -import android.content.ActivityNotFoundException -import android.content.Intent -import android.net.Uri import android.text.method.PasswordTransformationMethod import android.view.animation.AnimationUtils import androidx.appcompat.app.AlertDialog @@ -21,6 +18,7 @@ import org.oppia.android.domain.profile.ProfileManagementController import org.oppia.android.util.data.AsyncResult import org.oppia.android.util.data.DataProviders.Companion.toLiveData import javax.inject.Inject +import kotlin.system.exitProcess private const val TAG_ADMIN_SETTINGS_DIALOG = "ADMIN_SETTINGS_DIALOG" private const val TAG_RESET_PIN_DIALOG = "RESET_PIN_DIALOG" @@ -38,6 +36,7 @@ class PinPasswordActivityPresenter @Inject constructor( } private var profileId = -1 private lateinit var alertDialog: AlertDialog + private var confirmedDeletion = false fun handleOnCreate() { val adminPin = activity.intent.getStringExtra(PIN_PASSWORD_ADMIN_PIN_EXTRA_KEY) @@ -166,43 +165,71 @@ class PinPasswordActivityPresenter @Inject constructor( private fun showAdminForgotPin() { val appName = resourceHandler.getStringInLocale(R.string.app_name) pinViewModel.showAdminPinForgotPasswordPopUp.set(true) + val resetDataButtonText = + resourceHandler.getStringInLocaleWithWrapping( + R.string.admin_forgot_pin_reset_app_data_button_text, appName + ) alertDialog = AlertDialog.Builder(activity, R.style.OppiaAlertDialogTheme) .setTitle(R.string.pin_password_forgot_title) .setMessage( - resourceHandler.getStringInLocaleWithWrapping(R.string.pin_password_forgot_message, appName) + resourceHandler.getStringInLocaleWithWrapping(R.string.admin_forgot_pin_message, appName) ) .setNegativeButton(R.string.admin_settings_cancel) { dialog, _ -> pinViewModel.showAdminPinForgotPasswordPopUp.set(false) dialog.dismiss() } - .setPositiveButton(R.string.pin_password_play_store) { dialog, _ -> + .setPositiveButton(resetDataButtonText) { dialog, _ -> + // Show a confirmation dialog since this is a permanent action. + dialog.dismiss() + showConfirmAppResetDialog() + }.create() + alertDialog.setCanceledOnTouchOutside(false) + alertDialog.show() + } + + private fun showConfirmAppResetDialog() { + val appName = resourceHandler.getStringInLocale(R.string.app_name) + alertDialog = AlertDialog.Builder(activity, R.style.OppiaAlertDialogTheme) + .setTitle( + resourceHandler.getStringInLocaleWithWrapping( + R.string.admin_confirm_app_wipe_title, appName + ) + ) + .setMessage( + resourceHandler.getStringInLocaleWithWrapping( + R.string.admin_confirm_app_wipe_message, appName + ) + ) + .setNegativeButton(R.string.admin_confirm_app_wipe_negative_button_text) { dialog, _ -> pinViewModel.showAdminPinForgotPasswordPopUp.set(false) - try { - activity.startActivity( - Intent( - Intent.ACTION_VIEW, - Uri.parse("market://details?id=" + activity.packageName) - ) - ) - } catch (e: ActivityNotFoundException) { - activity.startActivity( - Intent( - Intent.ACTION_VIEW, - Uri.parse( - "https://play.google.com/store/apps/details?id=" + activity.packageName - ) - ) - ) - } dialog.dismiss() + } + .setPositiveButton(R.string.admin_confirm_app_wipe_positive_button_text) { dialog, _ -> + profileManagementController.deleteAllProfiles().toLiveData().observe( + activity, + { + // Regardless of the result of the operation, always restart the app. + confirmedDeletion = true + activity.finishAffinity() + } + ) }.create() + alertDialog.setCanceledOnTouchOutside(false) alertDialog.show() } - fun dismissAlertDialog() { + fun handleOnDestroy() { if (::alertDialog.isInitialized && alertDialog.isShowing) { alertDialog.dismiss() } + + if (confirmedDeletion) { + confirmedDeletion = false + + // End the process forcibly since the app is not designed to recover from major on-disk state + // changes that happen from underneath it (like deleting all profiles). + exitProcess(0) + } } private fun showSuccessDialog() { diff --git a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt index d78a40767e7..266ad9983c8 100644 --- a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt @@ -65,7 +65,7 @@ class ProfileChooserViewModel @Inject constructor( machineLocale.run { it.profile.name.toMachineLowerCase() } }.toMutableList() - val adminProfile = sortedProfileList.find { it.profile.isAdmin }!! + val adminProfile = sortedProfileList.find { it.profile.isAdmin } ?: return listOf() sortedProfileList.remove(adminProfile) adminPin = adminProfile.profile.pin diff --git a/app/src/main/java/org/oppia/android/app/splash/SplashActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/splash/SplashActivityPresenter.kt index 84fe06a1197..fd02c1ea32b 100644 --- a/app/src/main/java/org/oppia/android/app/splash/SplashActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/splash/SplashActivityPresenter.kt @@ -69,8 +69,11 @@ class SplashActivityPresenter @Inject constructor( // in the case of the deprecation dialog, blocks) the activity. liveData.removeObserver(this) - // First, initialize the app's initial locale. - appLanguageLocaleHandler.initializeLocale(initState.displayLocale) + // First, initialize the app's initial locale. Note that since the activity can be + // reopened, it's possible for this to be initialized more than once. + if (!appLanguageLocaleHandler.isInitialized()) { + appLanguageLocaleHandler.initializeLocale(initState.displayLocale) + } // Second, route the user to the correct destination. when (initState.startupMode) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 78eb62fc67f..145c532470f 100755 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -297,15 +297,19 @@ Please enter your PIN. Administrator’s 5-Digit PIN. User’s 3-Digit PIN. - I forgot my pin. + Forgot PIN? Incorrect PIN. show hide Close PIN change is successful Forgot PIN? - To reset your PIN, please uninstall %s and then reinstall it.\n\nKeep in mind that if the device has not been online, you may lose user progress on multiple accounts. - Go to the play store + To reset your PIN, you\'ll need to clear all saved data for %s.\n\nKeep in mind that this action will cause all profiles and user progress to be deleted, and it cannot be undone. Also, the app will close when this completes and will need to be reopened. + Reset %s Data + Confirm %s Data Reset + Are you sure that you want to delete all %s profiles on this device? This operation cannot be undone. + Yes + No Show/Hide password icon Password shown icon Password hidden icon diff --git a/app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt index f7f5fa8fe29..e8770e57d44 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt @@ -1125,7 +1125,7 @@ class PinPasswordActivityTest { private fun getAppName(): String = context.resources.getString(R.string.app_name) private fun getPinPasswordForgotMessage(): String = - context.resources.getString(R.string.pin_password_forgot_message, getAppName()) + context.resources.getString(R.string.admin_forgot_pin_message, getAppName()) // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. @Singleton diff --git a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt index 29f8ce50ac2..dac62b05666 100644 --- a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt +++ b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt @@ -132,7 +132,7 @@ class ProfileManagementController @Inject constructor( profileDataStore.primeInMemoryCacheAsync().invokeOnCompletion { it?.let { oppiaLogger.e( - "DOMAIN", + "ProfileManagementController", "Failed to prime cache ahead of data retrieval for ProfileManagementController.", it ) @@ -664,9 +664,7 @@ class ProfileManagementController @Inject constructor( * @return a [DataProvider] that indicates the success/failure of this delete operation. */ fun deleteProfile(profileId: ProfileId): DataProvider { - val deferred = profileDataStore.storeDataWithCustomChannelAsync( - updateInMemoryCache = true - ) { + val deferred = profileDataStore.storeDataWithCustomChannelAsync(updateInMemoryCache = true) { if (!it.profilesMap.containsKey(profileId.internalId)) { return@storeDataWithCustomChannelAsync Pair(it, ProfileActionStatus.PROFILE_NOT_FOUND) } @@ -683,6 +681,30 @@ class ProfileManagementController @Inject constructor( } } + /** + * Deletes all profiles installed on the device (and logs out the current user). + * + * Note that this will not update the in-memory cache as the app is expected to be forcibly closed + * after deletion (since there's no mechanism to notify existing cache stores that they need to + * reload/reset from their on-disk copies). + * + * Finally, this method attempts to never fail by forcibly deleting all profiles even if some are + * in a bad state (and would normally failed if attempted to be deleted via [deleteProfile]). + */ + fun deleteAllProfiles(): DataProvider { + val deferred = profileDataStore.storeDataWithCustomChannelAsync { + val installationId = loggingIdentifierController.fetchInstallationId() + it.profilesMap.forEach { (internalProfileId, profile) -> + directoryManagementUtil.deleteDir(internalProfileId.toString()) + learnerAnalyticsLogger.logDeleteProfile(installationId, profile.learnerId) + } + Pair(ProfileDatabase.getDefaultInstance(), ProfileActionStatus.SUCCESS) + } + return dataProviders.createInMemoryDataProviderAsync(DELETE_PROFILE_PROVIDER_ID) { + getDeferredResult(profileId = null, name = null, deferred) + } + } + /** * Returns the ProfileId of the current profile. The default value is -1 if currentProfileId * hasn't been set.