Fixes multiple onboarding events issue (#5551)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fixes multiple onboarding events issue:

From the previous implementation of the onboarding logs, it was possible
to have multiple onboarding logs belonging to the same user. This
happened because the onboarding flag used when making the log was not
updated until an app restart. For this reason, if a user went back to
the home screen several times or if the user changed orientation from
portrait to landscape and vice-versa, the onboarding complete log would
be triggered every time this happened.

This PR intends to fix this issue by centralizing the onboarding
complete log. It will also allow an optional profileId variable to be
passed as support for Onboarding V2 where onboarding will be tied to an
individual user.

## Manual testing process and results
To confirm that the bug was fixed, I manually performed a test using the
steps outlined below;

1. Clean the Oppia app storage from settings or uninstall the Oppia app
to prime it for the test then reinstall it.
2. Go through the onboarding screens, then select Administrator when you
reach the profile selection screen.
3. This opens the home screen. Click the hamburger menu at the top right
corner of the screen.
4. Click **Developer options** at the bottom of the side-bar menu. 
5. Under the **Event logs** section, click the event logs item. You
should see a list of events logged so far, including a **Complete App
Onboarding** event right above the **Open Home** event.
7. Click the back arrow at the top right corner to go back to the
developer options menu and then click the hamburger menu to open the
side pane again.
8. Click on Home to go back to the home screen and then repeat steps 3
to 8, three more times.
9. Once that is done, reopen the Event Logs screen. With the original
buggy flow, three Open Home and Complete App Onboarding events should be
displayed. With this fix, only one Complete App Onboarding event should
be logged but 3 Open Home events should be displayed.

|Buggy flow|New flow with fix|
|----|----|

|![Screenshot_20241021_102501](https://github.com/user-attachments/assets/8c6aaf59-4e10-4382-895d-619ffc2b1f18)|![Screenshot_20241021_101850](https://github.com/user-attachments/assets/ca576bb2-33e2-4ff5-ae19-8e7b0444474c)|

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Kenneth Murerwa <[email protected]>

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<AsyncResult<AppStartupState>> {
-        override fun onChanged(startUpStateResult: AsyncResult<AppStartupState>?) {
-          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(),

app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt

@@ -5,15 +5,13 @@ 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
 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<AsyncResult<AppStartupState>> {
-        override fun onChanged(startUpStateResult: AsyncResult<AppStartupState>?) {
-          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<HomeItemViewModel> {
     return multiTypeBuilderFactory.create<HomeItemViewModel, ViewType> { viewModel ->
       when (viewModel) {

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<HomeActivity>(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 ->

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<PlatformParameterValue<Boolean>>,
+  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)
+  }
 }

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",

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",

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 {

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",