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

Update login lib hash that allows wpcom site address login for JetPack app #16025

Merged

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Mar 1, 2022

References: wordpress-mobile/WordPress-Login-Flow-Android#80

Now that Jetpack site filtering is removed as part of Jetpack App - Enable Features (#15946), this PR updates login lib hash for a fix that matches wpcom site address login in the Jetpack app with the WordPress app.

CC @thehenrybyrd

To test:

  1. Install Jetpack app.
  2. Click "Enter your exiting site address" on the Landing screen.
  3. Enter a wpcom simple site address (note that the Jetpack app already allowed atomic site address login earlier as it has Jetpack installed, so make sure you specifically test a wpcom simple site).
  4. Notice that the app directs to the email login screen.
  5. Follow the log in steps.
  6. Notice that the login is successful.

Merge Instructions

This issue was noticed during beta testing 19.3-rc-1 (p5T066-31e-p2#comment-11484), and so targets release/19.3 branch.

  • Make sure that linked login lib is merged to trunk. I can take care of the merging tomorrow.
  • Update login lib version inside build.gradle.
  • Remove Not Ready for Merge label and merge the PR.

Regression Notes

  1. Potential unintended areas of impact
    Minimal changes are included in the linked login lib PR that filters out wpcom sites from the connect site info handling for the Jetpack app. These sites are handled by the fallback method that redirects them to WordPress.com email login similar to the WordPress app.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Tested manually.

  3. What automated tests I added (or what prevented me from doing so)
    Login lib does not support unit testing for this flow.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@ashiagr
Copy link
Contributor Author

ashiagr commented Mar 1, 2022

👋 @AliSoftware, pinging you as this issue was noticed during 19.3 beta testing yesterday and the fix targets release/19.3 branch. I'll also inform in the Slack channel for more visibility.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 1, 2022

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

@ashiagr - I reviewed and merged the Login Library changes. As per instructions, I'll leave this merge for you. 👍

@zwarm zwarm self-assigned this Mar 1, 2022
@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@ashiagr ashiagr merged commit 89e261c into release/19.3 Mar 2, 2022
@ashiagr ashiagr deleted the issue/fix-wpcom-site-address-login-update-login-lib-hash branch March 2, 2022 04:49
@ashiagr
Copy link
Contributor Author

ashiagr commented Mar 2, 2022

👋 @AliSoftware

I updated login lib version to trunk commit. I just realized I should have created a tag and updated that instead. Can you please confirm?

@AliSoftware
Copy link
Contributor

Yes for code frozen branch we should indeed use a tag 🙇‍♂️
In this case bumping the patch version (3rd digit) is usually the way to go for a betafix that happened after the freeze.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants