fix: Fixes according to code review feedback

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

Hide file tree

Showing 45 changed files with 144 additions and 138 deletions.
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
@@ -4,7 +4,6 @@ import android.content.Context
 import com.google.gson.Gson
 import org.openedx.app.BuildConfig
 import org.openedx.core.data.model.User
-import org.openedx.core.data.storage.CalendarPreferences
 import org.openedx.core.data.storage.CorePreferences
 import org.openedx.core.data.storage.InAppReviewPreferences
 import org.openedx.core.domain.model.AppConfig
@@ -13,6 +12,7 @@ import org.openedx.core.domain.model.VideoSettings
 import org.openedx.core.extension.replaceSpace
 import org.openedx.course.data.storage.CoursePreferences
 import org.openedx.profile.data.model.Account
+import org.openedx.profile.data.storage.CalendarPreferences
 import org.openedx.profile.data.storage.ProfilePreferences
 import org.openedx.profile.system.CalendarManager
 import org.openedx.whatsnew.data.storage.WhatsNewPreferences

diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt
@@ -28,7 +28,6 @@ import org.openedx.auth.presentation.sso.OAuthHelper
 import org.openedx.core.ImageProcessor
 import org.openedx.core.config.Config
 import org.openedx.core.data.model.CourseEnrollments
-import org.openedx.core.data.storage.CalendarPreferences
 import org.openedx.core.data.storage.CorePreferences
 import org.openedx.core.data.storage.InAppReviewPreferences
 import org.openedx.core.module.DownloadWorkerController
@@ -59,12 +58,13 @@ import org.openedx.discovery.presentation.DiscoveryRouter
 import org.openedx.discussion.presentation.DiscussionAnalytics
 import org.openedx.discussion.presentation.DiscussionRouter
 import org.openedx.discussion.system.notifier.DiscussionNotifier
+import org.openedx.profile.data.storage.CalendarPreferences
 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.system.notifier.calendar.CalendarNotifier
+import org.openedx.profile.system.notifier.profile.ProfileNotifier
 import org.openedx.profile.worker.CalendarSyncScheduler
 import org.openedx.whatsnew.WhatsNewManager
 import org.openedx.whatsnew.WhatsNewRouter

diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt
@@ -179,7 +179,7 @@ val screenModule = module {
     viewModel { CoursesToSyncViewModel(get(), get(), get()) }
     viewModel { NewCalendarDialogViewModel(get(), get(), get()) }
     viewModel { DisableCalendarSyncDialogViewModel(get(), get()) }
-    single { CalendarRepository(get(), get(), get()) }
+    factory { CalendarRepository(get(), get(), get()) }
     factory { CalendarInteractor(get()) }
 
     single { CourseRepository(get(), get(), get(), get(), get()) }

diff --git a/course/src/main/java/org/openedx/course/presentation/dates/CourseDatesViewModel.kt b/course/src/main/java/org/openedx/course/presentation/dates/CourseDatesViewModel.kt
@@ -136,21 +136,6 @@ class CourseDatesViewModel(
         }
     }
 
