From 6c4ff0b9425e1061b31d34dd4579ec7bd08f1b35 Mon Sep 17 00:00:00 2001 From: Oussama Hassine Date: Wed, 26 Jul 2023 16:26:09 +0200 Subject: [PATCH] chore: Unblock button is no longer visible when we block someone (WPB-2427) (#2012) Co-authored-by: Alexandre Ferris --- .../com/wire/android/model/ActionableState.kt | 8 +- .../wire/android/navigation/NavigationItem.kt | 1 - .../ui/connection/ConnectionActionButton.kt | 8 +- .../ConnectionActionButtonViewModel.kt | 36 +++-- .../ConnectionActionButtonViewModelTest.kt | 135 ++++++++++++------ 5 files changed, 114 insertions(+), 74 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/model/ActionableState.kt b/app/src/main/kotlin/com/wire/android/model/ActionableState.kt index 4bc6ebd34a5..ce179da1feb 100644 --- a/app/src/main/kotlin/com/wire/android/model/ActionableState.kt +++ b/app/src/main/kotlin/com/wire/android/model/ActionableState.kt @@ -25,11 +25,9 @@ package com.wire.android.model * like: * - blocking button action when some action is already triggered */ -data class ActionableState( - val state: T, +data class ActionableState( val isPerformingAction: Boolean = false ) -fun ActionableState.performAction(): ActionableState = this.copy(isPerformingAction = true) -fun ActionableState.finishAction(): ActionableState = this.copy(isPerformingAction = false) -fun ActionableState.updateState(newState: T): ActionableState = this.copy(state = newState, isPerformingAction = false) +fun ActionableState.performAction(): ActionableState = this.copy(isPerformingAction = true) +fun ActionableState.finishAction(): ActionableState = this.copy(isPerformingAction = false) diff --git a/app/src/main/kotlin/com/wire/android/navigation/NavigationItem.kt b/app/src/main/kotlin/com/wire/android/navigation/NavigationItem.kt index 634a87218fa..ba29c468eae 100644 --- a/app/src/main/kotlin/com/wire/android/navigation/NavigationItem.kt +++ b/app/src/main/kotlin/com/wire/android/navigation/NavigationItem.kt @@ -571,7 +571,6 @@ const val EXTRA_USER_ID = "extra_user_id" const val EXTRA_USER_NAME = "extra_user_name" const val EXTRA_USER_DOMAIN = "extra_user_domain" const val EXTRA_USER_HANDLE = "extra_user_handle" -const val EXTRA_CONNECTION_STATE = "extra_connection_state" const val EXTRA_NEW_EMAIL = "extra_new_email" diff --git a/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt b/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt index fafeae02bd6..22ad1798d02 100644 --- a/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt +++ b/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt @@ -34,7 +34,6 @@ import com.sebaslogen.resaca.hilt.hiltViewModelScoped import com.wire.android.R import com.wire.android.model.ClickBlockParams import com.wire.android.model.ActionableState -import com.wire.android.navigation.EXTRA_CONNECTION_STATE import com.wire.android.navigation.EXTRA_USER_ID import com.wire.android.navigation.EXTRA_USER_NAME import com.wire.android.ui.common.button.WireButtonState @@ -53,21 +52,20 @@ fun ConnectionActionButton( connectionStatus: ConnectionState ) { val viewModel: ConnectionActionButtonViewModel = if (LocalInspectionMode.current) { - ConnectionActionButtonPreviewModel(ActionableState(connectionStatus)) + ConnectionActionButtonPreviewModel(ActionableState()) } else { hiltViewModelScoped( key = "${ConnectionActionButtonViewModelImpl.ARGS_KEY}$userId", defaultArguments = bundleOf( EXTRA_USER_ID to userId.toString(), - EXTRA_USER_NAME to userName, - EXTRA_CONNECTION_STATE to connectionStatus.toString() + EXTRA_USER_NAME to userName ) ).also { LocalSnackbarHostState.current.collectAndShowSnackbar(snackbarFlow = it.infoMessage) } } - when (viewModel.actionableState().state) { + when (connectionStatus) { ConnectionState.SENT -> WireSecondaryButton( text = stringResource(R.string.connection_label_cancel_request), loading = viewModel.actionableState().isPerformingAction, diff --git a/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModel.kt index d381c935bed..07c64d1e7da 100644 --- a/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModel.kt @@ -29,10 +29,8 @@ import com.wire.android.appLogger import com.wire.android.model.ActionableState import com.wire.android.model.finishAction import com.wire.android.model.performAction -import com.wire.android.model.updateState import com.wire.android.navigation.BackStackMode import com.wire.android.navigation.EXTRA_CONNECTION_IGNORED_USER_NAME -import com.wire.android.navigation.EXTRA_CONNECTION_STATE import com.wire.android.navigation.EXTRA_USER_ID import com.wire.android.navigation.EXTRA_USER_NAME import com.wire.android.navigation.NavigationCommand @@ -43,7 +41,6 @@ import com.wire.android.util.ui.UIText import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.id.QualifiedIdMapper import com.wire.kalium.logic.data.id.toQualifiedID -import com.wire.kalium.logic.data.user.ConnectionState import com.wire.kalium.logic.feature.connection.AcceptConnectionRequestUseCase import com.wire.kalium.logic.feature.connection.AcceptConnectionRequestUseCaseResult import com.wire.kalium.logic.feature.connection.CancelConnectionRequestUseCase @@ -65,7 +62,7 @@ import javax.inject.Inject interface ConnectionActionButtonViewModel { - fun actionableState(): ActionableState + fun actionableState(): ActionableState fun onSendConnectionRequest() fun onCancelConnectionRequest() fun onAcceptConnectionRequest() @@ -91,27 +88,26 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( private val userId: QualifiedID = savedStateHandle.get(EXTRA_USER_ID)!!.toQualifiedID(qualifiedIdMapper) private val userName: String = savedStateHandle.get(EXTRA_USER_NAME)!! - private val extraConnectionState: ConnectionState = ConnectionState.valueOf(savedStateHandle.get(EXTRA_CONNECTION_STATE)!!) - private var state: ActionableState by mutableStateOf(ActionableState(extraConnectionState)) + private var state: ActionableState by mutableStateOf(ActionableState()) private val _infoMessage = MutableSharedFlow() val infoMessage = _infoMessage.asSharedFlow() - override fun actionableState(): ActionableState = state + override fun actionableState(): ActionableState = state override fun onSendConnectionRequest() { viewModelScope.launch { state = state.performAction() when (sendConnectionRequest(userId)) { is SendConnectionRequestResult.Failure -> { - appLogger.d(("Couldn't send a connect request to user $userId")) + appLogger.e(("Couldn't send a connection request to user ${userId.toLogString()}")) state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_sent_error)) } is SendConnectionRequestResult.Success -> { - state = state.updateState(ConnectionState.SENT) + state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_sent)) } } @@ -123,13 +119,13 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( state = state.performAction() when (cancelConnectionRequest(userId)) { is CancelConnectionRequestUseCaseResult.Failure -> { - appLogger.d(("Couldn't cancel a connect request to user $userId")) + appLogger.e(("Couldn't cancel a connection request to user ${userId.toLogString()}")) state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_cancel_error)) } is CancelConnectionRequestUseCaseResult.Success -> { - state = state.updateState(ConnectionState.NOT_CONNECTED) + state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_canceled)) } } @@ -141,13 +137,13 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( state = state.performAction() when (acceptConnectionRequest(userId)) { is AcceptConnectionRequestUseCaseResult.Failure -> { - appLogger.d(("Couldn't accept a connect request to user $userId")) + appLogger.e(("Couldn't accept a connection request to user ${userId.toLogString()}")) state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_accept_error)) } is AcceptConnectionRequestUseCaseResult.Success -> { - state = state.updateState(ConnectionState.ACCEPTED) + state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_accepted)) } } @@ -159,13 +155,13 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( state = state.performAction() when (ignoreConnectionRequest(userId)) { is IgnoreConnectionRequestUseCaseResult.Failure -> { - appLogger.d(("Couldn't ignore a connect request to user $userId")) + appLogger.e(("Couldn't ignore a connection request to user ${userId.toLogString()}")) state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.connection_request_ignore_error)) } is IgnoreConnectionRequestUseCaseResult.Success -> { - state = state.updateState(ConnectionState.IGNORED) + state = state.finishAction() navigationManager.navigateBack( mapOf( EXTRA_CONNECTION_IGNORED_USER_NAME to userName @@ -181,14 +177,14 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( state = state.performAction() when (val result = withContext(dispatchers.io()) { unblockUser(userId) }) { is UnblockUserResult.Failure -> { - appLogger.e("Error while unblocking user $userId ; Error ${result.coreFailure}") + appLogger.e("Error while unblocking user ${userId.toLogString()} ; Error ${result.coreFailure}") state = state.finishAction() _infoMessage.emit(UIText.StringResource(R.string.error_unblocking_user)) } UnblockUserResult.Success -> { - appLogger.i("User $userId was unblocked") - state = state.updateState(ConnectionState.ACCEPTED) + appLogger.i("User ${userId.toLogString()} was unblocked") + state = state.finishAction() } } } @@ -220,8 +216,8 @@ class ConnectionActionButtonViewModelImpl @Inject constructor( } @Suppress("EmptyFunctionBlock") -class ConnectionActionButtonPreviewModel(private val state: ActionableState) : ConnectionActionButtonViewModel { - override fun actionableState(): ActionableState = state +class ConnectionActionButtonPreviewModel(private val state: ActionableState) : ConnectionActionButtonViewModel { + override fun actionableState(): ActionableState = state override fun onSendConnectionRequest() {} override fun onCancelConnectionRequest() {} override fun onAcceptConnectionRequest() {} diff --git a/app/src/test/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModelTest.kt index 1f2722ae18c..3280da81ce9 100644 --- a/app/src/test/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModelTest.kt @@ -26,7 +26,6 @@ import com.wire.android.config.TestDispatcherProvider import com.wire.android.config.mockUri import com.wire.android.framework.TestConversation import com.wire.android.framework.TestUser -import com.wire.android.navigation.EXTRA_CONNECTION_STATE import com.wire.android.navigation.EXTRA_USER_ID import com.wire.android.navigation.EXTRA_USER_NAME import com.wire.android.navigation.NavigationManager @@ -35,7 +34,6 @@ import com.wire.android.util.ui.UIText import com.wire.android.util.ui.WireSessionImageLoader import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.data.id.QualifiedIdMapper -import com.wire.kalium.logic.data.user.ConnectionState import com.wire.kalium.logic.feature.connection.AcceptConnectionRequestUseCase import com.wire.kalium.logic.feature.connection.AcceptConnectionRequestUseCaseResult import com.wire.kalium.logic.feature.connection.CancelConnectionRequestUseCase @@ -65,14 +63,12 @@ import org.junit.jupiter.api.Test class ConnectionActionButtonViewModelTest { - @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `given a userId, when sending a connection request, then returns a Success result and update view state`() = runTest { + fun `given success, when sending a connection request, then emit success message`() = runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() .withSendConnectionRequest(SendConnectionRequestResult.Success) .arrange() - assertEquals(ConnectionState.NOT_CONNECTED, viewModel.actionableState().state) viewModel.infoMessage.test { // when @@ -81,58 +77,94 @@ class ConnectionActionButtonViewModelTest { // then val result = awaitItem() assertEquals(UIText.StringResource(R.string.connection_request_sent), result) - coVerify { arrangement.sendConnectionRequest.invoke(eq(TestUser.USER_ID)) } - assertEquals(ConnectionState.SENT, viewModel.actionableState().state) + coVerify(exactly = 1) { arrangement.sendConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) } } @Test - fun `given a userId, when sending a connection request a fails, then returns a Failure result and show error message`() = + fun `given a failure, when sending a connection request, then emit failure message`() = runTest { + // given + val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() + .withSendConnectionRequest(SendConnectionRequestResult.Failure(failure)) + .arrange() + + viewModel.infoMessage.test { + // when + viewModel.onSendConnectionRequest() + + // then + val result = awaitItem() + assertEquals(UIText.StringResource(R.string.connection_request_sent_error), result) + coVerify(exactly = 1) { arrangement.sendConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) + } + } + + @Test + fun `given success, when ignoring a connection request, then navigate back`() = + runTest { + // given + val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() + .withIgnoreConnectionRequest(IgnoreConnectionRequestUseCaseResult.Success) + .arrange() + + // when + viewModel.onIgnoreConnectionRequest() + + // then + coVerify(exactly = 1) { arrangement.ignoreConnectionRequest.invoke(eq(TestUser.USER_ID)) } + coVerify(exactly = 1) { arrangement.navigationManager.navigateBack(any()) } + assertEquals(false, viewModel.actionableState().isPerformingAction) + } + + @Test + fun `given failure, when ignoring a connection request, then emit error message`() = runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() - .withSendConnectionRequest(SendConnectionRequestResult.Failure(CoreFailure.Unknown(RuntimeException("some error")))) + .withIgnoreConnectionRequest(IgnoreConnectionRequestUseCaseResult.Failure(failure)) .arrange() - assertEquals(ConnectionState.NOT_CONNECTED, viewModel.actionableState().state) viewModel.infoMessage.test { // when - viewModel.onSendConnectionRequest() + viewModel.onIgnoreConnectionRequest() // then val result = awaitItem() - assertEquals(UIText.StringResource(R.string.connection_request_sent_error), result) - coVerify { arrangement.sendConnectionRequest.invoke(eq(TestUser.USER_ID)) } - assertEquals(ConnectionState.NOT_CONNECTED, viewModel.actionableState().state) + coVerify(exactly = 1) { arrangement.ignoreConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(UIText.StringResource(R.string.connection_request_ignore_error), result) + assertEquals(false, viewModel.actionableState().isPerformingAction) } } @Test - fun `given a userId, when ignoring a connection request, then returns a Success result and update view state`() = + fun `given success, when canceling a connection request, then emit success message`() = runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() - .withIgnoreConnectionRequest(IgnoreConnectionRequestUseCaseResult.Success) + .withCancelConnectionRequest(CancelConnectionRequestUseCaseResult.Success) .arrange() - assertEquals(ConnectionState.PENDING, viewModel.actionableState().state) - // when - viewModel.onIgnoreConnectionRequest() + viewModel.infoMessage.test { + // when + viewModel.onCancelConnectionRequest() - // then - coVerify { arrangement.ignoreConnectionRequest.invoke(eq(TestUser.USER_ID)) } - assertEquals(ConnectionState.IGNORED, viewModel.actionableState().state) - coVerify { arrangement.navigationManager.navigateBack(any()) } + // then + val result = awaitItem() + assertEquals(UIText.StringResource(R.string.connection_request_canceled), result) + coVerify(exactly = 1) { arrangement.cancelConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) + } } @Test - fun `given a userId, when canceling a connection request, then returns a Success result and update view state`() = + fun `given failure, when canceling a connection request, then emit failure message`() = runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() - .withCancelConnectionRequest(CancelConnectionRequestUseCaseResult.Success) + .withCancelConnectionRequest(CancelConnectionRequestUseCaseResult.Failure(failure)) .arrange() - assertEquals(ConnectionState.SENT, viewModel.actionableState().state) viewModel.infoMessage.test { // when @@ -140,29 +172,49 @@ class ConnectionActionButtonViewModelTest { // then val result = awaitItem() - assertEquals(UIText.StringResource(R.string.connection_request_canceled), result) - coVerify { arrangement.cancelConnectionRequest.invoke(eq(TestUser.USER_ID)) } - assertEquals(ConnectionState.NOT_CONNECTED, viewModel.actionableState().state) + assertEquals(UIText.StringResource(R.string.connection_request_cancel_error), result) + coVerify(exactly = 1) { arrangement.cancelConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) } } @Test - fun `given a userId, when accepting a connection request, then returns a Success result and update view state`() = + fun `given success, when accepting a connection request, then emit success message`() = runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() .withAcceptConnectionRequest(AcceptConnectionRequestUseCaseResult.Success) .arrange() - assertEquals(ConnectionState.PENDING, viewModel.actionableState().state) viewModel.infoMessage.test { // when viewModel.onAcceptConnectionRequest() // then - awaitItem() - coVerify { arrangement.acceptConnectionRequest.invoke(eq(TestUser.USER_ID)) } - assertEquals(ConnectionState.ACCEPTED, viewModel.actionableState().state) + val result = awaitItem() + assertEquals(UIText.StringResource(R.string.connection_request_accepted), result) + coVerify(exactly = 1) { arrangement.acceptConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) + } + } + + @Test + fun `given failure, when accepting a connection request, then emit failure message`() = + runTest { + // given + val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() + .withAcceptConnectionRequest(AcceptConnectionRequestUseCaseResult.Failure(failure)) + .arrange() + + viewModel.infoMessage.test { + // when + viewModel.onAcceptConnectionRequest() + + // then + val result = awaitItem() + assertEquals(UIText.StringResource(R.string.connection_request_accept_error), result) + coVerify(exactly = 1) { arrangement.acceptConnectionRequest.invoke(eq(TestUser.USER_ID)) } + assertEquals(false, viewModel.actionableState().isPerformingAction) } } @@ -189,7 +241,7 @@ class ConnectionActionButtonViewModelTest { runTest { // given val (arrangement, viewModel) = ConnectionActionButtonHiltArrangement() - .withGetOneToOneConversation(CreateConversationResult.Failure(CoreFailure.Unknown(RuntimeException("some error")))) + .withGetOneToOneConversation(CreateConversationResult.Failure(failure)) .arrange() // when @@ -201,6 +253,10 @@ class ConnectionActionButtonViewModelTest { arrangement.navigationManager wasNot Called } } + + companion object { + val failure = CoreFailure.Unknown(RuntimeException("some error")) + } } internal class ConnectionActionButtonHiltArrangement { @@ -259,8 +315,6 @@ internal class ConnectionActionButtonHiltArrangement { mockUri() every { savedStateHandle.get(EXTRA_USER_ID) } returns "some_value@some_domain" every { savedStateHandle.get(EXTRA_USER_NAME) } returns TestUser.OTHER_USER.name - every { savedStateHandle.get(EXTRA_CONNECTION_STATE) } returns - ConnectionState.NOT_CONNECTED.toString() coEvery { observeSelfUser() } returns flowOf(TestUser.SELF_USER) coEvery { getOrCreateOneToOneConversation(TestConversation.ID) } returns CreateConversationResult.Success( @@ -279,20 +333,15 @@ internal class ConnectionActionButtonHiltArrangement { } fun withAcceptConnectionRequest(result: AcceptConnectionRequestUseCaseResult) = apply { - every { savedStateHandle.get(EXTRA_CONNECTION_STATE) } returns - ConnectionState.PENDING.toString() coEvery { acceptConnectionRequest(any()) } returns result } fun withCancelConnectionRequest(result: CancelConnectionRequestUseCaseResult) = apply { - every { savedStateHandle.get(EXTRA_CONNECTION_STATE) } returns - ConnectionState.SENT.toString() + coEvery { cancelConnectionRequest(any()) } returns result } fun withIgnoreConnectionRequest(result: IgnoreConnectionRequestUseCaseResult) = apply { - every { savedStateHandle.get(EXTRA_CONNECTION_STATE) } returns - ConnectionState.PENDING.toString() coEvery { ignoreConnectionRequest(any()) } returns result }