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 19da63a9d68..6ced14b9960 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 @@ -28,7 +28,6 @@ import androidx.compose.ui.res.integerResource import androidx.compose.ui.unit.dp import androidx.databinding.ObservableList import androidx.fragment.app.Fragment -import androidx.lifecycle.Observer import org.oppia.android.R import org.oppia.android.app.classroom.classroomlist.AllClassroomsHeaderText import org.oppia.android.app.classroom.classroomlist.ClassroomList @@ -46,7 +45,6 @@ import org.oppia.android.app.home.promotedlist.ComingSoonTopicListViewModel import org.oppia.android.app.home.promotedlist.PromotedStoryListViewModel import org.oppia.android.app.home.topiclist.AllTopicsViewModel import org.oppia.android.app.home.topiclist.TopicSummaryViewModel -import org.oppia.android.app.model.AppStartupState import org.oppia.android.app.model.ClassroomSummary import org.oppia.android.app.model.LessonThumbnail import org.oppia.android.app.model.LessonThumbnailGraphic @@ -61,8 +59,6 @@ import org.oppia.android.domain.oppialogger.analytics.AnalyticsController import org.oppia.android.domain.profile.ProfileManagementController import org.oppia.android.domain.topic.TopicListController import org.oppia.android.domain.translation.TranslationController -import org.oppia.android.util.data.AsyncResult -import org.oppia.android.util.data.DataProviders.Companion.toLiveData import org.oppia.android.util.locale.OppiaLocale import org.oppia.android.util.parser.html.StoryHtmlParserEntityType import org.oppia.android.util.parser.html.TopicHtmlParserEntityType @@ -159,8 +155,6 @@ class ClassroomListFragmentPresenter @Inject constructor( } ) - logAppOnboardedEvent() - return binding.root } @@ -265,38 +259,6 @@ class ClassroomListFragmentPresenter @Inject constructor( } } - private fun logAppOnboardedEvent() { - val startupStateProvider = appStartupStateController.getAppStartupState() - val liveData = startupStateProvider.toLiveData() - liveData.observe( - activity, - object : Observer> { - override fun onChanged(startUpStateResult: AsyncResult?) { - when (startUpStateResult) { - null, is AsyncResult.Pending -> { - // Do nothing. - } - is AsyncResult.Success -> { - liveData.removeObserver(this) - - if (startUpStateResult.value.startupMode == - AppStartupState.StartupMode.USER_NOT_YET_ONBOARDED - ) { - analyticsController.logAppOnboardedEvent(profileId) - } - } - is AsyncResult.Failure -> { - oppiaLogger.e( - "ClassroomListFragment", - "Failed to retrieve app startup state" - ) - } - } - } - } - ) - } - private fun logHomeActivityEvent() { analyticsController.logImportantEvent( oppiaLogger.createOpenHomeContext(), 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 b3ef5d04e3f..17d41e62f1f 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 @@ -5,7 +5,6 @@ import android.view.View import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment -import androidx.lifecycle.Observer import androidx.recyclerview.widget.GridLayoutManager import org.oppia.android.R import org.oppia.android.app.fragment.FragmentScope @@ -13,7 +12,6 @@ import org.oppia.android.app.home.promotedlist.ComingSoonTopicListViewModel import org.oppia.android.app.home.promotedlist.PromotedStoryListViewModel import org.oppia.android.app.home.topiclist.AllTopicsViewModel import org.oppia.android.app.home.topiclist.TopicSummaryViewModel -import org.oppia.android.app.model.AppStartupState import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.TopicSummary import org.oppia.android.app.recyclerview.BindableAdapter @@ -25,14 +23,11 @@ import org.oppia.android.databinding.HomeFragmentBinding import org.oppia.android.databinding.PromotedStoryListBinding import org.oppia.android.databinding.TopicSummaryViewBinding import org.oppia.android.databinding.WelcomeBinding -import org.oppia.android.domain.onboarding.AppStartupStateController import org.oppia.android.domain.oppialogger.OppiaLogger import org.oppia.android.domain.oppialogger.analytics.AnalyticsController import org.oppia.android.domain.profile.ProfileManagementController import org.oppia.android.domain.topic.TopicListController import org.oppia.android.domain.translation.TranslationController -import org.oppia.android.util.data.AsyncResult -import org.oppia.android.util.data.DataProviders.Companion.toLiveData import org.oppia.android.util.parser.html.StoryHtmlParserEntityType import org.oppia.android.util.parser.html.TopicHtmlParserEntityType import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId @@ -53,7 +48,6 @@ class HomeFragmentPresenter @Inject constructor( private val dateTimeUtil: DateTimeUtil, private val translationController: TranslationController, private val multiTypeBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory, - private val appStartupStateController: AppStartupStateController ) { private val routeToTopicPlayStoryListener = activity as RouteToTopicPlayStoryListener private lateinit var binding: HomeFragmentBinding @@ -103,45 +97,9 @@ class HomeFragmentPresenter @Inject constructor( it.viewModel = homeViewModel } - logAppOnboardedEvent() - return binding.root } - private fun logAppOnboardedEvent() { - val startupStateProvider = appStartupStateController.getAppStartupState() - val liveData = startupStateProvider.toLiveData() - liveData.observe( - activity, - object : Observer> { - override fun onChanged(startUpStateResult: AsyncResult?) { - when (startUpStateResult) { - null, is AsyncResult.Pending -> { - // Do nothing - } - is AsyncResult.Success -> { - liveData.removeObserver(this) - - if (startUpStateResult.value.startupMode == - AppStartupState.StartupMode.USER_NOT_YET_ONBOARDED - ) { - analyticsController.logAppOnboardedEvent( - ProfileId.newBuilder().setInternalId(internalProfileId).build() - ) - } - } - is AsyncResult.Failure -> { - oppiaLogger.e( - "HomeFragment", - "Failed to retrieve app startup state" - ) - } - } - } - } - ) - } - private fun createRecyclerViewAdapter(): BindableAdapter { return multiTypeBuilderFactory.create { viewModel -> when (viewModel) { diff --git a/app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt b/app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt index 3f69bfff0d6..2d529beffd9 100644 --- a/app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt +++ b/app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt @@ -27,7 +27,6 @@ import org.oppia.android.app.application.testing.TestingBuildFlavorModule import org.oppia.android.app.devoptions.DeveloperOptionsModule import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.model.EventLog -import org.oppia.android.app.model.EventLog.Context.ActivityContextCase.COMPLETE_APP_ONBOARDING import org.oppia.android.app.model.EventLog.Context.ActivityContextCase.OPEN_HOME import org.oppia.android.app.model.ProfileId import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule @@ -141,18 +140,6 @@ class HomeActivityLocalTest { } } - @Test - fun testHomeActivity_onFirstLaunch_logsCompletedOnboardingEvent() { - setUpTestApplicationComponent() - launch(createHomeActivityIntent(profileId)).use { - testCoroutineDispatchers.runCurrent() - val event = fakeAnalyticsEventLogger.getMostRecentEvent() - - assertThat(event.priority).isEqualTo(EventLog.Priority.OPTIONAL) - assertThat(event.context.activityContextCase).isEqualTo(COMPLETE_APP_ONBOARDING) - } - } - @Test fun testHomeActivity_onSubsequentLaunch_doesNotLogCompletedOnboardingEvent() { executeInPreviousAppInstance { testComponent -> diff --git a/domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt b/domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt index ee30f69b061..43e959982c6 100644 --- a/domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt +++ b/domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt @@ -6,8 +6,10 @@ import org.oppia.android.app.model.AppStartupState.StartupMode import org.oppia.android.app.model.BuildFlavor import org.oppia.android.app.model.DeprecationResponseDatabase import org.oppia.android.app.model.OnboardingState +import org.oppia.android.app.model.ProfileId import org.oppia.android.data.persistence.PersistentCacheStore import org.oppia.android.domain.oppialogger.OppiaLogger +import org.oppia.android.domain.oppialogger.analytics.AnalyticsController import org.oppia.android.util.data.DataProvider import org.oppia.android.util.data.DataProviders.Companion.combineWith import org.oppia.android.util.extensions.getStringFromBundle @@ -31,6 +33,7 @@ class AppStartupStateController @Inject constructor( private val deprecationController: DeprecationController, @EnableAppAndOsDeprecation private val enableAppAndOsDeprecation: Provider>, + private val analyticsController: AnalyticsController, ) { private val onboardingFlowStore by lazy { cacheStoreFactory.create("on_boarding_flow", OnboardingState.getDefaultInstance()) @@ -65,8 +68,9 @@ class AppStartupStateController @Inject constructor( * Note that this does not notify existing subscribers of the changed state, nor can future * subscribers observe this state until the app restarts. */ - fun markOnboardingFlowCompleted() { + fun markOnboardingFlowCompleted(profileId: ProfileId? = null) { updateOnboardingState { alreadyOnboardedApp = true } + logAppOnboardedEvent(profileId) } /** @@ -190,4 +194,8 @@ class AppStartupStateController @Inject constructor( expirationDate?.isBeforeToday() ?: true } else false } + + private fun logAppOnboardedEvent(profileId: ProfileId?) { + analyticsController.logAppOnboardedEvent(profileId) + } } diff --git a/domain/src/main/java/org/oppia/android/domain/onboarding/BUILD.bazel b/domain/src/main/java/org/oppia/android/domain/onboarding/BUILD.bazel index 0c56bb8f283..34dc94addea 100644 --- a/domain/src/main/java/org/oppia/android/domain/onboarding/BUILD.bazel +++ b/domain/src/main/java/org/oppia/android/domain/onboarding/BUILD.bazel @@ -15,6 +15,7 @@ kt_android_library( ":exploration_meta_data_retriever", "//data/src/main/java/org/oppia/android/data/persistence:cache_store", "//domain/src/main/java/org/oppia/android/domain/oppialogger:oppia_logger", + "//domain/src/main/java/org/oppia/android/domain/oppialogger/analytics:controller", "//model/src/main/proto:deprecation_java_proto_lite", "//model/src/main/proto:onboarding_java_proto_lite", "//third_party:javax_inject_javax_inject", diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel index 3a91ed280e8..f27fbd4b280 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel @@ -20,7 +20,7 @@ kt_android_library( srcs = [ "AnalyticsController.kt", ], - visibility = ["//domain/src/main/java/org/oppia/android/domain/oppialogger:__subpackages__"], + visibility = ["//:oppia_api_visibility"], deps = [ "//:dagger", "//data/src/main/java/org/oppia/android/data/backends/gae:network_interceptors", diff --git a/domain/src/test/java/org/oppia/android/domain/onboarding/AppStartupStateControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/onboarding/AppStartupStateControllerTest.kt index 92c96177489..64aafa421d5 100644 --- a/domain/src/test/java/org/oppia/android/domain/onboarding/AppStartupStateControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/onboarding/AppStartupStateControllerTest.kt @@ -26,6 +26,7 @@ import org.oppia.android.app.model.AppStartupState.StartupMode.USER_NOT_YET_ONBO import org.oppia.android.app.model.BuildFlavor import org.oppia.android.app.model.DeprecationNoticeType import org.oppia.android.app.model.DeprecationResponse +import org.oppia.android.app.model.EventLog import org.oppia.android.app.model.OnboardingState import org.oppia.android.app.model.PlatformParameter import org.oppia.android.data.persistence.PersistentCacheStore @@ -41,6 +42,7 @@ import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule import org.oppia.android.domain.platformparameter.PlatformParameterController import org.oppia.android.domain.platformparameter.PlatformParameterModule import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule +import org.oppia.android.testing.FakeAnalyticsEventLogger import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.data.DataProviderTestMonitor import org.oppia.android.testing.junit.OppiaParameterizedTestRunner @@ -51,6 +53,7 @@ import org.oppia.android.testing.junit.ParameterizedRobolectricTestRunner import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers import org.oppia.android.testing.threading.TestDispatcherModule +import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.data.DataProvidersInjector import org.oppia.android.util.data.DataProvidersInjectorProvider import org.oppia.android.util.locale.LocaleProdModule @@ -87,6 +90,7 @@ class AppStartupStateControllerTest { @Inject lateinit var platformParameterController: PlatformParameterController @Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers @Inject lateinit var monitorFactory: DataProviderTestMonitor.Factory + @Inject lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger @Parameter lateinit var initialFlavorName: String // TODO(#3792): Remove this usage of Locale (probably by introducing a test utility in the locale @@ -122,6 +126,18 @@ class AppStartupStateControllerTest { assertThat(mode.startupMode).isEqualTo(USER_NOT_YET_ONBOARDED) } + @Test + fun testController_afterSettingAppOnboarded_logsCompletedOnboardingEvent() { + setUpDefaultTestApplicationComponent() + appStartupStateController.markOnboardingFlowCompleted() + testCoroutineDispatchers.runCurrent() + + val event = fakeAnalyticsEventLogger.getMostRecentEvent() + assertThat(event.priority).isEqualTo(EventLog.Priority.OPTIONAL) + assertThat(event.context.activityContextCase) + .isEqualTo(EventLog.Context.ActivityContextCase.COMPLETE_APP_ONBOARDING) + } + @Test fun testController_settingAppOnboarded_observedNewController_userOnboardedApp() { // Simulate the previous app already having completed onboarding. @@ -1063,7 +1079,7 @@ class AppStartupStateControllerTest { ExpirationMetaDataRetrieverModule::class, // Use real implementation to test closer to prod. LoggingIdentifierModule::class, ApplicationLifecycleModule::class, SyncStatusModule::class, PlatformParameterModule::class, - PlatformParameterSingletonModule::class + PlatformParameterSingletonModule::class, AssetModule::class ] ) interface TestApplicationComponent : DataProvidersInjector { diff --git a/domain/src/test/java/org/oppia/android/domain/onboarding/BUILD.bazel b/domain/src/test/java/org/oppia/android/domain/onboarding/BUILD.bazel index 16993f30a2c..7d59aa2021f 100644 --- a/domain/src/test/java/org/oppia/android/domain/onboarding/BUILD.bazel +++ b/domain/src/test/java/org/oppia/android/domain/onboarding/BUILD.bazel @@ -28,6 +28,7 @@ oppia_android_test( "//third_party:org_mockito_mockito-core", "//third_party:org_robolectric_robolectric", "//third_party:robolectric_android-all", + "//utility/src/main/java/org/oppia/android/util/caching:asset_prod_module", "//utility/src/main/java/org/oppia/android/util/locale:prod_module", "//utility/src/main/java/org/oppia/android/util/logging:prod_module", "//utility/src/main/java/org/oppia/android/util/networking:debug_module",