Issue/1482 step3 handle discovery error changes by anitaa1990 · Pull Request #1515 · woocommerce/woocommerce-android

anitaa1990 · 2019-10-30T15:02:06Z

Partially fixes #1482. This PR completes the third step of the Jetpack sign in with self credentials as defined in the master thread here.

Changes:

~~If a discovery error takes place for a site,~~
Further testing revealed that some errors such as XMLRPC_FORBIDDEN, XMLRPC_BLOCKED and MISSING_XMLRPC_METHOD errors can occur even with Jetpack available on the site. Furthermore, the CONNECT_SITE_INFO API returns that Jetpack does not exist, even when Jetpack is available and connected on the site. In order to address this issue, the logic for discovery errors has being modified as follows:
- For HTTP Auth error: HTTP Auth is not supported by Jetpack so we can safely assume that this error would only occur if Jetpack is not connected on a site. So we redirect the user to the Jetpack required screen. There is a possibility that user installs Jetpack while on this screen and clicks on Try again. In order to avoid confusion and displaying No jetpack available when it is, we are using the CONNECT_SITE_INFO API to verify if Jetpack is installed/active/connected.
  - If Jetpack is connected, redirect them back to the site credentials screen.
  - If Jetpack is still not connected, display an error snackbar asking them to install Jetpack.
- All other errors that can occur: Regardless of whether Jetpack is available or note, redirect them to the discovery error screen.
  - Added a new fragment called LoginDiscoveryErrorFragment that is displayed when a discovery error takes place for a site that has jetpack available.
  - Clicking on Sign in with WordPress.com redirects to the email screen.
  - Clicking on Read our troubleshooting tips redirects to this link inside a webview.
  - Clicking on Try again redirects to the site credentials screen.
Ported over discovery initiation from the LoginSiteAddressFragment to the LoginBaseDiscoveryFragment.

Notes:

The are TODO's added to the PR to add events to the button clicks. This will take place in another PR.
Note that this PR is to be merged to a feature branch.

Screenshots

Logging into a site with HTTP AUTH required when Jetpack is NOT available:

(LEFT: When jetpack is still not installed and user clicks on Try AGAIN button)
(RIGHT: When jetpack is installed when user is on the screen and clicks on TRY AGAIN button)
.

Logging into a site with XMLRPC missing when Jetpack is available:

Logging into a site with missing rsd tags:

Logging into a site with SSL Certificate error :

Logging into a site with the XML-RPC request is blocked :

Different discovery error messages:

(LEFT: HTTP AUTH required . RIGHT: SSL certificate needed)
.

(LEFT: XMLRPC errors . RIGHT: All other generic errors)
.

Testing:

Scenario I: Login to a site where the XMLRPC file is renamed.

(This scenario is applicable to the following discovery errors: MISSING_XMLRPC_METHOD, XMLRPC_BLOCKED, XMLRPC_FORBIDDEN, GENERIC_ERROR, NO_SITE_FOUND)

Login to anitaamurthy.com in wp-admin.
Install, activate and connect Jetpack.
Open the app and enter the above site address and click on Next.
Verify that you are redirected to the Sign in with Jetpack screen (even though Jetpack is installed on the site). This is because the XMLRPC file is renamed for this site and cannot be found so even if Jetpack is available, the API response returns false for the fields hasJetpack: true, isJetpackActive: false, isJetpackConnected: false.
Click on Sign in in the app.
Click on Login with site credentials and enter username + password.
Verify that you are redirected to the discovery error screen and the error message displayed is We were unable to access the XMLRPC file on your site. You will need to reach out to your host to resolve this..
Verify that clicking on Try again redirects to the Username + password screen.
Verify that clicking on Read our troubleshooting tips redirects to a webview from this link.
Verify that clicking on Help opens the help page.
Verify that clicking in Sign in with Wordpress.com redirects to the Email screen and you are able to login successfully to the app using wp.com email.

Scenario II: Login to a site with SSL Certificate required error:

(This scenario is the same as the above scenario but with different discovery error type: ERRONEOUS_SSL_CERTIFICATE)

