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

Merge login lib changes from Woo and implement new interface methods #10787

Merged
merged 9 commits into from
Nov 25, 2019

Conversation

anitaa1990
Copy link
Contributor

@anitaa1990 anitaa1990 commented Nov 13, 2019

This PR merges the woocommerce-android changes to the login library from this PR. There were several new interface methods added existing classes:

LoginListener

  • loginViaSiteCredentials
  • helpNoJetpackScreen
  • helpHandleDiscoveryError

I've implemented these new interface methods in LoginActivity but there is no logic needed since none of them are currently being used in WPAndroid.

Changes

  • The discovery process when validating a site address in LoginSiteAddressFragment is ported over to a new base fragment LoginBaseDiscoveryFragment so that it can be reused by multiple calling fragments.
  • Modified existing method gotWpcomEmail to include a boolean flag verifyEmail.

Full documentation around the changes to login library are documented in this PR, but the WPAndroid app should see no difference in the login experience.

Recommended Test Scenarios

  • Verify login flow is unchanged from the current login flow of WPAndroid
  • Verify the text in each of the login views is also unchanged
  • Login with email
  • Login by site url
  • Login w/ Magic Link
  • Login w/ Google

Note: This PR is not ready for merge. This PR must first be merged, and then a new branch/PR will be submitted with the fresh changes ready for final review.

4b8e83512d Added logic to redirect to discovery error if xmlrpc is blocked on site
2c3223bbe7 Reverted logic to check for Jetpack availability during discovery errors since it can return a false negative
846dc2ba99 Handle site not found discovery error which can occur even with Jetpack available
e1028df7f4 Modified logic to check for discovery errors first before checking for Jetpack
16c1bbe249 Modified error message for xmlrpc errors
732a0486e7 Modified logic to initiate discovery from LoginBaseDiscoveryFragment
fdfb9df9b7 Added logic to redirect to discovery error screen, even if Jetpack is available
0833d5ca77 Add flag to check if Jetpack is available if a discovery error takes place
aa7f8cc9d9 Added logic to port over changes from the LoginSiteAddressFragment to utilise LoginBaseDiscoveryFragment
e1f2ff2d37 Modified discovery error listener to handle the various error scenarios from the calling fragment
a1bc462454 Added option to verify email only if login is for self hosted credentials
c2f2bba7a8 Added new string labels for magic link sent label
b4b9dbcabb Merge branch 'feature/sign-in-with-self-hosted-credentials' into issue/1482-step2-magic-link-signin-changes
32d2c25fd9 Handle corner case in discovery process and redirect to email screen if site is wp.com
d96252d875 Fixed nitpicks in code/xml
9b227c817b Design changes for request magic link screen
a8712882c1 Hide keyboard before redirecting to the magic link/no jetpack screen
e4e8210da2 Added logic to initiate discovery process for xmlrpc endpoint
2c35450824 Added logic to pass the input site address to forgot password flow, if the xmlrpc url is empty
f9ff33ec87 Added separate method to redirect to username screen in LoginListener
f71a4065b0 Revert discovery initiation changes to the site fragment
1e3e9cf367 Added logic to fetch SiteModel based on the site url
71ec9bdaec Added logic to redirect to no Jetpack screen when site credentials are entered
ad42794306 Revert adding a new method to LoginListener, instead use an existing listener method
8f3649b4b5 Added logic to fetch jetpack user email from site credentials and redirect to magic link screen
aa0d39d792 Pass site xmlrpc url to email and jetpack required screen
360480c6ff Initiate discovery process even if jetpack is not installed/active/connected
39b0ee010f Added logic to redirect Woo users to email login or jetpack required screen
efd18a8cef Revert the connection info endpoint implementation and update error message when site is not found
96e0ff714f Added label to the top of the LoginUsernamePasswordFragment for Woo login only
082d1e1304 Display login with site credentials link button in login email fragment
27b07bf3e1 Merge commit 'f89f408ee83d29d59dfe5740b2ba26c51dcd00df' into issue/1446-login-lib-merge
2406304ffa Merge pull request #27 from wordpress-mobile/google_login_fix
18ddefd54b Merge pull request #10523 from wordpress-mobile/feature/move_api_client_builder_to_start_stop
a87f1ec203 Update Jitpack dependency urls to use the www subdomain
24bb535488 Implement PR comments
b63bb72af9 Remove api client cleanup from onDetach
bd3410e2ad Move GoogleApiClient.Builder to onStart/Stop methods
d70141070b Add commit message for jetpack validation fix
80b21579cf Use only the isJetpackConnected property for jetpack validation
c3f9b19157 Merge pull request #10321 from wordpress-mobile/login/fix-login-tracks
0b734cf390 Merge pull request #10270 from wordpress-mobile/issue/9720-login-subtree-update-squashed
eb90f72905 Merge pull request #1312 from woocommerce/merge-login-lib
bd6086802b Merge commit 'aec4a8317d3188866c02556c182cd8d09d9b0272' into merge-login-lib
378a45be3e Fix leak of anonymous OnCancelListener reference
71cfd27b21 Merge branch 'develop' into amanda/leak_canary
42cf2d56ec Merge pull request #1292 from woocommerce/release/2.3
67de44ca60 Merge branch 'release/2.3' into merge-release/2.2.1-release/2.3
227c48b323 Merge pull request #1277 from woocommerce/merge/WordPress-Login-Flow-Android/24
0123a10b6b Reverted changes to login build.gradle
ff16da7e70 Reverted back to SDK 28 (we can update to 29 once Android Q is out of beta)
d433fad0df Merge commit '9bb4617dddea713295ce4b53a60aa2a271f318a2' squashed
b0b4f17f8a Merge commit 'f6c4fbfadce780aaebc745154c29affe46978545' into issue/9720-login-subtree-update-squashed
66695843b0 Upgraded login library to SDK 29
ff4a69e142 Add leak canary and fix leaks

