fix: Fixes according to PR feedback

openedx · Jul 17, 2024 · 5cd96fb · 5cd96fb
1 parent 71ab360
commit 5cd96fb
Show file tree

Hide file tree

Showing 11 changed files with 133 additions and 58 deletions.
diff --git a/core/src/main/java/org/openedx/core/presentation/settings/calendarsync/CalendarSyncState.kt b/core/src/main/java/org/openedx/core/presentation/settings/calendarsync/CalendarSyncState.kt
@@ -30,7 +30,7 @@ enum class CalendarSyncState(
         Icons.Rounded.FreeCancellation
     ),
     SYNCED(
-        R.string.core_synced,
+        R.string.core_to_sync,
         R.string.core_synced_to_calendar,
         Icons.Rounded.EventRepeat
     ),

diff --git a/core/src/main/java/org/openedx/core/worker/CalendarSyncWorker.kt b/core/src/main/java/org/openedx/core/worker/CalendarSyncWorker.kt
@@ -42,6 +42,8 @@ class CalendarSyncWorker(
     private val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
     private val notificationBuilder = NotificationCompat.Builder(context, NOTIFICATION_CHANEL_ID)
 
+    private val failedCoursesSync = mutableSetOf<String>()
+
     override suspend fun doWork(): Result {
         return try {
             setForeground(createForegroundInfo())
@@ -98,7 +100,11 @@ class CalendarSyncWorker(
                 syncCalendar(enrollmentsStatus, courseId)
             }
             removeUnenrolledCourseEvents(enrollmentsStatus)
-            calendarNotifier.send(CalendarSynced)
+            if (failedCoursesSync.isEmpty()) {
+                calendarNotifier.send(CalendarSynced)
+            } else {
+                calendarNotifier.send(CalendarSyncFailed)
+            }
         }
     }
 
@@ -127,17 +133,22 @@ class CalendarSyncWorker(
     }
 
     private suspend fun syncCourseEvents(enrollmentStatus: EnrollmentStatus) {
-        createCalendarState(enrollmentStatus)
         val courseId = enrollmentStatus.courseId
-        if (enrollmentStatus.isActive && isCourseSyncEnabled(courseId)) {
-            val courseDates = calendarInteractor.getCourseDates(courseId)
-            val isCourseCalendarUpToDate = isCourseCalendarUpToDate(courseId, courseDates)
-            if (!isCourseCalendarUpToDate) {
+        try {
+            createCalendarState(enrollmentStatus)
+            if (enrollmentStatus.isActive && isCourseSyncEnabled(courseId)) {
+                val courseDates = calendarInteractor.getCourseDates(courseId)
+                val isCourseCalendarUpToDate = isCourseCalendarUpToDate(courseId, courseDates)
+                if (!isCourseCalendarUpToDate) {
+                    removeCalendarEvents(courseId)
+                    updateCourseEvents(courseDates, enrollmentStatus)
+                }
+            } else {
                 removeCalendarEvents(courseId)
-                updateCourseEvents(courseDates, enrollmentStatus)
             }
-        } else {
-            removeCalendarEvents(courseId)
+        } catch (e: Exception) {
+            failedCoursesSync.add(courseId)
+            e.printStackTrace()
         }
     }
 

diff --git a/...c/main/res/drawable/dashboard_ic_book.xml → core/src/main/res/drawable/core_ic_book.xml b/...c/main/res/drawable/dashboard_ic_book.xml → core/src/main/res/drawable/core_ic_book.xml
diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml
@@ -186,7 +186,7 @@
     <string name="core_calendar_sync_failed">Calendar Sync Failed</string>
     <string name="core_synced_to_calendar">Synced to Calendar</string>
     <string name="core_syncing_failed">Sync Failed</string>
-    <string name="core_synced">Synced</string>
+    <string name="core_to_sync">To Sync</string>
     <string name="core_not_synced">Not Synced</string>
     <string name="core_syncing_to_calendar">Syncing to calendar…</string>
 </resources>
diff --git a/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesView.kt b/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesView.kt
@@ -527,7 +527,7 @@ fun EmptyState(
             horizontalAlignment = Alignment.CenterHorizontally
         ) {
             Icon(
-                painter = painterResource(id = org.openedx.dashboard.R.drawable.dashboard_ic_book),
+                painter = painterResource(id = R.drawable.core_ic_book),
                 tint = MaterialTheme.appColors.textFieldBorder,
                 contentDescription = null
             )

diff --git a/dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryView.kt b/dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryView.kt
@@ -384,7 +384,7 @@ private fun ViewAllItem(
         ) {
             Icon(
                 modifier = Modifier.size(48.dp),
-                painter = painterResource(id = R.drawable.dashboard_ic_book),
+                painter = painterResource(id = CoreR.drawable.core_ic_book),
                 tint = MaterialTheme.appColors.textFieldBorder,
                 contentDescription = null
             )
@@ -749,7 +749,7 @@ private fun NoCoursesInfo(
             horizontalAlignment = Alignment.CenterHorizontally
         ) {
             Icon(
-                painter = painterResource(id = R.drawable.dashboard_ic_book),
+                painter = painterResource(id = CoreR.drawable.core_ic_book),
                 tint = MaterialTheme.appColors.textFieldBorder,
                 contentDescription = null
             )

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
@@ -69,6 +69,7 @@ class CalendarViewModel(
 
                     CalendarSyncFailed -> {
                         _uiState.update { it.copy(calendarSyncState = CalendarSyncState.SYNC_FAILED) }
+                        updateSyncedCoursesCount()
                     }
 
                     CalendarSyncOffline -> {

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
@@ -5,6 +5,7 @@ import android.view.LayoutInflater
 import android.view.ViewGroup
 import androidx.compose.foundation.background
 import androidx.compose.foundation.border
+import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
@@ -22,6 +23,7 @@ import androidx.compose.material.Checkbox
 import androidx.compose.material.CheckboxDefaults
 import androidx.compose.material.CircularProgressIndicator
 import androidx.compose.material.ExperimentalMaterialApi
+import androidx.compose.material.Icon
 import androidx.compose.material.LocalMinimumInteractiveComponentEnforcement
 import androidx.compose.material.MaterialTheme
 import androidx.compose.material.Scaffold
@@ -43,10 +45,12 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.platform.ViewCompositionStrategy
+import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.SpanStyle
 import androidx.compose.ui.text.buildAnnotatedString
 import androidx.compose.ui.text.font.FontWeight
+import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.text.withStyle
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.Dp
@@ -69,6 +73,7 @@ import org.openedx.core.ui.theme.appTypography
 import org.openedx.core.ui.theme.fontFamily
 import org.openedx.core.ui.windowSizeValue
 import org.openedx.profile.R
+import org.openedx.core.R as coreR
 
 class CoursesToSyncFragment : Fragment() {
 
@@ -274,58 +279,112 @@ private fun CourseCheckboxList(
                 .map { it.courseId }
             val filteredEnrollments = uiState.enrollmentsStatus
                 .filter { it.courseId in courseIds }
-            items(filteredEnrollments) { course ->
-                val isCourseSyncEnabled =
-                    uiState.coursesCalendarState.find { it.courseId == course.courseId }?.isCourseSyncEnabled ?: false
-                val annotatedString = buildAnnotatedString {
-                    append(course.courseName)
-                    if (!course.isActive) {
-                        append(" ")
-                        withStyle(
-                            style = SpanStyle(
-                                fontSize = 10.sp,
-                                fontWeight = FontWeight.Normal,
-                                letterSpacing = 0.sp,
-                                fontFamily = fontFamily,
-                                color = MaterialTheme.appColors.textFieldHint,
-                            )
-                        ) {
-                            append(stringResource(R.string.profile_inactive))
-                        }
+                .let { enrollments ->
+                    if (uiState.isHideInactiveCourses) {
+                        enrollments.filter { it.isActive }
+                    } else {
+                        enrollments
                     }
                 }
-
-                if (uiState.isHideInactiveCourses && !course.isActive) return@items
-                Row(
-                    modifier = Modifier
-                        .fillMaxWidth()
-                        .padding(vertical = 8.dp),
-                    verticalAlignment = Alignment.CenterVertically
-                ) {
-                    Checkbox(
-                        modifier = Modifier.size(24.dp),
-                        colors = CheckboxDefaults.colors(
-                            checkedColor = MaterialTheme.appColors.primary,
-                            uncheckedColor = MaterialTheme.appColors.textFieldText
-                        ),
-                        checked = isCourseSyncEnabled,
-                        enabled = course.isActive,
-                        onCheckedChange = { isEnabled ->
-                            onCourseSyncCheckChange(isEnabled, course.courseId)
-                        }
-                    )
-                    Spacer(modifier = Modifier.width(8.dp))
-                    Text(
-                        text = annotatedString,
-                        style = MaterialTheme.appTypography.labelLarge,
-                        color = MaterialTheme.appColors.textDark
+            if (filteredEnrollments.isEmpty()) {
+                item {
+                    EmptyListState(
+                        selectedTab = selectedTab
                     )
                 }
+            } else {
+                items(filteredEnrollments) { course ->
+                    val isCourseSyncEnabled =
+                        uiState.coursesCalendarState.find { it.courseId == course.courseId }?.isCourseSyncEnabled
+                            ?: false
+                    val annotatedString = buildAnnotatedString {
+                        append(course.courseName)
+                        if (!course.isActive) {
+                            append(" ")
+                            withStyle(
+                                style = SpanStyle(
+                                    fontSize = 10.sp,
+                                    fontWeight = FontWeight.Normal,
+                                    letterSpacing = 0.sp,
+                                    fontFamily = fontFamily,
+                                    color = MaterialTheme.appColors.textFieldHint,
+                                )
+                            ) {
+                                append(stringResource(R.string.profile_inactive))
+                            }
+                        }
+                    }
+                    Row(
+                        modifier = Modifier
+                            .fillMaxWidth()
+                            .padding(vertical = 8.dp),
+                        verticalAlignment = Alignment.CenterVertically
+                    ) {
+                        Checkbox(
+                            modifier = Modifier.size(24.dp),
+                            colors = CheckboxDefaults.colors(
+                                checkedColor = MaterialTheme.appColors.primary,
+                                uncheckedColor = MaterialTheme.appColors.textFieldText
+                            ),
+                            checked = isCourseSyncEnabled,
+                            enabled = course.isActive,
+                            onCheckedChange = { isEnabled ->
+                                onCourseSyncCheckChange(isEnabled, course.courseId)
+                            }
+                        )
+                        Spacer(modifier = Modifier.width(8.dp))
+                        Text(
+                            text = annotatedString,
+                            style = MaterialTheme.appTypography.labelLarge,
+                            color = MaterialTheme.appColors.textDark
+                        )
+                    }
+                }
             }
         }
     }
 }
 
+@Composable
+private fun EmptyListState(
+    modifier: Modifier = Modifier,
+    selectedTab: SyncCourseTab,
+) {
+    val description = if (selectedTab == SyncCourseTab.SYNCED) {
+        stringResource(id = R.string.profile_no_sync_courses)
+    } else {
+        stringResource(id = R.string.profile_no_courses_with_current_filter)
+    }
+    Column(
+        modifier = modifier
+            .fillMaxWidth()
+            .padding(horizontal = 40.dp, vertical = 60.dp),
+        horizontalAlignment = Alignment.CenterHorizontally,
+        verticalArrangement = Arrangement.spacedBy(12.dp)
+    ) {
+        Icon(
+            modifier = Modifier.size(96.dp),
+            painter = painterResource(id = coreR.drawable.core_ic_book),
+            tint = MaterialTheme.appColors.divider,
+            contentDescription = null
+        )
+        Text(
+            text = stringResource(
+                id = R.string.profile_no_courses,
+                stringResource(id = selectedTab.title)
+            ),
+            style = MaterialTheme.appTypography.titleMedium,
+            color = MaterialTheme.appColors.textDark
+        )
+        Text(
+            text = description,
+            style = MaterialTheme.appTypography.labelMedium,
+            color = MaterialTheme.appColors.textDark,
+            textAlign = TextAlign.Center
+        )
+    }
+}
+
 @OptIn(ExperimentalMaterialApi::class)
 @Composable
 private fun HideInactiveCoursesView(

diff --git a/...ile/src/main/java/org/openedx/profile/presentation/calendar/NewCalendarDialogViewModel.kt b/...ile/src/main/java/org/openedx/profile/presentation/calendar/NewCalendarDialogViewModel.kt
@@ -40,6 +40,7 @@ class NewCalendarDialogViewModel(
             if (networkConnection.isOnline()) {
                 calendarInteractor.resetChecksums()
                 val calendarId = calendarManager.createOrUpdateCalendar(
+                    calendarId = calendarPreferences.calendarId,
                     calendarTitle = calendarTitle,
                     calendarColor = calendarColor.color
                 )

diff --git a/profile/src/main/java/org/openedx/profile/presentation/calendar/SyncCourseTab.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/SyncCourseTab.kt
@@ -7,6 +7,6 @@ enum class SyncCourseTab(
     @StringRes
     val title: Int
 ) {
-    SYNCED(R.string.core_synced),
+    SYNCED(R.string.core_to_sync),
     NOT_SYNCED(R.string.core_not_synced)
 }
diff --git a/profile/src/main/res/values/strings.xml b/profile/src/main/res/values/strings.xml
@@ -75,5 +75,8 @@
     <string name="profile_disable_calendar_dialog_title">Disable Calendar Sync</string>
     <string name="profile_disable_calendar_dialog_description">Disabling calendar sync will delete the calendar “%1$s.” You can turn calendar sync back on at any time.</string>
     <string name="profile_disable_syncing">Disable Syncing</string>
+    <string name="profile_no_courses">No %1$s Courses</string>
+    <string name="profile_no_sync_courses">No courses are currently being synced to your calendar.</string>
+    <string name="profile_no_courses_with_current_filter">No courses match the current filter.</string>
 
 </resources>