woocommerce-android icon indicating copy to clipboard operation
woocommerce-android copied to clipboard

[Beta Fix] Fix navigation for "Collect payment" tap in order creation flow

Open samiuelson opened this issue 1 year ago • 4 comments

[Beta Fix] Tapping on "Collect Payment" in the order creation navigates to the order detail instead of the payment selection.

Closes: #12432

Description

It turned out that navigating programmatically to the "Take payment" screen we should not employ the CallThrottler singleton because it can prevent navigation to SelectPaymentMethodFragment if the OrderDetailViewModel is instantiated before the CallThrottler.DELAY (200 ms) threshold.

Brainstorming: p1724752391339309-slack-C025A8VV728

Steps to reproduce

To reproduce the bug:

  1. On trunk manually set the CallThrottler.DELAY const to some bigger number, e.g. 1200 ms.
  2. Create new order and add some product
  3. Click "Collect payment" in the bottom sheet
  4. Observe that "Order detail" is shown all the time instead of the "Take payment" screen

https://github.com/user-attachments/assets/f2546935-122c-46d6-b86d-28142b6c4344

Testing information

It should be verified that during the order creation flow, every time "Collect payment" is tapped, the app redirects to the SelectPaymentMethodFragment.

The tests that have been performed

I verified that this branch fixes the bug by testing the repro scenario on Pixel 7 Pro, Android 14.

Images/gif

Fix: https://github.com/user-attachments/assets/f4e7b17c-322d-491b-8ec1-921ba0338505

  • [x] 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.

samiuelson avatar Aug 28 '24 17:08 samiuelson

1 Warning
:warning: This PR is assigned to the milestone 20.1 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.
1 Message
:book: This PR contains changes to RELEASE-NOTES.txt.
Note that these changes won't affect the final version of the release notes as this version is in code freeze.
Please, get in touch with a release manager if you want to update the final release notes.

Generated by :no_entry_sign: Danger

dangermattic avatar Aug 28 '24 17:08 dangermattic

📲 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
FlavorJalapeno
Build TypeDebug
Commit540bbd592665467ec8d643ac94cf0db0c236e7bb
Direct Downloadwoocommerce-wear-prototype-build-pr12436-540bbd5.apk

wpmobilebot avatar Aug 28 '24 17:08 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
FlavorJalapeno
Build TypeDebug
Commit540bbd592665467ec8d643ac94cf0db0c236e7bb
Direct Downloadwoocommerce-prototype-build-pr12436-540bbd5.apk

wpmobilebot avatar Aug 28 '24 18:08 wpmobilebot

Codecov Report

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

Project coverage is 40.67%. Comparing base (b769c63) to head (540bbd5). Report is 3 commits behind head on release/20.1.

Files with missing lines Patch % Lines
...om/woocommerce/android/ui/orders/OrderNavigator.kt 0.00% 1 Missing :warning:
Additional details and impacted files
@@               Coverage Diff               @@
##             release/20.1   #12436   +/-   ##
===============================================
  Coverage           40.67%   40.67%           
  Complexity           5602     5602           
===============================================
  Files                1212     1212           
  Lines               68086    68086           
  Branches             9355     9355           
===============================================
  Hits                27691    27691           
  Misses              37856    37856           
  Partials             2539     2539           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Aug 28 '24 18:08 codecov-commenter

This has just been released in 20.1 :shipit:

malinajirka avatar Aug 30 '24 09:08 malinajirka