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

Unified Login: Site Address Flow #2773

Merged
merged 11 commits into from
Aug 25, 2020

Conversation

AmandaRiu
Copy link
Contributor

This PR is part of an epic to implement the unified login flow. This particular flow implements the site address login flow and includes the following scenarios:

  • Clicking "Enter store address" opens the site address screen
  • Login with email and using magic link to finish the login process
  • Login with email and password to finish the login process
  • Login with store credentials - match to WPcom account and use magic link to verify email and complete the login process
  • Login with store credentials - login wtih WPcom email and password to complete the login process

Not included:

  • Since the login with site address doesn't use the store selector, that screen has not been updated to the new styles yet.

Screenshots

Prologue site address screen email screen
Screenshot_1598306824 Screenshot_1598311156 Screenshot_1598313299
magic link request magic link sent password
Screenshot_1598311173 Screenshot_1598311179 Screenshot_1598311197
store creds verify with magic link magic link sent
Screenshot_1598311797 Screenshot_1598311828 Screenshot_1598311179

To Test

  • Verify entering site address and email, then logging in with magic link
  • Verify entering site address, then entering store credentials and using magic link
  • Verify entering site address, then entering store credentials and using password

Update release notes:

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

Only the "Continue with WordPress" button works at this point as none of the new logic has been added.
The "Enter your store address" button works and will now route to the site address fragment. The "Continue with WordPress.com" button does not do anything yet.
- Remove base login style overrides to reset UI
- Deleted old login_toolbar and login_menu overrides
This screen currently only used by Woo as it's a part of the flow where we attempt to connect to an email address already connected to a WordPress.com account based on the site address entered.
Hide the magic link screen while processing and only show the UI if there is an error.
@AmandaRiu AmandaRiu added this to the 5.0 milestone Aug 24, 2020
@AmandaRiu AmandaRiu requested a review from a team August 24, 2020 23:59
@peril-woocommerce
Copy link

peril-woocommerce bot commented Aug 24, 2020

Fails
🚫

Danger failed to run /app/danger-0.3jko55ajr0z.ts.

🚫

Danger failed to run /app/danger-0.gxu6r3vqiga.ts.

Warnings
⚠️ PR is not assigned to a milestone.
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 woocommerce-android
  2. git checkout issue/2656-wire-main-screens
  3. git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/2773
  4. Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/woocommerce-android/2773 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.3jko55ajr0z.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.gxu6r3vqiga.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

@AmandaRiu AmandaRiu linked an issue Aug 25, 2020 that may be closed by this pull request
@0nko 0nko self-assigned this Aug 25, 2020
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

After typing my site address and clicking on Continue, when I tap on the email EditText, I'm getting a crash:

kotlin.NotImplementedError: An operation is not implemented: Not yet implemented
        at com.woocommerce.android.ui.login.LoginAnalyticsTracker.trackSelectEmailField(LoginAnalyticsTracker.kt:263)
        at org.wordpress.android.login.LoginEmailFragment$2.onFocusChange(LoginEmailFragment.java:217)
        at android.view.View.onFocusChanged(View.java:8116)

Entering store credentials with password: When submitting my 2FA, I got this crash:

    kotlin.NotImplementedError: An operation is not implemented: Not yet implemented
        at com.woocommerce.android.ui.login.LoginAnalyticsTracker.trackSubmit2faCodeClicked(LoginAnalyticsTracker.kt:239)
        at org.wordpress.android.login.Login2FaFragment.next(Login2FaFragment.java:291)
        at org.wordpress.android.login.Login2FaFragment.onEditorCommit(Login2FaFragment.java:365)
        at org.wordpress.android.login.widgets.WPLoginInputRow$1.onEditorAction(WPLoginInputRow.java:152)

These are tracks events that will be populated in a later commit.
@AmandaRiu
Copy link
Contributor Author

@0nko Thanks for the review. The tracks stuff bit me! LOL! I've been removing the TODO() stuff as I hit them so I would know which ones we are definitely using, but apparently missed a part of the flow. Should be fixed now.

@AmandaRiu AmandaRiu requested a review from 0nko August 25, 2020 20:00
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

Works as expected 👍

:shipit:

@0nko 0nko merged commit 6196f4e into feature/unified-login Aug 25, 2020
@0nko 0nko deleted the issue/2656-wire-main-screens branch August 25, 2020 21:55
@AmandaRiu AmandaRiu mentioned this pull request Sep 15, 2020
1 task
@designsimply 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: login Related to any part of the log in or sign in flow, or authentication.
Projects
No open projects
Unified Login
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

Unified Login: Login with site address flow
3 participants