git-subtree-dir: libs/login
git-subtree-split: 4b8e83512da7628a11c4b075cf79ac6e3b9864ec
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 13, 2019

Warnings
⚠️ PR is not assigned to a milestone.
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.
Messages
📖

This PR contains changes in the subtree libs/login/. It is your responsibility to ensure these changes are merged back into wordpress-mobile/WordPress-Login-Flow-Android. Follow these handy steps!
WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history!

  1. cd WordPress-Android
  2. git checkout anitaa/woo-login-changes
  3. git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/WordPress-Android/10787
  4. Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/WordPress-Android/10787 and open a new PR.

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 13, 2019

You can test the changes on this Pull Request by downloading the APK here.

d8a4023afc Merge branch 'merge-wc-login-changes' of https://github.com/wordpress-mobile/WordPress-Login-Flow-Android into merge-wc-login-changes
de5b268726 Merge pull request #29 from wordpress-mobile/merge/WordPress-Android/10627
4d15fb0c8a Updates default FluxC hash to 1.5.1-beta-4
6806c9e538 Add signup flow name parameter to signup auth email request
7e1828705b Merge pull request #28 from wordpress-mobile/merge/WordPress-Android/develop-oguzkocer
761864335b Update the default FluxC hash to 1.5.1-beta-2
3a16385d8f Update Gradle to 5.4.1
9498aca6d2 Merge commit '4ac19a62bbfd7021e6890014122e189e1834f8c8' into issue/update-login-library-subtree
4aebf535c0 Bump Robolectric version to 4.3
4648de14f2 Upgrade Gradle to 5.4.1, gradle plugin to 3.5.1 and fix various errors

git-subtree-dir: libs/login
git-subtree-split: d8a4023afc4bf9d3d1036b9b3c011b8a6df78d5d
@@ -836,4 +836,28 @@ private void dismissSignupSheet() {
boolean hasJetpack) {
// Not used in WordPress app
}