Login to anitaastestwpsite2.com in wp-admin.
Install, activate and connect Jetpack.
Open the app and enter the above site address and click on Next.
Click on Login with site credentials and enter username + password.
Verify that you are redirected to the discovery error screen and the error message displayed is We were unable to access your site because of a problem with the SSL Certificate. You will need to reach out to your host to resolve this..
Verify that clicking on Try again redirects to the Username + password screen.
Verify that clicking on Read our troubleshooting tips redirects to a webview from this link.
Verify that clicking on Help opens the help page.
Verify that clicking in Sign in with Wordpress.com redirects to the Email screen and you are able to login successfully to the app using wp.com email.

Scenario III: Login to a site where HTTP AUTH plugin is installed and activated.

Login to anitaastestwpsite.com in wp-admin.
Uninstall Jetpack from the Plugins section.
Activate HTTP Auth plugin from the same page. This enables HTTP Auth to the site.
Open the app and enter the above site address and click on Next.
Verify that you are redirected to the Sign in with Jetpack screen (since Jetpack is not active on the site).
Click on Sign in in the app.
Click on Login with site credentials and enter username + password.
Verify you are redirected to the Jetpack required screen.
Click on Try again.
Verify that an error snackbar is displayed stating that Jetpack is not active on the site.
Go back to the Plugins page in wp-admin and deactivate HTTP Auth before activating and connecting Jetpack (This is because Jetpack does not support HTTP Auth plugin and you will only be able to connect Jetpack if HTTP Auth plugin is deactivated).
Go back to the app and click on Try again.
Verify that you are redirected back to the site credentials screen.
Click on Next and verify that you are successfully redirected to the Magic link screen.

Scenario IV: Login to a site where the XMLRPC request is blocked.

Login to any test site in wp-admin.
Install the plugin Disable XML-RPC.
Open the app and enter the above site address and click on Next.
If Jetpack is not active on the site, verify that:
- You are redirected to the Sign in with Jetpack screen.
- Click on Sign in in the app.
- Click on Login with site credentials and enter username + password.
If Jetpack is active on the site, verify that:
- You are redirected to the username password screen.
- Click on Login with site credentials and enter username + password.
In either case, verify that you are redirected to the discovery error screen and the error message displayed is We were unable to access the XMLRPC file on your site. You will need to reach out to your host to resolve this..

Update release notes:

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@AmandaRiu, I assigned the PR to you since you have reviewed the last two 😄 Could you trouble you for one more review please? Thank you!!

…os from the calling fragment

… utilise LoginBaseDiscoveryFragment

…place

…very process again

…e/1482-step3-discovery-error-changes

… available

peril-woocommerce · 2019-10-30T15:02:15Z

	Fails
🚫	Danger failed to run `/app/danger-0.hop2hk014d8.ts`.
🚫	Danger failed to run `/app/danger-0.cmns4qu35dd.ts`.

	Warnings
⚠️	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! `cd woocommerce-android` `git checkout issue/1482-step3-discovery-error-changes` `git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/1515` Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/woocommerce-android/1515 and open a new PR.

Error TypeError

Cannot read property 'added' of null
TypeError: Cannot read property 'added' of null
    at Object.exports.default (/app/danger-0.hop2hk014d8.ts:16:44)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Dangerfile

