From 5cd96fbb97a405a6d378d7c7bc5a8d1e487073a2 Mon Sep 17 00:00:00 2001 From: PavloNetrebchuk Date: Wed, 17 Jul 2024 13:51:05 +0300 Subject: [PATCH] fix: Fixes according to PR feedback --- .../calendarsync/CalendarSyncState.kt | 2 +- .../openedx/core/worker/CalendarSyncWorker.kt | 29 ++-- .../src/main/res/drawable/core_ic_book.xml | 0 core/src/main/res/values/strings.xml | 2 +- .../presentation/AllEnrolledCoursesView.kt | 2 +- .../presentation/DashboardGalleryView.kt | 4 +- .../calendar/CalendarViewModel.kt | 1 + .../calendar/CoursesToSyncFragment.kt | 145 ++++++++++++------ .../calendar/NewCalendarDialogViewModel.kt | 1 + .../presentation/calendar/SyncCourseTab.kt | 2 +- profile/src/main/res/values/strings.xml | 3 + 11 files changed, 133 insertions(+), 58 deletions(-) rename dashboard/src/main/res/drawable/dashboard_ic_book.xml => core/src/main/res/drawable/core_ic_book.xml (100%) 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 index 3e6188ed1..95a851442 100644 --- 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 index 986eb9b9d..2c36f075b 100644 --- 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() + 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/dashboard/src/main/res/drawable/dashboard_ic_book.xml b/core/src/main/res/drawable/core_ic_book.xml similarity index 100% rename from dashboard/src/main/res/drawable/dashboard_ic_book.xml rename to 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 index dc055138e..9aded8c31 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -186,7 +186,7 @@ Calendar Sync Failed Synced to Calendar Sync Failed - Synced + To Sync Not Synced Syncing to calendar… diff --git a/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesView.kt b/dashboard/src/main/java/org/openedx/courses/presentation/AllEnrolledCoursesView.kt index 3392ed7bd..c2668f766 100644 --- 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 index 7401f6304..a6d375569 100644 --- 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 index 29cb017a6..658d7ca8e 100644 --- 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 index b47d47d11..7b4d1d9d0 100644 --- 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/profile/src/main/java/org/openedx/profile/presentation/calendar/NewCalendarDialogViewModel.kt b/profile/src/main/java/org/openedx/profile/presentation/calendar/NewCalendarDialogViewModel.kt index 3085f7db9..e43f1b989 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/calendar/NewCalendarDialogViewModel.kt +++ b/profile/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 index a68fedc3e..ef65db249 100644 --- 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 index a36595f71..41535240c 100644 --- a/profile/src/main/res/values/strings.xml +++ b/profile/src/main/res/values/strings.xml @@ -75,5 +75,8 @@ Disable Calendar Sync Disabling calendar sync will delete the calendar “%1$s.” You can turn calendar sync back on at any time. Disable Syncing + No %1$s Courses + No courses are currently being synced to your calendar. + No courses match the current filter.