fix #90 don't dismiss failed message action (#137)

# *don't dismiss failed message action* ## ♻️ Current situation & Problem #90 ## ⚙️ Release Notes - don't dismiss failed message action ## 📝 Code of Conduct & Contributing Guidelines By submitting creating this pull request, you agree to follow our [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md). --------- Signed-off-by: Basler182 <[email protected]>
StanfordSpezi · Nov 3, 2024 · f7effec · f7effec
1 parent 891883b
commit f7effec
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 30 deletions.
diff --git a/app/src/main/kotlin/edu/stanford/bdh/engagehf/bluetooth/BluetoothViewModel.kt b/app/src/main/kotlin/edu/stanford/bdh/engagehf/bluetooth/BluetoothViewModel.kt
@@ -8,6 +8,7 @@ import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
 import dagger.hilt.android.qualifiers.ApplicationContext
+import edu.stanford.bdh.engagehf.R
 import edu.stanford.bdh.engagehf.bluetooth.component.AppScreenEvents
 import edu.stanford.bdh.engagehf.bluetooth.data.mapper.BluetoothUiStateMapper
 import edu.stanford.bdh.engagehf.bluetooth.data.models.Action
@@ -27,7 +28,9 @@ import edu.stanford.spezi.core.logging.speziLogger
 import edu.stanford.spezi.core.navigation.Navigator
 import edu.stanford.spezi.core.notification.notifier.FirebaseMessage
 import edu.stanford.spezi.core.notification.notifier.FirebaseMessage.Companion.FIREBASE_MESSAGE_KEY
+import edu.stanford.spezi.core.utils.MessageNotifier
 import edu.stanford.spezi.modules.education.EducationNavigationEvent
+import edu.stanford.spezi.modules.education.videos.Video
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.update
@@ -45,6 +48,7 @@ class BluetoothViewModel @Inject internal constructor(
     private val navigator: Navigator,
     private val engageEducationRepository: EngageEducationRepository,
     private val healthSummaryService: HealthSummaryService,
+    private val messageNotifier: MessageNotifier,
     @ApplicationContext private val context: Context,
 ) : ViewModel() {
     private val logger by speziLogger()
@@ -79,6 +83,7 @@ class BluetoothViewModel @Inject internal constructor(
                             it.copy(measurementDialog = dialog)
                         }
                     }
+
                     is EngageBLEServiceEvent.DeviceDiscovered,
                     is EngageBLEServiceEvent.DeviceConnected,
                     is EngageBLEServiceEvent.DevicePaired,
@@ -218,9 +223,18 @@ class BluetoothViewModel @Inject internal constructor(
         messageId: String,
         isDismissible: Boolean,
     ) {
+        var shouldDismissMessage = isDismissible
         when (messagesAction) {
             is MessagesAction.HealthSummaryAction -> {
-                handleHealthSummaryAction(messageId)
+                handleHealthSummaryAction(messageId).onFailure {
+                    shouldDismissMessage = false
+                }
+            }
+
+            is MessagesAction.VideoSectionAction -> {
+                handleVideoSectionAction(messagesAction).onFailure {
+                    shouldDismissMessage = false
+                }
             }
 
             is MessagesAction.MeasurementsAction -> {
@@ -242,32 +256,27 @@ class BluetoothViewModel @Inject internal constructor(
                     )
                 )
             }
-
-            is MessagesAction.VideoSectionAction -> {
-                viewModelScope.launch {
-                    handleVideoSectionAction(messagesAction)
-                }
-            }
         }
-        if (isDismissible) {
+        if (shouldDismissMessage) {
             messageRepository.completeMessage(messageId = messageId)
         }
     }
 
