wordpress-mobile / WordPress-Login-Flow-Android

Pluggable WordPress login flow for Android
GNU General Public License v2.0
14 stars 3 forks source link

Move `addMenuProvider` from `onCreate` to `onViewCreated` #142

Closed ThomazFB closed 2 months ago

ThomazFB commented 2 months ago

Summary

Fix issue https://github.com/wordpress-mobile/WordPress-Android/issues/20221 by adjusting the point where the menu provider added to the SignupConfirmationFragment view.

How to Test

  1. Import this PR to the WordPress or Woo app by using the 142-48e7fc1f82f011c246e23ac4b4a0c9195b3d7c21 version hash.
  2. Start the signup flow using an email account not registered on WordPress.com.
  3. Verify that during the Signup process, when landing in the view with the We’ll use this email address to create your new WordPress.com account (the SignupConfirmationFragment), the view loads as expected.

Update release notes:

thomashorta commented 2 months ago

Just a side note regarding the WordPress and Jetpack apps flow: it only opens the SignupConfirmationFragment if the user tries to do a Social Login (Login with Google) and the email is not yet registered on WordPress.com so the testing steps are slightly different for those apps:

  1. Import this PR to the WordPress app by using the 142-48e7fc1f82f011c246e23ac4b4a0c9195b3d7c21 version hash.
  2. Start the login/signup flow by tapping "Continue with WordPress.com".
  3. Use social login/signup by tapping "Continue with Google".
  4. Select an email that is not associated with a WordPress.com account.
  5. Verify that during the Signup process, when landing in the view with the "We’ll use this email address to create your new WordPress.com account" (the SignupConfirmationFragment), the view loads as expected.
zwarm commented 2 months ago

Just a side note regarding the WordPress and Jetpack apps flow: it only opens the SignupConfirmationFragment if the user tries to do a Social Login (Login with Google) and the email is not yet registered on WordPress.com so the testing steps are slightly different for those apps:

  1. Import this PR to the WordPress app by using the 142-48e7fc1f82f011c246e23ac4b4a0c9195b3d7c21 version hash.
  2. Start the login/signup flow by tapping "Continue with WordPress.com".
  3. Use social login/signup by tapping "Continue with Google".
  4. Select an email that is not associated with a WordPress.com account.
  5. Verify that during the Signup process, when landing in the view with the "We’ll use this email address to create your new WordPress.com account" (the SignupConfirmationFragment), the view loads as expected.

I didn't think you can use Google Sign in with a debug build? @thomashorta how did you test this for Jetpack?

thomashorta commented 2 months ago

I didn't think you can use Google Sign in with a debug build? @thomashorta how did you test this for Jetpack?

Yeah, that's correct, I was just trying to test the steps that I wrote but I was only able to do them to confirm the crash exists using the release beta build. I wasn't able to do them to confirm the crash was fixed. 😞

I think that, for testing purposes, I will just modify the gotUnregisteredEmail (here) to call gotUnregisteredSocialAccount, so it will open the SignupConfirmationFragment when trying to use a new email.

I didn't test it yet though.

zwarm commented 2 months ago

I think that, for testing purposes, I will just modify the gotUnregisteredEmail to call gotUnregisteredSocialAccount, so it will open the SignupConfirmationFragment when trying to use a new email.

I didn't test it yet though.

I started testing using the original steps, then saw the new ones. We can create a local release build for WP, but not JP (because it uses Google signing). I'll give that a go next

ThomazFB commented 2 months ago

Thank you, @thomashorta, @zwarm, and @atorresveiga, for the reviews! As we have the WP and Woo scenarios covered with the tests, we can move forward by merging this PR. I'll proceed with the new version release.