fix: Fixes according to QA feedback

openedx · Jun 24, 2024 · a8c9e27 · a8c9e27
1 parent 27069f7
commit a8c9e27
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 25 deletions.
diff --git a/core/src/main/java/org/openedx/core/data/api/CourseApi.kt b/core/src/main/java/org/openedx/core/data/api/CourseApi.kt
@@ -1,5 +1,6 @@
 package org.openedx.core.data.api
 
+import okhttp3.MultipartBody
 import org.openedx.core.data.model.AnnouncementModel
 import org.openedx.core.data.model.BlocksCompletionBody
 import org.openedx.core.data.model.CourseComponentStatus
@@ -10,11 +11,11 @@ import org.openedx.core.data.model.CourseStructureModel
 import org.openedx.core.data.model.HandoutsModel
 import org.openedx.core.data.model.ResetCourseDates
 import retrofit2.http.Body
-import retrofit2.http.FieldMap
-import retrofit2.http.FormUrlEncoded
 import retrofit2.http.GET
 import retrofit2.http.Header
+import retrofit2.http.Multipart
 import retrofit2.http.POST
+import retrofit2.http.Part
 import retrofit2.http.Path
 import retrofit2.http.Query
 
@@ -79,11 +80,11 @@ interface CourseApi {
         @Query("requested_fields") fields: List<String> = emptyList()
     ): CourseEnrollments
 
-    @FormUrlEncoded
+    @Multipart
     @POST("/courses/{course_id}/xblock/{block_id}/handler/xmodule_handler/problem_check")
     suspend fun submitOfflineXBlockProgress(
         @Path("course_id") courseId: String,
         @Path("block_id") blockId: String,
-        @FieldMap progress: Map<String, String>
+        @Part progress: List<MultipartBody.Part>
     )
 }
