From 5d849b3adaed2675fdda3e674f97af47cc1bbea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Borcz?= Date: Wed, 20 Jan 2021 17:42:19 +0100 Subject: [PATCH] Fix endless loading in grades (#1084) --- app/build.gradle | 6 +- .../ui/modules/grade/GradeAverageProvider.kt | 61 ++++--- .../grade/details/GradeDetailsPresenter.kt | 4 +- .../grade/summary/GradeSummaryPresenter.kt | 2 + .../modules/grade/GradeAverageProviderTest.kt | 163 +++++++++++++++++- 5 files changed, 193 insertions(+), 43 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 1118ac40..3bd6570c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -137,10 +137,6 @@ ext { moshi = "1.11.0" } -configurations.all { - resolutionStrategy.force "androidx.constraintlayout:constraintlayout:1.1.3" -} - dependencies { implementation "io.github.wulkanowy:sdk:edc165bb" @@ -161,7 +157,7 @@ dependencies { implementation "androidx.recyclerview:recyclerview:1.1.0" implementation "androidx.viewpager:viewpager:1.0.0" implementation "androidx.swiperefreshlayout:swiperefreshlayout:1.1.0" - implementation "androidx.constraintlayout:constraintlayout:2.0.1" + implementation "androidx.constraintlayout:constraintlayout:2.0.4" implementation "androidx.coordinatorlayout:coordinatorlayout:1.1.0" implementation "com.google.android.material:material:1.2.1" implementation "com.github.wulkanowy:material-chips-input:2.1.1" diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProvider.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProvider.kt index a608ac3b..0bd971ec 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProvider.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProvider.kt @@ -1,7 +1,6 @@ package io.github.wulkanowy.ui.modules.grade import io.github.wulkanowy.data.Resource -import io.github.wulkanowy.data.Status import io.github.wulkanowy.data.db.entities.Grade import io.github.wulkanowy.data.db.entities.GradeSummary import io.github.wulkanowy.data.db.entities.Semester @@ -18,10 +17,8 @@ import io.github.wulkanowy.utils.changeModifier import io.github.wulkanowy.utils.flowWithResourceIn import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.flatMapConcat -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import javax.inject.Inject @@ -50,20 +47,20 @@ class GradeAverageProvider @Inject constructor( val selectedSemester = semesters.single { it.semesterId == semesterId } val firstSemester = semesters.single { it.diaryId == selectedSemester.diaryId && it.semesterName == 1 } - return getSemesterDetailsWithAverage(student, selectedSemester, forceRefresh).flatMapConcat { selectedDetails -> - val isAnyAverage = selectedDetails.data.orEmpty().any { it.average != .0 } + val selectedSemesterDetailsWithAverage = getSemesterDetailsWithAverage(student, selectedSemester, forceRefresh) - if (selectedSemester != firstSemester) { - getSemesterDetailsWithAverage(student, firstSemester, forceRefresh).map { secondDetails -> - secondDetails.copy(data = selectedDetails.data?.map { selected -> - val second = secondDetails.data.orEmpty().singleOrNull { it.subject == selected.subject } - selected.copy(average = if (!isAnyAverage || preferencesRepository.gradeAverageForceCalc) { - val selectedGrades = selected.grades.updateModifiers(student).calcAverage() - (selectedGrades + (second?.grades?.updateModifiers(student)?.calcAverage() ?: selectedGrades)) / 2 - } else (selected.average + (second?.average ?: selected.average)) / 2) - }) - }.filter { it.status != Status.LOADING }.filter { it.data != null } - } else flowOf(selectedDetails) + return if (selectedSemester == firstSemester) selectedSemesterDetailsWithAverage else { + val firstSemesterDetailsWithAverage = getSemesterDetailsWithAverage(student, firstSemester, forceRefresh) + selectedSemesterDetailsWithAverage.combine(firstSemesterDetailsWithAverage) { selectedDetails, secondDetails -> + val isAnyAverage = selectedDetails.data.orEmpty().any { it.average != .0 } + secondDetails.copy(data = selectedDetails.data?.map { selected -> + val second = secondDetails.data.orEmpty().singleOrNull { it.subject == selected.subject } + selected.copy(average = if (!isAnyAverage || preferencesRepository.gradeAverageForceCalc) { + val selectedGrades = selected.grades.updateModifiers(student).calcAverage() + (selectedGrades + (second?.grades?.updateModifiers(student)?.calcAverage() ?: selectedGrades)) / 2 + } else (selected.average + (second?.average ?: selected.average)) / 2) + }) + } } } @@ -71,19 +68,19 @@ class GradeAverageProvider @Inject constructor( val selectedSemester = semesters.single { it.semesterId == semesterId } val firstSemester = semesters.single { it.diaryId == selectedSemester.diaryId && it.semesterName == 1 } - return getSemesterDetailsWithAverage(student, selectedSemester, forceRefresh).flatMapConcat { selectedDetails -> - val isAnyAverage = selectedDetails.data.orEmpty().any { it.average != .0 } + val selectedSemesterDetailsWithAverage = getSemesterDetailsWithAverage(student, selectedSemester, forceRefresh) - if (selectedSemester != firstSemester) { - getSemesterDetailsWithAverage(student, firstSemester, forceRefresh).map { secondDetails -> - secondDetails.copy(data = selectedDetails.data?.map { selected -> - val second = secondDetails.data.orEmpty().singleOrNull { it.subject == selected.subject } - selected.copy(average = if (!isAnyAverage || preferencesRepository.gradeAverageForceCalc) { - (selected.grades.updateModifiers(student) + second?.grades?.updateModifiers(student).orEmpty()).calcAverage() - } else selected.average) - }) - }.filter { it.status != Status.LOADING }.filter { it.data != null } - } else flowOf(selectedDetails) + return if (selectedSemester == firstSemester) selectedSemesterDetailsWithAverage else { + val firstSemesterDetailsWithAverage = getSemesterDetailsWithAverage(student, firstSemester, forceRefresh) + selectedSemesterDetailsWithAverage.combine(firstSemesterDetailsWithAverage) { selectedDetails, secondDetails -> + val isAnyAverage = selectedDetails.data.orEmpty().any { it.average != .0 } + secondDetails.copy(data = selectedDetails.data?.map { selected -> + val second = secondDetails.data.orEmpty().singleOrNull { it.subject == selected.subject } + selected.copy(average = if (!isAnyAverage || preferencesRepository.gradeAverageForceCalc) { + (selected.grades.updateModifiers(student) + second?.grades?.updateModifiers(student).orEmpty()).calcAverage() + } else selected.average) + }) + } } } @@ -93,7 +90,7 @@ class GradeAverageProvider @Inject constructor( val isAnyAverage = summaries.orEmpty().any { it.average != .0 } val allGrades = details.orEmpty().groupBy { it.subject } - Resource(res.status, summaries?.emulateEmptySummaries(student, semester, allGrades.toList(), isAnyAverage)?.map { summary -> + val items = summaries?.emulateEmptySummaries(student, semester, allGrades.toList(), isAnyAverage)?.map { summary -> val grades = allGrades[summary.subject].orEmpty() GradeDetailsWithAverage( subject = summary.subject, @@ -104,7 +101,9 @@ class GradeAverageProvider @Inject constructor( summary = summary, grades = grades ) - }, res.error) + } + + Resource(res.status, items, res.error) } } diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/details/GradeDetailsPresenter.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/details/GradeDetailsPresenter.kt index 9f0b237f..afec6b5e 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/details/GradeDetailsPresenter.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/details/GradeDetailsPresenter.kt @@ -143,16 +143,18 @@ class GradeDetailsPresenter @Inject constructor( val student = studentRepository.getCurrentStudent() averageProvider.getGradesDetailsWithAverage(student, semesterId, forceRefresh) }.onEach { + Timber.d("Loading grade details status: ${it.status}, data: ${it.data != null}") when (it.status) { Status.LOADING -> { val items = createGradeItems(it.data.orEmpty()) if (items.isNotEmpty()) { - Timber.i("Loading gradle details result: load cached data") + Timber.i("Loading grade details result: load cached data") view?.run { updateNewGradesAmount(it.data.orEmpty()) enableSwipe(true) showRefresh(true) showProgress(false) + showEmpty(false) showContent(true) updateData( data = items, diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/summary/GradeSummaryPresenter.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/summary/GradeSummaryPresenter.kt index 14d75d7c..17c14b85 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/grade/summary/GradeSummaryPresenter.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/grade/summary/GradeSummaryPresenter.kt @@ -43,6 +43,7 @@ class GradeSummaryPresenter @Inject constructor( val student = studentRepository.getCurrentStudent() averageProvider.getGradesDetailsWithAverage(student, semesterId, forceRefresh) }.onEach { + Timber.d("Loading grade summary status: ${it.status}, data: ${it.data != null}") when (it.status) { Status.LOADING -> { val items = createGradeSummaryItems(it.data.orEmpty()) @@ -52,6 +53,7 @@ class GradeSummaryPresenter @Inject constructor( enableSwipe(true) showRefresh(true) showProgress(false) + showEmpty(false) showContent(true) updateData(items) } diff --git a/app/src/test/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProviderTest.kt b/app/src/test/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProviderTest.kt index 0d523c01..4479965c 100644 --- a/app/src/test/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProviderTest.kt +++ b/app/src/test/java/io/github/wulkanowy/ui/modules/grade/GradeAverageProviderTest.kt @@ -1,6 +1,7 @@ package io.github.wulkanowy.ui.modules.grade import io.github.wulkanowy.data.Resource +import io.github.wulkanowy.data.Status import io.github.wulkanowy.data.db.entities.Grade import io.github.wulkanowy.data.db.entities.GradeSummary import io.github.wulkanowy.data.db.entities.Student @@ -15,6 +16,7 @@ import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.toList import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals @@ -99,13 +101,42 @@ class GradeAverageProviderTest { gradeAverageProvider = GradeAverageProvider(semesterRepository, gradeRepository, preferencesRepository) } + @Test + fun `calc current semester average with load from cache sequence`() { + every { preferencesRepository.gradeAverageForceCalc } returns true + every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.ONE_SEMESTER + coEvery { semesterRepository.getSemesters(student) } returns semesters + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(secondGradeWithModifier to secondSummariesWithModifier)) + emit(Resource.success(secondGradeWithModifier to secondSummariesWithModifier)) + } + + val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).toList() } + + with(items[0]) { + assertEquals(Status.LOADING, status) + assertEquals(null, data) + } + with(items[1]) { + assertEquals(Status.LOADING, status) + assertEquals(1, data!!.size) + } + with(items[2]) { + assertEquals(Status.SUCCESS, status) + assertEquals(1, data!!.size) + } + + assertEquals(3.5, items[1].data?.single { it.subject == "Język polski" }!!.average, .0) // from details and after set custom plus/minus + } + @Test fun `force calc average on no grades`() { every { preferencesRepository.gradeAverageForceCalc } returns true every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.BOTH_SEMESTERS - coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { emptyList() to emptyList() } - coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { emptyList() to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { emptyList() to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { emptyList() to emptyList() } val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).getResult() } @@ -214,6 +245,37 @@ class GradeAverageProviderTest { assertEquals(3.5, items.single { it.subject == "Fizyka" }.average, .0) // (from summary): 3,5 } + @Test + fun `calc full year average when current is first with load from cache sequence`() { + every { preferencesRepository.gradeAverageForceCalc } returns true + every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.ALL_YEAR + coEvery { semesterRepository.getSemesters(student) } returns semesters + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(firstGrades to firstSummaries)) + emit(Resource.success(firstGrades to firstSummaries)) + } + + val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[1].semesterId, true).toList() } + + with(items[0]) { + assertEquals(Status.LOADING, status) + assertEquals(null, data) + } + with(items[1]) { + assertEquals(Status.LOADING, status) + assertEquals(2, data!!.size) + } + with(items[2]) { + assertEquals(Status.SUCCESS, status) + assertEquals(2, data!!.size) + } + + assertEquals(2, items[2].data!!.size) + assertEquals(3.5, items[2].data!!.single { it.subject == "Matematyka" }.average, .0) // (from summary): 3,5 + assertEquals(3.5, items[2].data!!.single { it.subject == "Fizyka" }.average, .0) // (from summary): 3,5 + } + @Test fun `calc both semesters average`() { every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.BOTH_SEMESTERS @@ -238,6 +300,53 @@ class GradeAverageProviderTest { assertEquals(3.75, items.single { it.subject == "Fizyka" }.average, .0) // (from summaries ↑): 3,5 + 4,0 → 3,75 } + @Test + fun `calc both semesters average when current is second with load from cache sequence`() { + every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.BOTH_SEMESTERS + every { preferencesRepository.gradeAverageForceCalc } returns false + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(firstGrades to listOf( + getSummary(22, "Matematyka", 3.0), + getSummary(22, "Fizyka", 3.5) + ))) + emit(Resource.success(firstGrades to listOf( + getSummary(22, "Matematyka", 3.0), + getSummary(22, "Fizyka", 3.5) + ))) + } + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(secondGrades to listOf( + getSummary(22, "Matematyka", 3.5), + getSummary(22, "Fizyka", 4.0) + ))) + emit(Resource.success(secondGrades to listOf( + getSummary(22, "Matematyka", 3.5), + getSummary(22, "Fizyka", 4.0) + ))) + } + + val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).toList() } + + with(items[0]) { + assertEquals(Status.LOADING, status) + assertEquals(null, data) + } + with(items[1]) { + assertEquals(Status.LOADING, status) + assertEquals(2, data!!.size) + } + with(items[2]) { + assertEquals(Status.SUCCESS, status) + assertEquals(2, data!!.size) + } + + assertEquals(2, items[2].data!!.size) + assertEquals(3.25, items[2].data!!.single { it.subject == "Matematyka" }.average, .0) // (from summaries ↑): 3,0 + 3,5 → 3,25 + assertEquals(3.75, items[2].data!!.single { it.subject == "Fizyka" }.average, .0) // (from summaries ↑): 3,5 + 4,0 → 3,75 + } + @Test fun `force calc full year average`() { every { preferencesRepository.gradeAverageForceCalc } returns true @@ -257,13 +366,55 @@ class GradeAverageProviderTest { assertEquals(3.25, items.single { it.subject == "Fizyka" }.average, .0) // (from details): 3,5 + 3,0 → 3,25 } + @Test + fun `calc full year average when current is second with load from cache sequence`() { + every { preferencesRepository.gradeAverageForceCalc } returns true + every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.ALL_YEAR + coEvery { semesterRepository.getSemesters(student) } returns semesters + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(firstGrades to firstSummaries)) + emit(Resource.success(firstGrades to firstSummaries)) + } + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flow { + emit(Resource.loading()) + emit(Resource.loading(secondGrades to listOf( + getSummary(22, "Matematyka", 1.1), + getSummary(22, "Fizyka", 7.26) + ))) + emit(Resource.success(secondGrades to listOf( + getSummary(22, "Matematyka", 1.1), + getSummary(22, "Fizyka", 7.26) + ))) + } + + val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).toList() } + + with(items[0]) { + assertEquals(Status.LOADING, status) + assertEquals(null, data) + } + with(items[1]) { + assertEquals(Status.LOADING, status) + assertEquals(2, data!!.size) + } + with(items[2]) { + assertEquals(Status.SUCCESS, status) + assertEquals(2, data!!.size) + } + + assertEquals(2, items[2].data!!.size) + assertEquals(3.0, items[2].data!!.single { it.subject == "Matematyka" }.average, .0) // (from details): 3,5 + 2,5 → 3,0 + assertEquals(3.25, items[2].data!!.single { it.subject == "Fizyka" }.average, .0) // (from details): 3,5 + 3,0 → 3,25 + } + @Test fun `calc both semesters average when no summaries`() { every { preferencesRepository.gradeAverageForceCalc } returns true every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.BOTH_SEMESTERS - coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { firstGrades to emptyList() } - coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { secondGrades to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { firstGrades to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { secondGrades to emptyList() } val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).getResult() } @@ -277,8 +428,8 @@ class GradeAverageProviderTest { every { preferencesRepository.gradeAverageForceCalc } returns true every { preferencesRepository.gradeAverageMode } returns GradeAverageMode.ALL_YEAR - coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { firstGrades to emptyList() } - coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { secondGrades to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[1], true) } returns flowWithResource { firstGrades to emptyList() } + coEvery { gradeRepository.getGrades(student, semesters[2], true) } returns flowWithResource { secondGrades to emptyList() } val items = runBlocking { gradeAverageProvider.getGradesDetailsWithAverage(student, semesters[2].semesterId, true).getResult() }