fix: Fixes according to code review feedback

openedx · Jun 6, 2024 · 5a4a2e9 · 5a4a2e9
1 parent fe06870
commit 5a4a2e9
Show file tree

Hide file tree

Showing 59 changed files with 337 additions and 242 deletions.
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
@@ -114,6 +114,8 @@
             </intent-filter>
         </service>
 
+        <service android:name="org.openedx.core.service.CalendarSyncForegroundService" />
+
     </application>
 
 </manifest>
diff --git a/app/src/main/java/org/openedx/app/AppActivity.kt b/app/src/main/java/org/openedx/app/AppActivity.kt
@@ -27,8 +27,8 @@ 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.core.worker.CalendarSyncScheduler
 import org.openedx.profile.presentation.ProfileRouter
-import org.openedx.profile.worker.CalendarSyncScheduler
 import org.openedx.whatsnew.WhatsNewManager
 import org.openedx.whatsnew.presentation.whatsnew.WhatsNewFragment
 

diff --git a/app/src/main/java/org/openedx/app/AppViewModel.kt b/app/src/main/java/org/openedx/app/AppViewModel.kt
@@ -46,7 +46,7 @@ class AppViewModel(
             notifier.notifier.collect { event ->
                 if (event is LogoutEvent && System.currentTimeMillis() - logoutHandledAt > 5000) {
                     logoutHandledAt = System.currentTimeMillis()
-                    preferencesManager.clear()
+                    preferencesManager.clearCorePreferences()
                     withContext(dispatcher) {
                         room.clearAllTables()
                     }

diff --git a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt
@@ -11,10 +11,10 @@ import org.openedx.core.domain.model.AppConfig
 import org.openedx.core.domain.model.VideoQuality
 import org.openedx.core.domain.model.VideoSettings
 import org.openedx.core.extension.replaceSpace
+import org.openedx.core.system.CalendarManager
 import org.openedx.course.data.storage.CoursePreferences
 import org.openedx.profile.data.model.Account
 import org.openedx.profile.data.storage.ProfilePreferences
-import org.openedx.profile.system.CalendarManager
 import org.openedx.whatsnew.data.storage.WhatsNewPreferences
 
 class PreferencesManager(context: Context) : CorePreferences, ProfilePreferences,
@@ -51,7 +51,7 @@ class PreferencesManager(context: Context) : CorePreferences, ProfilePreferences
         return sharedPreferences.getBoolean(key, defValue)
     }
 
-    override fun clear() {
+    override fun clearCorePreferences() {
         sharedPreferences.edit().apply {
             remove(ACCESS_TOKEN)
             remove(REFRESH_TOKEN)
@@ -60,6 +60,14 @@ class PreferencesManager(context: Context) : CorePreferences, ProfilePreferences
         }.apply()
     }
 
+    override fun clearCalendarPreferences() {
+        sharedPreferences.edit().apply {
+            remove(CALENDAR_ID)
+            remove(IS_CALENDAR_SYNC_ENABLED)
+            remove(HIDE_INACTIVE_COURSES)
+        }.apply()
+    }
+
     override var accessToken: String
         set(value) {
             saveString(ACCESS_TOKEN, value)

diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt
@@ -41,14 +41,17 @@ import org.openedx.core.presentation.global.AppData
 import org.openedx.core.presentation.global.WhatsNewGlobalManager
 import org.openedx.core.presentation.global.app_upgrade.AppUpgradeRouter
 import org.openedx.core.system.AppCookieManager
+import org.openedx.core.system.CalendarManager
 import org.openedx.core.system.ResourceManager
 import org.openedx.core.system.connection.NetworkConnection
 import org.openedx.core.system.notifier.AppUpgradeNotifier
 import org.openedx.core.system.notifier.CourseNotifier
 import org.openedx.core.system.notifier.DiscoveryNotifier
 import org.openedx.core.system.notifier.DownloadNotifier
 import org.openedx.core.system.notifier.VideoNotifier
+import org.openedx.core.system.notifier.calendar.CalendarNotifier
 import org.openedx.core.utils.FileUtil
+import org.openedx.core.worker.CalendarSyncScheduler
 import org.openedx.course.data.storage.CoursePreferences
 import org.openedx.course.presentation.CourseAnalytics
 import org.openedx.course.presentation.CourseRouter
@@ -62,10 +65,7 @@ import org.openedx.discussion.system.notifier.DiscussionNotifier
 import org.openedx.profile.data.storage.ProfilePreferences
 import org.openedx.profile.presentation.ProfileAnalytics
 import org.openedx.profile.presentation.ProfileRouter
-import org.openedx.profile.system.CalendarManager
-import org.openedx.profile.system.notifier.CalendarNotifier
-import org.openedx.profile.system.notifier.ProfileNotifier
-import org.openedx.profile.worker.CalendarSyncScheduler
+import org.openedx.profile.system.notifier.profile.ProfileNotifier
 import org.openedx.whatsnew.WhatsNewManager
 import org.openedx.whatsnew.WhatsNewRouter
 import org.openedx.whatsnew.data.storage.WhatsNewPreferences

diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt
@@ -144,7 +144,7 @@ val screenModule = module {
         )
     }
 
-    factory { ProfileRepository(get(), get(), get(), get(), get()) }
+    factory { ProfileRepository(get(), get(), get(), get(), get(), get(), get()) }
     factory { ProfileInteractor(get()) }
     viewModel {
         ProfileViewModel(
@@ -177,9 +177,9 @@ val screenModule = module {
     viewModel { ManageAccountViewModel(get(), get(), get(), get(), get()) }
     viewModel { CalendarViewModel(get(), get(), get(), get(), get(), get(), get()) }
     viewModel { CoursesToSyncViewModel(get(), get(), get()) }
-    viewModel { NewCalendarDialogViewModel(get(), get(), get()) }
-    viewModel { DisableCalendarSyncDialogViewModel(get(), get()) }
-    single { CalendarRepository(get(), get(), get()) }
+    viewModel { NewCalendarDialogViewModel(get(), get(), get(), get()) }
+    viewModel { DisableCalendarSyncDialogViewModel(get(), get(), get(), get()) }
+    factory { CalendarRepository(get(), get(), get()) }
     factory { CalendarInteractor(get()) }
 
     single { CourseRepository(get(), get(), get(), get(), get()) }
@@ -313,17 +313,15 @@ val screenModule = module {
             get(),
         )
     }
-    viewModel { (courseId: String, courseTitle: String, enrollmentMode: String) ->
+    viewModel { (courseId: String, enrollmentMode: String) ->
         CourseDatesViewModel(
             courseId,
-            courseTitle,
             enrollmentMode,
             get(),
             get(),
             get(),
             get(),
             get(),
-            get(),
             get()
         )
     }

diff --git a/app/src/test/java/org/openedx/AppViewModelTest.kt b/app/src/test/java/org/openedx/AppViewModelTest.kt
@@ -88,7 +88,7 @@ class AppViewModelTest {
         every { notifier.notifier } returns flow {
             emit(LogoutEvent())
         }
-        every { preferencesManager.clear() } returns Unit
+        every { preferencesManager.clearCorePreferences() } returns Unit
         every { analytics.setUserIdForSession(any()) } returns Unit
         every { preferencesManager.user } returns user
         every { room.clearAllTables() } returns Unit
@@ -121,7 +121,7 @@ class AppViewModelTest {
             emit(LogoutEvent())
             emit(LogoutEvent())
         }
-        every { preferencesManager.clear() } returns Unit
+        every { preferencesManager.clearCorePreferences() } returns Unit
         every { analytics.setUserIdForSession(any()) } returns Unit
         every { preferencesManager.user } returns user
         every { room.clearAllTables() } returns Unit
@@ -145,7 +145,7 @@ class AppViewModelTest {
         advanceUntilIdle()
 
         verify(exactly = 1) { analytics.logoutEvent(true) }
-        verify(exactly = 1) { preferencesManager.clear() }
+        verify(exactly = 1) { preferencesManager.clearCorePreferences() }
         verify(exactly = 1) { analytics.setUserIdForSession(any()) }
         verify(exactly = 1) { preferencesManager.user }
         verify(exactly = 1) { room.clearAllTables() }

diff --git a/core/src/main/java/org/openedx/core/data/storage/CalendarPreferences.kt b/core/src/main/java/org/openedx/core/data/storage/CalendarPreferences.kt
@@ -4,4 +4,6 @@ interface CalendarPreferences {
     var calendarId: Long
     var isCalendarSyncEnabled: Boolean
     var isHideInactiveCourses: Boolean
+
+    fun clearCalendarPreferences()
 }
diff --git a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt
@@ -13,5 +13,5 @@ interface CorePreferences {
     var appConfig: AppConfig
     var canResetAppDirectory: Boolean
 
-    fun clear()
+    fun clearCorePreferences()
 }
diff --git a/core/src/main/java/org/openedx/core/domain/interactor/CalendarInteractor.kt b/core/src/main/java/org/openedx/core/domain/interactor/CalendarInteractor.kt
@@ -38,6 +38,14 @@ class CalendarInteractor(
         return repository.getAllCourseCalendarStateFromCache()
     }
 
+    suspend fun clearCalendarCachedData() {
+        repository.clearCalendarCachedData()
+    }
+
+    suspend fun resetChecksums() {
+        repository.resetChecksums()
+    }
+
     suspend fun updateCourseCalendarStateByIdInCache(
         courseId: String,
         checksum: Int? = null,

diff --git a/core/src/main/java/org/openedx/core/module/db/CalendarDao.kt b/core/src/main/java/org/openedx/core/module/db/CalendarDao.kt
@@ -20,6 +20,9 @@ interface CalendarDao {
     @Query("SELECT * FROM course_calendar_event_table WHERE course_id=:courseId")
     suspend fun readCourseCalendarEventsById(courseId: String): List<CourseCalendarEventEntity>
 
+    @Query("DELETE FROM course_calendar_event_table")
+    suspend fun clearCourseCalendarEventsCachedData()
+
     // region CourseCalendarStateEntity
     @Insert(onConflict = OnConflictStrategy.REPLACE)
     suspend fun insertCourseCalendarStateEntity(vararg courseCalendarStateEntity: CourseCalendarStateEntity)
@@ -30,6 +33,12 @@ interface CalendarDao {
     @Query("SELECT * FROM course_calendar_state_table")
     suspend fun readAllCourseCalendarState(): List<CourseCalendarStateEntity>
 
+    @Query("DELETE FROM course_calendar_state_table")
+    suspend fun clearCourseCalendarStateCachedData()
+
+    @Query("UPDATE course_calendar_state_table SET checksum = 0")
+    suspend fun resetChecksums()
+
     @Query(
         """
     UPDATE course_calendar_state_table

diff --git a/core/src/main/java/org/openedx/core/repository/CalendarRepository.kt b/core/src/main/java/org/openedx/core/repository/CalendarRepository.kt
@@ -1,5 +1,9 @@
 package org.openedx.core.repository
 
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
+import kotlinx.coroutines.launch
 import org.openedx.core.data.api.CourseApi
 import org.openedx.core.data.model.room.CourseCalendarEventEntity
 import org.openedx.core.data.model.room.CourseCalendarStateEntity
@@ -46,6 +50,19 @@ class CalendarRepository(
         return calendarDao.readAllCourseCalendarState().map { it.mapToDomain() }
     }
 
+    suspend fun resetChecksums() {
+        calendarDao.resetChecksums()
+    }
+
+    suspend fun clearCalendarCachedData() {
+        CoroutineScope(Dispatchers.Main).launch {
+            val clearCourseCalendarStateDeferred = async { calendarDao.clearCourseCalendarStateCachedData() }
+            val clearCourseCalendarEventsDeferred = async { calendarDao.clearCourseCalendarEventsCachedData() }
+            clearCourseCalendarStateDeferred.await()
+            clearCourseCalendarEventsDeferred.await()
+        }
+    }
+
     suspend fun updateCourseCalendarStateByIdInCache(
         courseId: String,
         checksum: Int? = null,

diff --git a/...nedx/profile/worker/CalendarSyncWorker.kt → ...enedx/core/service/CalendarSyncService.kt b/...nedx/profile/worker/CalendarSyncWorker.kt → ...enedx/core/service/CalendarSyncService.kt
@@ -1,55 +1,88 @@
-package org.openedx.profile.worker
+package org.openedx.core.service
 
+import android.app.Notification
+import android.app.NotificationChannel
+import android.app.NotificationManager
+import android.app.Service
 import android.content.Context
-import androidx.work.CoroutineWorker
-import androidx.work.WorkerParameters
+import android.content.Intent
+import android.os.Build
+import android.os.IBinder
+import androidx.core.app.NotificationCompat
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
+import org.openedx.core.R
 import org.openedx.core.data.model.CourseDates
 import org.openedx.core.data.model.room.CourseCalendarEventEntity
 import org.openedx.core.data.model.room.CourseCalendarStateEntity
 import org.openedx.core.data.storage.CalendarPreferences
 import org.openedx.core.domain.interactor.CalendarInteractor
 import org.openedx.core.domain.model.CourseDateBlock
 import org.openedx.core.domain.model.EnrollmentStatus
-import org.openedx.profile.system.CalendarManager
-import org.openedx.profile.system.notifier.CalendarNotifier
-import org.openedx.profile.system.notifier.CalendarSyncFailed
-import org.openedx.profile.system.notifier.CalendarSynced
-import org.openedx.profile.system.notifier.CalendarSyncing
+import org.openedx.core.system.CalendarManager
+import org.openedx.core.system.notifier.calendar.CalendarNotifier
+import org.openedx.core.system.notifier.calendar.CalendarSyncFailed
+import org.openedx.core.system.notifier.calendar.CalendarSynced
+import org.openedx.core.system.notifier.calendar.CalendarSyncing
 
-class CalendarSyncWorker(
-    context: Context,
-    workerParams: WorkerParameters
-) : CoroutineWorker(context, workerParams), KoinComponent {
+class CalendarSyncForegroundService : Service(), KoinComponent {
 
     private val calendarManager: CalendarManager by inject()
     private val calendarInteractor: CalendarInteractor by inject()
     private val calendarNotifier: CalendarNotifier by inject()
     private val calendarPreferences: CalendarPreferences by inject()
 
-    override suspend fun doWork(): Result {
-        return try {
-            val courseId = inputData.getString(ARG_COURSE_ID)
+    override fun onBind(intent: Intent?): IBinder? {
+        return null
+    }
+
+    override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
+        startForeground(NOTIFICATION_ID, createNotification())
+        val courseId = intent?.getStringExtra(ARG_COURSE_ID)
+        CoroutineScope(Dispatchers.Main).launch {
             tryToSyncCalendar(courseId)
-            Result.success()
-        } catch (e: Exception) {
-            calendarNotifier.send(CalendarSyncFailed)
-            Result.failure()
+            stopForeground(STOP_FOREGROUND_REMOVE)
+            stopSelf()
         }
+        return START_NOT_STICKY
+    }
+
+    private fun createNotification(): Notification {
+        val notificationManager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            val channel = NotificationChannel(
+                NOTIFICATION_CHANEL_ID,
+                getString(R.string.core_calendar_notification_channel_name),
+                NotificationManager.IMPORTANCE_DEFAULT
+            )
+            notificationManager.createNotificationChannel(channel)
+        }
+
+        return NotificationCompat.Builder(this, NOTIFICATION_CHANEL_ID)
+            .setSmallIcon(R.drawable.core_ic_calendar)
+            .setContentTitle(getString(R.string.core_calendar_notification_channel_name))
+            .setContentText(getString(R.string.core_title_syncing_calendar))
+            .build()
     }
 
     private suspend fun tryToSyncCalendar(courseId: String?) {
-        val isCalendarCreated = calendarPreferences.calendarId != CalendarManager.CALENDAR_DOES_NOT_EXIST
-        val isCalendarSyncEnabled = calendarPreferences.isCalendarSyncEnabled
-        if (isCalendarCreated && isCalendarSyncEnabled) {
-            calendarNotifier.send(CalendarSyncing)
-            if (courseId.isNullOrEmpty()) {
-                syncCalendar()
-            } else {
-                syncCalendar(courseId)
+        try {
+            val isCalendarCreated = calendarPreferences.calendarId != CalendarManager.CALENDAR_DOES_NOT_EXIST
+            val isCalendarSyncEnabled = calendarPreferences.isCalendarSyncEnabled
+            if (isCalendarCreated && isCalendarSyncEnabled) {
+                calendarNotifier.send(CalendarSyncing)
+                if (courseId.isNullOrEmpty()) {
+                    syncCalendar()
+                } else {
+                    syncCalendar(courseId)
+                }
+                calendarNotifier.send(CalendarSynced)
             }
-            calendarNotifier.send(CalendarSynced)
+        } catch (e: Exception) {
+            calendarNotifier.send(CalendarSyncFailed)
         }
     }
 
@@ -147,7 +180,8 @@ class CalendarSyncWorker(
     }
 
     companion object {
+        const val NOTIFICATION_ID = 1234
         const val ARG_COURSE_ID = "ARG_COURSE_ID"
-        const val WORKER_TAG = "calendar_sync_worker_tag"
+        const val NOTIFICATION_CHANEL_ID = "calendar_sync_channel"
     }
-}
+}
diff --git a/...openedx/profile/system/CalendarManager.kt → ...rg/openedx/core/system/CalendarManager.kt b/...openedx/profile/system/CalendarManager.kt → ...rg/openedx/core/system/CalendarManager.kt
@@ -1,4 +1,4 @@
-package org.openedx.profile.system
+package org.openedx.core.system
 
 import android.content.ContentUris
 import android.content.ContentValues
@@ -12,7 +12,6 @@ import org.openedx.core.R
 import org.openedx.core.data.storage.CorePreferences
 import org.openedx.core.domain.model.CalendarData
 import org.openedx.core.domain.model.CourseDateBlock
-import org.openedx.core.system.ResourceManager
 import org.openedx.core.utils.Logger
 import org.openedx.core.utils.toCalendar
 import java.util.TimeZone
@@ -225,7 +224,7 @@ class CalendarManager(
     /**
      * Method to delete the course calendar from the mobile calendar app
      */
-    private fun deleteCalendar(calendarId: Long) {
+    fun deleteCalendar(calendarId: Long) {
         context.contentResolver.delete(
             Uri.parse("content://com.android.calendar/calendars/$calendarId"),
             null,

diff --git a/core/src/main/java/org/openedx/core/system/notifier/calendar/CalendarCreated.kt b/core/src/main/java/org/openedx/core/system/notifier/calendar/CalendarCreated.kt
@@ -0,0 +1,3 @@
+package org.openedx.core.system.notifier.calendar
+
+object CalendarCreated : CalendarEvent