From 81e62b1090cc7857fec572d4d82fda34db23bb5c Mon Sep 17 00:00:00 2001 From: Hamza Israr Date: Thu, 20 Feb 2025 17:44:49 +0500 Subject: [PATCH 1/3] fix: Main Dashboard Learn Tab Events The MyCourses or MyPrograms screen event was being triggered whenever uiState updated, as it was being collected in the ViewModel. Fixed this by linking it to the tap click event instead. --- .../learn/presentation/LearnViewModel.kt | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/dashboard/src/main/java/org/openedx/learn/presentation/LearnViewModel.kt b/dashboard/src/main/java/org/openedx/learn/presentation/LearnViewModel.kt index f985f4205..27a0f32e7 100644 --- a/dashboard/src/main/java/org/openedx/learn/presentation/LearnViewModel.kt +++ b/dashboard/src/main/java/org/openedx/learn/presentation/LearnViewModel.kt @@ -50,15 +50,6 @@ class LearnViewModel( val getProgramFragment get() = dashboardRouter.getProgramFragment() init { - viewModelScope.launch { - _uiState.collect { uiState -> - if (uiState.learnType == LearnType.COURSES) { - logMyCoursesTabClickedEvent() - } else { - logMyProgramsTabClickedEvent() - } - } - } viewModelScope.launch { pushNotifier.notifier.collect { event -> if (event is PushEvent.RefreshBadgeCount) { @@ -66,12 +57,16 @@ class LearnViewModel( } } } + logTabClickedEvent(_uiState.value.learnType) checkNotificationCount() } fun updateLearnType(learnType: LearnType) { viewModelScope.launch { - _uiState.update { it.copy(learnType = learnType) } + if (learnType != _uiState.value.learnType) { + _uiState.update { it.copy(learnType = learnType) } + logTabClickedEvent(learnType) + } } } @@ -93,12 +88,11 @@ class LearnViewModel( _uiState.update { it.copy(hasUnreadNotifications = false) } } - private fun logMyCoursesTabClickedEvent() { - logScreenEvent(DashboardAnalyticsEvent.MY_COURSES) - } - - private fun logMyProgramsTabClickedEvent() { - logScreenEvent(DashboardAnalyticsEvent.MY_PROGRAMS) + private fun logTabClickedEvent(learnType: LearnType) { + when (learnType) { + LearnType.COURSES -> logScreenEvent(DashboardAnalyticsEvent.MY_COURSES) + LearnType.PROGRAMS -> logScreenEvent(DashboardAnalyticsEvent.MY_PROGRAMS) + } } private fun logScreenEvent(event: DashboardAnalyticsEvent) { From b8dc7f7d2049a78b477543c91222703ae70e252a Mon Sep 17 00:00:00 2001 From: Hamza Israr Date: Thu, 20 Feb 2025 17:45:26 +0500 Subject: [PATCH 2/3] fix: Events Triggered on Screen Orientation Change Some events were being triggered on orientation change since most were placed in onCreate block. Moved them to the ViewModel's init block to resolve the issue. --- app/src/main/java/org/openedx/app/AppActivity.kt | 1 - app/src/main/java/org/openedx/app/AppViewModel.kt | 10 ++++++---- app/src/main/java/org/openedx/app/MainViewModel.kt | 5 ++++- app/src/test/java/org/openedx/AppViewModelTest.kt | 11 +++++------ .../container/CourseContainerViewModel.kt | 5 ++--- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/openedx/app/AppActivity.kt b/app/src/main/java/org/openedx/app/AppActivity.kt index e03d9f2cd..2f4b57872 100644 --- a/app/src/main/java/org/openedx/app/AppActivity.kt +++ b/app/src/main/java/org/openedx/app/AppActivity.kt @@ -85,7 +85,6 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder { installSplashScreen() binding = ActivityAppBinding.inflate(layoutInflater) lifecycle.addObserver(viewModel) - viewModel.logAppLaunchEvent() setContentView(binding.root) val container = binding.rootLayout diff --git a/app/src/main/java/org/openedx/app/AppViewModel.kt b/app/src/main/java/org/openedx/app/AppViewModel.kt index 20b3b0c97..d9b9af6b7 100644 --- a/app/src/main/java/org/openedx/app/AppViewModel.kt +++ b/app/src/main/java/org/openedx/app/AppViewModel.kt @@ -50,13 +50,15 @@ class AppViewModel( val isBranchEnabled get() = config.getBranchConfig().enabled private val canResetAppDirectory get() = preferencesManager.canResetAppDirectory + init { + logAppLaunchEvent() + setUserId(preferencesManager.user) + } + override fun onCreate(owner: LifecycleOwner) { super.onCreate(owner) val user = preferencesManager.user - - setUserId(user) - if (user != null && preferencesManager.pushToken.isNotEmpty()) { SyncFirebaseTokenWorker.schedule(context) } @@ -76,7 +78,7 @@ class AppViewModel( } } - fun logAppLaunchEvent() { + private fun logAppLaunchEvent() { analytics.logEvent( event = AppAnalyticsEvent.LAUNCH.eventName, params = buildMap { diff --git a/app/src/main/java/org/openedx/app/MainViewModel.kt b/app/src/main/java/org/openedx/app/MainViewModel.kt index 45ef687f9..9d1b5c439 100644 --- a/app/src/main/java/org/openedx/app/MainViewModel.kt +++ b/app/src/main/java/org/openedx/app/MainViewModel.kt @@ -38,6 +38,10 @@ class MainViewModel( val isDiscoveryTypeWebView get() = config.getDiscoveryConfig().isViewTypeWebView() val getDiscoveryFragment get() = DiscoveryNavigator(isDiscoveryTypeWebView).getDiscoveryFragment() + init { + logNotificationPermissionStatusEvent() + } + override fun onCreate(owner: LifecycleOwner) { super.onCreate(owner) notifier.notifier @@ -48,7 +52,6 @@ class MainViewModel( } .distinctUntilChanged() .launchIn(viewModelScope) - logNotificationPermissionStatusEvent() } fun enableBottomBar(enable: Boolean) { diff --git a/app/src/test/java/org/openedx/AppViewModelTest.kt b/app/src/test/java/org/openedx/AppViewModelTest.kt index 87a34e790..5755e020b 100644 --- a/app/src/test/java/org/openedx/AppViewModelTest.kt +++ b/app/src/test/java/org/openedx/AppViewModelTest.kt @@ -22,15 +22,15 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule import org.openedx.app.AppAnalytics -import org.openedx.app.deeplink.DeepLinkRouter import org.openedx.app.AppViewModel import org.openedx.app.data.storage.PreferencesManager +import org.openedx.app.deeplink.DeepLinkRouter import org.openedx.app.room.AppDatabase -import org.openedx.core.system.notifier.app.LogoutEvent import org.openedx.core.config.Config import org.openedx.core.config.FirebaseConfig import org.openedx.core.data.model.User import org.openedx.core.system.notifier.app.AppNotifier +import org.openedx.core.system.notifier.app.LogoutEvent import org.openedx.core.utils.FileUtil @ExperimentalCoroutinesApi @@ -55,6 +55,8 @@ class AppViewModelTest { @Before fun before() { Dispatchers.setMain(dispatcher) + every { analytics.logEvent(any(), any()) } returns Unit + every { preferencesManager.user } returns user } @After @@ -65,7 +67,6 @@ class AppViewModelTest { @Test fun setIdSuccess() = runTest { every { analytics.setUserIdForSession(any()) } returns Unit - every { preferencesManager.user } returns user every { notifier.notifier } returns flow { } every { preferencesManager.canResetAppDirectory } returns false every { preferencesManager.pushToken } returns "" @@ -98,7 +99,6 @@ class AppViewModelTest { } every { preferencesManager.clear() } returns Unit every { analytics.setUserIdForSession(any()) } returns Unit - every { preferencesManager.user } returns user every { room.clearAllTables() } returns Unit every { analytics.logoutEvent(true) } returns Unit every { preferencesManager.canResetAppDirectory } returns false @@ -135,7 +135,6 @@ class AppViewModelTest { } every { preferencesManager.clear() } returns Unit every { analytics.setUserIdForSession(any()) } returns Unit - every { preferencesManager.user } returns user every { room.clearAllTables() } returns Unit every { analytics.logoutEvent(true) } returns Unit every { preferencesManager.canResetAppDirectory } returns false @@ -163,7 +162,7 @@ class AppViewModelTest { verify(exactly = 1) { analytics.logoutEvent(true) } verify(exactly = 1) { preferencesManager.clear() } verify(exactly = 1) { analytics.setUserIdForSession(any()) } - verify(exactly = 1) { preferencesManager.user } + verify(exactly = 2) { preferencesManager.user } verify(exactly = 1) { room.clearAllTables() } verify(exactly = 1) { analytics.logoutEvent(true) } } diff --git a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt index f72291493..01f448b1e 100644 --- a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt +++ b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt @@ -234,12 +234,11 @@ class CourseContainerViewModel( } } }.distinctUntilChanged().launchIn(viewModelScope) + + courseDashboardViewed() } fun fetchCourseDetails(isIAPFlow: Boolean = false, isExpiredCoursePurchase: Boolean = false) { - if (isIAPFlow.not()) { - courseDashboardViewed() - } _showProgress.value = true viewModelScope.launch { try { From 779b8fd038704b9820b51c02539fee52d4eb78d4 Mon Sep 17 00:00:00 2001 From: Hamza Israr Date: Thu, 20 Feb 2025 18:56:22 +0500 Subject: [PATCH 3/3] fix: Course Tab Events on Deeplinks The Course Tab events weren't being triggered when navigating via a deeplink; they were only triggered on click events. --- .../container/CourseContainerFragment.kt | 23 +++++---- .../container/CourseContainerViewModel.kt | 1 - .../container/CourseContainerViewModelTest.kt | 51 +------------------ 3 files changed, 15 insertions(+), 60 deletions(-) diff --git a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt index 737d98518..d0768b368 100644 --- a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt +++ b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt @@ -353,17 +353,19 @@ fun CourseDashboard( val refreshing by viewModel.refreshing.collectAsState(true) val courseImage by viewModel.courseImage.collectAsState() val uiMessage by viewModel.uiMessage.collectAsState(null) - val requiredTab = when (openTab.uppercase()) { - CourseContainerTab.HOME.name -> CourseContainerTab.HOME - CourseContainerTab.VIDEOS.name -> CourseContainerTab.VIDEOS - CourseContainerTab.DATES.name -> CourseContainerTab.DATES - CourseContainerTab.DISCUSSIONS.name -> CourseContainerTab.DISCUSSIONS - CourseContainerTab.MORE.name -> CourseContainerTab.MORE - else -> CourseContainerTab.HOME - } + val requiredTabIndex = CourseContainerTab.entries.indexOf( + when (openTab.uppercase()) { + CourseContainerTab.HOME.name -> CourseContainerTab.HOME + CourseContainerTab.VIDEOS.name -> CourseContainerTab.VIDEOS + CourseContainerTab.DATES.name -> CourseContainerTab.DATES + CourseContainerTab.DISCUSSIONS.name -> CourseContainerTab.DISCUSSIONS + CourseContainerTab.MORE.name -> CourseContainerTab.MORE + else -> CourseContainerTab.HOME + } + ) val pagerState = rememberPagerState( - initialPage = CourseContainerTab.entries.indexOf(requiredTab), + initialPage = requiredTabIndex, pageCount = { CourseContainerTab.entries.size } ) val dataReady = viewModel.dataReady.observeAsState() @@ -388,6 +390,7 @@ fun CourseDashboard( LaunchedEffect(pagerState.currentPage) { tabState.animateScrollToItem(pagerState.currentPage) + viewModel.courseContainerTabClickedEvent(pagerState.currentPage) } Column( @@ -450,7 +453,7 @@ fun CourseDashboard( rowState = tabState, pagerState = pagerState, withPager = true, - onTabClicked = viewModel::courseContainerTabClickedEvent + onTabClicked = { } ) } }, diff --git a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt index 01f448b1e..975dfe686 100644 --- a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt +++ b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt @@ -672,7 +672,6 @@ class CourseContainerViewModel( private fun courseDashboardViewed() { logCourseContainerEvent(CourseAnalyticsEvent.DASHBOARD) - courseTabClickedEvent() } private fun courseTabClickedEvent() { diff --git a/course/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt b/course/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt index a55162106..582c6bb2e 100644 --- a/course/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt +++ b/course/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt @@ -7,7 +7,6 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.spyk -import io.mockk.verify import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.emptyFlow @@ -233,6 +232,7 @@ class CourseContainerViewModelTest { coEvery { interactor.getEnrollmentDetails(any()) } returns courseDetails every { imageProcessor.loadImage(any(), any(), any()) } returns Unit every { imageProcessor.applyBlur(any(), any()) } returns mockBitmap + every { courseAnalytics.logScreenEvent(any(), any()) } returns Unit } @After @@ -279,18 +279,6 @@ class CourseContainerViewModelTest { advanceUntilIdle() coVerify(exactly = 1) { interactor.getEnrollmentDetails(any()) } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.DASHBOARD.eventName, - any() - ) - } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.HOME_TAB.eventName, - any() - ) - } assert(!viewModel.refreshing.value) assert(viewModel.courseAccessStatus.value == CourseAccessError.UNKNOWN) } @@ -318,34 +306,11 @@ class CourseContainerViewModelTest { ) every { networkConnection.isOnline() } returns true coEvery { interactor.getEnrollmentDetails(any()) } returns enrollmentDetails - every { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.DASHBOARD.eventName, - any() - ) - } returns Unit - every { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.HOME_TAB.eventName, - any() - ) - } returns Unit + viewModel.fetchCourseDetails() advanceUntilIdle() coVerify(exactly = 1) { interactor.getEnrollmentDetails(any()) } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.DASHBOARD.eventName, - any() - ) - } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.HOME_TAB.eventName, - any() - ) - } assert(viewModel.errorMessage.value == null) assert(!viewModel.refreshing.value) assert(viewModel.courseAccessStatus.value != null) @@ -389,18 +354,6 @@ class CourseContainerViewModelTest { viewModel.fetchCourseDetails() advanceUntilIdle() coVerify(exactly = 0) { courseApi.getEnrollmentDetails(any()) } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.DASHBOARD.eventName, - any() - ) - } - verify(exactly = 1) { - courseAnalytics.logScreenEvent( - CourseAnalyticsEvent.HOME_TAB.eventName, - any() - ) - } assert(viewModel.errorMessage.value == null) assert(!viewModel.refreshing.value)