fix: course load exception and removed ui result class

Ahsan Arif · Ahsan Arif · commit addb8a5752a4 · 2025-02-12T15:10:20.000+05:00
diff --git a/core/src/main/java/org/openedx/core/domain/model/CourseInfoOverview.kt b/core/src/main/java/org/openedx/core/domain/model/CourseInfoOverview.kt
@@ -11,7 +11,7 @@ data class CourseInfoOverview(
     val number: String,
     val org: String,
     val start: Date?,
-    val startDisplay: String,
+    val startDisplay: String?,
     val startType: String,
     val end: Date?,
     val isSelfPaced: Boolean,
@@ -27,7 +27,7 @@ data class CourseInfoOverview(
         number = number,
         org = org,
         start = start,
-        startDisplay = startDisplay,
+        startDisplay = startDisplay ?: "",
         startType = startType,
         end = end,
         isSelfPaced = isSelfPaced,
diff --git a/core/src/main/java/org/openedx/core/ui/Result.kt b/core/src/main/java/org/openedx/core/ui/Result.kt
diff --git a/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt b/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
@@ -47,11 +47,30 @@ class CourseRepository(
 
     suspend fun getCourseStructureFlow(courseId: String, forceRefresh: Boolean = true): Flow<CourseStructure> =
         channelFlowWithAwait {
-            // Send the local result first
-            trySend(getCourseStructureFromCache(courseId))
-            // Send the updated network result if needed
-            if (networkConnection.isOnline() && forceRefresh) {
-                trySend(getCourseStructure(courseId, true))
+            var hasCourseStructure = false
+            val cachedCourseStructure =
+                courseStructure[courseId] ?: (courseDao.getCourseStructureById(courseId)
+                    ?.mapToDomain())
+            if (cachedCourseStructure != null) {
+                hasCourseStructure = true
+                trySend(cachedCourseStructure)
+            }
+            val fetchRemoteCourse = !hasCourseStructure || forceRefresh
+            if (networkConnection.isOnline() && fetchRemoteCourse) {
+                val response = api.getCourseStructure(
+                    "stale-if-error=0",
+                    "v4",
+                    preferencesManager.user?.username,
+                    courseId
+                )
+                courseDao.insertCourseStructureEntity(response.mapToRoomEntity())
+                val courseDomainModel = response.mapToDomain()
+                courseStructure[courseId] = courseDomainModel
+                trySend(courseDomainModel)
+                hasCourseStructure = true
+            }
+            if (!hasCourseStructure) {
+                throw NoCachedDataException()
             }
         }
 
diff --git a/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt b/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt
@@ -15,7 +15,7 @@ class CourseInteractor(
     suspend fun getCourseStructureFlow(
         courseId: String,
         forceRefresh: Boolean = true
-    ): Flow<CourseStructure> {
+    ): Flow<CourseStructure?> {
         return repository.getCourseStructureFlow(courseId, forceRefresh)
     }
 
@@ -30,7 +30,7 @@ class CourseInteractor(
         return repository.getCourseStructureFromCache(courseId)
     }
 
-    suspend fun getEnrollmentDetailsFlow(courseId: String): Flow<CourseEnrollmentDetails> {
+    suspend fun getEnrollmentDetailsFlow(courseId: String): Flow<CourseEnrollmentDetails?> {
         return repository.getEnrollmentDetailsFlow(courseId)
     }
 
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
@@ -13,8 +13,8 @@ import kotlinx.coroutines.flow.SharedFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.combine
-import kotlinx.coroutines.flow.take
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import org.openedx.core.config.Config
@@ -38,9 +38,6 @@ import org.openedx.core.system.notifier.CourseOpenBlock
 import org.openedx.core.system.notifier.CourseStructureUpdated
 import org.openedx.core.system.notifier.RefreshDates
 import org.openedx.core.system.notifier.RefreshDiscussions
-import org.openedx.core.ui.Result
-import org.openedx.core.ui.asResult
-import org.openedx.core.ui.isLoading
 import org.openedx.core.worker.CalendarSyncScheduler
 import org.openedx.course.DatesShiftedSnackBar
 import org.openedx.course.domain.interactor.CourseInteractor
@@ -170,45 +167,40 @@ class CourseContainerViewModel(
         // If data is already loaded, do nothing
         if (_dataReady.value != null) return
 
+        _showProgress.value = true
+
         viewModelScope.launch {
-            val courseFlow = interactor.getCourseStructureFlow(courseId)
-            val enrollmentFlow = interactor.getEnrollmentDetailsFlow(courseId)
-            // Combining the first emission from both flows, ensuring they emit at least once before combining
-            courseFlow.take(1).combine(enrollmentFlow.take(1)) { course, enrollment ->
-                course to enrollment
-            }.asResult().collect { result ->
-                _showProgress.value = result.isLoading
-                if (result is Result.Success) {
-                    result.data.let { (structure, enrollment) ->
-                        processCourseData(structure, enrollment)
+            try {
+                val courseStructureFlow = interactor.getCourseStructureFlow(courseId)
+                    .catch { e ->
+                        handleFetchError(e)
+                        emit(null)
                     }
-                }
-                if (result is Result.Error) {
-                    result.exception?.let { e ->
-                        e.printStackTrace()
-                        if (isNetworkRelatedError(e)) {
-                            _errorMessage.value =
-                                resourceManager.getString(CoreR.string.core_error_no_connection)
-                        } else {
-                            _courseAccessStatus.value = CourseAccessError.UNKNOWN
-                        }
-                    } ?: run {
-                        _courseAccessStatus.value = CourseAccessError.UNKNOWN
+                val courseDetailsFlow = interactor.getEnrollmentDetailsFlow(courseId)
+                    .catch { emit(null) }
+                courseStructureFlow.combine(courseDetailsFlow) { courseStructure, courseEnrollmentDetails ->
+                    courseStructure to courseEnrollmentDetails
+                }.collect { (courseStructure, courseEnrollmentDetails) ->
+                    when {
+                        courseEnrollmentDetails != null -> handleCourseEnrollment(courseEnrollmentDetails)
+                        courseStructure != null -> handleCourseStructureOnly(courseStructure)
+                        else -> _courseAccessStatus.value = CourseAccessError.UNKNOWN
                     }
                 }
+            } catch (e: Exception) {
+                handleFetchError(e)
             }
         }
     }
 
-    private fun processCourseData(
-        courseStructure: CourseStructure,
-        courseDetails: CourseEnrollmentDetails
-    ) {
+    /**
+     * Handles the scenario where [CourseEnrollmentDetails] is successfully fetched.
+     */
+    private fun handleCourseEnrollment(courseDetails: CourseEnrollmentDetails) {
         _courseDetails = courseDetails
         courseName = courseDetails.courseInfoOverview.name
-        val courseImage = courseDetails.courseInfoOverview.media?.image?.large
-            ?: courseStructure.media?.image?.large
-        loadCourseImage(courseImage)
+        loadCourseImage(courseDetails.courseInfoOverview.media?.image?.large)
+
         if (courseDetails.hasAccess.isFalse()) {
             _dataReady.value = false
             _courseAccessStatus.value = when {
@@ -233,6 +225,35 @@ class CourseContainerViewModel(
         }
     }
 
+    /**
+     * Handles the scenario where we only have [CourseStructure] but no enrollment details.
+     */
+    private fun handleCourseStructureOnly(courseStructure: CourseStructure) {
+        loadCourseImage(courseStructure.media?.image?.large)
+        _courseAccessStatus.value = CourseAccessError.NONE
+        _isNavigationEnabled.value = true
+        _calendarSyncUIState.update { state ->
+            state.copy(isCalendarSyncEnabled = isCalendarSyncEnabled())
+        }
+        if (resumeBlockId.isNotEmpty()) {
+            viewModelScope.launch {
+                delay(500L)
+                courseNotifier.send(CourseOpenBlock(resumeBlockId))
+            }
+        }
+        _dataReady.value = true
+    }
+
+    private fun handleFetchError(e: Throwable) {
+        e.printStackTrace()
+        if (isNetworkRelatedError(e)) {
+            _errorMessage.value = resourceManager.getString(CoreR.string.core_error_no_connection)
+        } else {
+            _courseAccessStatus.value = CourseAccessError.UNKNOWN
+        }
+        _showProgress.value = false
+    }
+
     private fun isNetworkRelatedError(e: Throwable): Boolean {
         return e.isInternetError() || e is NoCachedDataException
     }
diff --git a/course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt b/course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt
@@ -9,7 +9,6 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.combine
-import kotlinx.coroutines.flow.take
 import kotlinx.coroutines.launch
 import org.openedx.core.BlockType
 import org.openedx.core.R
@@ -35,9 +34,6 @@ import org.openedx.core.system.notifier.CourseDatesShifted
 import org.openedx.core.system.notifier.CourseNotifier
 import org.openedx.core.system.notifier.CourseOpenBlock
 import org.openedx.core.system.notifier.CourseStructureUpdated
-import org.openedx.core.ui.Result
-import org.openedx.core.ui.asResult
-import org.openedx.core.ui.error
 import org.openedx.course.domain.interactor.CourseInteractor
 import org.openedx.course.presentation.CourseAnalytics
 import org.openedx.course.presentation.CourseAnalyticsEvent
@@ -192,26 +188,18 @@ class CourseOutlineViewModel(
                 val courseStatusFlow = interactor.getCourseStatusFlow(courseId)
                 val courseDatesFlow = interactor.getCourseDatesFlow(courseId)
                 combine(
-                    courseStructureFlow.take(1),
-                    courseStatusFlow.take(1),
-                    courseDatesFlow.take(1)
+                    courseStructureFlow, courseStatusFlow, courseDatesFlow
                 ) { courseStructure, courseStatus, courseDatesResult ->
                     Triple(courseStructure, courseStatus, courseDatesResult)
-                }.asResult().collect {
-                    if (it is Result.Success) {
-                        it.data.let { (courseStructure, courseStatus, courseDatesResult) ->
-                            val blocks = courseStructure.blockData
-                            val datesBannerInfo = courseDatesResult.courseBanner
+                }.collect { (courseStructure, courseStatus, courseDates) ->
+                    if (courseStructure == null) return@collect
+                    val blocks = courseStructure.blockData
+                    val datesBannerInfo = courseDates.courseBanner
 
-                            checkIfCalendarOutOfDate(courseDatesResult.datesSection.values.flatten())
-                            updateOutdatedOfflineXBlocks(courseStructure)
+                    checkIfCalendarOutOfDate(courseDates.datesSection.values.flatten())
+                    updateOutdatedOfflineXBlocks(courseStructure)
 
-                            initializeCourseData(blocks, courseStructure, courseStatus, datesBannerInfo)
-                        }
-                    }
-                    if (it is Result.Error) {
-                        handleCourseDataError(it.error)
-                    }
+                    initializeCourseData(blocks, courseStructure, courseStatus, datesBannerInfo)
                 }
             } catch (e: Exception) {
                 handleCourseDataError(e)
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
@@ -41,8 +41,6 @@ import org.openedx.core.domain.model.EnrollmentDetails
 import org.openedx.core.system.connection.NetworkConnection
 import org.openedx.core.system.notifier.CourseNotifier
 import org.openedx.core.system.notifier.CourseStructureUpdated
-import org.openedx.core.ui.Result
-import org.openedx.core.ui.asResult
 import org.openedx.core.worker.CalendarSyncScheduler
 import org.openedx.course.domain.interactor.CourseInteractor
 import org.openedx.course.presentation.CourseAnalytics
@@ -237,11 +235,11 @@ class CourseContainerViewModelTest {
         )
         every { networkConnection.isOnline() } returns true
         coEvery {
-            interactor.getCourseStructureFlow(any(), any()).asResult()
-        } returns flowOf(Result.Error(Exception()))
+            interactor.getCourseStructureFlow(any(), any())
+        } returns flowOf(courseStructure)
         coEvery {
-            interactor.getEnrollmentDetailsFlow(any()).asResult()
-        } returns flowOf(Result.Error(Exception()))
+            interactor.getEnrollmentDetailsFlow(any())
+        } throws Exception()
         every {
             analytics.logScreenEvent(
                 CourseAnalyticsEvent.DASHBOARD.eventName,
diff --git a/course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt b/course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt
@@ -55,7 +55,6 @@ import org.openedx.core.presentation.CoreAnalyticsEvent
 import org.openedx.core.system.connection.NetworkConnection
 import org.openedx.core.system.notifier.CourseNotifier
 import org.openedx.core.system.notifier.CourseStructureUpdated
-import org.openedx.core.ui.asResult
 import org.openedx.course.domain.interactor.CourseInteractor
 import org.openedx.course.presentation.CourseAnalytics
 import org.openedx.course.presentation.CourseRouter
@@ -307,7 +306,7 @@ class CourseOutlineViewModelTest {
         coEvery { interactor.getCourseStructureFlow(any(), any()) } returns flowOf(courseStructure)
         every { networkConnection.isOnline() } returns true
         every { downloadDao.getAllDataFlow() } returns flow { emit(emptyList()) }
-        coEvery { interactor.getCourseStatusFlow(any()).asResult() } throws Exception()
+        coEvery { interactor.getCourseStatusFlow(any()) } throws Exception()
         val viewModel = CourseOutlineViewModel(
             "",
             "",
diff --git a/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesViewModel.kt b/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesViewModel.kt
@@ -58,8 +58,18 @@ class AllEnrolledCoursesViewModel(
 
     init {
         collectDiscoveryNotifier()
-        loadCachedCourses()
-        getCourses(currentFilter.value, showLoadingProgress = false)
+        loadInitialCourses()
+    }
+
+    private fun loadInitialCourses() {
+        viewModelScope.launch {
+            _uiState.update { it.copy(showProgress = true) }
+            val cachedList = interactor.getEnrolledCoursesFromCache()
+            if (cachedList.isNotEmpty()) {
+                _uiState.update { it.copy(courses = cachedList.toList(), showProgress = false) }
+            }
+            getCourses(showLoadingProgress = false)
+        }
     }
 
     fun getCourses(courseStatusFilter: CourseStatusFilter? = null, showLoadingProgress: Boolean = true) {
@@ -107,13 +117,6 @@ class AllEnrolledCoursesViewModel(
         }
     }
 
-    private fun loadCachedCourses() {
-        viewModelScope.launch {
-            val cachedList = interactor.getEnrolledCoursesFromCache()
-            _uiState.update { it.copy(courses = cachedList.toList()) }
-        }
-    }
-
     private fun internalLoadingCourses(courseStatusFilter: CourseStatusFilter? = null) {
         if (courseStatusFilter != null) {
             page = 1
diff --git a/dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryViewModel.kt b/dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryViewModel.kt
@@ -67,18 +67,22 @@ class DashboardGalleryViewModel(
     fun getCourses() {
         viewModelScope.launch {
             try {
-                isLoading = true
-                val courseEnrollments = fileUtil.getObjectFromFile<CourseEnrollments>()
-                if (courseEnrollments == null) {
-                    _uiState.value = DashboardGalleryUIState.Empty
+                val cachedCourseEnrollments = fileUtil.getObjectFromFile<CourseEnrollments>()
+                if (cachedCourseEnrollments == null) {
+                    if (networkConnection.isOnline()) {
+                        _uiState.value = DashboardGalleryUIState.Loading
+                    } else {
+                        _uiState.value = DashboardGalleryUIState.Empty
+                    }
                 } else {
                     _uiState.value =
                         DashboardGalleryUIState.Courses(
-                            courseEnrollments.mapToDomain(),
+                            cachedCourseEnrollments.mapToDomain(),
                             corePreferences.isRelativeDatesEnabled
                         )
                 }
                 if (networkConnection.isOnline()) {
+                    isLoading = true
                     val pageSize = if (windowSize.isTablet) {
                         PAGE_SIZE_TABLET
                     } else {

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ class CourseInteractor(`
`15`	`15`	`suspend fun getCourseStructureFlow(`
`16`	`16`	`courseId: String,`
`17`	`17`	`forceRefresh: Boolean = true`
`18`		`- ): Flow<CourseStructure> {`
	`18`	`+ ): Flow<CourseStructure?> {`
`19`	`19`	`return repository.getCourseStructureFlow(courseId, forceRefresh)`
`20`	`20`	`}`
`21`	`21`
`@@ -30,7 +30,7 @@ class CourseInteractor(`
`30`	`30`	`return repository.getCourseStructureFromCache(courseId)`
`31`	`31`	`}`
`32`	`32`
`33`		`- suspend fun getEnrollmentDetailsFlow(courseId: String): Flow<CourseEnrollmentDetails> {`
	`33`	`+ suspend fun getEnrollmentDetailsFlow(courseId: String): Flow<CourseEnrollmentDetails?> {`
`34`	`34`	`return repository.getEnrollmentDetailsFlow(courseId)`
`35`	`35`	`}`
`36`	`36`