Block sign ups to WordPress.com accounts. #3286

ctarda · 2020-12-04T05:14:07Z

⚠️ Please note this PR is not against develop, but against the feature branch for UL&S 🙇

	Light	Dark
Error screen
Error screen + alert

And a gif showing the interactions:

I am not sure if the actual link that can be opened in a web view from the alert is correct. Changing that takes 30 seconds.

Changes

Pointed WPAuthenticator to the branch that provides support for short-circuiting the navigation flow provided by WPAuthenticator, and allows us to block the account creation flow.
A new implementation of the ULErrorViewModel that provides support for this new error screen.
A couple of minor touches to the constraints in the actual UI, so that the button that is styled as a link can stretch horizontally to occupy all the available space.

Testing

Checkout the branch. Run bundle exec pod install
Log out if necessary
Select Continue With WordPress.com
Enter an email address that does not match a WP.com account.
You should see the custom error UI instead of the flow that allows creating a new WP.com account

Update release notes:

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

… flow

…ail address does not match a wp.com account

…all available horzontal space

Garance91540

👏👏👏 The only thing I mentioned on slack is that the link "need help finding your email" should open the Help center

peril-woocommerce · 2020-12-07T02:05:39Z

You can trigger an installable build for these changes by visiting CircleCI here.

peril-woocommerce · 2020-12-07T02:05:39Z

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

…the alert

ctarda · 2020-12-07T02:20:27Z

Thanks for the review @Garance91540

I just pushed an update to:

Point WPAuthenticator to the latest beta
Present the Help when tapping "need more help" in the alert:

I guess this is ready for a close look at the code 😊

…ld trigger a new build on CI

jaclync

Looks good!

Code: ✅
Testing: ✅ (all CTAs work as expected on the error screen!)

I noticed a non-blocking UI issue in landscape:

jaclync · 2020-12-07T10:28:05Z