11|         warn("The PlayStoreStrings.po file must be updated any time changes are made to release notes");
12|     }
13| 
14|     // If changes were made to the strings, make sure they follow our guidelines
15|     const modifiedStrings = modifiedFiles.filter((path: string) => path.endsWith('values/strings.xml'))
----------------------------------------------^
16| 
17|     for (let file of modifiedStrings) {        
18|         const stringDiffs = await danger.git.diffForFile(file)
19|

Error TypeError

Cannot read property 'diff' of null
TypeError: Cannot read property 'diff' of null
    at checkCommitDiffs (/app/danger-0.cmns4qu35dd.ts:43:49)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Dangerfile

38|         if (git === undefined) {
39|             console.log("About to crash due to an error")
40|             console.log("File:", thisFile)
41|             console.log("Danger Object: ", danger)
42| 
---------------------------------------------------^
43|             if (danger !== undefined) {
44|                 console.log("Danger is no longer defined")
45|             }
46|             else {

Generated by 🚫 dangerJS

peril-woocommerce · 2019-10-30T15:05:42Z

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

AmandaRiu

@anitaa1990 finished the first review round and left a code comment. I tried testing through the scenarios using our test sites page but couldn't get to the FragmentLoginDiscoveryError view. I tried to sign in with a WP test site that had HTTP AUTH required, but couldn't get past the first screen because CONNECTED-SITE-INFO was flagging it as a site that was not a WordPress site. Logging in with a site that required an SSL cert didn't give me this screen either - it would just drop me in the "Jetpack Required" screen.

Do you have some tests sites I can use?

WooCommerce/src/main/res/layout/fragment_login_discovery_error.xml

…r Jetpack

anitaa1990 · 2019-10-31T08:24:24Z

Thanks for the quick review @AmandaRiu.

I've invited you to three of my test sites. The invites are sent to your official email account. Do let me know if you face any issues. I've also updated the PR description with the testing steps and explanation.

PR ready for another round 👍

…ck available

…ors since it can return a false negative

AmandaRiu · 2019-11-01T18:37:43Z

@anitaa1990 Thank you for the test accounts and testing steps! All of them worked great except for this one: Scenario II: Login to a site with missing rsd tags. With that scenario I never get the "Jetpack required" view. It allows me to get all the way to the "Not a woocommerce site" view.

anitaa1990 · 2019-11-03T15:07:56Z

Hey @AmandaRiu, thanks for testing!

All of them worked great except for this one: Scenario II. With that scenario I never get the "Jetpack required" view. It allows me to get all the way to the "Not a woocommerce site" view.

Ah, yes! I was testing SSL certification with that test site and had to revoke the changes I made that caused this error. I forgot yo update you 🤦‍♀️ I’ve set an invalid certificate to the site now so you should be able to see the below behaviour when you try it now. The steps to reproduce this are updated in the PR as well under Scenario II: Login to a site with SSL Certificate required error.

I also faced another error when trying to login with a site that has the Disable XMLRPC plugin. This plugin blocks the XMLRPC calls but interestingly, the discovery API returns a successful response. But when trying to access the site details using wp.getUserBlogs API endpoint, results in an error. I’ve modified this behaviour in the app as well in d58f81f and updated the test scenario to include this test scenario as well. To reproduce this error:

Login to any test site in wp-admin.
Install the plugin Disable XML-RPC.
Open the app and enter the above site address and click on Next.
If Jetpack is not active on the site, verify that:
- You are redirected to the Sign in with Jetpack screen.
- Click on Sign in in the app.
- Click on Login with site credentials and enter username + password.
If Jetpack is active on the site, verify that:
- You are redirected to the username password screen.
- Click on Login with site credentials and enter username + password.
In either case, verify that you are redirected to the discovery error screen and the error message displayed is We were unable to access the XMLRPC file on your site. You will need to reach out to your host to resolve this..

Let me know if you face any other issues! PR ready for another round 👍

AmandaRiu

Brilliant work @anitaa1990! I've run through the new test scenarios and they were all successful! 🎆 Approving this PR

anitaa1990 added 10 commits October 29, 2019 17:59

Modified discovery error listener to handle the various error scenari…

da6ab92

…os from the calling fragment

Added logic to port over changes from the LoginSiteAddressFragment to…

1d56658

… utilise LoginBaseDiscoveryFragment

Add flag to check if Jetpack is available if a discovery error takes …

91014f3

…place

Added logic to verify if jetpack is installed before initiating disco…

b7c51dc

…very process again

Merge branch 'feature/sign-in-with-self-hosted-credentials' into issu…

3933948

…e/1482-step3-discovery-error-changes

Added xml + fragment for discovery error screen

00bf42a

Added logic to redirect to discovery error screen, even if Jetpack is…

4ed344b

… available

Added click handlers for discovery error screen buttons

0ceb413

Modified logic to initiate discovery from LoginBaseDiscoveryFragment

52369aa

Modified error message for xmlrpc errors

df80a06

anitaa1990 added type: enhancement A request for an enhancement. Login labels Oct 30, 2019

anitaa1990 added this to the 3.0 milestone Oct 30, 2019

anitaa1990 requested a review from AmandaRiu October 30, 2019 15:02

anitaa1990 mentioned this pull request Oct 30, 2019

Jetpack sign-in with self-hosted credentials - Master checklist #1482

Closed

11 tasks

anitaa1990 assigned AmandaRiu Oct 30, 2019

AmandaRiu reviewed Oct 30, 2019

View reviewed changes

WooCommerce/src/main/res/layout/fragment_login_discovery_error.xml Outdated Show resolved Hide resolved

anitaa1990 added 2 commits October 31, 2019 12:28

Modified logic to check for discovery errors first before checking fo…

a4d804d

…r Jetpack

Fixed alignment issues in discovery error fragment

d75b227

anitaa1990 added Login and removed Login labels Oct 31, 2019

anitaa1990 added 2 commits October 31, 2019 19:01

Handle site not found discovery error which can occur even with Jetpa…

03dd045

…ck available

Reverted logic to check for Jetpack availability during discovery err…

59b2548

…ors since it can return a false negative

anitaa1990 mentioned this pull request Nov 1, 2019

Issue/1482 step4 track events #1518

Merged

3 tasks

Added logic to redirect to discovery error if xmlrpc is blocked on site

d58f81f

AmandaRiu self-requested a review November 5, 2019 01:42

AmandaRiu approved these changes Nov 5, 2019

View reviewed changes

AmandaRiu merged commit 8dc0d06 into feature/sign-in-with-self-hosted-credentials Nov 5, 2019

AmandaRiu deleted the issue/1482-step3-discovery-error-changes branch November 5, 2019 20:08

designsimply added feature: login Related to any part of the log in or sign in flow, or authentication. and removed Login labels May 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/1482 step3 handle discovery error changes#1515

Issue/1482 step3 handle discovery error changes#1515
AmandaRiu merged 15 commits intofeature/sign-in-with-self-hosted-credentialsfrom
issue/1482-step3-discovery-error-changes

anitaa1990 commented Oct 30, 2019 •

edited

Loading

Uh oh!

peril-woocommerce bot commented Oct 30, 2019 •

edited

Loading

Uh oh!

peril-woocommerce bot commented Oct 30, 2019 •

edited

Loading

Uh oh!

AmandaRiu left a comment

Uh oh!

Uh oh!

anitaa1990 commented Oct 31, 2019 •

edited

Loading

Uh oh!

AmandaRiu commented Nov 1, 2019

Uh oh!

anitaa1990 commented Nov 3, 2019

Uh oh!

AmandaRiu left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

anitaa1990 commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Notes:

Screenshots

Logging into a site with HTTP AUTH required when Jetpack is NOT available:

Logging into a site with XMLRPC missing when Jetpack is available:

Logging into a site with missing rsd tags:

Logging into a site with SSL Certificate error :

Logging into a site with the XML-RPC request is blocked :

Different discovery error messages:

Testing:

Scenario I: Login to a site where the XMLRPC file is renamed.

Scenario II: Login to a site with SSL Certificate required error:

Scenario III: Login to a site where HTTP AUTH plugin is installed and activated.

Scenario IV: Login to a site where the XMLRPC request is blocked.

Uh oh!

peril-woocommerce bot commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Error TypeError

Dangerfile

Error TypeError

Dangerfile

Uh oh!

peril-woocommerce bot commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmandaRiu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anitaa1990 commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmandaRiu commented Nov 1, 2019

Uh oh!

anitaa1990 commented Nov 3, 2019

Uh oh!

AmandaRiu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anitaa1990 commented Oct 30, 2019 •

edited

Loading

peril-woocommerce bot commented Oct 30, 2019 •

edited

Loading

peril-woocommerce bot commented Oct 30, 2019 •

edited

Loading

anitaa1990 commented Oct 31, 2019 •

edited

Loading