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.

Sign up for GitHub

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

Jump to bottom

[Woo POS] Improve Card Reader Payment Flow Error Handling #12543

Merged
merged 8 commits into from
Sep 13, 2024
Merged

[Woo POS] Improve Card Reader Payment Flow Error Handling #12543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e94d2b9
Check for the network before payment
backwardstruck Sep 12, 2024
eaa43c5
Add a test
backwardstruck Sep 12, 2024
66036ba
Update the tests
backwardstruck Sep 12, 2024
862db8f
Fix imports
backwardstruck Sep 12, 2024
af2aa22
Merge branch 'trunk' into 12542-woo-pos-error-message-doesnt-feel-rig…
backwardstruck Sep 12, 2024
516edef
Merge branch 'issue/12497-pos-internet-issue' into 12542-woo-pos-erro…
backwardstruck Sep 12, 2024
45bdb08
Merge branch 'trunk' into 12542-woo-pos-error-message-doesnt-feel-rig…
backwardstruck Sep 13, 2024
ab4d89a
Add a new test
backwardstruck Sep 13, 2024
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
14 changes: 11 additions & 3 deletions ...ce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.woocommerce.android.ui.woopos.home.ChildToParentEvent
import com.woocommerce.android.ui.woopos.home.ParentToChildrenEvent
import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender
import com.woocommerce.android.ui.woopos.home.WooPosParentToChildrenEventReceiver
import com.woocommerce.android.ui.woopos.util.WooPosNetworkStatus
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker
import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice
Expand All @@ -35,6 +36,7 @@ class WooPosTotalsViewModel @Inject constructor(
private val totalsRepository: WooPosTotalsRepository,
private val priceFormat: WooPosFormatPrice,
private val analyticsTracker: WooPosAnalyticsTracker,
private val networkStatus: WooPosNetworkStatus,
savedState: SavedStateHandle,
) : ViewModel() {

Expand Down Expand Up @@ -80,9 +82,15 @@ class WooPosTotalsViewModel @Inject constructor(
}

private fun collectPayment() {
val orderId = dataState.value.orderId
check(orderId != EMPTY_ORDER_ID)
cardReaderFacade.collectPayment(orderId)
if (!networkStatus.isConnected()) {
viewModelScope.launch {
childrenToParentEventSender.sendToParent(ChildToParentEvent.NoInternet)
}
} else {
val orderId = dataState.value.orderId
check(orderId != EMPTY_ORDER_ID)
cardReaderFacade.collectPayment(orderId)
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ What do you think about adding test to verify that cardReaderFacade.collectPayment is not called when there is no internet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you had suggested that earlier but I failed to add that test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

}
}

private fun listenUpEvents() {
Expand Down
118 changes: 112 additions & 6 deletions ...rc/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.woocommerce.android.ui.woopos.home.ParentToChildrenEvent
import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender
import com.woocommerce.android.ui.woopos.home.WooPosParentToChildrenEventReceiver
import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule
import com.woocommerce.android.ui.woopos.util.WooPosNetworkStatus
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker
import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice
Expand All @@ -22,6 +23,7 @@ import org.assertj.core.api.Assertions.assertThat
import org.junit.Rule
import org.mockito.kotlin.any
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
Expand All @@ -40,6 +42,10 @@ class WooPosTotalsViewModelTest {
@JvmField
val coroutinesTestRule = WooPosCoroutineTestRule()

private val networkStatus: WooPosNetworkStatus = mock()

private val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock()

private fun createMockSavedStateHandle(): SavedStateHandle {
return SavedStateHandle(
mapOf(
Expand Down Expand Up @@ -196,11 +202,9 @@ class WooPosTotalsViewModelTest {
on { events }.thenReturn(mock())
}
val savedState = createMockSavedStateHandle()
val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock()
val viewModel = createViewModel(
savedState = savedState,
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
childrenToParentEventSender = childrenToParentEventSender,
)

// WHEN
Expand Down Expand Up @@ -320,6 +324,7 @@ class WooPosTotalsViewModelTest {
@Test
fun `when CollectPaymentClicked is emitted, then should collect payment`() = runTest {
// GIVEN
whenever(networkStatus.isConnected()).thenReturn(true)
val productIds = listOf(1L, 2L, 3L)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(productIds))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
Expand Down Expand Up @@ -463,12 +468,12 @@ class WooPosTotalsViewModelTest {
@Test
fun `given payment status is success, when payment flow started, then OrderSuccessfullyPaid event and update state to PaymentSuccess`() = runTest {
// GIVEN
whenever(networkStatus.isConnected()).thenReturn(true)
val productIds = listOf(1L, 2L, 3L)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(productIds))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
on { events }.thenReturn(parentToChildrenEventFlow)
}
val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock()

val order = Order.getEmptyOrder(
dateCreated = Date(),
Expand Down Expand Up @@ -500,7 +505,6 @@ class WooPosTotalsViewModelTest {
val viewModel = createViewModel(
savedState = savedState,
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
childrenToParentEventSender = childrenToParentEventSender,
totalsRepository = totalsRepository,
priceFormat = priceFormat,
)
Expand All @@ -518,10 +522,111 @@ class WooPosTotalsViewModelTest {
verify(childrenToParentEventSender).sendToParent(ChildToParentEvent.OrderSuccessfullyPaid)
}

@org.junit.Test
fun `given there is no internet, when trying to complete payment, then trigger proper event`() = runTest {
// GIVEN
whenever(networkStatus.isConnected()).thenReturn(false)
val productIds = listOf(1L, 2L, 3L)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(productIds))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
on { events }.thenReturn(parentToChildrenEventFlow)
}
val order = Order.getEmptyOrder(
dateCreated = Date(),
dateModified = Date()
).copy(
totalTax = BigDecimal("2.00"),
items = listOf(
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
)
),
productsTotal = BigDecimal("3.00"),
total = BigDecimal("5.00"),
)
val totalsRepository: WooPosTotalsRepository = mock {
onBlocking { createOrderWithProducts(productIds = productIds) }.thenReturn(
Result.success(order)
)
}
val priceFormat: WooPosFormatPrice = mock {
onBlocking { invoke(BigDecimal("2.00")) }.thenReturn("2.00$")
onBlocking { invoke(BigDecimal("3.00")) }.thenReturn("3.00$")
onBlocking { invoke(BigDecimal("5.00")) }.thenReturn("5.00$")
}

// WHEN
val viewModel = createViewModel(
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
totalsRepository = totalsRepository,
priceFormat = priceFormat,
)
viewModel.onUIEvent(WooPosTotalsUIEvent.CollectPaymentClicked)

// THEN
verify(childrenToParentEventSender).sendToParent(ChildToParentEvent.NoInternet)
}

@org.junit.Test
fun `given there is no internet, when trying to complete payment, then collect payment method is not called`() = runTest {
// GIVEN
whenever(networkStatus.isConnected()).thenReturn(false)
val productIds = listOf(1L, 2L, 3L)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(productIds))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
on { events }.thenReturn(parentToChildrenEventFlow)
}
val order = Order.getEmptyOrder(
dateCreated = Date(),
dateModified = Date()
).copy(
totalTax = BigDecimal("2.00"),
items = listOf(
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
)
),
productsTotal = BigDecimal("3.00"),
total = BigDecimal("5.00"),
)
val totalsRepository: WooPosTotalsRepository = mock {
onBlocking { createOrderWithProducts(productIds = productIds) }.thenReturn(
Result.success(order)
)
}
val priceFormat: WooPosFormatPrice = mock {
onBlocking { invoke(BigDecimal("2.00")) }.thenReturn("2.00$")
onBlocking { invoke(BigDecimal("3.00")) }.thenReturn("3.00$")
onBlocking { invoke(BigDecimal("5.00")) }.thenReturn("5.00$")
}

// WHEN
val viewModel = createViewModel(
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
totalsRepository = totalsRepository,
priceFormat = priceFormat,
)
viewModel.onUIEvent(WooPosTotalsUIEvent.CollectPaymentClicked)

// THEN
verify(cardReaderFacade, never()).collectPayment(any())
}

private fun createViewModel(
resourceProvider: ResourceProvider = mock(),
parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock(),
childrenToParentEventSender: WooPosChildrenToParentEventSender = mock(),
totalsRepository: WooPosTotalsRepository = mock(),
priceFormat: WooPosFormatPrice = mock(),
savedState: SavedStateHandle = SavedStateHandle(),
Expand All @@ -533,6 +638,7 @@ class WooPosTotalsViewModelTest {
totalsRepository,
priceFormat,
analyticsTracker,
savedState,
networkStatus,
savedState
)
}
Loading