WooCommerce/Classes/Authentication/AuthenticationManager.swift


    /// Maps error codes emitted by WPAuthenticator to a domain error object
    enum AuthenticationError: Int, Error {
+        case emailDoesNotMatchWPAcount = 7


nit:

Suggested change

case emailDoesNotMatchWPAcount = 7

case emailDoesNotMatchWPAccount = 7

jaclync · 2020-12-07T10:29:00Z

WooCommerce/Classes/Authentication/AuthenticationManager.swift

+
+    func isSupportedError(_ error: Error) -> Bool {
+        let wooAuthError = AuthenticationError.make(with: error)
+        return wooAuthError == .emailDoesNotMatchWPAcount || wooAuthError == .notWPSite


nit: how about just checking if the error is not unknown, since we handle all the known cases?

Suggested change

return wooAuthError == .emailDoesNotMatchWPAcount || wooAuthError == .notWPSite

return wooAuthError != .unknown

I went with the long form because it seemed more explicit to me, but will update, thanks!

jaclync · 2020-12-07T10:35:02Z

WooCommerce/Classes/Authentication/Navigation Exceptions/NotWPAccountViewModel.swift

+    // MARK: - Data and configuration
+    let image: UIImage = .loginNoWordPressError
+
+    let text: NSAttributedString = NSMutableAttributedString(string: Localization.errorMessage)


nit: any reasons for using the mutable version?

Suggested change

let text: NSAttributedString = NSMutableAttributedString(string: Localization.errorMessage)

let text: NSAttributedString = .init(string: Localization.errorMessage)

Nope, good point, thanks!

ctarda · 2020-12-07T23:11:42Z

Thanks for the review @jaclync 🙇

I pushed an update to incorporate your feedback.

This is blocking progress a little bit, so I will defer the layout bug on landscape to a separate issue that I will attack later this week: #3300

ctarda added 10 commits December 4, 2020 10:53

Point WPAuthenticator to branch that supports intercepting the signup…

a2117ee

… flow

Extract view model instantation to a private method

492eb43

First iteration of viewmodel specific for error presented when the em…

0f9e69b

…ail address does not match a wp.com account

Make primary button navigate to enter site

f07918b

Bail out when there in an unkown error

b6b2ef7

Add fancy alert for a mismatched email address

b6ec3ce

Present alert

00d24e5

fix a typo and silence two line lenght violation warnings

4735014

Update dependency

d2e9187

Add custom insets to extra info button to ensure it expands to cover …

b57d110

…all available horzontal space

ctarda added the feature: login Related to any part of the log in or sign in flow, or authentication. label Dec 4, 2020

ctarda added this to the 5.7 milestone Dec 4, 2020

ctarda requested review from a team December 4, 2020 05:14

ctarda self-assigned this Dec 4, 2020

ctarda mentioned this pull request Dec 4, 2020

Allow host apps to insert custom UI instead of WP.com account creation flow wordpress-mobile/WordPressAuthenticator-iOS#534

Merged

Garance91540 approved these changes Dec 4, 2020

View reviewed changes

ctarda added 2 commits December 7, 2020 10:04

Point WPAuthenticator to 1.31.0-beta.3

51eae2a

Merge branch 'feat/unified-login' into issue/3146-no-signup

aa6b2af

ctarda added 2 commits December 7, 2020 10:16

Present Help view controller odally when tapping "need more help" in …

c633949

…the alert

Silence a linter warning

28abbb5

A bit of cleanup of the Podfile. A good side effect is that this shou…

8817acc

…ld trigger a new build on CI

jaclync self-assigned this Dec 7, 2020

jaclync approved these changes Dec 7, 2020

View reviewed changes

ctarda added 3 commits December 8, 2020 07:02

Merge branch 'feat/unified-login' into issue/3146-no-signup

74b37bb

Fix typo in emailDoesNotMatchWPAcount

45b6629

Update logic deciding what errors are supported

ae1404f

Instaintate non-mutable string

43ae140

ctarda mentioned this pull request Dec 7, 2020

[UL&S] Error screens in landscape #3300

Closed

ctarda merged commit 2f2e34e into feat/unified-login Dec 7, 2020

ctarda deleted the issue/3146-no-signup branch December 7, 2020 23:28

ctarda mentioned this pull request Dec 9, 2020

Make UL error view support rotation only for iPad, in order to blend in with the rest of WPAuthenticator #3311

Merged

1 task

shiki mentioned this pull request Apr 14, 2021

Automatic Sign-ups Should Be Disabled #3962

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Block sign ups to WordPress.com accounts. #3286

Block sign ups to WordPress.com accounts. #3286

Uh oh!

ctarda commented Dec 4, 2020 •

edited

Loading

Uh oh!

Garance91540 left a comment

Uh oh!

peril-woocommerce bot commented Dec 7, 2020 •

edited

Loading

Uh oh!

peril-woocommerce bot commented Dec 7, 2020 •

edited

Loading

Uh oh!

ctarda commented Dec 7, 2020

Uh oh!

jaclync left a comment

Uh oh!

jaclync Dec 7, 2020

Uh oh!

jaclync Dec 7, 2020

Uh oh!

ctarda Dec 7, 2020

Uh oh!

jaclync Dec 7, 2020

Uh oh!

ctarda Dec 7, 2020

Uh oh!

ctarda commented Dec 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	case emailDoesNotMatchWPAcount = 7
	case emailDoesNotMatchWPAccount = 7

	return wooAuthError == .emailDoesNotMatchWPAcount \|\| wooAuthError == .notWPSite
	return wooAuthError != .unknown

	let text: NSAttributedString = NSMutableAttributedString(string: Localization.errorMessage)
	let text: NSAttributedString = .init(string: Localization.errorMessage)

Block sign ups to WordPress.com accounts. #3286

Block sign ups to WordPress.com accounts. #3286

Uh oh!

Conversation

ctarda commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Uh oh!

Garance91540 left a comment

Choose a reason for hiding this comment

Uh oh!

peril-woocommerce bot commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-woocommerce bot commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctarda commented Dec 7, 2020

Uh oh!

jaclync left a comment

Choose a reason for hiding this comment

Uh oh!

jaclync Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

jaclync Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

ctarda Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

jaclync Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

ctarda Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

ctarda commented Dec 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ctarda commented Dec 4, 2020 •

edited

Loading

peril-woocommerce bot commented Dec 7, 2020 •

edited

Loading

peril-woocommerce bot commented Dec 7, 2020 •

edited

Loading