From fc4e2725736a362b66d3b59ab324d7aefe794689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Mon, 24 Jun 2024 17:07:15 +0200 Subject: [PATCH] fix: missing remove device button [WPB-8819] (#3110) Co-authored-by: Yamil Medina --- .../ui/authentication/devices/DeviceItem.kt | 65 +++++----- .../devices/remove/RemoveDeviceScreen.kt | 42 +++++-- .../devices/remove/RemoveDeviceViewModel.kt | 11 +- .../devices/DeviceDetailsViewModel.kt | 2 +- .../ui/settings/devices/SelfDevicesScreen.kt | 45 ++++--- .../settings/devices/SelfDevicesViewModel.kt | 16 +-- .../devices/model/SelfDevicesState.kt | 2 +- .../other/OtherUserDevicesScreen.kt | 16 ++- app/src/main/res/values/strings.xml | 1 + .../test/kotlin/com/wire/android/TestUtil.kt | 3 + .../remove/RemoveDeviceViewModelTest.kt | 117 ++++++++++++++++++ .../devices/DeviceDetailsViewModelTest.kt | 64 ++++++---- .../devices/SelfDevicesViewModelTest.kt | 35 ++++-- 13 files changed, 310 insertions(+), 109 deletions(-) create mode 100644 app/src/test/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModelTest.kt diff --git a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt index 7d945ccb578..dcbe53947cb 100644 --- a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt +++ b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/DeviceItem.kt @@ -18,21 +18,18 @@ package com.wire.android.ui.authentication.devices -import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentWidth import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ChevronRight import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -40,7 +37,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource @@ -52,7 +48,7 @@ import com.wire.android.BuildConfig import com.wire.android.R import com.wire.android.ui.authentication.devices.model.Device import com.wire.android.ui.authentication.devices.model.lastActiveDescription -import com.wire.android.ui.common.Icon +import com.wire.android.ui.common.ArrowRightIcon import com.wire.android.ui.common.MLSVerificationIcon import com.wire.android.ui.common.ProteusVerifiedIcon import com.wire.android.ui.common.button.WireSecondaryButton @@ -62,9 +58,9 @@ import com.wire.android.ui.theme.WireTheme import com.wire.android.ui.theme.wireColorScheme import com.wire.android.ui.theme.wireDimensions import com.wire.android.ui.theme.wireTypography +import com.wire.android.util.deviceDateTimeFormat import com.wire.android.util.extension.formatAsFingerPrint import com.wire.android.util.extension.formatAsString -import com.wire.android.util.deviceDateTimeFormat import com.wire.android.util.ui.PreviewMultipleThemes import com.wire.android.util.ui.UIText @@ -73,23 +69,23 @@ fun DeviceItem( device: Device, placeholder: Boolean, shouldShowVerifyLabel: Boolean, + icon: @Composable (() -> Unit), + modifier: Modifier = Modifier, isCurrentClient: Boolean = false, shouldShowE2EIInfo: Boolean = false, - background: Color? = null, - icon: @Composable (() -> Unit), isWholeItemClickable: Boolean = false, onClickAction: ((Device) -> Unit)? = null ) { DeviceItemContent( device = device, placeholder = placeholder, - background = background, icon = icon, onClickAction = onClickAction, isWholeItemClickable = isWholeItemClickable, shouldShowVerifyLabel = shouldShowVerifyLabel, isCurrentClient = isCurrentClient, - shouldShowE2EIInfo = shouldShowE2EIInfo + shouldShowE2EIInfo = shouldShowE2EIInfo, + modifier = modifier, ) } @@ -97,17 +93,17 @@ fun DeviceItem( private fun DeviceItemContent( device: Device, placeholder: Boolean, - background: Color? = null, icon: @Composable (() -> Unit), onClickAction: ((Device) -> Unit)?, isWholeItemClickable: Boolean, shouldShowVerifyLabel: Boolean, isCurrentClient: Boolean, - shouldShowE2EIInfo: Boolean + shouldShowE2EIInfo: Boolean, + modifier: Modifier = Modifier, ) { Row( verticalAlignment = Alignment.Top, - modifier = (if (background != null) Modifier.background(color = background) else Modifier) + modifier = modifier .clickable(enabled = isWholeItemClickable) { if (isWholeItemClickable) { onClickAction?.invoke(device) @@ -129,12 +125,16 @@ private fun DeviceItemContent( modifier = Modifier .padding(start = MaterialTheme.wireDimensions.removeDeviceItemPadding) .weight(1f) - ) { DeviceItemTexts(device, placeholder, shouldShowVerifyLabel, isCurrentClient, shouldShowE2EIInfo) } + ) { + DeviceItemTexts(device, placeholder, shouldShowVerifyLabel, isCurrentClient, shouldShowE2EIInfo) + } } if (!placeholder) { if (onClickAction != null && !isWholeItemClickable) { WireSecondaryButton( - modifier = Modifier.testTag("remove device button"), + modifier = Modifier + .padding(end = MaterialTheme.wireDimensions.removeDeviceItemPadding) + .testTag("remove device button"), onClick = { onClickAction(device) }, leadingIcon = icon, fillMaxWidth = false, @@ -143,11 +143,17 @@ private fun DeviceItemContent( shape = RoundedCornerShape(size = MaterialTheme.wireDimensions.buttonSmallCornerSize), contentPadding = PaddingValues(0.dp), colors = wireSecondaryButtonColors().copy( - enabled = background ?: MaterialTheme.wireColorScheme.secondaryButtonEnabled + enabled = MaterialTheme.wireColorScheme.secondaryButtonEnabled ) ) } else { - Box(modifier = Modifier.padding(MaterialTheme.wireDimensions.removeDeviceItemPadding)) { + Box( + modifier = Modifier + .padding( + top = MaterialTheme.wireDimensions.removeDeviceItemPadding, + end = MaterialTheme.wireDimensions.removeDeviceItemPadding, + ) + ) { icon() } } @@ -156,13 +162,13 @@ private fun DeviceItemContent( } @Composable -private fun DeviceItemTexts( +private fun ColumnScope.DeviceItemTexts( device: Device, placeholder: Boolean, shouldShowVerifyLabel: Boolean, isCurrentClient: Boolean = false, shouldShowE2EIInfo: Boolean = false, - isDebug: Boolean = BuildConfig.DEBUG + isDebug: Boolean = BuildConfig.DEBUG, ) { val displayZombieIndicator = remember { if (isDebug) { @@ -172,7 +178,11 @@ private fun DeviceItemTexts( } } - Row(Modifier.fillMaxWidth()) { + Row( + modifier = Modifier + .fillMaxWidth() + .shimmerPlaceholder(visible = placeholder) + ) { Text( style = MaterialTheme.wireTypography.body02, color = MaterialTheme.wireColorScheme.onBackground, @@ -185,7 +195,6 @@ private fun DeviceItemTexts( if (shouldShowE2EIInfo) { MLSVerificationIcon(device.e2eiCertificate?.status) } - Spacer(modifier = Modifier.width(MaterialTheme.wireDimensions.spacing8x)) if (device.isVerifiedProteus && !isCurrentClient) { ProteusVerifiedIcon( Modifier @@ -202,7 +211,6 @@ private fun DeviceItemTexts( text = "this client is invalid", modifier = Modifier .fillMaxWidth() - .shimmerPlaceholder(visible = placeholder) ) } @@ -249,6 +257,7 @@ private fun DeviceItemTexts( style = MaterialTheme.wireTypography.subline01, color = MaterialTheme.wireColorScheme.labelText, text = proteusDetails, + minLines = 2, modifier = Modifier .fillMaxWidth() .shimmerPlaceholder(visible = placeholder) @@ -260,13 +269,12 @@ private fun DeviceItemTexts( fun PreviewDeviceItemWithActionIcon() { WireTheme { DeviceItem( - device = Device(name = UIText.DynamicString("name"), isVerifiedProteus = true), + device = Device(name = UIText.DynamicString("Name"), isVerifiedProteus = true, registrationTime = "2024-01-01T12:00:00.000Z"), placeholder = false, shouldShowVerifyLabel = true, isCurrentClient = true, shouldShowE2EIInfo = true, - background = null, - { Icon(painter = painterResource(id = R.drawable.ic_remove), contentDescription = "") } + icon = { Icon(painter = painterResource(id = R.drawable.ic_remove), contentDescription = "") } ) {} } } @@ -276,12 +284,11 @@ fun PreviewDeviceItemWithActionIcon() { fun PreviewDeviceItem() { WireTheme { DeviceItem( - device = Device(name = UIText.DynamicString("name"), isVerifiedProteus = true), + device = Device(name = UIText.DynamicString("Name"), isVerifiedProteus = true, registrationTime = "2024-01-01T12:00:00.000Z"), placeholder = false, shouldShowVerifyLabel = true, - background = null, isWholeItemClickable = true, - icon = Icons.Filled.ChevronRight.Icon() + icon = { ArrowRightIcon() } ) {} } } diff --git a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt index 2469ff4e62e..d0a3a0c53e8 100644 --- a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceScreen.kt @@ -33,6 +33,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.window.DialogProperties import androidx.hilt.navigation.compose.hiltViewModel import com.ramcosta.composedestinations.annotation.Destination import com.ramcosta.composedestinations.annotation.RootNavGraph @@ -63,6 +64,7 @@ import com.wire.android.ui.destinations.InitialSyncScreenDestination import com.wire.android.ui.theme.WireTheme import com.wire.android.util.dialogErrorStrings import com.wire.android.util.ui.PreviewMultipleThemes +import com.wire.kalium.logic.data.conversation.ClientId @RootNavGraph @Destination( @@ -96,6 +98,25 @@ fun RemoveDeviceScreen( onCancelLoginClicked = { clearSessionViewModel.onCancelLoginClicked(NavigationSwitchAccountActions(navigator::navigate)) }, onProceedLoginClicked = clearSessionViewModel::onProceedLoginClicked ) + + if (viewModel.state.error is RemoveDeviceError.InitError) { + WireDialog( + properties = DialogProperties(dismissOnBackPress = false, dismissOnClickOutside = false, usePlatformDefaultWidth = false), + title = stringResource(id = R.string.label_general_error), + text = stringResource(id = R.string.devices_loading_error), + onDismiss = viewModel::clearDeleteClientError, + dismissButtonProperties = WireDialogButtonProperties( + onClick = clearSessionViewModel::onBackButtonClicked, + text = stringResource(id = R.string.label_cancel), + type = WireDialogButtonType.Secondary, + ), + optionButton1Properties = WireDialogButtonProperties( + onClick = viewModel::retryFetch, + text = stringResource(id = R.string.label_retry), + type = WireDialogButtonType.Primary, + ) + ) + } } @Composable @@ -118,12 +139,8 @@ private fun RemoveDeviceContent( val cancelLoginDialogState = rememberVisibilityState() CancelLoginDialogContent( dialogState = cancelLoginDialogState, - onActionButtonClicked = { - onCancelLoginClicked() - }, - onProceedButtonClicked = { - onProceedLoginClicked() - } + onActionButtonClicked = onCancelLoginClicked, + onProceedButtonClicked = onProceedLoginClicked, ) if (clearSessionState.showCancelLoginDialog) { cancelLoginDialogState.show( @@ -144,10 +161,15 @@ private fun RemoveDeviceContent( } ) { internalPadding -> Box(modifier = Modifier.padding(internalPadding)) { - when (state.isLoadingClientsList) { - true -> RemoveDeviceItemsList(lazyListState, List(10) { Device() }, true, onItemClicked) - false -> RemoveDeviceItemsList(lazyListState, state.deviceList, false, onItemClicked) - } + RemoveDeviceItemsList( + lazyListState = lazyListState, + items = when (state.isLoadingClientsList) { + true -> List(4) { Device(clientId = ClientId("placeholder_$it")) } + false -> state.deviceList + }, + placeholders = state.isLoadingClientsList, + onItemClicked = onItemClicked + ) } // TODO handle list loading errors if (!state.isLoadingClientsList && state.removeDeviceDialogState is RemoveDeviceDialogState.Visible) { diff --git a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt index 6eaf697c97e..dd47f050754 100644 --- a/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModel.kt @@ -84,12 +84,14 @@ class RemoveDeviceViewModel @Inject constructor( viewModelScope.launch { state = state.copy(isLoadingClientsList = true) val selfClientsResult = fetchSelfClientsFromRemote() - if (selfClientsResult is SelfClientsResult.Success) { - state = state.copy( + state = if (selfClientsResult is SelfClientsResult.Success) { + state.copy( isLoadingClientsList = false, deviceList = selfClientsResult.clients.filter { it.type == ClientType.Permanent }.map { Device(it) }, removeDeviceDialogState = RemoveDeviceDialogState.Hidden ) + } else { + state.copy(isLoadingClientsList = false, error = RemoveDeviceError.InitError) } } } @@ -103,6 +105,11 @@ class RemoveDeviceViewModel @Inject constructor( updateStateIfDialogVisible { state.copy(error = RemoveDeviceError.None) } } + fun retryFetch() { + state = state.copy(isLoadingClientsList = true, error = RemoveDeviceError.None) + loadClientsList() + } + fun onItemClicked(device: Device, onCompleted: (initialSyncCompleted: Boolean, isE2EIRequired: Boolean) -> Unit) { viewModelScope.launch { val isPasswordRequired: Boolean = when (val passwordRequiredResult = isPasswordRequired()) { diff --git a/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt index f9f83bcadeb..6be29b071d8 100644 --- a/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModel.kt @@ -197,7 +197,7 @@ class DeviceDetailsViewModel @Inject constructor( device = state.device.updateFromClient(result.client), isCurrentDevice = result.isCurrentClient, removeDeviceDialogState = RemoveDeviceDialogState.Hidden, - canBeRemoved = !result.isCurrentClient && isSelfClient && result.client.type == ClientType.Permanent, + canBeRemoved = !result.isCurrentClient && isSelfClient && result.client.type != ClientType.LegalHold, ) } } diff --git a/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesScreen.kt b/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesScreen.kt index 485f3f8e16c..b4625034cfe 100644 --- a/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesScreen.kt @@ -18,13 +18,12 @@ package com.wire.android.ui.settings.devices +import androidx.compose.foundation.background import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.rememberLazyListState -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ChevronRight import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable @@ -43,13 +42,15 @@ import com.wire.android.navigation.NavigationCommand import com.wire.android.navigation.Navigator import com.wire.android.ui.authentication.devices.DeviceItem import com.wire.android.ui.authentication.devices.model.Device -import com.wire.android.ui.common.Icon +import com.wire.android.ui.common.ArrowRightIcon +import com.wire.android.ui.common.rememberTopBarElevationState import com.wire.android.ui.common.scaffold.WireScaffold import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar import com.wire.android.ui.destinations.DeviceDetailsScreenDestination import com.wire.android.ui.settings.devices.model.SelfDevicesState import com.wire.android.ui.theme.wireColorScheme import com.wire.android.util.extension.folderWithElements +import com.wire.kalium.logic.data.conversation.ClientId import com.wire.android.util.lifecycle.rememberLifecycleEvent @RootNavGraph @@ -73,17 +74,20 @@ fun SelfDevicesScreen( @Composable fun SelfDevicesScreenContent( + state: SelfDevicesState, + modifier: Modifier = Modifier, onNavigateBack: () -> Unit = {}, onDeviceClick: (Device) -> Unit = {}, - state: SelfDevicesState ) { val lazyListState = rememberLazyListState() val context = LocalContext.current WireScaffold( + modifier = modifier, topBar = { TopBarHeader( - onNavigateBack = onNavigateBack + onNavigateBack = onNavigateBack, + elevation = lazyListState.rememberTopBarElevationState().value, ) }, content = { paddingValues -> @@ -94,7 +98,16 @@ fun SelfDevicesScreenContent( .fillMaxSize() ) { when (state.isLoadingClientsList) { - true -> items(count = 4, itemContent = { Device() }) + true -> { + folderDeviceItems( + header = null, + items = List(4) { Device(clientId = ClientId("placeholder_$it")) }, + shouldShowVerifyLabel = false, + isCurrentClient = false, + isE2EIEnabled = false, + placeholders = true, + ) + } false -> { state.currentDevice?.let { currentDevice -> folderDeviceItems( @@ -104,8 +117,7 @@ fun SelfDevicesScreenContent( isCurrentClient = true, isE2EIEnabled = state.isE2EIEnabled, onDeviceClick = onDeviceClick, - - ) + ) } folderDeviceItems( header = context.getString(R.string.other_devices_label), @@ -124,15 +136,16 @@ fun SelfDevicesScreenContent( @Suppress("LongParameterList") private fun LazyListScope.folderDeviceItems( - header: String, + header: String?, items: List, shouldShowVerifyLabel: Boolean, isCurrentClient: Boolean, isE2EIEnabled: Boolean, + placeholders: Boolean = false, onDeviceClick: (Device) -> Unit = {} ) { folderWithElements( - header = header.uppercase(), + header = header?.uppercase(), items = items.associateBy { it.clientId.value }, divider = { HorizontalDivider( @@ -142,11 +155,12 @@ private fun LazyListScope.folderDeviceItems( } ) { item: Device -> DeviceItem( - item, - background = MaterialTheme.wireColorScheme.surface, - placeholder = false, + device = item, + modifier = Modifier + .background(MaterialTheme.wireColorScheme.surface), + placeholder = placeholders, onClickAction = onDeviceClick, - icon = Icons.Filled.ChevronRight.Icon(), + icon = { ArrowRightIcon() }, isWholeItemClickable = true, shouldShowVerifyLabel = shouldShowVerifyLabel, isCurrentClient = isCurrentClient, @@ -157,11 +171,12 @@ private fun LazyListScope.folderDeviceItems( @Composable private fun TopBarHeader( + elevation: Dp = 0.dp, onNavigateBack: () -> Unit ) { WireCenterAlignedTopAppBar( onNavigationPressed = onNavigateBack, title = stringResource(id = R.string.devices_title), - elevation = 0.dp + elevation = elevation, ) } diff --git a/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModel.kt index b89f7073076..c1ee370724c 100644 --- a/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModel.kt @@ -59,14 +59,14 @@ class SelfDevicesViewModel @Inject constructor( private val observeUserE2eiCertificates = refreshE2eiCertificates.map { getUserE2eiCertificates(currentAccountId) } init { - // this will cause the list to be refreshed - loadClientsList() - observeClientList() + fetchAndObserveClientList() } - private fun observeClientList() { + private fun fetchAndObserveClientList() { viewModelScope.launch { - observeClientList(currentAccountId).combine(observeUserE2eiCertificates, ::Pair) + fetchSelfClientsFromRemote() // this will cause the list to be refreshed + observeClientList(currentAccountId) + .combine(observeUserE2eiCertificates, ::Pair) .collect { (result, e2eiCertificates) -> state = when (result) { is ObserveClientsByUserIdUseCase.Result.Failure -> state.copy(isLoadingClientsList = false) @@ -87,12 +87,6 @@ class SelfDevicesViewModel @Inject constructor( } } - private fun loadClientsList() { - viewModelScope.launch { - fetchSelfClientsFromRemote() - } - } - fun loadCertificates() { viewModelScope.launch { refreshE2eiCertificates.emit(Unit) diff --git a/app/src/main/kotlin/com/wire/android/ui/settings/devices/model/SelfDevicesState.kt b/app/src/main/kotlin/com/wire/android/ui/settings/devices/model/SelfDevicesState.kt index 26d12dd815c..26851de19c2 100644 --- a/app/src/main/kotlin/com/wire/android/ui/settings/devices/model/SelfDevicesState.kt +++ b/app/src/main/kotlin/com/wire/android/ui/settings/devices/model/SelfDevicesState.kt @@ -24,5 +24,5 @@ data class SelfDevicesState ( val currentDevice: Device?, val deviceList: List, val isLoadingClientsList: Boolean, - val isE2EIEnabled: Boolean = false + val isE2EIEnabled: Boolean = false, ) diff --git a/app/src/main/kotlin/com/wire/android/ui/userprofile/other/OtherUserDevicesScreen.kt b/app/src/main/kotlin/com/wire/android/ui/userprofile/other/OtherUserDevicesScreen.kt index 5fb689a69e3..73e1a6062d8 100644 --- a/app/src/main/kotlin/com/wire/android/ui/userprofile/other/OtherUserDevicesScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/userprofile/other/OtherUserDevicesScreen.kt @@ -27,8 +27,6 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.rememberLazyListState -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ChevronRight import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -41,7 +39,7 @@ import com.wire.android.BuildConfig import com.wire.android.R import com.wire.android.ui.authentication.devices.DeviceItem import com.wire.android.ui.authentication.devices.model.Device -import com.wire.android.ui.common.Icon +import com.wire.android.ui.common.ArrowRightIcon import com.wire.android.ui.common.colorsScheme import com.wire.android.ui.common.dimensions import com.wire.android.ui.common.divider.WireDivider @@ -54,14 +52,14 @@ import com.wire.android.util.ui.LinkTextData @Composable fun OtherUserDevicesScreen( - lazyListState: LazyListState = rememberLazyListState(), state: OtherUserProfileState, + lazyListState: LazyListState = rememberLazyListState(), onDeviceClick: (Device) -> Unit ) { if (state.otherUserDevices.isEmpty()) { OtherUserEmptyDevicesContent() } else { - OtherUserDevicesContent(lazyListState, state, onDeviceClick) + OtherUserDevicesContent(state, lazyListState, onDeviceClick) } } @@ -84,8 +82,8 @@ private fun OtherUserEmptyDevicesContent() { @Composable private fun OtherUserDevicesContent( - lazyListState: LazyListState = rememberLazyListState(), state: OtherUserProfileState, + lazyListState: LazyListState = rememberLazyListState(), onDeviceClick: (Device) -> Unit ) { val context = LocalContext.current @@ -117,12 +115,12 @@ private fun OtherUserDevicesContent( itemsIndexed(otherUserDevices) { index, item -> DeviceItem( - item, + device = item, placeholder = false, - background = MaterialTheme.wireColorScheme.surface, + modifier = Modifier.background(MaterialTheme.wireColorScheme.surface), isWholeItemClickable = true, onClickAction = onDeviceClick, - icon = Icons.Filled.ChevronRight.Icon(), + icon = { ArrowRightIcon() }, shouldShowVerifyLabel = true, shouldShowE2EIInfo = item.e2eiCertificate != null ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 89fb91e9716..fdfb715a6bb 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1032,6 +1032,7 @@ In group conversations, the group admin can overwrite this setting. Your Devices Current Device Other Devices + There was an error when trying to fetch devices. Please check your Internet connection and try again. Unblock user? %1$s will be able to contact you or add you to group conversations. You will be able to access a 1:1 conversation with them. diff --git a/app/src/test/kotlin/com/wire/android/TestUtil.kt b/app/src/test/kotlin/com/wire/android/TestUtil.kt index 91af5338ddc..5c934c8661f 100644 --- a/app/src/test/kotlin/com/wire/android/TestUtil.kt +++ b/app/src/test/kotlin/com/wire/android/TestUtil.kt @@ -17,6 +17,7 @@ */ package com.wire.android +import org.junit.jupiter.api.Assertions import kotlin.random.Random val charPoolWithNumbers: List = ('a'..'z') + ('A'..'Z') + ('0'..'9') @@ -29,3 +30,5 @@ fun Random.stringWithNumbers(length: Int) = (1..length) fun Random.string(length: Int) = (1..length) .map { Random.nextInt(0, charPool.size).let { charPool[it] } } .joinToString("") + +inline fun assertIs(actualValue: Any): T = Assertions.assertInstanceOf(T::class.java, actualValue) diff --git a/app/src/test/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModelTest.kt new file mode 100644 index 00000000000..018b139a49c --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/authentication/devices/remove/RemoveDeviceViewModelTest.kt @@ -0,0 +1,117 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.ui.authentication.devices.remove + +import com.wire.android.config.CoroutineTestExtension +import com.wire.android.config.SnapshotExtension +import com.wire.android.datastore.UserDataStore +import com.wire.android.framework.TestClient.CLIENT +import com.wire.kalium.logic.NetworkFailure +import com.wire.kalium.logic.feature.client.DeleteClientUseCase +import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCase +import com.wire.kalium.logic.feature.client.GetOrRegisterClientUseCase +import com.wire.kalium.logic.feature.client.SelfClientsResult +import com.wire.kalium.logic.feature.user.IsPasswordRequiredUseCase +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.amshove.kluent.internal.assertEquals +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith + +@OptIn(ExperimentalCoroutinesApi::class) +@ExtendWith(CoroutineTestExtension::class, SnapshotExtension::class) +class RemoveDeviceViewModelTest { + + @Test + fun `given success, when fetching devices, then show devices list`() = runTest { + val (_, viewModel) = Arrangement() + .withFetchSelfClientsResult(SelfClientsResult.Success(clients = listOf(CLIENT), currentClientId = CLIENT.id)) + .arrange() + advanceUntilIdle() + + assertEquals(false, viewModel.state.isLoadingClientsList) + assertEquals(RemoveDeviceError.None, viewModel.state.error) + assertEquals(1, viewModel.state.deviceList.size) + assertEquals(CLIENT.id, viewModel.state.deviceList.first().clientId) + } + + @Test + fun `given error, when fetching devices, then show init error`() = runTest { + val (_, viewModel) = Arrangement() + .withFetchSelfClientsResult(SelfClientsResult.Failure.Generic(NetworkFailure.NoNetworkConnection(null))) + .arrange() + advanceUntilIdle() + + assertEquals(false, viewModel.state.isLoadingClientsList) + assertEquals(RemoveDeviceError.InitError, viewModel.state.error) + assertEquals(emptyList(), viewModel.state.deviceList) + } + + @Test + fun `given error, when retrying, then execute fetch properly`() = runTest { + val (arrangement, viewModel) = Arrangement() + .withFetchSelfClientsResult(SelfClientsResult.Failure.Generic(NetworkFailure.NoNetworkConnection(null))) + .arrange() + advanceUntilIdle() + + viewModel.retryFetch() + + coVerify(exactly = 2) { // first time during init, second time when retrying + arrangement.fetchSelfClientsFromRemote() + } + } + + inner class Arrangement { + + @MockK + lateinit var fetchSelfClientsFromRemote: FetchSelfClientsFromRemoteUseCase + + @MockK + lateinit var deleteClientUseCase: DeleteClientUseCase + + @MockK + lateinit var registerClientUseCase: GetOrRegisterClientUseCase + + @MockK + lateinit var isPasswordRequired: IsPasswordRequiredUseCase + + @MockK + lateinit var userDataStore: UserDataStore + + init { + MockKAnnotations.init(this, relaxUnitFun = true) + } + + fun withFetchSelfClientsResult(result: SelfClientsResult) = apply { + coEvery { fetchSelfClientsFromRemote() } returns result + } + + fun arrange() = this to RemoveDeviceViewModel( + fetchSelfClientsFromRemote = fetchSelfClientsFromRemote, + deleteClientUseCase = deleteClientUseCase, + registerClientUseCase = registerClientUseCase, + isPasswordRequired = isPasswordRequired, + userDataStore = userDataStore + ) + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModelTest.kt index 652a9d5962c..63e80a72688 100644 --- a/app/src/test/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/settings/devices/DeviceDetailsViewModelTest.kt @@ -18,6 +18,7 @@ package com.wire.android.ui.settings.devices import androidx.lifecycle.SavedStateHandle +import com.wire.android.assertIs import com.wire.android.config.CoroutineTestExtension import com.wire.android.config.NavigationTestExtension import com.wire.android.framework.TestClient @@ -59,8 +60,6 @@ import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import okio.IOException import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import kotlin.time.Duration.Companion.days @@ -93,7 +92,7 @@ class DeviceDetailsViewModelTest { .arrange() // then - assert(viewModel.state.error is RemoveDeviceError.InitError) + assertIs(viewModel.state.error) } @Test @@ -129,8 +128,8 @@ class DeviceDetailsViewModelTest { arrangement.deleteClientUseCase(any()) wasNot Called } verify { arrangement.onSuccess wasNot Called } - assertTrue(viewModel.state.removeDeviceDialogState is RemoveDeviceDialogState.Visible) - assertTrue(viewModel.state.error is RemoveDeviceError.None) + assertIs(viewModel.state.removeDeviceDialogState) + assertIs(viewModel.state.error) } @Test @@ -148,8 +147,8 @@ class DeviceDetailsViewModelTest { arrangement.deleteClientUseCase(any()) wasNot Called } verify { arrangement.onSuccess wasNot Called } - assertTrue(viewModel.state.removeDeviceDialogState is RemoveDeviceDialogState.Hidden) - assertTrue(viewModel.state.error is RemoveDeviceError.None) + assertIs(viewModel.state.removeDeviceDialogState) + assertIs(viewModel.state.error) } @Test @@ -169,8 +168,8 @@ class DeviceDetailsViewModelTest { arrangement.deleteClientUseCase.invoke(any()) wasNot Called } verify { arrangement.onSuccess wasNot Called } - assertTrue(viewModel.state.removeDeviceDialogState is RemoveDeviceDialogState.Visible) - assertTrue(viewModel.state.error is RemoveDeviceError.None) + assertIs(viewModel.state.removeDeviceDialogState) + assertIs(viewModel.state.error) } @Test @@ -191,9 +190,10 @@ class DeviceDetailsViewModelTest { arrangement.deleteClientUseCase.invoke(any()) } verify { arrangement.onSuccess() } - assertTrue(viewModel.state.removeDeviceDialogState is RemoveDeviceDialogState.Visible) - assertTrue((viewModel.state.removeDeviceDialogState as? RemoveDeviceDialogState.Visible)?.removeEnabled == false) - assertTrue(viewModel.state.error is RemoveDeviceError.None) + assertIs(viewModel.state.removeDeviceDialogState).let { + assertEquals(true, it.removeEnabled == false) + } + assertIs(viewModel.state.error) } @Test @@ -212,8 +212,8 @@ class DeviceDetailsViewModelTest { arrangement.deleteClientUseCase(any()) } verify { arrangement.onSuccess() } - assertTrue(viewModel.state.removeDeviceDialogState is RemoveDeviceDialogState.Hidden) - assertTrue(viewModel.state.error is RemoveDeviceError.None) + assertIs(viewModel.state.removeDeviceDialogState) + assertIs(viewModel.state.error) } @Test @@ -224,7 +224,7 @@ class DeviceDetailsViewModelTest { .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.LegalHold), false)) .arrange() // then - assertFalse(viewModel.state.canBeRemoved) + assertEquals(false, viewModel.state.canBeRemoved) } @Test @@ -235,7 +235,7 @@ class DeviceDetailsViewModelTest { .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.Temporary), false)) .arrange() // then - assertFalse(viewModel.state.canBeRemoved) + assertEquals(true, viewModel.state.canBeRemoved) } @Test @@ -246,7 +246,7 @@ class DeviceDetailsViewModelTest { .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.Permanent), false)) .arrange() // then - assertTrue(viewModel.state.canBeRemoved) + assertEquals(true, viewModel.state.canBeRemoved) } @Test @@ -257,7 +257,29 @@ class DeviceDetailsViewModelTest { .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.Permanent), true)) .arrange() // then - assertFalse(viewModel.state.canBeRemoved) + assertEquals(false, viewModel.state.canBeRemoved) + } + + @Test + fun `given self client with null type, when fetching state, then canBeRemoved is true`() = runTest { + // given + val (_, viewModel) = Arrangement() + .withRequiredMockSetup() + .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = null), false)) + .arrange() + // then + assertEquals(true, viewModel.state.canBeRemoved) + } + + @Test + fun `given other user temporary client, when fetching state, then canBeRemoved is false`() = runTest { + // given + val (_, viewModel) = Arrangement() + .withRequiredMockSetup(userId = UserId("otherUserId", "otherUserDomain")) + .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.Temporary), false)) + .arrange() + // then + assertEquals(false, viewModel.state.canBeRemoved) } @Test @@ -268,7 +290,7 @@ class DeviceDetailsViewModelTest { .withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT.copy(type = ClientType.Permanent), false)) .arrange() // then - assertFalse(viewModel.state.canBeRemoved) + assertEquals(false, viewModel.state.canBeRemoved) } @Test @@ -281,8 +303,8 @@ class DeviceDetailsViewModelTest { viewModel.enrollE2EICertificate() - assertTrue(viewModel.state.isLoadingCertificate) - assertTrue(viewModel.state.startGettingE2EICertificate) + assertEquals(true, viewModel.state.isLoadingCertificate) + assertEquals(true, viewModel.state.startGettingE2EICertificate) } @Test diff --git a/app/src/test/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModelTest.kt index 2a17c3fda94..126d7330df7 100644 --- a/app/src/test/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/settings/devices/SelfDevicesViewModelTest.kt @@ -23,6 +23,7 @@ package com.wire.android.ui.settings.devices import com.wire.android.config.CoroutineTestExtension import com.wire.android.framework.TestClient import com.wire.android.ui.authentication.devices.model.Device +import com.wire.kalium.logic.data.conversation.ClientId import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCase import com.wire.kalium.logic.feature.client.ObserveClientsByUserIdUseCase @@ -37,6 +38,7 @@ import io.mockk.impl.annotations.MockK import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest +import org.amshove.kluent.internal.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @@ -45,18 +47,25 @@ import org.junit.jupiter.api.extension.ExtendWith class SelfDevicesViewModelTest { @Test - fun `given a self client id, when fetching self clients, then returns devices list without current device`() = + fun `given a self client id, when observing self clients, then returns devices list without current device`() = runTest { // given + val currentClient = TestClient.CLIENT + val otherClient = TestClient.CLIENT.copy(id = ClientId("anotherId")) val (_, viewModel) = Arrangement() + .withCurrentClientId(currentClient.id) + .withObserveClientsByUserIdResult(ObserveClientsByUserIdUseCase.Result.Success(listOf(currentClient, otherClient))) .arrange() - val currentDevice = Device(TestClient.CLIENT) + val currentDevice = Device(currentClient) + val otherDevice = Device(otherClient) // when viewModel.loadCertificates() // then - assert(!viewModel.state.deviceList.contains(currentDevice)) + assertEquals(false, viewModel.state.isLoadingClientsList) + assertEquals(false, viewModel.state.deviceList.contains(currentDevice)) + assertEquals(true, viewModel.state.deviceList.contains(otherDevice)) } @Test @@ -109,17 +118,23 @@ class SelfDevicesViewModelTest { coEvery { currentClientId.invoke() } returns flowOf(TestClient.CLIENT_ID) coEvery { fetchSelfClientsFromRemote.invoke() } returns SelfClientsResult.Success(listOf(), null) - coEvery { observeClientsByUserId(any()) } returns flowOf( - ObserveClientsByUserIdUseCase.Result.Success( - listOf( - TestClient.CLIENT - ) - ) - ) + coEvery { observeClientsByUserId(any()) } returns flowOf(ObserveClientsByUserIdUseCase.Result.Success(listOf())) coEvery { getUserE2eiCertificates.invoke(any()) } returns mapOf() coEvery { isE2EIEnabledUseCase() } returns true } + fun withCurrentClientId(clientId: ClientId) = apply { + coEvery { currentClientId.invoke() } returns flowOf(clientId) + } + + fun withFetchSelfClientsResult(result: SelfClientsResult) = apply { + coEvery { fetchSelfClientsFromRemote() } returns result + } + + fun withObserveClientsByUserIdResult(result: ObserveClientsByUserIdUseCase.Result) = apply { + coEvery { observeClientsByUserId(any()) } returns flowOf(result) + } + fun arrange() = this to viewModel } }