Skip to content

Commit

Permalink
Merge branch 'ensure-we-use-collectsideeffectwithlifecycle-droid-1186'
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawa committed Sep 2, 2024
2 parents d0040f9 + 0cdcfb8 commit 71170a3
Show file tree
Hide file tree
Showing 21 changed files with 96 additions and 118 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import io.mockk.MockKAnnotations
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension
import net.mullvad.mullvadvpn.compose.setContentWithTheme
import net.mullvad.mullvadvpn.compose.state.PaymentState
Expand All @@ -21,7 +19,6 @@ import net.mullvad.mullvadvpn.lib.payment.model.PaymentStatus
import net.mullvad.mullvadvpn.lib.payment.model.ProductId
import net.mullvad.mullvadvpn.lib.payment.model.ProductPrice
import net.mullvad.mullvadvpn.viewmodel.AccountUiState
import net.mullvad.mullvadvpn.viewmodel.AccountViewModel
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.RegisterExtension
Expand All @@ -48,8 +45,7 @@ class AccountScreenTest {
accountNumber = DUMMY_ACCOUNT_NUMBER,
accountExpiry = null,
showSitePayment = false,
),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
)
}

Expand All @@ -72,8 +68,6 @@ class AccountScreenTest {
accountNumber = DUMMY_ACCOUNT_NUMBER,
accountExpiry = null,
),
uiSideEffect =
MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
onManageAccountClick = mockedClickHandler,
)
}
Expand All @@ -99,8 +93,6 @@ class AccountScreenTest {
accountExpiry = null,
showSitePayment = false,
),
uiSideEffect =
MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
onRedeemVoucherClick = mockedClickHandler,
)
}
Expand All @@ -126,8 +118,6 @@ class AccountScreenTest {
accountExpiry = null,
showSitePayment = false,
),
uiSideEffect =
MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
onLogoutClick = mockedClickHandler,
)
}
Expand All @@ -147,8 +137,7 @@ class AccountScreenTest {
AccountScreen(
state =
AccountUiState.default()
.copy(billingPaymentState = PaymentState.Error.Billing),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
.copy(billingPaymentState = PaymentState.Error.Billing)
)
}

Expand All @@ -170,8 +159,7 @@ class AccountScreenTest {
.copy(
billingPaymentState =
PaymentState.PaymentAvailable(listOf(mockPaymentProduct))
),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
)
}

Expand All @@ -193,8 +181,7 @@ class AccountScreenTest {
.copy(
billingPaymentState =
PaymentState.PaymentAvailable(listOf(mockPaymentProduct))
),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
)
}

Expand All @@ -218,8 +205,6 @@ class AccountScreenTest {
billingPaymentState =
PaymentState.PaymentAvailable(listOf(mockPaymentProduct))
),
uiSideEffect =
MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
navigateToVerificationPendingDialog = mockNavigateToVerificationPending,
)
}
Expand All @@ -245,8 +230,7 @@ class AccountScreenTest {
.copy(
billingPaymentState =
PaymentState.PaymentAvailable(listOf(mockPaymentProduct))
),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
)
}

