fix: crash when opening service details screen [WPB-4586] (#2230)

wireapp · Sep 13, 2023 · 395cf3b · 395cf3b
1 parent 0107a02
commit 395cf3b
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 50 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/common/CallerDetails.kt b/app/src/main/kotlin/com/wire/android/ui/calling/common/CallerDetails.kt
@@ -43,7 +43,7 @@ import com.wire.android.model.UserAvatarData
 import com.wire.android.ui.calling.ConversationName
 import com.wire.android.ui.common.MembershipQualifierLabel
 import com.wire.android.ui.common.UserProfileAvatar
-import com.wire.android.ui.common.banner.SecurityClassificationBanner
+import com.wire.android.ui.common.banner.SecurityClassificationBannerForConversation
 import com.wire.android.ui.common.colorsScheme
 import com.wire.android.ui.common.dimensions
 import com.wire.android.ui.common.spacers.VerticalSpace
@@ -114,9 +114,10 @@ fun CallerDetails(
             MembershipQualifierLabel(membership)
         }
 
-        SecurityClassificationBanner(
+        SecurityClassificationBannerForConversation(
             conversationId = conversationId,
-            modifier = Modifier.padding(top = dimensions().spacing8x)
+            modifier = Modifier.padding(top = dimensions().spacing8x),
+
         )
 
         if (!isCameraOn && conversationType == ConversationType.OneOnOne) {

diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt b/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt
@@ -71,7 +71,7 @@ import com.wire.android.ui.calling.ongoing.fullscreen.DoubleTapToast
 import com.wire.android.ui.calling.ongoing.fullscreen.FullScreenTile
 import com.wire.android.ui.calling.ongoing.fullscreen.SelectedParticipant
 import com.wire.android.ui.calling.ongoing.participantsview.VerticalCallingPager
-import com.wire.android.ui.common.banner.SecurityClassificationBanner
+import com.wire.android.ui.common.banner.SecurityClassificationBannerForConversation
 import com.wire.android.ui.common.bottomsheet.WireBottomSheetScaffold
 import com.wire.android.ui.common.colorsScheme
 import com.wire.android.ui.common.dimensions
@@ -349,7 +349,7 @@ private fun CallingControls(
             )
         }
         Spacer(modifier = Modifier.weight(1F))
-        SecurityClassificationBanner(conversationId)
+        SecurityClassificationBannerForConversation(conversationId)
     }
 }
 

diff --git a/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationArgs.kt b/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationArgs.kt
@@ -18,19 +18,38 @@
 package com.wire.android.ui.common.banner
 
 import com.wire.android.di.ScopedArgs
+import com.wire.kalium.logic.data.id.ConversationId
 import com.wire.kalium.logic.data.id.QualifiedID
+import com.wire.kalium.logic.data.user.UserId
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
 
 @Serializable
