Check null site for the mismatch account case

irfano commented 2 weeks ago

Closes: #12868

Description

This fixes a crash on the site picker screen when the sites data is null. I couldn't reproduce the crash. But when the sites.value is null for any reason, it makes sense to hide the primary button on the site picker screen. This case may occur rarely and temporarily when the screen is initializing.

Steps to reproduce

~~I couldn't reproduce.~~ @JorgeMucientes could reproduce it, see comments.

Testing information

No need to test since we can't reproduce.

The tests that have been performed

Tested normal mismatch account case.

[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:

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

dangermattic commented 2 weeks ago

	1 Warning
:warning:	This PR is assigned to the milestone 21.1. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by :no_entry_sign: Danger

wpmobilebot commented 2 weeks ago

📲 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	8219ffa0b7d5a58daf2d81bc9ed201d6f7a39561
	Direct Download	`woocommerce-wear-prototype-build-pr12889-8219ffa.apk`

wpmobilebot commented 2 weeks ago

📲 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	8219ffa0b7d5a58daf2d81bc9ed201d6f7a39561
	Direct Download	`woocommerce-prototype-build-pr12889-8219ffa.apk`

codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 40.21%. Comparing base (133b9d9) to head (8219ffa). Report is 5 commits behind head on trunk.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## trunk #12889 +/- ## ========================================= Coverage 40.21% 40.21% - Complexity 5788 5789 +1 ========================================= Files 1251 1251 Lines 71261 71261 Branches 9955 9955 ========================================= + Hits 28660 28661 +1 Misses 39966 39966 + Partials 2635 2634 -1 ```

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

JorgeMucientes commented 2 weeks ago

Hey @irfano I was able to reproduce the crash. And I can confirm the changes fix the crash. Although the changes won't hide the main CTA, we will end up showing the following primary CTA, which doesn't make sense.:

I think we should keep showing the Connect to the site button no matter what, since it is possible the user might want to connect this new WP.com account to the site address they entered. So, maybe we can just set the primary button visibility in the account mismatch function to true always. Wdyt?

To reproduce the issue, you need a WordPress.com account with 0 sites. Not even a blog. Which is tricky to get through the normal flow because WP.com will create 1 blog for you as soon as you create a new account. However, if you delete that blog (and wait a few hours until the deletion is completed) you'll en up with a WP.com account with 0 sites. Using that account o log into any Jetpack connected site will result in the account mismatch screen and the app crashing.

To create an empty account you can use this endpoint:

https://public-api.wordpress.com/rest/v1.1/users/new?

Body:

{
    "client_secret": "wc.oauth.app_secret in your local gradle.properties",
    "client_id": "wc.oauth.app_id in your local gradle.properties",
    "signup_flow_name": "mobile-ios",
    "flow": "signup",
    "scheme": "woocommerce",
    "password": "",
    "email": "@gmail.com",
    "username": "",
    "validate": false,
    "send_verification_email": true 
}

irfano commented 2 weeks ago

Good catch, @JorgeMucientes! This PR doesn't change the primary button on the mismatch account screen. Mismatch account screen is different (see AccountMismatchErrorFragment). I updated the PR description according to this.

If you tap the back button from the mismatch account screen, you'll see the site picker screen. This PR hides the primary button on that screen:

woocommerce / woocommerce-android