Skip to content

Commit

Permalink
Fix profile migration bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
adhiamboperes committed Aug 28, 2024
1 parent b3d6ee3 commit daea15f
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,15 @@ class ExitProfileDialogFragment : InjectableDialogFragment() {
}
.setPositiveButton(R.string.home_activity_back_dialog_exit) { _, _ ->
if (soleLearnerProfile) {
requireActivity().finishAffinity()
requireActivity().finish()
} else {
// TODO(#3641): Investigate on using finish instead of intent.
val intent = ProfileChooserActivity.createProfileChooserActivity(requireActivity())
if (!restoreLastCheckedItem) {
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
}
requireActivity().startActivity(intent)
requireActivity().finish()
}
}
.create()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.oppia.android.app.model.IntroActivityParams
import org.oppia.android.app.model.Profile
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ProfileOnboardingMode
import org.oppia.android.app.model.ProfileType
import org.oppia.android.app.notice.AutomaticAppDeprecationNoticeDialogFragment
import org.oppia.android.app.notice.BetaNoticeDialogFragment
import org.oppia.android.app.notice.DeprecationNoticeActionResponse
Expand Down Expand Up @@ -278,7 +279,7 @@ class SplashActivityPresenter @Inject constructor(
OsDeprecationNoticeDialogFragment::newInstance
)
}
StartupMode.USER_NOT_YET_ONBOARDED -> fetchProfiles()
StartupMode.USER_NOT_YET_ONBOARDED -> fetchProfile()
else -> {
// In all other cases (including errors when the startup state fails to load or is
// defaulted), assume the user needs to be onboarded.
Expand All @@ -297,7 +298,7 @@ class SplashActivityPresenter @Inject constructor(
AutomaticAppDeprecationNoticeDialogFragment::newInstance
)
}
StartupMode.USER_NOT_YET_ONBOARDED -> fetchProfiles()
StartupMode.USER_NOT_YET_ONBOARDED -> fetchProfile()
else -> {
// In all other cases (including errors when the startup state fails to load or is
// defaulted), assume the user needs to be onboarded.
Expand All @@ -316,7 +317,7 @@ class SplashActivityPresenter @Inject constructor(
}

private fun getProfileOnboardingState() {
profileManagementController.getProfileOnboardingState().toLiveData().observe(
profileManagementController.getProfileOnboardingMode().toLiveData().observe(
activity,
{ result ->
when (result) {
Expand All @@ -337,15 +338,15 @@ class SplashActivityPresenter @Inject constructor(
ProfileOnboardingMode.NEW_INSTALL -> {
launchOnboardingActivity()
}
ProfileOnboardingMode.SOLE_LEARNER_PROFILE -> fetchProfiles()
ProfileOnboardingMode.SOLE_LEARNER_PROFILE_ONLY -> fetchProfile()
else -> {
activity.startActivity(ProfileChooserActivity.createProfileChooserActivity(activity))
activity.finish()
}
}
}

private fun fetchProfiles() {
private fun fetchProfile() {
profileManagementController.getProfiles().toLiveData().observe(activity) { result ->
when (result) {
is AsyncResult.Success -> handleProfiles(result.value)
Expand All @@ -358,7 +359,7 @@ class SplashActivityPresenter @Inject constructor(
}

private fun handleProfiles(profiles: List<Profile>) {
val soleLearnerProfile = profiles.find { it.isAdmin && it.pin.isNullOrBlank() }
val soleLearnerProfile = profiles.find { it.profileType == ProfileType.SOLE_LEARNER }
if (soleLearnerProfile != null) {
proceedBasedOnProfileState(soleLearnerProfile)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import org.oppia.android.app.model.OppiaLanguage.NIGERIAN_PIDGIN
import org.oppia.android.app.model.OppiaLocaleContext
import org.oppia.android.app.model.OppiaRegion
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ProfileType
import org.oppia.android.app.model.ScreenName
import org.oppia.android.app.onboarding.IntroActivity
import org.oppia.android.app.onboarding.OnboardingActivity
Expand Down Expand Up @@ -1072,6 +1073,7 @@ class SplashActivityTest {
initializeTestApplication(onboardingV2Enabled = true)
profileTestHelper.addOnlyAdminProfileWithoutPin()
val profileId = ProfileId.newBuilder().setInternalId(0).build()
profileTestHelper.updateProfileType(profileId, ProfileType.SOLE_LEARNER)
profileTestHelper.markProfileOnboardingStarted(profileId)
val params = IntroActivityParams.newBuilder()
.setProfileNickname("Admin")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,17 +405,22 @@ class ProfileManagementController @Inject constructor(
}

/** Returns the state of the app based on the number and type of existing profiles. */
fun getProfileOnboardingState(): DataProvider<ProfileOnboardingMode> {
fun getProfileOnboardingMode(): DataProvider<ProfileOnboardingMode> {
return getProfiles().transform(PROFILE_ONBOARDING_MODE_PROVIDER_ID) { profileList ->
val profileCount = profileList.size
when {
profileCount > 1 -> ProfileOnboardingMode.MULTIPLE_PROFILES
profileCount == 1 -> {
val profileType = profileList.first().profileType
if (profileType == ProfileType.SUPERVISOR) {
ProfileOnboardingMode.ADMIN_PROFILE_ONLY
} else {
ProfileOnboardingMode.SOLE_LEARNER_PROFILE
when (profileList.first().profileType) {
ProfileType.SUPERVISOR -> {
ProfileOnboardingMode.SUPERVISOR_PROFILE_ONLY
}
ProfileType.SOLE_LEARNER -> {
ProfileOnboardingMode.SOLE_LEARNER_PROFILE_ONLY
}
else -> {
ProfileOnboardingMode.UNKNOWN_PROFILE_TYPE
}
}
}
else -> ProfileOnboardingMode.NEW_INSTALL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,28 @@ import javax.inject.Singleton
@LooperMode(LooperMode.Mode.PAUSED)
@Config(application = ProfileManagementControllerTest.TestApplication::class)
class ProfileManagementControllerTest {
@get:Rule val oppiaTestRule = OppiaTestRule()
@Inject lateinit var context: Context
@Inject lateinit var profileTestHelper: ProfileTestHelper
@Inject lateinit var profileManagementController: ProfileManagementController
@Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
@Inject lateinit var monitorFactory: DataProviderTestMonitor.Factory
@Inject lateinit var machineLocale: OppiaLocale.MachineLocale
@field:[BackgroundDispatcher Inject] lateinit var backgroundDispatcher: CoroutineDispatcher
@Inject lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger
@Inject lateinit var loggingIdentifierController: LoggingIdentifierController
@Inject lateinit var oppiaClock: FakeOppiaClock
@get:Rule
val oppiaTestRule = OppiaTestRule()
@Inject
lateinit var context: Context
@Inject
lateinit var profileTestHelper: ProfileTestHelper
@Inject
lateinit var profileManagementController: ProfileManagementController
@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
@Inject
lateinit var monitorFactory: DataProviderTestMonitor.Factory
@Inject
lateinit var machineLocale: OppiaLocale.MachineLocale
@field:[BackgroundDispatcher Inject]
lateinit var backgroundDispatcher: CoroutineDispatcher
@Inject
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger
@Inject
lateinit var loggingIdentifierController: LoggingIdentifierController
@Inject
lateinit var oppiaClock: FakeOppiaClock

private companion object {
private val PROFILES_LIST = listOf<Profile>(
Expand Down Expand Up @@ -1726,16 +1737,21 @@ class ProfileManagementControllerTest {
}

@Test
fun testProfileOnboardingState_oneAdminProfileWithoutPassword_returnsSoleLeanerState() {
fun testProfileOnboardingState_oneAdminProfileWithoutPassword_returnsSoleLeanerTypeMode() {
setUpTestWithOnboardingV2Enabled(true)
addAdminProfileAndWait(name = "James", pin = "")

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingState()
val updateProfileProvider =
profileManagementController.updateProfileType(ADMIN_PROFILE_ID_0, ProfileType.SOLE_LEARNER)
monitorFactory.ensureDataProviderExecutes(updateProfileProvider)

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingMode()
val profileOnboardingModeResult =
monitorFactory.waitForNextSuccessfulResult(profileOnboardingModeProvider)

assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.SOLE_LEARNER_PROFILE)
assertThat(profileOnboardingModeResult).isEqualTo(
ProfileOnboardingMode.SOLE_LEARNER_PROFILE_ONLY
)
}

@Test
Expand All @@ -1745,44 +1761,52 @@ class ProfileManagementControllerTest {

val updateProfileProvider =
profileManagementController.updateProfileType(ADMIN_PROFILE_ID_0, ProfileType.SUPERVISOR)

monitorFactory.ensureDataProviderExecutes(updateProfileProvider)

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingState()

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingMode()
val profileOnboardingModeResult =
monitorFactory.waitForNextSuccessfulResult(profileOnboardingModeProvider)

assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.ADMIN_PROFILE_ONLY)
assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.SUPERVISOR_PROFILE_ONLY)
}

@Test
fun testProfileOnboardingState_multipleProfiles_returnsMultipleProfilesState() {
fun testProfileOnboardingState_multipleProfiles_returnsMultipleProfilesTypeMode() {
setUpTestWithOnboardingV2Enabled(true)
addAdminProfileAndWait(name = "James")
addNonAdminProfileAndWait(name = "Rajat", pin = "01234")
addNonAdminProfileAndWait(name = "Rohit", pin = "")

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingState()

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingMode()
val profileOnboardingModeResult =
monitorFactory.waitForNextSuccessfulResult(profileOnboardingModeProvider)

assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.MULTIPLE_PROFILES)
}

@Test
fun testProfileOnboardingState_noProfilesFound_returnsNewInstallState() {
fun testProfileOnboardingState_noProfilesFound_returnsNewInstallTypeMode() {
setUpTestWithOnboardingV2Enabled(true)

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingState()

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingMode()
val profileOnboardingModeResult =
monitorFactory.waitForNextSuccessfulResult(profileOnboardingModeProvider)

assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.NEW_INSTALL)
}

@Test
fun testProfileOnboardingState_existingProfilesV1_returnsUnknownProfileTypeMode() {
setUpTestWithOnboardingV2Enabled(true)
addAdminProfileAndWait(name = "James")

val profileOnboardingModeProvider = profileManagementController.getProfileOnboardingMode()
val profileOnboardingModeResult =
monitorFactory.waitForNextSuccessfulResult(profileOnboardingModeProvider)

assertThat(profileOnboardingModeResult).isEqualTo(ProfileOnboardingMode.UNKNOWN_PROFILE_TYPE)
}

@Test
fun testGetProfile_createAdmin_returnsSupervisorType() {
setUpTestWithOnboardingV2Enabled(true)
Expand Down
8 changes: 6 additions & 2 deletions model/src/main/proto/profile.proto
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,15 @@ enum ProfileOnboardingMode {
NEW_INSTALL = 1;

// Indicates that there is only one profile and it is a sole learner profile.
SOLE_LEARNER_PROFILE = 2;
SOLE_LEARNER_PROFILE_ONLY = 2;

// Indicates that there is only one profile and it is an admin profile.
ADMIN_PROFILE_ONLY = 3;
SUPERVISOR_PROFILE_ONLY = 3;

// Indicates that there are multiple profiles on the device.
MULTIPLE_PROFILES = 4;

// Indicates that there is only one profile and the profile type is unknown, indicating that
// migration is required.
UNKNOWN_PROFILE_TYPE = 5;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.oppia.android.testing.profile

import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ProfileType
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.testing.data.DataProviderTestMonitor
import org.oppia.android.util.data.AsyncResult
Expand Down Expand Up @@ -129,6 +130,11 @@ class ProfileTestHelper @Inject constructor(
return profileManagementController.markProfileOnboardingStarted(profileId)
}

/** Updates the [ProfileType] of an existing profile. */
fun updateProfileType(profileId: ProfileId, profileType: ProfileType): DataProvider<Any?> {
return profileManagementController.updateProfileType(profileId, profileType)
}

/** Returns the continue button animation seen for profile. */
fun getContinueButtonAnimationSeenStatus(profileId: ProfileId): Boolean {
return monitorFactory.waitForNextSuccessfulResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import dagger.Provides
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.oppia.android.app.model.ProfileType
import org.oppia.android.domain.oppialogger.LogStorageModule
import org.oppia.android.domain.oppialogger.LoggingIdentifierModule
import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule
Expand Down Expand Up @@ -147,13 +148,38 @@ class ProfileTestHelperTest {
}

@Test
fun testProfileOnboarding_markOnboardingCompleted_chekIsSuccessful() {
fun testProfileOnboarding_markOnboardingStarted_checkIsSuccessful() {
profileTestHelper.addOnlyAdminProfile()
val profileId = profileManagementController.getCurrentProfileId()
val onboardingProvider = profileTestHelper.markProfileOnboardingStarted(profileId!!)
monitorFactory.waitForNextSuccessfulResult(onboardingProvider)
}

@Test
fun testProfileOnboarding_markOnboardingCompleted_checkIsSuccessful() {
profileTestHelper.addOnlyAdminProfile()
val profileId = profileManagementController.getCurrentProfileId()
val onboardingProvider = profileTestHelper.markProfileOnboardingEnded(profileId!!)
monitorFactory.waitForNextSuccessfulResult(onboardingProvider)
}

@Test
fun testUpdateProfile_updateProfileType_checkIsSuccessful() {
profileTestHelper.addOnlyAdminProfile()
val profileId = profileManagementController.getCurrentProfileId()
val updateProvider = profileTestHelper.updateProfileType(profileId!!, ProfileType.SUPERVISOR)
monitorFactory.ensureDataProviderExecutes(updateProvider)

val profilesProvider = profileManagementController.getProfiles()
testCoroutineDispatchers.runCurrent()

val profiles = monitorFactory.waitForNextSuccessfulResult(profilesProvider)
assertThat(profiles.size).isEqualTo(1)
assertThat(profiles[0].name).isEqualTo("Admin")
assertThat(profiles[0].isAdmin).isTrue()
assertThat(profiles[0].profileType).isEqualTo(ProfileType.SUPERVISOR)
}

// TODO(#89): Move this to a common test application component.
@Module
class TestModule {
Expand Down

0 comments on commit daea15f

Please sign in to comment.