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

[Woo POS] Crash when plugin either attempted to be installed or activated by a show manager #13203

Conversation

kidinov
Copy link
Contributor

@kidinov kidinov commented Dec 25, 2024

Closes: #12957

Description

It's been crashing when in the POS during the onboarding flow a shop manager was trying to install or activate WooPayments/Stripe

The crash caused by uiMessageResolver.showSnack(event.message) inside of CardReaderOnboardingFragment, as UiMessageResolver uses hardcoded snack_root id for CoordinatorLayout, which was called differently in WooPosCardReaderActivity

Steps to reproduce

  • Login with a user in a shop manager role
  • Disable Woo Payments
  • Go to the POS
  • Try to connect to a reader
  • Try to activate the plugin from the app
  • Notice the crash

The tests that have been performed

Above

Images/gif

12-25--12-02.mp4
  • 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.

…o snack_root as this is hardcoded value used by UIMessageResolver
@kidinov kidinov added the feature: point of sale POS project label Dec 25, 2024
@kidinov kidinov added this to the 21.4 milestone Dec 25, 2024
@kidinov kidinov requested a review from AnirudhBhat December 25, 2024 14:03
@kidinov kidinov added the type: crash The worst kind of bug. label Dec 25, 2024
@wpmobilebot
Copy link
Collaborator

📲 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
Commitaa27885
Direct Downloadwoocommerce-wear-prototype-build-pr13203-aa27885.apk

@wpmobilebot
Copy link
Collaborator

📲 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
Commitaa27885
Direct Downloadwoocommerce-prototype-build-pr13203-aa27885.apk

@AnirudhBhat AnirudhBhat self-assigned this Dec 27, 2024
Copy link
Contributor

@AnirudhBhat AnirudhBhat left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @kidinov. LGTM :shipit:

Nitpick: I noticed that the exit animation from the onboarding screen in POS doesn't match with the enter animation.

onboarding_back_animation.mp4

Another observation was that we connect to the card reader without dismissing the onboarding error state and then come back to the POS screen after the connection. I'm not sure if this is intended behaviour, I just wanted to bring this to your attention.

pending_requirement.mp4

@kidinov
Copy link
Contributor Author

kidinov commented Dec 27, 2024

@AnirudhBhat thanks for the review!

Nitpick: I noticed that the exit animation from the onboarding screen in POS doesn't match with the enter animation.

Good catch. I've made a ticket:
#13210

Another observation was that we connect to the card reader without dismissing the onboarding error state and then come back to the POS screen after the connection. I'm not sure if this is intended behaviour, I just wanted to bring this to your attention.

Yeah, that's known. There is something with navigation library which due to very low impact I decided not to spend time one

@kidinov kidinov merged commit 9a2b63d into trunk Dec 27, 2024
17 of 19 checks passed
@kidinov kidinov deleted the 12957-mobile-payments-crash-when-shop-managers-tries-to-installactivate-woopayments branch December 27, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: point of sale POS project type: crash The worst kind of bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mobile Payments] Crash when Shop Managers tries to install/activate WooPayments
3 participants