fix: Fixes according to PR feedback

openedx · Jun 22, 2024 · b9f4a46 · b9f4a46
1 parent 74072e2
commit b9f4a46
Show file tree

Hide file tree

Showing 37 changed files with 243 additions and 168 deletions.
diff --git a/app/src/main/java/org/openedx/app/AppActivity.kt b/app/src/main/java/org/openedx/app/AppActivity.kt
@@ -12,9 +12,11 @@ import androidx.core.view.WindowCompat
 import androidx.core.view.WindowInsetsCompat
 import androidx.core.view.WindowInsetsControllerCompat
 import androidx.fragment.app.Fragment
+import androidx.lifecycle.lifecycleScope
 import androidx.window.layout.WindowMetricsCalculator
 import io.branch.referral.Branch
 import io.branch.referral.Branch.BranchUniversalReferralInitListener
+import kotlinx.coroutines.launch
 import org.koin.android.ext.android.inject
 import org.koin.androidx.viewmodel.ext.android.viewModel
 import org.openedx.app.databinding.ActivityAppBinding
@@ -28,6 +30,7 @@ import org.openedx.core.presentation.global.WindowSizeHolder
 import org.openedx.core.ui.WindowSize
 import org.openedx.core.ui.WindowType
 import org.openedx.core.utils.Logger
+import org.openedx.course.presentation.download.DownloadDialogManager
 import org.openedx.profile.presentation.ProfileRouter
 import org.openedx.whatsnew.WhatsNewManager
 import org.openedx.whatsnew.presentation.whatsnew.WhatsNewFragment
@@ -49,6 +52,7 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
     private val whatsNewManager by inject<WhatsNewManager>()
     private val corePreferencesManager by inject<CorePreferences>()
     private val profileRouter by inject<ProfileRouter>()
+    private val downloadDialogManager by inject<DownloadDialogManager>()
 
     private val branchLogger = Logger(BRANCH_TAG)
 
@@ -73,7 +77,6 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
         viewModel.logAppLaunchEvent()
         setContentView(binding.root)
         val container = binding.rootLayout
