-
Notifications
You must be signed in to change notification settings - Fork 112
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
[REST API] Initial support for WPCom account creation during Jetpack setup #14431
Changes from 6 commits
a1cf891
ced1d21
b80ceb7
c558367
25f2815
aa7a587
99ad008
20a0bfc
f910f76
74eff52
8f5e0d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,17 @@ import Combine | |
import UIKit | ||
import WordPressAuthenticator | ||
import protocol WooFoundation.Analytics | ||
import enum WordPressKit.WordPressAPIError | ||
import struct WordPressKit.WordPressComRestApiEndpointError | ||
|
||
/// A protocol used to mock `WordPressComAccountService` for unit tests. | ||
protocol WordPressComAccountServiceProtocol { | ||
func isPasswordlessAccount(username: String, success: @escaping (Bool) -> Void, failure: @escaping (Error) -> Void) | ||
func requestAuthenticationLink(for email: String, jetpackLogin: Bool, success: @escaping () -> Void, failure: @escaping (Error) -> Void) | ||
func requestAuthenticationLink(for email: String, | ||
jetpackLogin: Bool, | ||
createAccountIfNotFound: Bool, | ||
success: @escaping () -> Void, | ||
failure: @escaping (Error) -> Void) | ||
} | ||
|
||
/// Conformance | ||
|
@@ -21,6 +27,7 @@ final class WPComEmailLoginViewModel: ObservableObject { | |
|
||
let termsAttributedString: NSAttributedString | ||
|
||
private let allowAccountCreation: Bool | ||
private let accountService: WordPressComAccountServiceProtocol | ||
private let analytics: Analytics | ||
private let onPasswordUIRequest: (String) -> Void | ||
|
@@ -31,12 +38,14 @@ final class WPComEmailLoginViewModel: ObservableObject { | |
|
||
init(siteURL: String, | ||
requiresConnectionOnly: Bool, | ||
allowAccountCreation: Bool, | ||
debounceDuration: Double = Constants.fieldDebounceDuration, | ||
accountService: WordPressComAccountServiceProtocol = WordPressComAccountService(), | ||
analytics: Analytics = ServiceLocator.analytics, | ||
onPasswordUIRequest: @escaping (String) -> Void, | ||
onMagicLinkUIRequest: @escaping (String) -> Void, | ||
onError: @escaping (String) -> Void) { | ||
self.allowAccountCreation = allowAccountCreation | ||
self.analytics = analytics | ||
self.accountService = accountService | ||
self.onPasswordUIRequest = onPasswordUIRequest | ||
|
@@ -78,8 +87,16 @@ final class WPComEmailLoginViewModel: ObservableObject { | |
} | ||
await startAuthentication(email: email, isPasswordlessAccount: passwordless) | ||
} catch { | ||
analytics.track(event: .JetpackSetup.loginFlow(step: .emailAddress, failure: error)) | ||
onError(error.localizedDescription) | ||
if allowAccountCreation, | ||
let apiError = error as? WordPressAPIError<WordPressComRestApiEndpointError>, | ||
case let .endpointError(endpointError) = apiError, | ||
endpointError.apiErrorCode == Constants.unknownUserErrorCode { | ||
// The user does not exist yet, trigger magic link flow for account creation | ||
await requestAuthenticationLink(email: email, forAccountCreation: true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not specifically related to functionality of this PR: I feel it might be valuable to add tracking for this case. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's already planned, I'll add some tracking before releasing the feature. |
||
} else { | ||
analytics.track(event: .JetpackSetup.loginFlow(step: .emailAddress, failure: error)) | ||
onError(error.localizedDescription) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -93,10 +110,13 @@ final class WPComEmailLoginViewModel: ObservableObject { | |
} | ||
|
||
@MainActor | ||
func requestAuthenticationLink(email: String) async { | ||
func requestAuthenticationLink(email: String, forAccountCreation: Bool = false) async { | ||
do { | ||
try await withCheckedThrowingContinuation { continuation in | ||
accountService.requestAuthenticationLink(for: email, jetpackLogin: false, success: { | ||
accountService.requestAuthenticationLink(for: email, | ||
jetpackLogin: false, | ||
createAccountIfNotFound: forAccountCreation, | ||
success: { | ||
continuation.resume() | ||
}, failure: { error in | ||
continuation.resume(throwing: error) | ||
|
@@ -116,6 +136,7 @@ extension WPComEmailLoginViewModel { | |
static let jetpackTermsURL = "https://jetpack.com/redirect/?source=wpcom-tos&site=" | ||
static let jetpackShareDetailsURL = "https://jetpack.com/redirect/?source=jetpack-support-what-data-does-jetpack-sync&site=" | ||
static let wpcomErrorCodeKey = "WordPressComRestApiErrorCodeKey" | ||
static let unknownUserErrorCode = "unknown_user" | ||
} | ||
|
||
enum Localization { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt about replacing the if-else with guard?
I think
guard
helps with early return pattern and communicates what conditions are required, and reduces nesting a bit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied in 99ad008