-data class SecurityClassificationArgs(
-    @SerialName("conversation_id") val conversationId: QualifiedID?,
-    @SerialName("user_id") val userId: QualifiedID?
-) : ScopedArgs {
+sealed interface SecurityClassificationArgs : ScopedArgs {
 
-    override val key = "$ARGS_KEY:${conversationId?.toString().orEmpty()}_${userId?.toString().orEmpty()}"
+    val id: QualifiedID
 
-    companion object {
-        const val ARGS_KEY = "SecurityClassificationArgsKey"
+    @Serializable
+    data class Conversation(
+        @SerialName("conversationId")
+        override val id: ConversationId,
+    ) : SecurityClassificationArgs {
+        override val key = "$ARGS_KEY:$id"
+
+        companion object {
+            const val ARGS_KEY = "SecurityClassificationConversationArgsKey"
+        }
+    }
+
+    @Serializable
+    data class User(
+        @SerialName("userId")
+        override val id: UserId
+    ) : SecurityClassificationArgs {
+        override val key = "$ARGS_KEY:$id"
+
+        companion object {
+            const val ARGS_KEY = "SecurityClassificationUserArgsKey"
+        }
     }
 }
diff --git a/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationBanner.kt b/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationBanner.kt
@@ -49,42 +49,68 @@ import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.conversation.SecurityClassificationType
 
 @Composable
-fun SecurityClassificationBanner(
-    conversationId: ConversationId? = null,
-    userId: UserId? = null,
+fun SecurityClassificationBannerForConversation(
+    conversationId: ConversationId,
     viewModel: SecurityClassificationViewModel =
-        hiltViewModelScoped<SecurityClassificationViewModelImpl, SecurityClassificationArgs>(SecurityClassificationArgs(
-            userId = userId,
-            conversationId = conversationId,
-        )),
+        hiltViewModelScoped<SecurityClassificationViewModelImpl, SecurityClassificationArgs>(
+            SecurityClassificationArgs.Conversation(
+                id = conversationId,
+            )
+        ),
     modifier: Modifier = Modifier
 ) {
-    val securityClassificationType = viewModel.state()
+    SecurityClassificationBanner(
+        state = viewModel.state(),
+        modifier = modifier
+    )
+}
+
+@Composable
+fun SecurityClassificationBannerForUser(
+    userId: UserId,
+    viewModel: SecurityClassificationViewModel =
+        hiltViewModelScoped<SecurityClassificationViewModelImpl, SecurityClassificationArgs>(
+            SecurityClassificationArgs.User(
+                id = userId,
+            )
+        ),
+    modifier: Modifier = Modifier
+) {
+    SecurityClassificationBanner(
+        state = viewModel.state(),
+        modifier = modifier
+    )
+}
 
-    if (securityClassificationType != SecurityClassificationType.NONE) {
+@Composable
+private fun SecurityClassificationBanner(
+    state: SecurityClassificationType,
+    modifier: Modifier = Modifier
+) {
+    if (state != SecurityClassificationType.NONE) {
         Column(modifier = modifier) {
-            Divider(color = getDividerColorFor(securityClassificationType))
+            Divider(color = getDividerColorFor(state))
             Row(
                 horizontalArrangement = Arrangement.Center,
                 verticalAlignment = Alignment.CenterVertically,
                 modifier = Modifier
-                    .background(getBackgroundColorFor(securityClassificationType))
+                    .background(getBackgroundColorFor(state))
                     .height(dimensions().spacing24x)
                     .fillMaxWidth()
             ) {
                 Icon(
-                    painter = getIconFor(securityClassificationType),
-                    tint = getColorTextFor(securityClassificationType),
-                    contentDescription = getTextFor(securityClassificationType),
+                    painter = getIconFor(state),
+                    tint = getColorTextFor(state),
+                    contentDescription = getTextFor(state),
                     modifier = Modifier.padding(end = dimensions().spacing8x)
                 )
                 Text(
-                    text = getTextFor(securityClassificationType),
-                    color = getColorTextFor(securityClassificationType),
+                    text = getTextFor(state),
+                    color = getColorTextFor(state),
                     style = MaterialTheme.wireTypography.label03
                 )
             }
-            Divider(color = getDividerColorFor(securityClassificationType))
+            Divider(color = getDividerColorFor(state))
         }
     }
 }
@@ -141,15 +167,15 @@ fun PreviewClassifiedIndicator() {
         Surface {
             Column(modifier = Modifier.fillMaxWidth()) {
                 SecurityClassificationBanner(
-                    viewModel = SecurityClassificationPreviewModel(SecurityClassificationType.NONE)
+                    state = SecurityClassificationType.NONE
                 )
                 Divider()
                 SecurityClassificationBanner(
-                    viewModel = SecurityClassificationPreviewModel(SecurityClassificationType.NOT_CLASSIFIED)
+                    state = SecurityClassificationType.NOT_CLASSIFIED
                 )
                 Divider()
                 SecurityClassificationBanner(
-                    viewModel = SecurityClassificationPreviewModel(SecurityClassificationType.CLASSIFIED)
+                    state = SecurityClassificationType.CLASSIFIED
                 )
             }
         }

diff --git a/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/common/banner/SecurityClassificationViewModel.kt
@@ -26,7 +26,6 @@ import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import com.wire.android.di.scopedArgs
 import com.wire.kalium.logic.data.id.ConversationId
-import com.wire.kalium.logic.data.id.QualifiedID
 import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.conversation.ObserveOtherUserSecurityClassificationLabelUseCase
 import com.wire.kalium.logic.feature.conversation.ObserveSecurityClassificationLabelUseCase
@@ -47,18 +46,15 @@ class SecurityClassificationViewModelImpl @Inject constructor(
 ) : SecurityClassificationViewModel, ViewModel() {
 
     private val args: SecurityClassificationArgs = savedStateHandle.scopedArgs()
-    private val conversationId: QualifiedID? = args.conversationId
-    private val userId: QualifiedID? = args.userId
 
     private var state by mutableStateOf(SecurityClassificationType.NONE)
 
     override fun state(): SecurityClassificationType = state
 
     init {
-        when {
-            conversationId != null -> fetchConversationClassificationType(conversationId)
-            userId != null -> fetchUserClassificationType(userId)
-            else -> error("Either conversationId or userId should be provided to SecurityClassificationViewModel")
+        when (args) {
+            is SecurityClassificationArgs.Conversation -> fetchConversationClassificationType(args.id)
+            is SecurityClassificationArgs.User -> fetchUserClassificationType(args.id)
         }
     }
 

diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/MessageComposer.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/MessageComposer.kt
@@ -62,7 +62,7 @@ import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
 import com.wire.android.R
 import com.wire.android.ui.common.KeyboardHelper
-import com.wire.android.ui.common.banner.SecurityClassificationBanner
+import com.wire.android.ui.common.banner.SecurityClassificationBannerForConversation
 import com.wire.android.ui.common.bottomsheet.WireModalSheetState
 import com.wire.android.ui.common.colorsScheme
 import com.wire.android.ui.common.dimensions
@@ -214,7 +214,7 @@ private fun DisabledInteractionMessageComposer(
                     )
                 }
             }
-            SecurityClassificationBanner(conversationId = conversationId)
+            SecurityClassificationBannerForConversation(conversationId = conversationId)
         }
     }
 }
@@ -296,7 +296,7 @@ private fun InactiveMessageComposer(
             ) {
                 messageListContent()
             }
-            SecurityClassificationBanner(conversationId)
+            SecurityClassificationBannerForConversation(conversationId)
             Divider(color = MaterialTheme.wireColorScheme.outline)
             Row(
                 verticalAlignment = Alignment.CenterVertically,
@@ -413,7 +413,7 @@ private fun ActiveMessageComposer(
                         ) {
                             Column {
                                 Box(Modifier.wrapContentSize()) {
-                                    SecurityClassificationBanner(
+                                    SecurityClassificationBannerForConversation(
                                         conversationId = conversationId
                                     )
                                 }

diff --git a/app/src/main/kotlin/com/wire/android/ui/userprofile/common/UserProfileInfo.kt b/app/src/main/kotlin/com/wire/android/ui/userprofile/common/UserProfileInfo.kt
@@ -58,7 +58,7 @@ import com.wire.android.model.UserAvatarData
 import com.wire.android.ui.common.Icon
 import com.wire.android.ui.common.UserBadge
 import com.wire.android.ui.common.UserProfileAvatar
-import com.wire.android.ui.common.banner.SecurityClassificationBanner
+import com.wire.android.ui.common.banner.SecurityClassificationBannerForUser
 import com.wire.android.ui.common.dimensions
 import com.wire.android.ui.common.progress.WireCircularProgressIndicator
 import com.wire.android.ui.home.conversationslist.model.Membership
@@ -213,10 +213,13 @@ fun UserProfileInfo(
                 )
             }
         }
-        SecurityClassificationBanner(
-            userId = userId,
-            modifier = Modifier.padding(top = dimensions().spacing8x)
-        )
+
+        userId?.also {
+            SecurityClassificationBannerForUser(
+                userId = it,
+                modifier = Modifier.padding(top = dimensions().spacing8x)
+            )
+        }
     }
 }
 

diff --git a/app/src/main/kotlin/com/wire/android/ui/userprofile/service/ServiceDetailsScreen.kt b/app/src/main/kotlin/com/wire/android/ui/userprofile/service/ServiceDetailsScreen.kt
@@ -128,7 +128,7 @@ private fun ServiceDetailsProfileInfo(
 ) {
     state.serviceDetails?.let { serviceDetails ->
         UserProfileInfo(
-            userId = null,
+            userId = state.serviceMemberId,
             isLoading = state.isAvatarLoading,
             avatarAsset = state.serviceAvatarAsset,
             fullName = serviceDetails.name,

diff --git a/...tlin/com/wire/android/ui/home/conversations/banner/SecurityClassificationViewModelTest.kt b/...tlin/com/wire/android/ui/home/conversations/banner/SecurityClassificationViewModelTest.kt
@@ -113,8 +113,13 @@ class SecurityClassificationViewModelTest {
         val classificationUserChannel = Channel<SecurityClassificationType>(capacity = Channel.UNLIMITED)
 
         init {
+            val navArg = when {
+                conversationId != null -> SecurityClassificationArgs.Conversation(conversationId)
+                userId != null -> SecurityClassificationArgs.User(userId)
+                else -> throw IllegalArgumentException("Either conversationId or userId must be provided")
+            }
             MockKAnnotations.init(this, relaxUnitFun = true)
-            every { savedStateHandle.scopedArgs<SecurityClassificationArgs>() } returns SecurityClassificationArgs(conversationId, userId)
+            every { savedStateHandle.scopedArgs<SecurityClassificationArgs>() } returns navArg
             coEvery { observeSecurityClassificationLabel(any()) } returns classificationChannel.consumeAsFlow()
             coEvery { getOtherUserSecurityClassificationLabel(any()) } returns classificationUserChannel.consumeAsFlow()
         }