-    private suspend fun handleVideoSectionAction(messageAction: MessagesAction.VideoSectionAction) {
-        engageEducationRepository.getVideoBySectionAndVideoId(
+    private suspend fun handleVideoSectionAction(messageAction: MessagesAction.VideoSectionAction): Result<Video> {
+        return engageEducationRepository.getVideoBySectionAndVideoId(
             messageAction.videoSectionVideo.videoSectionId,
             messageAction.videoSectionVideo.videoId
-        ).getOrNull()?.let { video ->
-            navigator.navigateTo(
-                EducationNavigationEvent.VideoSectionClicked(
-                    video = video
-                )
+        ).onSuccess { video ->
+            navigator.navigateTo(EducationNavigationEvent.VideoSectionClicked(video))
+        }.onFailure {
+            messageNotifier.notify(
+                messageId = R.string.error_while_handling_message_action
             )
+            logger.e(it) { "Error while getting video by section and video id" }
         }
     }
 
-    private fun handleHealthSummaryAction(messageId: String) {
+    private suspend fun handleHealthSummaryAction(messageId: String): Result<Unit> {
         val setLoading = { loading: Boolean ->
             _uiState.update {
                 it.copy(
@@ -281,11 +290,10 @@ class BluetoothViewModel @Inject internal constructor(
                 )
             }
         }
-        viewModelScope.launch {
-            setLoading(true)
-            healthSummaryService.generateHealthSummaryPdf()
-            setLoading(false)
-        }
+        setLoading(true)
+        val result = healthSummaryService.generateHealthSummaryPdf()
+        setLoading(false)
+        return result
     }
 
     public override fun onCleared() {

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -121,4 +121,5 @@
     <string name="specific_symptoms_score_description">This represents the frequency you are experiencing heart-related symptoms. Higher scores mean less frequent symptoms and lower scores mean more frequent symptoms.</string>
     <string name="dizziness_score_title">Dizziness Score</string>
     <string name="dizziness_score_description">Your dizziness is important to keep track of because dizziness can be a side effect of your heart or your heart medicines.</string>
+    <string name="error_while_handling_message_action">Error while handling message action.</string>
 </resources>
diff --git a/app/src/test/kotlin/edu/stanford/bdh/engagehf/bluetooth/BluetoothViewModelTest.kt b/app/src/test/kotlin/edu/stanford/bdh/engagehf/bluetooth/BluetoothViewModelTest.kt
@@ -5,6 +5,7 @@ import androidx.health.connect.client.records.BloodPressureRecord
 import androidx.health.connect.client.records.HeartRateRecord
 import androidx.health.connect.client.records.WeightRecord
 import com.google.common.truth.Truth.assertThat
+import edu.stanford.bdh.engagehf.R
 import edu.stanford.bdh.engagehf.bluetooth.component.AppScreenEvents
 import edu.stanford.bdh.engagehf.bluetooth.data.mapper.BluetoothUiStateMapper
 import edu.stanford.bdh.engagehf.bluetooth.data.models.Action
@@ -28,6 +29,7 @@ import edu.stanford.bdh.engagehf.navigation.screens.BottomBarItem
 import edu.stanford.spezi.core.navigation.Navigator
 import edu.stanford.spezi.core.testing.CoroutineTestRule
 import edu.stanford.spezi.core.testing.runTestUnconfined
+import edu.stanford.spezi.core.utils.MessageNotifier
 import edu.stanford.spezi.modules.education.EducationNavigationEvent
 import edu.stanford.spezi.modules.education.videos.Video
 import io.mockk.Runs
@@ -53,6 +55,7 @@ class BluetoothViewModelTest {
     private val engageEducationRepository = mockk<EngageEducationRepository>(relaxed = true)
     private val healthSummaryService = mockk<HealthSummaryService>(relaxed = true)
     private val context: Context = mockk(relaxed = true)
+    private val messageNotifier = mockk<MessageNotifier>(relaxed = true)
 
     private val bleServiceState =
         MutableStateFlow<EngageBLEServiceState>(EngageBLEServiceState.Idle)
@@ -312,6 +315,53 @@ class BluetoothViewModelTest {
         coVerify { messageRepository.completeMessage(messageId = messageId) }
     }
 
+    @Test
+    fun `it should not dismiss health summary message on error result`() {
+        // given
+        val action = Action.MessageItemClicked(message = message)
+        every {
+            uiStateMapper.mapMessagesAction(messageAction)
+        } returns Result.success(MessagesAction.HealthSummaryAction)
+        coEvery {
+            healthSummaryService.generateHealthSummaryPdf()
+        } returns Result.failure(Throwable())
+        createViewModel()
+
+        // when
+        bluetoothViewModel.onAction(action = action)
+
+        // then
+        coVerify(
+            exactly = 0
+        ) { messageRepository.completeMessage(messageId = messageId) }
+    }
+
+    @Test
+    fun `it should not dismiss video section message on error result`() {
+        // given
+        val action = Action.MessageItemClicked(message = message)
+        every {
+            uiStateMapper.mapMessagesAction(messageAction)
+        } returns Result.success(MessagesAction.VideoSectionAction(VideoSectionVideo("1", "2")))
+        coEvery {
+            engageEducationRepository.getVideoBySectionAndVideoId(any(), any())
+        } returns Result.failure(Throwable())
+        createViewModel()
+
+        // when
+        bluetoothViewModel.onAction(action = action)
+
+        // then
+        coVerify(
+            exactly = 0
+        ) { messageRepository.completeMessage(messageId = messageId) }
+        coVerify {
+            messageNotifier.notify(
+                messageId = R.string.error_while_handling_message_action
+            )
+        }
+    }
+
     @Test
     fun `it should handle MeasurementsAction correctly`() {
         // given
@@ -509,6 +559,7 @@ class BluetoothViewModelTest {
             engageEducationRepository = engageEducationRepository,
             healthSummaryService = healthSummaryService,
             context = context,
+            messageNotifier = messageNotifier
         )
     }
 }
diff --git a/...n/src/androidTest/java/edu/stanford/spezi/modules/education/videos/EducationScreenTest.kt b/...n/src/androidTest/java/edu/stanford/spezi/modules/education/videos/EducationScreenTest.kt
@@ -4,6 +4,8 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
 import edu.stanford.spezi.core.design.component.ComposeContentActivity
+import edu.stanford.spezi.core.design.component.StringResource
+import edu.stanford.spezi.modules.education.R
 import org.junit.Rule
 import org.junit.Test
 
@@ -48,7 +50,7 @@ class EducationScreenTest {
     fun `education screen should display retry button`() {
         composeTestRule.activity.setScreen {
             EducationScreen(
-                uiState = UiState.Error("Could not load videos"),
+                uiState = UiState.Error(StringResource(R.string.failed_to_load_video_sections)),
                 onAction = {},
             )
         }

diff --git a/...es/education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationScreen.kt b/...es/education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationScreen.kt
@@ -19,12 +19,14 @@ import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import androidx.hilt.navigation.compose.hiltViewModel
 import edu.stanford.spezi.core.design.component.Button
 import edu.stanford.spezi.core.design.component.RepeatingLazyColumn
+import edu.stanford.spezi.core.design.component.StringResource
 import edu.stanford.spezi.core.design.component.VerticalSpacer
 import edu.stanford.spezi.core.design.theme.Colors
 import edu.stanford.spezi.core.design.theme.Spacings
 import edu.stanford.spezi.core.design.theme.SpeziTheme
 import edu.stanford.spezi.core.design.theme.ThemePreviews
 import edu.stanford.spezi.core.utils.extensions.testIdentifier
+import edu.stanford.spezi.modules.education.R
 import edu.stanford.spezi.modules.education.videos.component.ExpandableVideoSection
 import edu.stanford.spezi.modules.education.videos.component.LoadingVideoCard
 
@@ -64,7 +66,7 @@ fun EducationScreen(
             ) {
                 Column {
                     Text(
-                        text = uiState.message,
+                        text = uiState.message.text(),
                         modifier = Modifier.align(Alignment.CenterHorizontally),
                         color = Colors.error
                     )
@@ -114,7 +116,7 @@ private class EducationUiStatePreviewProvider :
     PreviewParameterProvider<Pair<UiState, (Action) -> Unit>> {
     override val values: Sequence<Pair<UiState, (Action) -> Unit>> = sequenceOf(
         Pair(UiState.Loading) {},
-        Pair(UiState.Error("An error occurred")) {},
+        Pair(UiState.Error(StringResource(R.string.failed_to_load_video_sections))) {},
         Pair(
             UiState.Success(
                 EducationUiState(

diff --git a/...s/education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationUiState.kt b/...s/education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationUiState.kt
@@ -1,5 +1,6 @@
 package edu.stanford.spezi.modules.education.videos
 
+import edu.stanford.spezi.core.design.component.StringResource
 import kotlinx.serialization.Serializable
 
 data class EducationUiState(
@@ -9,7 +10,7 @@ data class EducationUiState(
 sealed interface UiState {
     data object Loading : UiState
     data class Success(val data: EducationUiState) : UiState
-    data class Error(val message: String) : UiState
+    data class Error(val message: StringResource) : UiState
 }
 
 data class VideoSection(

diff --git a/...education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationViewModel.kt b/...education/src/main/java/edu/stanford/spezi/modules/education/videos/EducationViewModel.kt
@@ -3,8 +3,11 @@ package edu.stanford.spezi.modules.education.videos
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
+import edu.stanford.spezi.core.design.component.StringResource
+import edu.stanford.spezi.core.logging.speziLogger
 import edu.stanford.spezi.core.navigation.Navigator
 import edu.stanford.spezi.modules.education.EducationNavigationEvent
+import edu.stanford.spezi.modules.education.R
 import edu.stanford.spezi.modules.education.videos.data.repository.EducationRepository
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.asStateFlow
@@ -17,6 +20,7 @@ internal class EducationViewModel @Inject constructor(
     private val educationRepository: EducationRepository,
     private val navigator: Navigator,
 ) : ViewModel() {
+    private val logger by speziLogger()
     private val _uiState = MutableStateFlow<UiState>(UiState.Loading)
     val uiState = _uiState.asStateFlow()
 
@@ -27,11 +31,12 @@ internal class EducationViewModel @Inject constructor(
     private fun loadVideoSections() {
         viewModelScope.launch {
             _uiState.value = UiState.Loading
-            val result = educationRepository.getVideoSections()
-            _uiState.value = if (result.isSuccess) {
-                UiState.Success(EducationUiState(videoSections = result.getOrNull() ?: emptyList()))
-            } else {
-                UiState.Error("Failed to load video sections")
+            educationRepository.getVideoSections().onFailure {
+                logger.e(it) { "Failed to load video sections" }
+                _uiState.value =
+                    UiState.Error(StringResource(R.string.failed_to_load_video_sections))
+            }.onSuccess { videoSections ->
+                _uiState.value = UiState.Success(EducationUiState(videoSections = videoSections))
             }
         }
     }

diff --git a/modules/education/src/main/res/values/strings.xml b/modules/education/src/main/res/values/strings.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+    <string name="failed_to_load_video_sections">Failed to load video sections</string>
+</resources>