-    private fun setCalendarSyncDialogType(dialog: CalendarSyncDialogType) {
-        val value = _uiState.value
-        if (value is DatesUIState.Dates) {
-            viewModelScope.launch {
-                courseNotifier.send(
-                    CreateCalendarSyncEvent(
-                        courseDates = value.courseDatesResult.datesSection.values.flatten(),
-                        dialogType = dialog.name,
-                        checkOutOfSync = false,
-                    )
-                )
-            }
-        }
-    }
-
     private fun checkIfCalendarOutOfDate() {
         val value = _uiState.value
         if (value is DatesUIState.Dates) {
@@ -166,12 +151,6 @@ class CourseDatesViewModel(
         }
     }
 
-    private fun isCalendarSyncEnabled(): Boolean {
-        val calendarSync = corePreferences.appConfig.courseDatesCalendarSync
-        return calendarSync.isEnabled && ((calendarSync.isSelfPacedEnabled && isSelfPaced) ||
-                (calendarSync.isInstructorPacedEnabled && !isSelfPaced))
-    }
-
     fun logPlsBannerViewed() {
         logPLSBannerEvent(CourseAnalyticsEvent.PLS_BANNER_VIEWED)
     }
@@ -195,18 +174,6 @@ class CourseDatesViewModel(
         logDatesEvent(CourseAnalyticsEvent.DATES_COURSE_COMPONENT_CLICKED, params)
     }
 
-    private fun logCalendarSyncToggle(isChecked: Boolean) {
-        logDatesEvent(
-            CourseAnalyticsEvent.DATES_CALENDAR_SYNC_TOGGLE,
-            buildMap {
-                put(
-                    CourseAnalyticsKey.ACTION.key,
-                    if (isChecked) CourseAnalyticsKey.ON.key else CourseAnalyticsKey.OFF.key
-                )
-            }
-        )
-    }
-
     private fun logDatesEvent(
         event: CourseAnalyticsEvent,
         param: Map<String, Any> = emptyMap(),

diff --git a/.../core/data/storage/CalendarPreferences.kt → ...ofile/data/storage/CalendarPreferences.kt b/.../core/data/storage/CalendarPreferences.kt → ...ofile/data/storage/CalendarPreferences.kt
@@ -1,4 +1,4 @@
-package org.openedx.core.data.storage
+package org.openedx.profile.data.storage
 
 interface CalendarPreferences {
     var calendarId: Long

diff --git a/...e/src/main/java/org/openedx/profile/presentation/calendar/CalendarAccessDialogFragment.kt b/...e/src/main/java/org/openedx/profile/presentation/calendar/CalendarAccessDialogFragment.kt
@@ -16,6 +16,8 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.rememberScrollState
+import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.MaterialTheme
 import androidx.compose.material.Text
 import androidx.compose.material.icons.Icons
@@ -86,13 +88,15 @@ private fun CalendarAccessDialog(
     onCancelClick: () -> Unit,
     onGrantCalendarAccessClick: () -> Unit
 ) {
+    val scrollState = rememberScrollState()
     DefaultDialogBox(
         modifier = modifier,
         onDismissClick = onCancelClick
     ) {
         Column(
             modifier = Modifier
                 .fillMaxWidth()
+                .verticalScroll(scrollState)
                 .padding(20.dp),
             horizontalAlignment = Alignment.CenterHorizontally,
             verticalArrangement = Arrangement.spacedBy(16.dp)

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarSetUpView.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarSetUpView.kt
@@ -10,6 +10,8 @@ import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.widthIn
+import androidx.compose.foundation.rememberScrollState
+import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.Card
 import androidx.compose.material.Icon
 import androidx.compose.material.MaterialTheme
@@ -57,6 +59,7 @@ fun CalendarSetUpView(
     onBackClick: () -> Unit
 ) {
     val scaffoldState = rememberScaffoldState()
+    val scrollState = rememberScrollState()
 
     Scaffold(
         modifier = Modifier
@@ -119,11 +122,13 @@ fun CalendarSetUpView(
                     contentAlignment = Alignment.TopCenter
                 ) {
                     Column(
-                        modifier = contentWidth.padding(vertical = 28.dp),
+                        modifier = contentWidth
+                            .verticalScroll(scrollState)
+                            .padding(vertical = 28.dp),
                     ) {
                         Text(
-                            modifier = Modifier.testTag("txt_settings"),
-                            text = stringResource(id = org.openedx.core.R.string.core_settings),
+                            modifier = Modifier.testTag("txt_calendar_sync"),
+                            text = stringResource(id = R.string.profile_calendar_sync),
                             style = MaterialTheme.appTypography.labelLarge,
                             color = MaterialTheme.appColors.textSecondary
                         )

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarSettingsView.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarSettingsView.kt
@@ -13,7 +13,9 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.widthIn
+import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.shape.CircleShape
+import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.Card
 import androidx.compose.material.CircularProgressIndicator
 import androidx.compose.material.ExperimentalMaterialApi
@@ -69,6 +71,7 @@ fun CalendarSettingsView(
     onBackClick: () -> Unit
 ) {
     val scaffoldState = rememberScaffoldState()
+    val scrollState = rememberScrollState()
 
     Scaffold(
         modifier = Modifier
@@ -131,10 +134,10 @@ fun CalendarSettingsView(
                     contentAlignment = Alignment.TopCenter
                 ) {
                     Column(
-                        modifier = contentWidth.padding(vertical = 28.dp),
+                        modifier = contentWidth
+                            .verticalScroll(scrollState)
+                            .padding(vertical = 28.dp),
                     ) {
-                        val coursesSynced = uiState.coursesSynced
-
                         if (uiState.calendarData != null) {
                             CalendarSyncSection(
                                 isCourseCalendarSyncEnabled = uiState.isCalendarSyncEnabled,
@@ -145,10 +148,12 @@ fun CalendarSettingsView(
                             )
                         }
                         Spacer(modifier = Modifier.height(20.dp))
-                        CoursesToSyncSection(
-                            coursesSynced = coursesSynced,
-                            onCourseToSyncClick = onCourseToSyncClick
-                        )
+                        if (uiState.coursesSynced != null) {
+                            CoursesToSyncSection(
+                                coursesSynced = uiState.coursesSynced,
+                                onCourseToSyncClick = onCourseToSyncClick
+                            )
+                        }
                     }
                 }
             }

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarUIState.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarUIState.kt
@@ -7,5 +7,5 @@ data class CalendarUIState(
     val calendarData: CalendarData? = null,
     val calendarSyncState: CalendarSyncState,
     val isCalendarSyncEnabled: Boolean,
-    val coursesSynced: Int
+    val coursesSynced: Int?
 )
diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarViewModel.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CalendarViewModel.kt
@@ -9,17 +9,17 @@ import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import org.openedx.core.BaseViewModel
-import org.openedx.core.data.storage.CalendarPreferences
 import org.openedx.core.domain.interactor.CalendarInteractor
 import org.openedx.core.system.connection.NetworkConnection
+import org.openedx.profile.data.storage.CalendarPreferences
 import org.openedx.profile.presentation.ProfileRouter
 import org.openedx.profile.system.CalendarManager
-import org.openedx.profile.system.notifier.CalendarCreated
-import org.openedx.profile.system.notifier.CalendarNotifier
-import org.openedx.profile.system.notifier.CalendarSyncDisabled
-import org.openedx.profile.system.notifier.CalendarSyncFailed
-import org.openedx.profile.system.notifier.CalendarSynced
-import org.openedx.profile.system.notifier.CalendarSyncing
+import org.openedx.profile.system.notifier.calendar.CalendarCreated
+import org.openedx.profile.system.notifier.calendar.CalendarNotifier
+import org.openedx.profile.system.notifier.calendar.CalendarSyncDisabled
+import org.openedx.profile.system.notifier.calendar.CalendarSyncFailed
+import org.openedx.profile.system.notifier.calendar.CalendarSynced
+import org.openedx.profile.system.notifier.calendar.CalendarSyncing
 import org.openedx.profile.worker.CalendarSyncScheduler
 
 class CalendarViewModel(
@@ -38,7 +38,7 @@ class CalendarViewModel(
             calendarData = null,
             calendarSyncState = if (networkConnection.isOnline()) CalendarSyncState.SYNCED else CalendarSyncState.OFFLINE,
             isCalendarSyncEnabled = calendarPreferences.isCalendarSyncEnabled,
-            coursesSynced = 0
+            coursesSynced = null
         )
     )
     val uiState: StateFlow<CalendarUIState>
@@ -49,7 +49,6 @@ class CalendarViewModel(
             calendarNotifier.notifier.collect { calendarEvent ->
                 when (calendarEvent) {
                     CalendarCreated -> {
-                        calendarSyncScheduler.scheduleDailySync()
                         calendarSyncScheduler.requestImmediateSync()
                         _uiState.update { it.copy(isCalendarExist = true) }
                         getCalendarData()

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CoursesToSyncFragment.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CoursesToSyncFragment.kt
@@ -168,7 +168,8 @@ private fun CoursesToSyncView(
                     contentAlignment = Alignment.TopCenter
                 ) {
                     Column(
-                        modifier = contentWidth.padding(vertical = 28.dp),
+                        modifier = contentWidth
+                            .padding(vertical = 28.dp),
                     ) {
                         Text(
                             text = stringResource(R.string.profile_courses_to_sync_title),

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/CoursesToSyncViewModel.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/CoursesToSyncViewModel.kt
@@ -7,8 +7,8 @@ import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import org.openedx.core.BaseViewModel
-import org.openedx.core.data.storage.CalendarPreferences
 import org.openedx.core.domain.interactor.CalendarInteractor
+import org.openedx.profile.data.storage.CalendarPreferences
 import org.openedx.profile.worker.CalendarSyncScheduler
 
 class CoursesToSyncViewModel(

diff --git a/.../main/java/org/openedx/profile/presentation/calendar/DisableCalendarSyncDialogFragment.kt b/.../main/java/org/openedx/profile/presentation/calendar/DisableCalendarSyncDialogFragment.kt
@@ -15,7 +15,9 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.shape.CircleShape
+import androidx.compose.foundation.verticalScroll
 import androidx.compose.material.MaterialTheme
 import androidx.compose.material.Text
 import androidx.compose.runtime.Composable
@@ -94,13 +96,15 @@ private fun DisableCalendarSyncDialogView(
     onCancelClick: () -> Unit,
     onDisableSyncingClick: () -> Unit
 ) {
+    val scrollState = rememberScrollState()
     DefaultDialogBox(
         modifier = modifier,
         onDismissClick = onCancelClick
     ) {
         Column(
             modifier = Modifier
                 .fillMaxWidth()
+                .verticalScroll(scrollState)
                 .padding(20.dp),
             horizontalAlignment = Alignment.CenterHorizontally,
             verticalArrangement = Arrangement.spacedBy(16.dp)
@@ -148,11 +152,13 @@ private fun DisableCalendarSyncDialogView(
             }
             Text(
                 modifier = Modifier.fillMaxWidth(),
-                text = stringResource(id = org.openedx.profile.R.string.profile_disable_calendar_dialog_description),
+                text = stringResource(
+                    id = org.openedx.profile.R.string.profile_disable_calendar_dialog_description,
+                    calendarData?.title ?: ""
+                ),
                 style = MaterialTheme.appTypography.bodyMedium,
                 color = MaterialTheme.appColors.textDark
             )
-
             OpenEdXOutlinedButton(
                 modifier = Modifier.fillMaxWidth(),
                 text = stringResource(id = R.string.profile_disable_syncing),

diff --git a/...main/java/org/openedx/profile/presentation/calendar/DisableCalendarSyncDialogViewModel.kt b/...main/java/org/openedx/profile/presentation/calendar/DisableCalendarSyncDialogViewModel.kt
@@ -3,9 +3,9 @@ package org.openedx.profile.presentation.calendar
 import androidx.lifecycle.viewModelScope
 import kotlinx.coroutines.launch
 import org.openedx.core.BaseViewModel
-import org.openedx.core.data.storage.CalendarPreferences
-import org.openedx.profile.system.notifier.CalendarNotifier
-import org.openedx.profile.system.notifier.CalendarSyncDisabled
+import org.openedx.profile.data.storage.CalendarPreferences
+import org.openedx.profile.system.notifier.calendar.CalendarNotifier
+import org.openedx.profile.system.notifier.calendar.CalendarSyncDisabled
 
 class DisableCalendarSyncDialogViewModel(
     private val calendarPreferences: CalendarPreferences,