Skip to content

Commit

Permalink
Fix #4366, #3611, #4323: Add app data reset flow to facilitate being …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
BenHenning authored Sep 7, 2022
1 parent c639763 commit ca2660f
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ class PinPasswordActivity : InjectableAppCompatActivity(), ProfileRouteDialogInt

override fun onDestroy() {
super.onDestroy()
pinPasswordActivityPresenter.dismissAlertDialog()
pinPasswordActivityPresenter.handleOnDestroy()
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 7 additions & 3 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,19 @@
<string name="pin_password_user_enter">Please enter your PIN.</string>
<string name="input_pin_password_as_admin">Administrator’s 5-Digit PIN.</string>
<string name="input_pin_password_as_user">User’s 3-Digit PIN.</string>
<string name="pin_password_forgot_pin">I forgot my pin.</string>
<string name="pin_password_forgot_pin">Forgot PIN?</string>
<string name="pin_password_incorrect_pin">Incorrect PIN.</string>
<string name="pin_password_show">show</string>
<string name="pin_password_hide">hide</string>
<string name="pin_password_close">Close</string>
<string name="pin_password_success">PIN change is successful</string>
<string name="pin_password_forgot_title">Forgot PIN?</string>
<string name="pin_password_forgot_message">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. </string>
<string name="pin_password_play_store">Go to the play store</string>
<string name="admin_forgot_pin_message">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.</string>
<string name="admin_forgot_pin_reset_app_data_button_text">Reset %s Data</string>
<string name="admin_confirm_app_wipe_title">Confirm %s Data Reset</string>
<string name="admin_confirm_app_wipe_message">Are you sure that you want to delete all %s profiles on this device? This operation cannot be undone.</string>
<string name="admin_confirm_app_wipe_positive_button_text">Yes</string>
<string name="admin_confirm_app_wipe_negative_button_text">No</string>
<string name="show_hide_password_icon">Show/Hide password icon</string>
<string name="password_shown_icon">Password shown icon</string>
<string name="password_hidden_icon">Password hidden icon</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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<Any?> {
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)
}
Expand All @@ -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<Any?> {
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.
Expand Down

0 comments on commit ca2660f

Please sign in to comment.