Skip to content

fix: Course screen requests optimization #473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PavloNetrebchuk wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: Course screen requests optimization #473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
294 changes: 225 additions & 69 deletions course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
View file
Open in desktop

Large diffs are not rendered by default.

28 changes: 22 additions & 6 deletions course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ class CourseInteractor(
private val repository: CourseRepository
) : CourseInteractor {

fun startCourseSession(courseId: String) {
repository.startCourseSession(courseId)
}

suspend fun getCourseStructureFlow(
courseId: String,
forceRefresh: Boolean = true
forceRefresh: Boolean = false
): Flow<CourseStructure?> {
return repository.getCourseStructureFlow(courseId, forceRefresh)
}
Expand All @@ -31,8 +35,11 @@ class CourseInteractor(
return repository.getCourseStructureFromCache(courseId)
}

suspend fun getEnrollmentDetailsFlow(courseId: String): Flow<CourseEnrollmentDetails?> {
return repository.getEnrollmentDetailsFlow(courseId)
suspend fun getEnrollmentDetailsFlow(
courseId: String,
forceRefresh: Boolean = false
): Flow<CourseEnrollmentDetails?> {
return repository.getEnrollmentDetailsFlow(courseId, forceRefresh)
}

suspend fun getEnrollmentDetails(courseId: String): CourseEnrollmentDetails {
Expand Down Expand Up @@ -87,13 +94,22 @@ class CourseInteractor(
}
}

suspend fun getCourseStatusFlow(courseId: String) = repository.getCourseStatusFlow(courseId)
suspend fun getCourseStatusFlow(
courseId: String,
forceRefresh: Boolean = false
) = repository.getCourseStatusFlow(courseId, forceRefresh)

suspend fun getCourseStatus(courseId: String) = repository.getCourseStatus(courseId)

suspend fun getCourseDatesFlow(courseId: String) = repository.getCourseDatesFlow(courseId)
suspend fun getCourseDatesFlow(
courseId: String,
forceRefresh: Boolean = false
) = repository.getCourseDatesFlow(courseId, forceRefresh)

suspend fun getCourseDates(courseId: String) = repository.getCourseDates(courseId)
suspend fun getCourseDates(
courseId: String,
forceRefresh: Boolean = false
) = repository.getCourseDates(courseId, forceRefresh)

suspend fun resetCourseDates(courseId: String) = repository.resetCourseDates(courseId)

Expand Down
2 changes: 1 addition & 1 deletion ...se/src/main/java/org/openedx/course/presentation/assignments/CourseAssignmentViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class CourseAssignmentViewModel(
private fun collectData() {
viewModelScope.launch {
val courseProgressFlow = interactor.getCourseProgress(courseId, false, true)
val courseStructureFlow = interactor.getCourseStructureFlow(courseId)
val courseStructureFlow = interactor.getCourseStructureFlow(courseId, false)

combine(
courseProgressFlow,
Expand Down
7 changes: 0 additions & 7 deletions course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,6 @@ class CourseContainerFragment : Fragment(R.layout.fragment_course_container) {
observe()
}

override fun onResume() {
super.onResume()
if (viewModel.courseAccessStatus.value == CourseAccessError.NONE) {
viewModel.updateData()
}
}

override fun onDestroyView() {
snackBar?.dismiss()
super.onDestroyView()
Expand Down
7 changes: 6 additions & 1 deletion course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,18 @@ class CourseContainerViewModel(

_showProgress.value = true

interactor.startCourseSession(courseId)

viewModelScope.launch {
val courseStructureFlow = interactor.getCourseStructureFlow(courseId)
.catch { e ->
handleFetchError(e)
emit(null)
}
val courseDetailsFlow = interactor.getEnrollmentDetailsFlow(courseId)
val courseDetailsFlow = interactor.getEnrollmentDetailsFlow(
courseId,
forceRefresh = true
)
.catch { emit(null) }
courseStructureFlow.combine(courseDetailsFlow) { courseStructure, courseEnrollmentDetails ->
courseStructure to courseEnrollmentDetails
Expand Down
20 changes: 14 additions & 6 deletions course/src/main/java/org/openedx/course/presentation/dates/CourseDatesViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class CourseDatesViewModel(
courseNotifier.notifier.collect { event ->
when (event) {
is RefreshDates -> {
loadingCourseDatesInternal()
loadingCourseDatesInternal(forceRefresh = true)
}
}
}
Expand All @@ -82,15 +82,23 @@ class CourseDatesViewModel(
}
}

loadingCourseDatesInternal()
loadingCourseDatesInternal(forceRefresh = false)
}

private fun loadingCourseDatesInternal() {
private fun loadingCourseDatesInternal(forceRefresh: Boolean) {
viewModelScope.launch {
try {
courseStructure = interactor.getCourseStructure(courseId = courseId)
courseStructure = if (forceRefresh) {
interactor.getCourseStructure(courseId = courseId, isNeedRefresh = true)
} else {
interactor.getCourseStructure(courseId = courseId)
}
isSelfPaced = courseStructure?.isSelfPaced ?: false
val datesResponse = interactor.getCourseDates(courseId = courseId)
val datesResponse = if (forceRefresh) {
interactor.getCourseDates(courseId = courseId, forceRefresh = true)
} else {
interactor.getCourseDates(courseId = courseId)
}
if (datesResponse.datesSection.isEmpty()) {
_uiState.value = CourseDatesUIState.Error
} else {
Expand All @@ -117,7 +125,7 @@ class CourseDatesViewModel(
viewModelScope.launch {
try {
interactor.resetCourseDates(courseId = courseId)
loadingCourseDatesInternal()
loadingCourseDatesInternal(forceRefresh = true)
courseNotifier.send(CourseDatesShifted)
onResetDates(true)
} catch (e: Exception) {
Expand Down
2 changes: 1 addition & 1 deletion course/src/main/java/org/openedx/course/presentation/progress/CourseProgressViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CourseProgressViewModel(
private fun collectData(isRefresh: Boolean) {
viewModelScope.launch {
val courseProgressFlow = interactor.getCourseProgress(courseId, isRefresh, false)
val courseStructureFlow = interactor.getCourseStructureFlow(courseId)
val courseStructureFlow = interactor.getCourseStructureFlow(courseId, false)

combine(
courseProgressFlow,
Expand Down
12 changes: 7 additions & 5 deletions ...e/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.justRun
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
Expand Down Expand Up @@ -81,6 +82,7 @@ class CourseContainerViewModelTest {
every { corePreferences.appConfig } returns CoreMocks.mockAppConfig
every { courseNotifier.notifier } returns emptyFlow()
every { config.getApiHostURL() } returns "baseUrl"
justRun { interactor.startCourseSession(any()) }
coEvery { interactor.getEnrollmentDetails(any()) } returns CoreMocks.mockCourseEnrollmentDetails
every { imageProcessor.loadImage(any(), any(), any()) } returns Unit
every { imageProcessor.applyBlur(any(), any()) } returns mockBitmap
Expand Down Expand Up @@ -114,7 +116,7 @@ class CourseContainerViewModelTest {
interactor.getCourseStructureFlow(any(), any())
} returns flowOf(null)
coEvery {
interactor.getEnrollmentDetailsFlow(any())
interactor.getEnrollmentDetailsFlow(any(), any())
} returns flow { throw Exception() }
every {
analytics.logScreenEvent(
Expand All @@ -131,7 +133,7 @@ class CourseContainerViewModelTest {
viewModel.fetchCourseDetails()
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getEnrollmentDetailsFlow(any()) }
coVerify(exactly = 1) { interactor.getEnrollmentDetailsFlow(any(), any()) }
verify(exactly = 1) {
analytics.logScreenEvent(
CourseAnalyticsEvent.DASHBOARD.eventName,
Expand Down Expand Up @@ -169,7 +171,7 @@ class CourseContainerViewModelTest {
coEvery { interactor.getCourseStructureFlow(any(), any()) } returns flowOf(
CoreMocks.mockCourseStructure
)
coEvery { interactor.getEnrollmentDetailsFlow(any()) } returns flowOf(
coEvery { interactor.getEnrollmentDetailsFlow(any(), any()) } returns flowOf(
CoreMocks.mockCourseEnrollmentDetails
)
every {
Expand All @@ -187,7 +189,7 @@ class CourseContainerViewModelTest {
viewModel.fetchCourseDetails()
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getEnrollmentDetailsFlow(any()) }
coVerify(exactly = 1) { interactor.getEnrollmentDetailsFlow(any(), any()) }
verify(exactly = 1) {
analytics.logScreenEvent(
CourseAnalyticsEvent.DASHBOARD.eventName,
Expand Down Expand Up @@ -226,7 +228,7 @@ class CourseContainerViewModelTest {
coEvery { interactor.getCourseStructureFlow(any(), any()) } returns flowOf(
CoreMocks.mockCourseStructure
)
coEvery { interactor.getEnrollmentDetailsFlow(any()) } returns flowOf(
coEvery { interactor.getEnrollmentDetailsFlow(any(), any()) } returns flowOf(
CoreMocks.mockCourseEnrollmentDetails
)
every {
Expand Down
21 changes: 13 additions & 8 deletions course/src/test/java/org/openedx/course/presentation/dates/CourseDatesViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ class CourseDatesViewModelTest {
calendarRouter,
resourceManager,
)
coEvery { interactor.getCourseDates(any()) } throws UnknownHostException()
coEvery { interactor.getCourseDates(any(), any()) } throws UnknownHostException()
val message = async {
withTimeoutOrNull(5000) {
viewModel.uiMessage.first() as? UIMessage.SnackBarMessage
}
}
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getCourseDates(any()) }
coVerify(exactly = 1) { interactor.getCourseDates(any(), any()) }

Assert.assertEquals(noInternet, message.await()?.message)
assert(viewModel.uiState.value is CourseDatesUIState.Error)
Expand All @@ -142,15 +142,15 @@ class CourseDatesViewModelTest {
calendarRouter,
resourceManager,
)
coEvery { interactor.getCourseDates(any()) } throws Exception()
coEvery { interactor.getCourseDates(any(), any()) } throws Exception()
val message = async {
withTimeoutOrNull(5000) {
viewModel.uiMessage.first() as? UIMessage.SnackBarMessage
}
}
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getCourseDates(any()) }
coVerify(exactly = 1) { interactor.getCourseDates(any(), any()) }

assert(message.await()?.message.isNullOrEmpty())
assert(viewModel.uiState.value is CourseDatesUIState.Error)
Expand All @@ -172,15 +172,20 @@ class CourseDatesViewModelTest {
calendarRouter,
resourceManager,
)
coEvery { interactor.getCourseDates(any()) } returns CourseMocks.courseDatesResultWithData
coEvery {
interactor.getCourseDates(
any(),
any()
)
} returns CourseMocks.courseDatesResultWithData
val message = async {
withTimeoutOrNull(5000) {
viewModel.uiMessage.first() as? UIMessage.SnackBarMessage
}
}
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getCourseDates(any()) }
coVerify(exactly = 1) { interactor.getCourseDates(any(), any()) }

assert(message.await()?.message.isNullOrEmpty())
assert(viewModel.uiState.value is CourseDatesUIState.CourseDates)
Expand All @@ -202,7 +207,7 @@ class CourseDatesViewModelTest {
calendarRouter,
resourceManager,
)
coEvery { interactor.getCourseDates(any()) } returns CourseDatesResult(
coEvery { interactor.getCourseDates(any(), any()) } returns CourseDatesResult(
datesSection = linkedMapOf(),
courseBanner = CoreMocks.mockCourseDatesBannerInfo,
)
Expand All @@ -213,7 +218,7 @@ class CourseDatesViewModelTest {
}
advanceUntilIdle()

coVerify(exactly = 1) { interactor.getCourseDates(any()) }
coVerify(exactly = 1) { interactor.getCourseDates(any(), any()) }

assert(message.await()?.message.isNullOrEmpty())
assert(viewModel.uiState.value is CourseDatesUIState.Error)
Expand Down
Loading
Loading