diff --git a/core/src/main/java/org/openedx/core/extension/LongExt.kt b/core/src/main/java/org/openedx/core/extension/LongExt.kt
@@ -9,7 +9,7 @@ fun Long.toFileSize(round: Int = 2, space: Boolean = true): String {
         val units = arrayOf("B", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB")
         val digitGroups = (log10(this.toDouble()) / log10(1024.0)).toInt()
         val size = this / 1024.0.pow(digitGroups.toDouble())
-        val formatString = if (size % 1 < 0.05) "%.0f" else "%.${round}f"
+        val formatString = if (size % 1 < 0.05 || size % 1 >= 0.95) "%.0f" else "%.${round}f"
         return String.format(formatString, size) + if (space) " " else "" + units[digitGroups]
     } catch (e: Exception) {
         println(e.toString())

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
@@ -1,6 +1,7 @@
 package org.openedx.course.data.repository
 
 import kotlinx.coroutines.flow.map
+import okhttp3.MultipartBody
 import org.openedx.core.ApiConstants
 import org.openedx.core.data.api.CourseApi
 import org.openedx.core.data.model.BlocksCompletionBody
@@ -13,6 +14,8 @@ import org.openedx.core.exception.NoCachedDataException
 import org.openedx.core.module.db.DownloadDao
 import org.openedx.core.system.connection.NetworkConnection
 import org.openedx.course.data.storage.CourseDao
+import java.net.URLDecoder
+import java.nio.charset.StandardCharsets
 
 class CourseRepository(
     private val api: CourseApi,
@@ -120,11 +123,14 @@ class CourseRepository(
 
     private suspend fun submitOfflineXBlockProgress(blockId: String, courseId: String, jsonProgressData: String?) {
         if (!jsonProgressData.isNullOrEmpty()) {
-            val progressMap = jsonProgressData
-                .split("&")
-                .map { it.split("=") }
-                .associate { it[0] to it[1] }
-            api.submitOfflineXBlockProgress(courseId, blockId, progressMap)
+            val parts = mutableListOf<MultipartBody.Part>()
+            val decodedQuery = URLDecoder.decode(jsonProgressData, StandardCharsets.UTF_8.name())
+            val keyValuePairs = decodedQuery.split("&")
+            for (pair in keyValuePairs) {
+                val (key, value) = pair.split("=")
+                parts.add(MultipartBody.Part.createFormData(key, value))
+            }
+            api.submitOfflineXBlockProgress(courseId, blockId, parts)
             downloadDao.removeOfflineXBlockProgress(listOf(blockId))
         }
     }

diff --git a/...rc/main/java/org/openedx/course/presentation/unit/container/CourseUnitContainerAdapter.kt b/...rc/main/java/org/openedx/course/presentation/unit/container/CourseUnitContainerAdapter.kt
@@ -28,26 +28,25 @@ class CourseUnitContainerAdapter(
         val offlineUrl = downloadedModel?.let { it.path + File.separator + "index.html" } ?: ""
         val noNetwork = !viewModel.hasNetworkConnection
 
-        if (noNetwork) {
-            if (block.isDownloadable && offlineUrl.isEmpty()) {
-                return createNotAvailableUnitFragment(block, NotAvailableUnitType.NOT_DOWNLOADED)
+        return when {
+            noNetwork && block.isDownloadable && offlineUrl.isEmpty() -> {
+                createNotAvailableUnitFragment(block, NotAvailableUnitType.NOT_DOWNLOADED)
             }
-            if (!block.isDownloadable) {
-                return createNotAvailableUnitFragment(block, NotAvailableUnitType.OFFLINE_UNSUPPORTED)
+
+            noNetwork && !block.isDownloadable -> {
+                createNotAvailableUnitFragment(block, NotAvailableUnitType.OFFLINE_UNSUPPORTED)
             }
-        }
 
-        when {
             block.isVideoBlock && block.studentViewData?.encodedVideos?.run { hasVideoUrl || hasYoutubeUrl } == true -> {
-                return createVideoFragment(block)
+                createVideoFragment(block)
             }
 
             block.isDiscussionBlock && !block.studentViewData?.topicId.isNullOrEmpty() -> {
-                return createDiscussionFragment(block)
+                createDiscussionFragment(block)
             }
 
             !block.studentViewMultiDevice -> {
-                return createNotAvailableUnitFragment(block, NotAvailableUnitType.MOBILE_UNSUPPORTED)
+                createNotAvailableUnitFragment(block, NotAvailableUnitType.MOBILE_UNSUPPORTED)
             }
 
             block.isHTMLBlock || block.isProblemBlock || block.isOpenAssessmentBlock || block.isDragAndDropBlock ||
@@ -57,7 +56,7 @@ class CourseUnitContainerAdapter(
                 } else {
                     ""
                 }
-                return HtmlUnitFragment.newInstance(
+                HtmlUnitFragment.newInstance(
                     block.id,
                     block.studentViewUrl,
                     viewModel.courseId,
@@ -67,7 +66,7 @@ class CourseUnitContainerAdapter(
             }
 
             else -> {
-                return createNotAvailableUnitFragment(block, NotAvailableUnitType.MOBILE_UNSUPPORTED)
+                createNotAvailableUnitFragment(block, NotAvailableUnitType.MOBILE_UNSUPPORTED)
             }
         }
     }

diff --git a/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt b/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt
@@ -170,7 +170,7 @@ class HtmlUnitFragment : Fragment() {
                                     },
                                     saveXBlockProgress = { jsonProgress ->
                                         viewModel.saveXBlockProgress(jsonProgress)
-                                    }
+                                    },
                                 )
                             } else {
                                 ConnectionErrorView(

diff --git a/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitViewModel.kt b/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitViewModel.kt
@@ -2,6 +2,7 @@ package org.openedx.course.presentation.unit.html
 
 import android.content.res.AssetManager
 import androidx.lifecycle.viewModelScope
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
@@ -42,7 +43,6 @@ class HtmlUnitViewModel(
 
     init {
         tryToSyncProgress()
-        getXBlockProgress(blockId)
     }
 
     fun setWebPageLoaded(assets: AssetManager) {
@@ -56,6 +56,7 @@ class HtmlUnitViewModel(
         assets.readAsText("js_injection/survey_css.js")?.let { jsList.add(it) }
 
         _injectJSList.value = jsList
+        getXBlockProgress()
     }
 
     fun notifyCompletionSet() {
@@ -84,10 +85,11 @@ class HtmlUnitViewModel(
         }
     }
 
-    private fun getXBlockProgress(blockId: String) {
+    private fun getXBlockProgress() {
         viewModelScope.launch {
             if (!isOnline) {
                 val xBlockProgress = courseInteractor.getXBlockProgress(blockId)
+                delay(500)
                 _uiState.update { it.copy(jsonProgress = xBlockProgress?.jsonProgress?.toJson()) }
             }
         }

diff --git a/course/src/main/java/org/openedx/course/worker/OfflineProgressSyncWorker.kt b/course/src/main/java/org/openedx/course/worker/OfflineProgressSyncWorker.kt
@@ -5,6 +5,7 @@ import android.app.NotificationManager
 import android.content.Context
 import android.content.pm.ServiceInfo
 import android.os.Build
+import android.util.Log
 import androidx.annotation.RequiresApi
 import androidx.core.app.NotificationCompat
 import androidx.work.CoroutineWorker
@@ -33,6 +34,7 @@ class OfflineProgressSyncWorker(
             tryToSyncProgress()
             Result.success()
         } catch (e: Exception) {
+            Log.e(WORKER_TAG, "$e")
             Firebase.crashlytics.log("$e")
             Result.failure()
         }