@Override public void helpHandleDiscoveryError(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the annotations should be on separate lines

Copy link
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

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

I've tested all the scenarios and all seems to work the same way as it does in production. The code added just to WPAndroid seems simple and I have one minor comment. The stuff in the login library is much more complex but I assume that was already reviewed and tested with the Woo changes, right? The login library doesn't have Kotlin yet, right? I'd consider adding it and moving the new classes to Kotlin (especially LoginBaseDiscoveryFragment).

f58a4c88ab Merge pull request #30 from wordpress-mobile/merge-wc-login-changes

git-subtree-dir: libs/login
git-subtree-split: f58a4c88ab81449c1f002711c260be9d46628b97
@anitaa1990 anitaa1990 marked this pull request as ready for review November 25, 2019 04:19
Copy link
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

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

looks great, thanks 👍

@planarvoid planarvoid merged commit e34b9ef into develop Nov 25, 2019
@planarvoid planarvoid deleted the anitaa/woo-login-changes branch November 25, 2019 08:14
AmandaRiu added a commit that referenced this pull request May 23, 2020
cfc0675d21 Add title to the layout override for the site help dialog
63fc73efa0 Use wrap_content for image width so wider images will scale better
68b0c58437 Merge commit '8487e6f4cfee09193f48b803fcf1eb2252355022' into darkmode/login-theme-1
c826be2ccb Merge pull request #35 from wordpress-mobile/feature/login-style-changes-v2
1f6c6e4bca Gutenberg/integrate release 1.25.0 with dark mode (#11580)
1d668f54a5 Merge pull request #11537 from wordpress-mobile/fix/10930-email-error-dissapears-on-rotation
fd6665242f store the res id instead of the boolean so it supports multiple errors.
858decb8b1 utilized runnable that's posted when the UI has been drawn.
ab37113dbf removed clearing in text watcher.
3691e848b8 Fixed config change issues.
fd0c8c97e8 Merge pull request #34 from wordpress-mobile/merge/WordPress-Android/11492
c134376ee6 Merge commit 'e80a69322fe65ad994bee1854a2343c2089af323' into issue/fix-login-subtree
526919648d Ignore onDiscoverySucceeded events if LoginBaseDiscoveryFragment is detached
775f096826 Revert "Feature/material theme and Dark Theme support (#11469)" (#11486)
65d5c8f67b Feature/material theme and Dark Theme support (#11469)
e80a69322f Merge pull request #33 from wordpress-mobile/merge/WordPress-Android/11051
40824a4333 Merge commit '0c3930794ed0c77e4926d334674e85263ef2a651' into update_login_lib_with_signup_sheet_nav_bar_buttons_visibility_fix
fec863d2f8 Resolved merge conflict by removing SignupBottomSheetDialog.java file.
f2d8c102ad Merge branch 'develop' into issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet
7e46000166 Fix validation in input of Email
fccc72b9a4 Merge pull request #32 from wordpress-mobile/merge/WordPress-Android/11172
d634c91e9f Merge commit '371f14160a780fbd7797d71921859d0fee5764d5' into update-login-lib-with-password-toggle-fix
cd81dd1ce9 Add custom selector for password button
8f444b2d13 Merge branch 'issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet' of https://github.com/wordpress-mobile/WordPress-Android into issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet
b53dda144c Add null check for design_bottom_sheet layout
521d09f42c Update libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/widgets/WPBottomSheetDialogFragment.java
7178f5c123 Show full width navigation bar and restrict max width for large screens
8a768d3dd2 Partial fix for terms of service announcement bering read twice.
989fb1d5a2 Remove empty line after brace
2d5d733080 Extract parts of code from onCreateView to onViewCreated
a24d5289ab Eliminate setRetainInstance(true)
2e71a00135 Remove dialog.setOnDismissListener
cfff7a060d Avoid using a parameter in Fragment's constructor
8b09b2d836 Convert BottomSheetDialog to BottomSheetDialogFragment and update styles
a87d595f7a Merge pull request #1796 from woocommerce/release/merge-3.3
f18c390619 Fix signup bottom sheet navigation bar buttons not visible issue
4f4657fb64 Add requested changes
e407a7f304 Revert erroneously deleted code during merge
821d29b90b Revert erroneously deleted code during merge
188cc0ee64 Check if view is null before accessing property
1a4414eec7 Merge commit 'bd659986940153549bfc26a8f6a4104bf748fe3c' into hotfix/1778-npe
d2b8135e1e Added logic to fetch the correct SiteModel from the local db for the incoming url
d80e8e616d Remove whitespaces
b18703520d Issue/10930 email address error is preserved on rotation
3421ca9534 Merge branch 'develop' into test
a20471e8c1 Merge branch 'develop' of https://github.com/woocommerce/woocommerce-android into feature/refund-by-items-master
7757f052b7 Merge branch 'develop' into feature/edit-product-master
f869be0bfc Merge branch 'feature/refund-by-items-master' into 0nko/refund-items-list
ea896f6172 Merge pull request #10833 from wordpress-mobile/issue/10832-fix-npe-in-login-fragment
64678168e4 check if listener is null before handling discovery error
1de03a5522 check if listener is null before handling discovery error
a6e0946af0 Update gradle plugin to version 3.5.3
a23213aa25 Fixed wrong key variable
c89899f001 Updated Glide version to 4.10.0 in the login module
2dd431ccdd Updated Glide version in the login lib
e3211148cd Merge pull request #10787 from wordpress-mobile/anitaa/woo-login-changes
00f6bd2e62 Added flag that checks if login has started.
d5e8683cf6 Added period to login with site credentials
eee4c53fa7 Fixed bug when the forgot password url in username password screen was not valid
2810b14e28 Upgrade gradle plugin to version 3.5.2
daf7384622 Merge pull request #1552 from woocommerce/login-lib-changes
f75cbaaacd Merge commit '4e588147352f8fe88f1417a03a00114dd7d51640' into login-lib-changes
b2d28c652d Add signup flow name parameter to signup auth email request

git-subtree-dir: libs/login
git-subtree-split: cfc0675d2167b938369cf1896083453a593e9b8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants