Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

23.0
-----
- [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953]


22.9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,6 @@ private extension StorePickerViewController {
}

func presentSiteDiscovery() {
ServiceLocator.authenticationManager.initialize()
guard let viewController = WordPressAuthenticator.siteDiscoveryUI() else {
return
}
Expand Down
11 changes: 2 additions & 9 deletions WooCommerce/Classes/ViewRelated/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ final class AppCoordinator {
self.displayLoggedInStateWithoutDefaultStore()
case (true, false):
self.validateRoleEligibility {
self.configureAuthenticator()
self.displayLoggedInUI()
self.synchronizeAndShowWhatsNew()
}
Expand Down Expand Up @@ -200,19 +199,13 @@ private extension AppCoordinator {
presentLoginOnboarding { [weak self] in
guard let self = self else { return }
// Only displays the authenticator when dismissing onboarding to allow time for A/B test setup.
self.configureAndDisplayAuthenticator()
self.displayAuthenticator()
}
} else {
configureAndDisplayAuthenticator()
displayAuthenticator()
}
}

/// Configures the WPAuthenticator and sets the authenticator UI as the window's root view.
func configureAndDisplayAuthenticator() {
configureAuthenticator()
displayAuthenticator()
}

/// Configures the WPAuthenticator for usage in both logged-in and logged-out states.
func configureAuthenticator() {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is another configureAuthenticator call on line 292. Should it removed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! Removed in b4f38e6.

authenticationManager.initialize()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ final class JetpackSetupCoordinator {
self.stores = stores
self.analytics = analytics
self.featureFlagService = featureFlagService

/// the authenticator needs to be initialized with configs
/// to be used for requesting authentication link and handle login later.
WordPressAuthenticator.initializeWithCustomConfigs(dotcomAuthScheme: dotcomAuthScheme)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the dotcomAuthScheme argument of initializeWithCustomConfigs method is no longer being passed and the ApiCredentials.dotcomAuthScheme default value is always used there. May be it's worth removing the method argument at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 40282f6.

}

func showBenefitModal() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ import WordPressUI
unifiedStyle: WordPressAuthenticatorUnifiedStyle?,
displayImages: WordPressAuthenticatorDisplayImages = .defaultImages,
displayStrings: WordPressAuthenticatorDisplayStrings = .defaultStrings) {
guard privateInstance == nil else {
if isRunningTests() == false { // avoid crashing in tests
assertionFailure("Attempting to initialize WordPressAuthenticator more than once - this would cause the delegate to be reset. Aborting... 🙅⛔️")
}
return
}
privateInstance = WordPressAuthenticator(configuration: configuration,
style: style,
unifiedStyle: unifiedStyle,
Expand Down Expand Up @@ -487,3 +493,9 @@ public extension WordPressAuthenticator {
appleIDCredentialObserver = nil
}
}

private extension WordPressAuthenticator {
static func isRunningTests() -> Bool {
return NSClassFromString("XCTestCase") != nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate {

var authenticationDelegate: WordPressAuthenticatorDelegate {
guard let delegate = WordPressAuthenticator.shared.delegate else {
fatalError()
fatalError("No delegate found for WordPressAuthenticator")
}

return delegate
Expand Down Expand Up @@ -140,7 +140,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate {

func showLoginEpilogue(for credentials: AuthenticatorCredentials) {
guard let navigationController = navigationController else {
fatalError()
fatalError("No navigation controller found to show login epilogue")
}

authenticationDelegate.presentLoginEpilogue(in: navigationController,
Expand Down