[Woo POS] Improve Card Reader Payment Flow Error Handling

[backwardstruck]

Closes: #12542

Branched off of this PR from @AnirudhBhat (thanks for the help):
#12569

Description

Addresses an issue with error handling in the card reader payment flow. Ensures that network connectivity is checked before attempting to fetch an order.

Steps to reproduce

0. Add products and connect reader in POS mode
1. Ensure the device is in airplane mode.
2. Attempt to collect payment.
3. Observe the error message indicating network unavailability.
4. Disable airplane mode, ensuring the device has an active internet connection.
5. Attempt to collect payment again.
6. Observe that the payment flow now proceeds as expected.

Testing information

Devices used:

• Tablet emualtor

Tested scenarios:

• Network unavailable (airplane mode).
• Normal card reader payment flow with network available and order fetch succeeds.

The tests that have been performed

• Verified normal payment flow with successful order fetch and payment collection.

• Manual smoke tests on both tablet and phone devices to ensure no regressions were introduced.

• I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

• The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
• Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
• Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

[backwardstruck]

@AnirudhBhat this is still in draft because I want to make sure I have the right approach. What do you think about me checking for connection before attempting to fetch the order?

Should I instead watch the connection state throughout the payment process or try to return an error from the order detail repository?

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	ab4d89a
	Direct Download	woocommerce-wear-prototype-build-pr12543-ab4d89a.apk

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	ab4d89a
	Direct Download	woocommerce-prototype-build-pr12543-ab4d89a.apk

[AnirudhBhat]

Thanks for working on this improvement @backwardstruck!

this is still in draft because I want to make sure I have the right approach. What do you think about me checking for connection before attempting to fetch the order?
Should I instead watch the connection state throughout the payment process or try to return an error from the order detail repository?

The approach sounds fine. However, as I understand it, one of the key principles we’ve tried to maintain since the inception of POS is to minimise changes to the existing IPP store management code in order to avoid potential regressions. With that in mind, we could consider the following alternative:

In WooPosTotalsViewModel, we can check the internet connection before initiating the payment process, like this:

private fun collectPayment() {
    if (!wooPosNetworkStatus.isConnected()) {
        viewModelScope.launch {
            childrenToParentEventSender.sendToParent(ChildToParentEvent.NoInternet) // This event is being handled in the WooPosHomeViewModel - PR 12569
        }
    } else {
        val orderId = dataState.value.orderId
        check(orderId != EMPTY_ORDER_ID)
        cardReaderFacade.collectPayment(orderId)
    }
}

I implemented a similar solution while resolving #12497, and you can refer to the relevant code here: #12569. You can merge this changes into your branch and start modifying if you agree with this approach.

This approach allows us to resolve the issue without modifying the existing IPP-related code.

That said, I agree that in the current IPP store management solution, when there is no internet and we try to collect a payment, the "error fetching order" dialog is shown instead of a "No internet" message. However, in store management mode, a persistent offline banner is displayed at the bottom whenever we lose connectivity, giving users an indication of the issue. While I agree that the UI could be improved here, I think that can be addressed in a separate PR.

Let me know your thoughts on this approach!

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 40.61%. Comparing base (1c2cc6c) to head (ab4d89a).
Report is 9 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...oid/ui/woopos/home/totals/WooPosTotalsViewModel.kt	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##              trunk   #12543   +/-   ##
=========================================
  Coverage     40.61%   40.61%           
- Complexity     5680     5681    +1     
=========================================
  Files          1229     1229           
  Lines         69123    69128    +5     
  Branches       9573     9573           
=========================================
+ Hits          28074    28079    +5     
  Misses        38466    38466           
  Partials       2583     2583           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AnirudhBhat]

Thanks for working on this @backwardstruck. LGTM! I've added a minor comment regarding the inclusion of a unit test. I'm holding off on merging this since it's based on another PR; we'll proceed once that has been merged.

[AnirudhBhat]

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

[backwardstruck]

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

[backwardstruck]

Added.