Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[POS] Simultaneous cash and card payment handling #13277

Merged
merged 15 commits into from
Jan 15, 2025
Merged
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -2,13 +2,17 @@ package com.woocommerce.android.ui.woopos.cashpayment

import androidx.compose.animation.slideInHorizontally
import androidx.compose.animation.slideOutHorizontally
import androidx.compose.runtime.LaunchedEffect
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.navigation.NavController
import androidx.navigation.NavGraphBuilder
import androidx.navigation.NavType
import androidx.navigation.compose.composable
import androidx.navigation.navArgument
import com.woocommerce.android.ui.woopos.home.ChildToParentEvent.NavigationEvent
import com.woocommerce.android.ui.woopos.home.HOME_ROUTE
import com.woocommerce.android.ui.woopos.home.IsHomePaymentCompletedViaCash
import com.woocommerce.android.ui.woopos.home.WooPosHomeViewModel
import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent
import com.woocommerce.android.ui.woopos.root.navigation.navigateOnce

@@ -49,6 +53,18 @@ fun NavGraphBuilder.cashPaymentScreen(
}
},
) { backStackEntry ->
val homeViewModel = hiltViewModel<WooPosHomeViewModel>()
LaunchedEffect(homeViewModel) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if we need to have this code at all? Don't we handle the event in WooPosHomeScreen already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LaunchedEffect coroutine in WooPosHomeScreen is not active when we navigate to CashPayment screen. That's why I subscribed to the events in the CashPayment screen here again.

An alternative approach could be to move the subscription to navigation events up in the compose navigation hierarchy, to theWooPosRootHost, in order to avoid duplicate subscriptions – 5f9d68e. Let me know what you think, @kidinov !

homeViewModel.navigationEvent.collect { event ->
when (event) {
NavigationEvent.ReturnHomeFromCashPayment -> {
// interrupting cash payment in case buyer pays with card
onNavigationEvent(WooPosNavigationEvent.ReturnHomeFromCashPayment)
}
else -> Unit
}
}
}
WooPosCashPaymentScreen(
onNavigationEvent = onNavigationEvent,
)
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@ sealed class ChildToParentEvent {
sealed class NavigationEvent : ChildToParentEvent() {
data class ToCashPayment(val orderId: Long) : NavigationEvent()
data class ToEmailReceipt(val orderId: Long) : NavigationEvent()
data object ReturnHomeFromCashPayment : NavigationEvent()
data object ExitPos : NavigationEvent()
}
}
Original file line number Diff line number Diff line change
@@ -30,6 +30,15 @@ fun NavController.navigateToHomeScreenAfterSuccessfulCashPayment() {
}
}

fun NavController.navigateToHomeScreenIfHomeScreenNotOpen() {
if (currentDestination?.route != HOME_ROUTE) {
popBackStack(
HOME_ROUTE,
false
)
}
}

fun NavGraphBuilder.homeScreen(
onNavigationEvent: (WooPosNavigationEvent) -> Unit
) {
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent
import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent.ExitPosClicked
import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent.OpenCashPayment
import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent.OpenEmailReceipt
import com.woocommerce.android.ui.woopos.root.navigation.WooPosNavigationEvent.ReturnHomeFromCashPayment
import org.wordpress.android.util.ToastUtils

@Composable
@@ -79,6 +80,7 @@ fun WooPosHomeScreen(
is NavigationEvent.ToCashPayment -> onNavigationEvent(OpenCashPayment(it.orderId))
is NavigationEvent.ToEmailReceipt -> onNavigationEvent(OpenEmailReceipt(it.orderId))
NavigationEvent.ExitPos -> onNavigationEvent(ExitPosClicked)
NavigationEvent.ReturnHomeFromCashPayment -> onNavigationEvent(ReturnHomeFromCashPayment)
}
}
}
Original file line number Diff line number Diff line change
@@ -138,6 +138,7 @@ class WooPosHomeViewModel @Inject constructor(
_state.value = _state.value.copy(
screenPositionState = ScreenPositionState.Checkout.FullScreenTotals
)
_navigationEvent.emit(NavigationEvent.ReturnHomeFromCashPayment)
}

is ChildToParentEvent.GoBackToCheckoutAfterFailedPayment,
Original file line number Diff line number Diff line change
@@ -278,9 +278,10 @@ class WooPosTotalsViewModel @Inject constructor(
uiState.value = InitialState
}

is ParentToChildrenEvent.OrderSuccessfullyPaid -> showSuccessfulPaymentState(
event.paymentMethod
)
is ParentToChildrenEvent.OrderSuccessfullyPaid -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking that it's ok that cancelation will be called also in the card reader payments cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow works correctly, but I switched to canceling payment intent only in case the payment success comes from cash payment to avoid redundant logic – 60ef6e2

cancelPaymentAction()
showSuccessfulPaymentState(event.paymentMethod)
}

is ParentToChildrenEvent.ItemClickedInProductSelector -> Unit
}
Original file line number Diff line number Diff line change
@@ -8,4 +8,5 @@ sealed class WooPosNavigationEvent {
data class OpenEmailReceipt(val orderId: Long) : WooPosNavigationEvent()
data object GoBack : WooPosNavigationEvent()
data object OpenHomeFromCashPaymentAfterSuccessfulPayment : WooPosNavigationEvent()
data object ReturnHomeFromCashPayment : WooPosNavigationEvent()
}
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ import com.woocommerce.android.ui.woopos.cashpayment.navigateToCashPaymentScreen
import com.woocommerce.android.ui.woopos.emailreceipt.navigateToEmailReceipt
import com.woocommerce.android.ui.woopos.home.navigateToHomeScreen
import com.woocommerce.android.ui.woopos.home.navigateToHomeScreenAfterSuccessfulCashPayment
import com.woocommerce.android.ui.woopos.home.navigateToHomeScreenIfHomeScreenNotOpen

fun NavHostController.handleNavigationEvent(
event: WooPosNavigationEvent,
@@ -22,5 +23,6 @@ fun NavHostController.handleNavigationEvent(
navigateToHomeScreenAfterSuccessfulCashPayment()

is WooPosNavigationEvent.OpenEmailReceipt -> navigateToEmailReceipt(event.orderId)
WooPosNavigationEvent.ReturnHomeFromCashPayment -> navigateToHomeScreenIfHomeScreenNotOpen()
}
}