fix: Address PR comments

edx · Jan 13, 2025 · b7cc5cd · b7cc5cd
1 parent 8561166
commit b7cc5cd
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 28 deletions.
diff --git a/.../src/main/java/org/openedx/notifications/presentation/inbox/NotificationsInboxFragment.kt b/.../src/main/java/org/openedx/notifications/presentation/inbox/NotificationsInboxFragment.kt
@@ -21,7 +21,6 @@ import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.lazy.items
 import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.foundation.shape.CircleShape
-import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material.CircularProgressIndicator
 import androidx.compose.material.DropdownMenu
 import androidx.compose.material.DropdownMenuItem
@@ -71,14 +70,12 @@ import org.openedx.core.ui.displayCutoutForLandscape
 import org.openedx.core.ui.rememberWindowSize
 import org.openedx.core.ui.shouldLoadMore
 import org.openedx.core.ui.statusBarsInset
-import org.openedx.core.ui.theme.AppShapes
 import org.openedx.core.ui.theme.OpenEdXTheme
 import org.openedx.core.ui.theme.appColors
 import org.openedx.core.ui.theme.appShapes
 import org.openedx.core.ui.theme.appTypography
 import org.openedx.core.ui.windowSizeValue
 import org.openedx.notifications.R
-import org.openedx.notifications.data.model.NotificationsMenuType
 import org.openedx.notifications.domain.model.InboxSection
 import org.openedx.notifications.domain.model.NotificationContent
 import org.openedx.notifications.domain.model.NotificationItem

diff --git a/...src/main/java/org/openedx/notifications/presentation/inbox/NotificationsInboxViewModel.kt b/...src/main/java/org/openedx/notifications/presentation/inbox/NotificationsInboxViewModel.kt
@@ -117,7 +117,7 @@ class NotificationsInboxViewModel(
     ) {
         viewModelScope.launch {
             try {
-                if (interactor.markNotificationAsRead(notification.id)) {
+                if (notification.isUnread() && interactor.markNotificationAsRead(notification.id)) {
                     val currentSection = notifications[inboxSection] ?: return@launch
 
                     val index = currentSection.indexOfFirst { it.id == notification.id }
@@ -142,6 +142,30 @@ class NotificationsInboxViewModel(
         }
     }
 
+    fun markAllNotificationsAsRead() {
+        viewModelScope.launch {
+            try {
+                if (_uiState.value is InboxUIState.Data) {
+                    interactor.markAllNotificationsAsRead()
+
+                    notifications.forEach { (section, notificationItems) ->
+                        notifications[section] = notificationItems
+                            .map { it.copy(lastRead = Date()) }
+                            .toMutableList()
+                    }
+                    _uiState.value = InboxUIState.Data(notifications = notifications.toMap())
+                }
+            } catch (e: Exception) {
+                e.printStackTrace()
+                emitErrorMessage(e)
+            }
+        }
+    }
+
+    fun navigateToPushNotificationsSettings(fm: FragmentManager) {
+        notificationsRouter.navigateToPushNotificationsSettings(fm)
+    }
+
     private suspend fun emitErrorMessage(e: Exception) {
         if (e.isInternetError()) {
             _uiMessage.emit(
@@ -153,26 +177,4 @@ class NotificationsInboxViewModel(
             )
         }
     }
-
-    fun navigateToPushNotificationsSettings(fm: FragmentManager) {
-        notificationsRouter.navigateToPushNotificationsSettings(fm)
-    }
-
-    fun markAllNotificationsAsRead() {
-        viewModelScope.launch {
-            try {
-                interactor.markAllNotificationsAsRead()
-
-                notifications.forEach { (section, notificationItems) ->
-                    notifications[section] =
-                        notificationItems.map { it.copy(lastRead = Date()) }.toMutableList()
-                }
-
-                _uiState.value = InboxUIState.Data(notifications = notifications.toMap())
-            } catch (e: Exception) {
-                e.printStackTrace()
-                emitErrorMessage(e)
-            }
-        }
-    }
 }
diff --git a/...tions/data/model/NotificationsMenuType.kt → ...esentation/inbox/NotificationsMenuType.kt b/...tions/data/model/NotificationsMenuType.kt → ...esentation/inbox/NotificationsMenuType.kt
@@ -1,4 +1,4 @@
-package org.openedx.notifications.data.model
+package org.openedx.notifications.presentation.inbox
 
 import androidx.annotation.StringRes
 import org.openedx.notifications.R

diff --git a/notifications/src/main/res/values/strings.xml b/notifications/src/main/res/values/strings.xml
@@ -9,7 +9,7 @@
     <string name="notifications_no_notifications_yet">No notifications yet</string>
     <string name="notifications_no_notifications_yet_description">When you receive notifications they’ll show up here</string>
 
-    <string name="notifications_menu_settings">Notification Settings</string>
+    <string name="notifications_menu_settings">Notification settings</string>
     <string name="notifications_menu_mark_all_read">Mark all as read</string>
     <plurals name="notifications_date_format_seconds">
         <item quantity="one">%1$s second</item>