Expand All @@ -272,7 +256,6 @@ class AccountScreenTest {
PaymentState.PaymentAvailable(listOf(mockPaymentProduct))
),
onPurchaseBillingProductClick = clickHandler,
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import com.ramcosta.composedestinations.result.ResultBackNavigator
import com.ramcosta.composedestinations.spec.DestinationStyle
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.state.DeleteApiAccessMethodUiState
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.viewmodel.DeleteApiAccessMethodConfirmationSideEffect
Expand All @@ -34,7 +34,7 @@ fun DeleteApiAccessMethodConfirmation(navigator: ResultBackNavigator<Boolean>) {
val viewModel = koinViewModel<DeleteApiAccessMethodConfirmationViewModel>()
val state = viewModel.uiState.collectAsStateWithLifecycle()

LaunchedEffectCollect(viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
is DeleteApiAccessMethodConfirmationSideEffect.Deleted ->
navigator.navigateBack(result = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import com.ramcosta.composedestinations.spec.DestinationStyle
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.communication.CustomListActionResultData
import net.mullvad.mullvadvpn.compose.state.DeleteCustomListUiState
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.CustomListId
import net.mullvad.mullvadvpn.lib.model.CustomListName
import net.mullvad.mullvadvpn.lib.theme.AppTheme
Expand Down Expand Up @@ -42,7 +42,7 @@ fun DeleteCustomList(navigator: ResultBackNavigator<CustomListActionResultData.S
val viewModel: DeleteCustomListConfirmationViewModel = koinViewModel()
val state by viewModel.uiState.collectAsStateWithLifecycle()

LaunchedEffectCollect(viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
is DeleteCustomListConfirmationSideEffect.ReturnWithResult ->
navigator.navigateBack(result = it.result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import net.mullvad.mullvadvpn.compose.button.NegativeButton
import net.mullvad.mullvadvpn.compose.button.PrimaryButton
import net.mullvad.mullvadvpn.compose.communication.DnsDialogResult
import net.mullvad.mullvadvpn.compose.textfield.DnsTextField
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens
import net.mullvad.mullvadvpn.viewmodel.DnsDialogSideEffect
Expand Down Expand Up @@ -57,7 +57,7 @@ data class DnsDialogNavArgs(val index: Int? = null, val initialValue: String? =
fun Dns(resultNavigator: ResultBackNavigator<DnsDialogResult>) {
val viewModel = koinViewModel<DnsDialogViewModel>()

LaunchedEffectCollect(viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
DnsDialogSideEffect.Complete ->
resultNavigator.navigateBack(result = DnsDialogResult.Success)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import net.mullvad.mullvadvpn.compose.communication.CustomListActionResultData
import net.mullvad.mullvadvpn.compose.state.EditCustomListNameUiState
import net.mullvad.mullvadvpn.compose.test.EDIT_CUSTOM_LIST_DIALOG_INPUT_TEST_TAG
import net.mullvad.mullvadvpn.compose.textfield.CustomListNameTextField
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.CustomListId
import net.mullvad.mullvadvpn.lib.model.CustomListName
import net.mullvad.mullvadvpn.lib.model.GetCustomListError
Expand Down Expand Up @@ -53,7 +53,7 @@ fun EditCustomListName(
backNavigator: ResultBackNavigator<CustomListActionResultData.Success.Renamed>
) {
val vm: EditCustomListNameDialogViewModel = koinViewModel()
LaunchedEffectCollect(vm.uiSideEffect) { sideEffect ->
CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect ->
when (sideEffect) {
is EditCustomListNameDialogSideEffect.ReturnWithResult -> {
backNavigator.navigateBack(result = sideEffect.result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.NegativeButton
import net.mullvad.mullvadvpn.compose.button.PrimaryButton
import net.mullvad.mullvadvpn.compose.textfield.MtuTextField
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.constant.MTU_MAX_VALUE
import net.mullvad.mullvadvpn.constant.MTU_MIN_VALUE
import net.mullvad.mullvadvpn.lib.model.Mtu
Expand All @@ -48,7 +48,7 @@ fun Mtu(navigator: ResultBackNavigator<Boolean>) {
val viewModel = koinViewModel<MtuDialogViewModel>()

val uiState by viewModel.uiState.collectAsStateWithLifecycle()
LaunchedEffectCollect(viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
MtuDialogSideEffect.Complete -> navigator.navigateBack(result = true)
MtuDialogSideEffect.Error -> navigator.navigateBack(result = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import net.mullvad.mullvadvpn.compose.state.TestApiAccessMethodState
import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_CANCEL_BUTTON_TEST_TAG
import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_LOADING_SPINNER_TEST_TAG
import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_SAVE_BUTTON_TEST_TAG
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.ApiAccessMethod
import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId
import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodName
Expand Down Expand Up @@ -59,7 +59,7 @@ data class SaveApiAccessMethodNavArgs(
fun SaveApiAccessMethod(backNavigator: ResultBackNavigator<Boolean>) {
val viewModel = koinViewModel<SaveApiAccessMethodViewModel>()

LaunchedEffectCollect(sideEffect = viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
SaveApiAccessMethodSideEffect.CouldNotSaveApiAccessMethod ->
backNavigator.navigateBack(result = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import com.ramcosta.composedestinations.spec.DestinationStyle
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.PrimaryButton
import net.mullvad.mullvadvpn.compose.component.MullvadCircularProgressIndicatorMedium
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.payment.model.ProductId
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.util.getActivity
Expand Down Expand Up @@ -125,7 +125,7 @@ fun Payment(productId: ProductId, resultBackNavigator: ResultBackNavigator<Boole
val vm = koinViewModel<PaymentViewModel>()
val state by vm.uiState.collectAsStateWithLifecycle()

LaunchedEffectCollect(vm.uiSideEffect) { sideEffect ->
CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect ->
when (sideEffect) {
PaymentUiSideEffect.PaymentCancelled -> resultBackNavigator.navigateBack(result = false)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ import com.ramcosta.composedestinations.generated.destinations.VerificationPendi
import com.ramcosta.composedestinations.navigation.DestinationsNavigator
import com.ramcosta.composedestinations.result.NavResult
import com.ramcosta.composedestinations.result.ResultRecipient
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.ExternalButton
Expand All @@ -54,7 +51,7 @@ import net.mullvad.mullvadvpn.compose.extensions.createOpenAccountPageHook
import net.mullvad.mullvadvpn.compose.extensions.dropUnlessResumed
import net.mullvad.mullvadvpn.compose.state.PaymentState
import net.mullvad.mullvadvpn.compose.transitions.SlideInFromBottomTransition
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.compose.util.SecureScreenWhileInView
import net.mullvad.mullvadvpn.compose.util.createCopyToClipboardHandle
import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately
Expand Down Expand Up @@ -98,8 +95,7 @@ private fun PreviewAccountScreen() {
),
)
),
),
uiSideEffect = MutableSharedFlow<AccountViewModel.UiSideEffect>().asSharedFlow(),
)
)
}
}
Expand All @@ -114,6 +110,29 @@ fun Account(
val vm = koinViewModel<AccountViewModel>()
val state by vm.uiState.collectAsStateWithLifecycle()

val snackbarHostState = remember { SnackbarHostState() }
val copyTextString = stringResource(id = R.string.copied_mullvad_account_number)
val errorString = stringResource(id = R.string.error_occurred)
val copyToClipboard = createCopyToClipboardHandle(snackbarHostState = snackbarHostState)
val openAccountPage = LocalUriHandler.current.createOpenAccountPageHook()

CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect ->
when (sideEffect) {
AccountViewModel.UiSideEffect.NavigateToLogin -> {
navigator.navigate(LoginDestination(null)) {
launchSingleTop = true
popUpTo(NavGraphs.root) { inclusive = true }
}
}
is AccountViewModel.UiSideEffect.OpenAccountManagementPageInBrowser ->
openAccountPage(sideEffect.token)
is AccountViewModel.UiSideEffect.CopyAccountNumber ->
launch { copyToClipboard(sideEffect.accountNumber, copyTextString) }
AccountViewModel.UiSideEffect.GenericError ->
snackbarHostState.showSnackbarImmediately(message = errorString)
}
}

playPaymentResultRecipient.onNavResult {
when (it) {
NavResult.Canceled -> {
Expand All @@ -125,16 +144,10 @@ fun Account(

AccountScreen(
state = state,
uiSideEffect = vm.uiSideEffect,
snackbarHostState = snackbarHostState,
onRedeemVoucherClick = dropUnlessResumed { navigator.navigate(RedeemVoucherDestination) },
onManageAccountClick = vm::onManageAccountClick,
onLogoutClick = vm::onLogoutClick,
navigateToLogin = {
navigator.navigate(LoginDestination(null)) {
launchSingleTop = true
popUpTo(NavGraphs.root) { inclusive = true }
}
},
onCopyAccountNumber = vm::onCopyAccountNumber,
onBackClick = dropUnlessResumed { navigator.navigateUp() },
navigateToDeviceInfo = dropUnlessResumed { navigator.navigate(DeviceNameInfoDestination) },
Expand All @@ -149,37 +162,19 @@ fun Account(
@Composable
fun AccountScreen(
state: AccountUiState,
uiSideEffect: Flow<AccountViewModel.UiSideEffect>,
snackbarHostState: SnackbarHostState = remember { SnackbarHostState() },
onCopyAccountNumber: (String) -> Unit = {},
onRedeemVoucherClick: () -> Unit = {},
onManageAccountClick: () -> Unit = {},
onLogoutClick: () -> Unit = {},
onPurchaseBillingProductClick: (productId: ProductId) -> Unit = { _ -> },
navigateToLogin: () -> Unit = {},
navigateToDeviceInfo: () -> Unit = {},
navigateToVerificationPendingDialog: () -> Unit = {},
onBackClick: () -> Unit = {},
) {
// This will enable SECURE_FLAG while this screen is visible to preview screenshot
SecureScreenWhileInView()

val snackbarHostState = remember { SnackbarHostState() }
val copyTextString = stringResource(id = R.string.copied_mullvad_account_number)
val errorString = stringResource(id = R.string.error_occurred)
val copyToClipboard = createCopyToClipboardHandle(snackbarHostState = snackbarHostState)
val openAccountPage = LocalUriHandler.current.createOpenAccountPageHook()
LaunchedEffectCollect(uiSideEffect) { sideEffect ->
when (sideEffect) {
AccountViewModel.UiSideEffect.NavigateToLogin -> navigateToLogin()
is AccountViewModel.UiSideEffect.OpenAccountManagementPageInBrowser ->
openAccountPage(sideEffect.token)
is AccountViewModel.UiSideEffect.CopyAccountNumber ->
launch { copyToClipboard(sideEffect.accountNumber, copyTextString) }
AccountViewModel.UiSideEffect.GenericError ->
snackbarHostState.showSnackbarImmediately(message = errorString)
}
}

ScaffoldWithMediumTopBar(
appBarTitle = stringResource(id = R.string.settings_account),
navigationIcon = { NavigateBackDownIconButton(onBackClick) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import net.mullvad.mullvadvpn.compose.test.API_ACCESS_TEST_METHOD_BUTTON
import net.mullvad.mullvadvpn.compose.test.API_ACCESS_USE_METHOD_BUTTON
import net.mullvad.mullvadvpn.compose.test.DELETE_DROPDOWN_MENU_ITEM_TEST_TAG
import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.compose.util.OnNavResultValue
import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately
import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId
Expand Down Expand Up @@ -94,7 +94,7 @@ fun ApiAccessMethodDetails(
val context = LocalContext.current
val coroutineScope = rememberCoroutineScope()

LaunchedEffectCollect(sideEffect = viewModel.uiSideEffect) {
CollectSideEffectWithLifecycle(viewModel.uiSideEffect) {
when (it) {
ApiAccessMethodDetailsSideEffect.GenericError ->
launch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import net.mullvad.mullvadvpn.compose.test.CIRCULAR_PROGRESS_INDICATOR
import net.mullvad.mullvadvpn.compose.test.SAVE_BUTTON_TEST_TAG
import net.mullvad.mullvadvpn.compose.textfield.SearchTextField
import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition
import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.CustomListId
import net.mullvad.mullvadvpn.lib.model.RelayItem
import net.mullvad.mullvadvpn.lib.theme.Dimens
Expand Down Expand Up @@ -85,7 +85,7 @@ fun CustomListLocations(
}
}

LaunchedEffectCollect(customListsViewModel.uiSideEffect) { sideEffect ->
CollectSideEffectWithLifecycle(customListsViewModel.uiSideEffect) { sideEffect ->
when (sideEffect) {
is CustomListLocationsSideEffect.ReturnWithResultData ->
backNavigator.navigateBack(result = sideEffect.result)
Expand Down
Loading

0 comments on commit 71170a3

Please sign in to comment.