-        viewModel.fragmentManager = supportFragmentManager
 
         container.addView(object : View(this) {
             override fun onConfigurationChanged(newConfig: Configuration?) {
@@ -141,6 +144,15 @@ class AppActivity : AppCompatActivity(), InsetHolder, WindowSizeHolder {
         viewModel.logoutUser.observe(this) {
             profileRouter.restartApp(supportFragmentManager, viewModel.isLogistrationEnabled)
         }
+
+        lifecycleScope.launch {
+            viewModel.downloadFailedDialog.collect {
+                downloadDialogManager.showDownloadFailedPopup(
+                    downloadModel = it.downloadModel,
+                    fragmentManager = supportFragmentManager,
+                )
+            }
+        }
     }
 
     override fun onStart() {

diff --git a/app/src/main/java/org/openedx/app/AppViewModel.kt b/app/src/main/java/org/openedx/app/AppViewModel.kt
@@ -6,6 +6,9 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.viewModelScope
 import androidx.room.RoomDatabase
 import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.SharedFlow
+import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import org.openedx.app.deeplink.DeepLink
@@ -19,7 +22,6 @@ import org.openedx.core.data.storage.CorePreferences
 import org.openedx.core.system.notifier.DownloadFailed
 import org.openedx.core.system.notifier.DownloadNotifier
 import org.openedx.core.utils.FileUtil
-import org.openedx.course.presentation.download.DownloadDialogManager
 
 class AppViewModel(
     private val config: Config,
@@ -31,22 +33,24 @@ class AppViewModel(
     private val deepLinkRouter: DeepLinkRouter,
     private val fileUtil: FileUtil,
     private val downloadNotifier: DownloadNotifier,
-    private val downloadDialogManager: DownloadDialogManager,
 ) : BaseViewModel() {
 
     private val _logoutUser = SingleEventLiveData<Unit>()
     val logoutUser: LiveData<Unit>
         get() = _logoutUser
 
+    private val _downloadFailedDialog = MutableSharedFlow<DownloadFailed>()
+    val downloadFailedDialog: SharedFlow<DownloadFailed>
+        get() = _downloadFailedDialog.asSharedFlow()
+
+
     val isLogistrationEnabled get() = config.isPreLoginExperienceEnabled()
 
     private var logoutHandledAt: Long = 0
 
     val isBranchEnabled get() = config.getBranchConfig().enabled
     private val canResetAppDirectory get() = preferencesManager.canResetAppDirectory
 
-    var fragmentManager: FragmentManager? = null
-
     override fun onCreate(owner: LifecycleOwner) {
         super.onCreate(owner)
         setUserId()
@@ -69,12 +73,7 @@ class AppViewModel(
         viewModelScope.launch {
             downloadNotifier.notifier.collect { event ->
                 if (event is DownloadFailed) {
-                    fragmentManager?.let {
-                        downloadDialogManager.showDownloadFailedPopup(
-                            downloadModel = event.downloadModel,
-                            fragmentManager = it,
-                        )
-                    }
+                    _downloadFailedDialog.emit(event)
                 }
             }
         }

diff --git a/app/src/main/java/org/openedx/app/MainViewModel.kt b/app/src/main/java/org/openedx/app/MainViewModel.kt
@@ -10,7 +10,6 @@ import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
-import org.openedx.app.worker.OfflineProgressSyncScheduler
 import org.openedx.core.BaseViewModel
 import org.openedx.core.config.Config
 import org.openedx.core.system.notifier.DiscoveryNotifier
@@ -21,7 +20,6 @@ class MainViewModel(
     private val config: Config,
     private val notifier: DiscoveryNotifier,
     private val analytics: AppAnalytics,
-    private val offlineProgressSyncScheduler: OfflineProgressSyncScheduler,
 ) : BaseViewModel() {
 
     private val _isBottomBarEnabled = MutableLiveData(true)
@@ -37,7 +35,6 @@ class MainViewModel(
 
     override fun onCreate(owner: LifecycleOwner) {
         super.onCreate(owner)
-        offlineProgressSyncScheduler.scheduleHourlySync()
         notifier.notifier
             .onEach {
                 if (it is NavigationToDiscovery) {

diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt
@@ -19,7 +19,6 @@ import org.openedx.app.deeplink.DeepLinkRouter
 import org.openedx.app.room.AppDatabase
 import org.openedx.app.room.DATABASE_NAME
 import org.openedx.app.system.notifier.AppNotifier
-import org.openedx.app.worker.OfflineProgressSyncScheduler
 import org.openedx.auth.presentation.AgreementProvider
 import org.openedx.auth.presentation.AuthAnalytics
 import org.openedx.auth.presentation.AuthRouter
@@ -56,6 +55,7 @@ import org.openedx.course.data.storage.CoursePreferences
 import org.openedx.course.presentation.CourseAnalytics
 import org.openedx.course.presentation.CourseRouter
 import org.openedx.course.presentation.download.DownloadDialogManager
+import org.openedx.course.worker.OfflineProgressSyncScheduler
 import org.openedx.dashboard.presentation.DashboardAnalytics
 import org.openedx.dashboard.presentation.DashboardRouter
 import org.openedx.discovery.presentation.DiscoveryAnalytics

diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt
@@ -80,10 +80,9 @@ val screenModule = module {
             get(),
             get(),
             get(),
-            get(),
         )
     }
-    viewModel { MainViewModel(get(), get(), get(), get()) }
+    viewModel { MainViewModel(get(), get(), get()) }
 
     factory { AuthRepository(get(), get(), get()) }
     factory { AuthInteractor(get()) }
@@ -438,7 +437,8 @@ val screenModule = module {
             get(),
             get(),
             get(),
-            get()
+            get(),
+            get(),
         )
     }
 
@@ -456,6 +456,7 @@ val screenModule = module {
             get(),
             get(),
             get(),
+            get(),
         )
     }
 

diff --git a/app/src/main/java/org/openedx/app/worker/OfflineProgressSyncScheduler.kt b/app/src/main/java/org/openedx/app/worker/OfflineProgressSyncScheduler.kt
diff --git a/app/src/test/java/org/openedx/AppViewModelTest.kt b/app/src/test/java/org/openedx/AppViewModelTest.kt
@@ -31,7 +31,6 @@ import org.openedx.core.config.Config
 import org.openedx.core.data.model.User
 import org.openedx.core.system.notifier.DownloadNotifier
 import org.openedx.core.utils.FileUtil
-import org.openedx.course.presentation.download.DownloadDialogManager
 
 @ExperimentalCoroutinesApi
 class AppViewModelTest {
@@ -48,15 +47,13 @@ class AppViewModelTest {
     private val analytics = mockk<AppAnalytics>()
     private val fileUtil = mockk<FileUtil>()
     private val deepLinkRouter = mockk<DeepLinkRouter>()
-    private val downloadDialogManager = mockk<DownloadDialogManager>()
     private val downloadNotifier = mockk<DownloadNotifier>()
 
     private val user = User(0, "", "", "")
 
     @Before
     fun before() {
         Dispatchers.setMain(dispatcher)
-        every { downloadDialogManager.showDownloadFailedPopup(any(), any()) } returns Unit
         every { downloadNotifier.notifier } returns flow { }
     }
 
@@ -82,7 +79,6 @@ class AppViewModelTest {
                 deepLinkRouter,
                 fileUtil,
                 downloadNotifier,
-                downloadDialogManager,
             )
 
         val mockLifeCycleOwner: LifecycleOwner = mockk()
@@ -116,7 +112,6 @@ class AppViewModelTest {
                 deepLinkRouter,
                 fileUtil,
                 downloadNotifier,
-                downloadDialogManager,
             )
 
         val mockLifeCycleOwner: LifecycleOwner = mockk()
@@ -152,7 +147,6 @@ class AppViewModelTest {
                 deepLinkRouter,
                 fileUtil,
                 downloadNotifier,
-                downloadDialogManager,
             )
 
         val mockLifeCycleOwner: LifecycleOwner = mockk()

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
@@ -84,6 +84,6 @@ interface CourseApi {
     suspend fun submitOfflineXBlockProgress(
         @Path("course_id") courseId: String,
         @Path("block_id") blockId: String,
-        @FieldMap(encoded = false) progress: Map<String, String>
+        @FieldMap progress: Map<String, String>
     )
 }
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.0) "%.0f" else "%.${round}f"
+        val formatString = if (size % 1 < 0.05) "%.0f" else "%.${round}f"
         return String.format(formatString, size) + if (space) " " else "" + units[digitGroups]
     } catch (e: Exception) {
         println(e.toString())

diff --git a/core/src/main/java/org/openedx/core/module/DownloadWorker.kt b/core/src/main/java/org/openedx/core/module/DownloadWorker.kt
@@ -135,9 +135,14 @@ class DownloadWorker(
             when (downloadResult) {
                 DownloadResult.SUCCESS -> {
                     val updatedModel = downloadHelper.updateDownloadStatus(downloadTask)
-                    downloadDao.updateDownloadModel(
-                        DownloadModelEntity.createFrom(updatedModel)
-                    )
+                    if (updatedModel == null) {
+                        downloadDao.removeDownloadModel(downloadTask.id)
+                        downloadError.add(downloadTask)
+                    } else {
+                        downloadDao.updateDownloadModel(
+                            DownloadModelEntity.createFrom(updatedModel)
+                        )
+                    }
                 }
 
                 DownloadResult.CANCELED -> {

diff --git a/core/src/main/java/org/openedx/core/module/db/DownloadModel.kt b/core/src/main/java/org/openedx/core/module/db/DownloadModel.kt
@@ -32,4 +32,4 @@ enum class DownloadedState {
 
 enum class FileType {
     VIDEO, X_BLOCK
-}
+}
diff --git a/core/src/main/java/org/openedx/core/module/download/AbstractDownloader.kt b/core/src/main/java/org/openedx/core/module/download/AbstractDownloader.kt
@@ -73,7 +73,6 @@ abstract class AbstractDownloader : KoinComponent {
         }
     }
 
-
     suspend fun cancelDownloading() {
         isCanceled = true
         withContext(Dispatchers.IO) {
@@ -95,5 +94,4 @@ abstract class AbstractDownloader : KoinComponent {
     enum class DownloadResult {
         SUCCESS, CANCELED, ERROR
     }
-
-}
+}
diff --git a/core/src/main/java/org/openedx/core/module/download/BaseDownloadViewModel.kt b/core/src/main/java/org/openedx/core/module/download/BaseDownloadViewModel.kt
@@ -217,17 +217,6 @@ abstract class BaseDownloadViewModel(
         }
     }
 
-    protected fun addDownloadableChildrenForVerticalBlock(verticalBlock: Block) {
-        for (unitBlockId in verticalBlock.descendants) {
-            val block = allBlocks[unitBlockId]
-            if (block?.isDownloadable == true) {
-                val id = verticalBlock.id
-                val children = downloadableChildrenMap[id] ?: listOf()
-                downloadableChildrenMap[id] = children + block.id
-            }
-        }
-    }
-
     fun logBulkDownloadToggleEvent(toggle: Boolean) {
         logEvent(
             CoreAnalyticsEvent.VIDEO_BULK_DOWNLOAD_TOGGLE,

diff --git a/core/src/main/java/org/openedx/core/module/download/DownloadHelper.kt b/core/src/main/java/org/openedx/core/module/download/DownloadHelper.kt
@@ -71,7 +71,7 @@ class DownloadHelper(
         }
     }
 
-    suspend fun updateDownloadStatus(downloadModel: DownloadModel): DownloadModel {
+    suspend fun updateDownloadStatus(downloadModel: DownloadModel): DownloadModel? {
         return when (downloadModel.type) {
             FileType.VIDEO -> {
                 downloadModel.copy(
@@ -81,13 +81,33 @@ class DownloadHelper(
             }
 
             FileType.X_BLOCK -> {
-                val unzippedFolderPath = fileUtil.unzipFile(downloadModel.path)
+                val unzippedFolderPath = fileUtil.unzipFile(downloadModel.path) ?: return null
                 downloadModel.copy(
                     downloadedState = DownloadedState.DOWNLOADED,
-                    size = File(unzippedFolderPath ?: "").length(),
-                    path = unzippedFolderPath ?: ""
+                    size = calculateDirectorySize(File(unzippedFolderPath)),
+                    path = unzippedFolderPath
                 )
             }
         }
     }
+
+    private fun calculateDirectorySize(directory: File): Long {
+        var size: Long = 0
+
+        if (directory.exists()) {
+            val files = directory.listFiles()
+
+            if (files != null) {
+                for (file in files) {
+                    size += if (file.isDirectory) {
+                        calculateDirectorySize(file)
+                    } else {
+                        file.length()
+                    }
+                }
+            }
+        }
+
+        return size
+    }
 }
diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml
@@ -193,4 +193,5 @@
     <string name="course_confirm_download">Confirm Download</string>
     <string name="core_edit">Edit</string>
     <string name="core_offline_progress_sync">Offline Progress Sync</string>
+    <string name="core_close">Close</string>
 </resources>
diff --git a/core/src/openedx/org/openedx/core/ui/theme/Colors.kt b/core/src/openedx/org/openedx/core/ui/theme/Colors.kt
@@ -8,7 +8,7 @@ val light_secondary = Color(0xFF94D3DD)
 val light_secondary_variant = Color(0xFF94D3DD)
 val light_background = Color.White
 val light_surface = Color(0xFFF7F7F8)
-val light_error = Color(0xFFFF3D71)
+val light_error = Color(0xFFE8174F)
 val light_onPrimary = Color.White
 val light_onSecondary = Color.White
 val light_onBackground = Color.Black