From 063b97ab1a1e0891dda72715cd2c43b50e683480 Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Tue, 10 Sep 2024 00:39:41 +0300 Subject: [PATCH] Address general comments --- .../app/classroom/ClassroomListFragmentPresenter.kt | 2 +- .../org/oppia/android/app/home/ExitProfileListener.kt | 5 +++-- .../org/oppia/android/app/home/HomeFragmentPresenter.kt | 2 +- .../app/onboarding/AudioLanguageFragmentPresenter.kt | 4 ++-- .../app/profile/ProfileChooserActivityPresenter.kt | 3 ++- .../app/profile/ProfileChooserFragmentPresenter.kt | 8 ++++---- .../oppia/android/app/splash/SplashActivityPresenter.kt | 4 ++-- .../domain/profile/ProfileManagementController.kt | 4 ++-- model/src/main/proto/arguments.proto | 9 --------- model/src/main/proto/profile.proto | 8 ++++---- .../org/oppia/android/testing/logging/EventLogSubject.kt | 3 +-- .../android/testing/profile/ProfileTestHelperTest.kt | 2 +- 12 files changed, 23 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt index ee54f6316ca..f5a90d703fc 100644 --- a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt @@ -342,7 +342,7 @@ class ClassroomListFragmentPresenter @Inject constructor( private fun handleProfileOnboardingState(profile: Profile) { // App onboarding is completed by the first profile on the app(SOLE_LEARNER or SUPERVISOR), // while profile onboarding is completed by each profile. - if (!profile.completedProfileOboarding) { + if (!profile.completedProfileOnboarding) { markProfileOnboardingEnded(profileId) if (profile.profileType == ProfileType.SOLE_LEARNER || profile.profileType == ProfileType.SUPERVISOR diff --git a/app/src/main/java/org/oppia/android/app/home/ExitProfileListener.kt b/app/src/main/java/org/oppia/android/app/home/ExitProfileListener.kt index 84467168f49..d866cce75df 100644 --- a/app/src/main/java/org/oppia/android/app/home/ExitProfileListener.kt +++ b/app/src/main/java/org/oppia/android/app/home/ExitProfileListener.kt @@ -1,13 +1,14 @@ package org.oppia.android.app.home import org.oppia.android.app.model.ProfileType + /** Listener for when a user wishes to exit their profile. */ interface ExitProfileListener { + /** * Called when back press is clicked on the HomeScreen. * - * A SOLE_LEARNER exits the app completely while other [ProfileType]s are routed to the - * [ProfileChooserActivity]. + * Routing behaviour may change based on [ProfileType] */ fun exitProfile(profileType: ProfileType) } diff --git a/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt index 3df529c3763..0a442ff6442 100644 --- a/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt @@ -181,7 +181,7 @@ class HomeFragmentPresenter @Inject constructor( private fun handleProfileOnboardingState(profile: Profile) { // App onboarding is completed by the first profile on the app(SOLE_LEARNER or SUPERVISOR), // while profile onboarding is completed by each profile. - if (!profile.completedProfileOboarding) { + if (!profile.completedProfileOnboarding) { markProfileOnboardingEnded(profileId) if (profile.profileType == ProfileType.SOLE_LEARNER || profile.profileType == ProfileType.SUPERVISOR diff --git a/app/src/main/java/org/oppia/android/app/onboarding/AudioLanguageFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/onboarding/AudioLanguageFragmentPresenter.kt index 8fa5a13a1da..3a7736edb17 100644 --- a/app/src/main/java/org/oppia/android/app/onboarding/AudioLanguageFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/onboarding/AudioLanguageFragmentPresenter.kt @@ -123,7 +123,7 @@ class AudioLanguageFragmentPresenter @Inject constructor( binding.onboardingNavigationContinue.setOnClickListener { updateSelectedAudioLanguage(selectedLanguage, profileId).also { - loginToProfile(profileId) + logInToProfile(profileId) } } @@ -159,7 +159,7 @@ class AudioLanguageFragmentPresenter @Inject constructor( } } - private fun loginToProfile(profileId: ProfileId) { + private fun logInToProfile(profileId: ProfileId) { profileManagementController.loginToProfile(profileId).toLiveData().observe( fragment, { result -> diff --git a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt index 049d9d4ca9d..6bfe0bb3122 100644 --- a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt @@ -31,7 +31,8 @@ class ProfileChooserActivityPresenter @Inject constructor( isAdmin = true ) } else { - // TODO(#482): Ensures that an admin profile is present. Remove when there is proper admin account creation. + // TODO(#482): Ensures that an admin profile is present. + // This can be removed once the new onboarding flow is finalized, as it will handle the creation of an admin profile. profileManagementController.addProfile( name = "Admin", pin = "", diff --git a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt index c1c5e86a74d..2cab08277ff 100644 --- a/app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt @@ -184,7 +184,7 @@ class ProfileChooserFragmentPresenter @Inject constructor( if (enableOnboardingFlowV2.value) { ensureProfileOnboarded(model.profile) } else { - loginToProfile(model.profile) + logInToProfile(model.profile) } } } @@ -256,8 +256,8 @@ class ProfileChooserFragmentPresenter @Inject constructor( } private fun ensureProfileOnboarded(profile: Profile) { - if (profile.profileType == ProfileType.SUPERVISOR || profile.completedProfileOboarding) { - loginToProfile(profile) + if (profile.profileType == ProfileType.SUPERVISOR || profile.completedProfileOnboarding) { + logInToProfile(profile) } else { launchOnboardingScreen(profile.id, profile.name) } @@ -277,7 +277,7 @@ class ProfileChooserFragmentPresenter @Inject constructor( activity.startActivity(intent) } - private fun loginToProfile(profile: Profile) { + private fun logInToProfile(profile: Profile) { if (profile.pin.isNullOrBlank()) { profileManagementController.loginToProfile(profile.id).toLiveData().observe( fragment, 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 3b3a027d663..51891c6ed0c 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 @@ -369,10 +369,10 @@ class SplashActivityPresenter @Inject constructor( private fun proceedBasedOnProfileState(profile: Profile) { when { - profile.startedProfileOboarding && !profile.completedProfileOboarding -> { + profile.startedProfileOnboarding && !profile.completedProfileOnboarding -> { resumeOnboarding(profile.id, profile.name) } - profile.startedProfileOboarding && profile.completedProfileOboarding -> { + profile.startedProfileOnboarding && profile.completedProfileOnboarding -> { loginToProfile(profile.id) } else -> launchOnboardingActivity() 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 cd458f3ef50..674f6c0d58f 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 @@ -364,7 +364,7 @@ class ProfileManagementController @Inject constructor( it, ProfileActionStatus.PROFILE_NOT_FOUND ) - val updatedProfile = profile.toBuilder().setStartedProfileOboarding(true).build() + val updatedProfile = profile.toBuilder().setStartedProfileOnboarding(true).build() val profileDatabaseBuilder = it.toBuilder().putProfiles( profileId.internalId, updatedProfile @@ -392,7 +392,7 @@ class ProfileManagementController @Inject constructor( it, ProfileActionStatus.PROFILE_NOT_FOUND ) - val updatedProfile = profile.toBuilder().setCompletedProfileOboarding(true).build() + val updatedProfile = profile.toBuilder().setCompletedProfileOnboarding(true).build() val profileDatabaseBuilder = it.toBuilder().putProfiles( profileId.internalId, updatedProfile diff --git a/model/src/main/proto/arguments.proto b/model/src/main/proto/arguments.proto index bc593ef258a..8540563d3ee 100644 --- a/model/src/main/proto/arguments.proto +++ b/model/src/main/proto/arguments.proto @@ -260,9 +260,6 @@ message ReadingTextSizeFragmentStateBundle { message AudioLanguageActivityParams { // The default audio language previously selected by the user (upon opening the activity). AudioLanguage audio_language = 1; - - // The internal Id associated with the profile. - int32 profile_id = 2; } // The bundle of properties that are saved upon configuration changes in AudioLanguageActivity. @@ -281,9 +278,6 @@ message AudioLanguageActivityResultBundle { message AudioLanguageFragmentArguments { // The default audio language previously selected by the user (upon opening the fragment). AudioLanguage audio_language = 1; - - // The internal Id associated with the profile. - int32 profile_id = 2; } // The bundle of properties that are saved upon configuration changes in AudioLanguageFragment. @@ -897,9 +891,6 @@ message WalkthroughFinalFragmentArguments { message IntroActivityParams { // The nickname associated with a newly created profile. string profile_nickname = 1; - - // The internal Id associated with the newly created profile. - int32 profile_id = 2; } // Arguments required when creating a new IntroFragment. diff --git a/model/src/main/proto/profile.proto b/model/src/main/proto/profile.proto index e58fd585e0b..11755096bc4 100644 --- a/model/src/main/proto/profile.proto +++ b/model/src/main/proto/profile.proto @@ -94,11 +94,11 @@ message Profile { // Represents the type of user which informs the configuration options available to them. ProfileType profile_type = 20; - // Indicates whether this profile has started the onboarding flow. - bool started_profile_oboarding = 21; + // Indicates that this profile has viewed the relevant onboarding introduction screen. + bool started_profile_onboarding = 21; - // Indicates whether this profile has completed the onboarding flow. - bool completed_profile_oboarding = 22; + // Indicates that this profile has reached the home screen for the first time. + bool completed_profile_onboarding = 22; } // Represents the type of user using the app. diff --git a/testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt b/testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt index f2b3a0b49c9..31681e04e4c 100644 --- a/testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt +++ b/testing/src/main/java/org/oppia/android/testing/logging/EventLogSubject.kt @@ -7,7 +7,6 @@ import com.google.common.truth.IntegerSubject import com.google.common.truth.IterableSubject import com.google.common.truth.LongSubject import com.google.common.truth.StringSubject -import com.google.common.truth.Subject import com.google.common.truth.Truth.assertAbout import com.google.common.truth.Truth.assertThat import com.google.common.truth.extensions.proto.LiteProtoSubject @@ -2473,7 +2472,7 @@ class EventLogSubject private constructor( * This method never fails since the underlying property defaults to empty string if it's not * defined in the context. */ - fun hasProfileIdThat(): Subject = assertThat(actual.profileId) + fun hasProfileIdThat(): LiteProtoSubject = assertThat(actual.profileId) companion object { /** diff --git a/testing/src/test/java/org/oppia/android/testing/profile/ProfileTestHelperTest.kt b/testing/src/test/java/org/oppia/android/testing/profile/ProfileTestHelperTest.kt index 3683a0016af..abe33ac86a7 100644 --- a/testing/src/test/java/org/oppia/android/testing/profile/ProfileTestHelperTest.kt +++ b/testing/src/test/java/org/oppia/android/testing/profile/ProfileTestHelperTest.kt @@ -164,7 +164,7 @@ class ProfileTestHelperTest { } @Test - fun testUpdateProfile_updateProfileType_checkIsSuccessful() { + fun testUpdateProfile_updateProfileType_profileTypeShouldBeUpdated() { profileTestHelper.addOnlyAdminProfile() val profileId = profileManagementController.getCurrentProfileId() val updateProvider = profileTestHelper.updateProfileType(profileId!!, ProfileType.SUPERVISOR)