From cf69dbb6570ec2212b51fcda9cf284a3dd5b597a Mon Sep 17 00:00:00 2001 From: Hafiz Rahman Date: Thu, 31 Oct 2024 16:23:34 +0700 Subject: [PATCH 01/19] First pass at converting error dialog into an error screen for disabled application password. --- ...cationPasswordAuthorizationViewModel.swift | 2 +- ...sswordAuthorizationWebViewController.swift | 36 ++++++++++++++++++- ...ApplicationPasswordDisabledViewModel.swift | 20 ++++++++++- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationViewModel.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationViewModel.swift index f4af2404173..17dcd02737c 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationViewModel.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationViewModel.swift @@ -4,7 +4,7 @@ import Yosemite /// View model for `ApplicationPasswordAuthorizationWebViewController`. /// final class ApplicationPasswordAuthorizationViewModel { - private let siteURL: String + let siteURL: String private let stores: StoresManager init(siteURL: String, diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index 836f54d9f74..376e5463e54 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -157,7 +157,20 @@ private extension ApplicationPasswordAuthorizationWebViewController { guard let url = try await viewModel.fetchAuthURL() else { DDLogError("⛔️ No authorization URL found for application passwords") analytics.track(.applicationPasswordAuthorizationURLNotAvailable) - return showErrorAlert(message: Localization.applicationPasswordDisabled) + + let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL) + // When the error view controller is popped, also pop the web view + errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem( + image: UIImage(systemName: "chevron.backward"), + style: .plain, + target: self, + action: #selector(popBothControllers) + ) + + // Push instead of present + navigationController?.pushViewController(errorUI, animated: true) + + return } loadAuthorizationPage(url: url) } catch { @@ -171,6 +184,12 @@ private extension ApplicationPasswordAuthorizationWebViewController { } } + @objc private func popBothControllers() { + // Pop back two view controllers to remove both error and web view + navigationController?.popViewController(animated: false) + navigationController?.popViewController(animated: true) + } + func loadAuthorizationPage(url: URL) { let parameters: [URLQueryItem] = [ URLQueryItem(name: Constants.Query.appName, value: appName), @@ -220,6 +239,14 @@ private extension ApplicationPasswordAuthorizationWebViewController { } present(alertController, animated: true) } + + /// The error screen to be displayed when the user tries to log in with site credentials + /// with application password disabled. + /// + func applicationPasswordDisabledUI(for siteURL: String) -> UIViewController { + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL) + return ULErrorViewController(viewModel: viewModel) + } } extension ApplicationPasswordAuthorizationWebViewController: WKNavigationDelegate { @@ -254,6 +281,13 @@ extension ApplicationPasswordAuthorizationWebViewController: WKNavigationDelegat } } +extension ApplicationPasswordAuthorizationWebViewController: UIAdaptivePresentationControllerDelegate { + func presentationControllerDidDismiss(_ presentationController: UIPresentationController) { + // This will be called when the error UI is dismissed + navigationController?.dismiss(animated: true) + } +} + private extension ApplicationPasswordAuthorizationWebViewController { enum Constants { static let successURL = "woocommerce://application-password" diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index 7a9a747a7c2..1267e74c283 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -5,11 +5,14 @@ import WordPressAuthenticator /// modeling an error when application password is disabled. /// struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { - init(siteURL: String) { + init(siteURL: String, + authentication: Authentication = ServiceLocator.authenticationManager) { self.siteURL = siteURL + self.authentication = authentication } let siteURL: String + let authentication: Authentication let image: UIImage = .errorImage // TODO: update this if needed var text: NSAttributedString { @@ -55,6 +58,17 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { } WebviewHelper.launch(Constants.applicationPasswordLink, with: viewController) } + + var rightBarButtonItemTitle: String? { + return Localization.helpButtonTitle + } + + func didTapRightBarButtonItem(in viewController: UIViewController?) { + guard let viewController else { + return + } + authentication.presentSupport(from: viewController, screen: .noWooError) + } } private extension ApplicationPasswordDisabledViewModel { @@ -78,6 +92,10 @@ private extension ApplicationPasswordDisabledViewModel { "Log in with WordPress.com", comment: "Button that will navigate to the authentication flow with WP.com" ) + static let helpButtonTitle = NSLocalizedString( + "Help", + comment: "Button that will navigate to the support area" + ) } enum Constants { static let applicationPasswordLink = "https://make.wordpress.org/core/2020/11/05/application-passwords-integration-guide/" From b292c92c9e2d10f21af79481c3319c9575f8e29c Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 14:20:09 +0100 Subject: [PATCH 02/19] Update navigation logic and button titles in ApplicationPasswordDisabledViewModel.swift - Modify navigation logic to pop to the third last view controller or root if fewer than three - Update primary button title to "Retry" for retrying application password authorization - Remove outdated primary button title "Log in with WordPress.com" --- ...ApplicationPasswordDisabledViewModel.swift | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index 1267e74c283..a824d0187eb 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -13,7 +13,7 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { let siteURL: String let authentication: Authentication - let image: UIImage = .errorImage // TODO: update this if needed + let image: UIImage = .errorImage var text: NSAttributedString { let font: UIFont = .body @@ -40,11 +40,16 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { // TODO: add tracks if necessary } + // Navigates back to the third last view controller in the stack if possible, + // otherwise it navigates to the root view controller. func didTapPrimaryButton(in viewController: UIViewController?) { - guard let viewController else { - return + guard let navigationController = viewController?.navigationController else { return } + let viewControllers = navigationController.viewControllers + if viewControllers.count >= 3 { + navigationController.popToViewController(viewControllers[viewControllers.count - 3], animated: true) + } else { + navigationController.popToRootViewController(animated: true) } - WordPressAuthenticator.showLoginForJustWPCom(from: viewController) } func didTapSecondaryButton(in viewController: UIViewController?) { @@ -79,6 +84,11 @@ private extension ApplicationPasswordDisabledViewModel { "Reads like: It seems that your site google.com has Application Password disabled. " + "Please enable it to use the WooCommerce app." ) + static let primaryButtonTitle = NSLocalizedString( + "applicationPasswordDisabled.retry.buttonTitle", + value: "Retry", + comment: "Button to retry fetching application password authorization if application password is disabled" + ) static let secondaryButtonTitle = NSLocalizedString( "Log In With Another Account", comment: "Action button that will restart the login flow." @@ -88,10 +98,6 @@ private extension ApplicationPasswordDisabledViewModel { "What is Application Password?", comment: "Button that will navigate to a web page explaining Application Password" ) - static let primaryButtonTitle = NSLocalizedString( - "Log in with WordPress.com", - comment: "Button that will navigate to the authentication flow with WP.com" - ) static let helpButtonTitle = NSLocalizedString( "Help", comment: "Button that will navigate to the support area" From 170d58e672cef09956678fed17896396683899f0 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 14:26:04 +0100 Subject: [PATCH 03/19] update: localization keys and message formatting in `ApplicationPasswordDisabledViewModel` using Woo mobile guidelines --- .../ApplicationPasswordDisabledViewModel.swift | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index a824d0187eb..973e117acd3 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -23,7 +23,7 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { attributes: [.font: boldFont]) let message = NSMutableAttributedString(string: Localization.errorMessage) - message.replaceFirstOccurrence(of: "%@", with: boldSiteAddress) + message.replaceFirstOccurrence(of: "%1$@", with: boldSiteAddress) return message } @@ -79,7 +79,8 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { private extension ApplicationPasswordDisabledViewModel { enum Localization { static let errorMessage = NSLocalizedString( - "It seems that your site %@ has Application Password disabled. Please enable it to use the WooCommerce app.", + "applicationPasswordDisabled.errorMessage", + value: "It seems that your site %1$@ has Application Password disabled. Please enable it to use the WooCommerce app.", comment: "An error message displayed when the user tries to log in to the app with site credentials but has application password disabled. " + "Reads like: It seems that your site google.com has Application Password disabled. " + "Please enable it to use the WooCommerce app." @@ -90,16 +91,19 @@ private extension ApplicationPasswordDisabledViewModel { comment: "Button to retry fetching application password authorization if application password is disabled" ) static let secondaryButtonTitle = NSLocalizedString( - "Log In With Another Account", + "applicationPasswordDisabled.secondaryButtonTitle", + value: "Log In With Another Account", comment: "Action button that will restart the login flow." + "Presented when the user tries to log in to the app with site credentials but has application password disabled." ) static let auxiliaryButtonTitle = NSLocalizedString( - "What is Application Password?", + "applicationPasswordDisabled.auxiliaryButtonTitle", + value: "What is Application Password?", comment: "Button that will navigate to a web page explaining Application Password" ) static let helpButtonTitle = NSLocalizedString( - "Help", + "applicationPasswordDisabled.helpButtonTitle", + value: "Help", comment: "Button that will navigate to the support area" ) } From 28874912b3849fa387e84c112cdf4aa7e3c6a639 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 15:52:06 +0100 Subject: [PATCH 04/19] update: authentication tests and localization keys in `ApplicationPasswordViewModelTests` --- ...cationPasswordDisabledViewModelTests.swift | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift b/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift index 332c70671d4..121b244d19a 100644 --- a/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift @@ -40,7 +40,7 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_error_message() { // Given let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) - let expectation = Expectations.errorMessage.replacingOccurrences(of: "%@", with: "test.com") + let expectation = Expectations.errorMessage.replacingOccurrences(of: "%1$@", with: "test.com") // When let errorMessage = viewModel.text.string @@ -129,18 +129,30 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { XCTAssertTrue(navigationController.presentedViewController is SFSafariViewController) } - func test_didTapPrimaryButton_presents_LoginNavigationController() { + func test_didTapPrimaryButton_navigates_to_correct_view_controller() { // Given let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewController1 = UIViewController() + let viewController2 = UIViewController() + let viewController3 = UIViewController() + + navigationController.viewControllers = [viewController1, viewController2, viewController3] + window.rootViewController = navigationController + window.makeKeyAndVisible() + + XCTAssertEqual(viewController3.navigationController, navigationController, "viewController3 should be part of the navigationController's stack") // When - viewModel.didTapPrimaryButton(in: navigationController) + viewModel.didTapPrimaryButton(in: viewController3) - // Then - waitUntil { - self.navigationController.presentedViewController != nil + waitUntil(timeout: Constants.expectationTimeout) { + return self.navigationController.viewControllers.count == 1 && + self.navigationController.topViewController === viewController1 } - XCTAssertTrue(navigationController.presentedViewController is LoginNavigationController) + + // Then + XCTAssertEqual(navigationController.viewControllers.count, 1, "After the action, navigationController's stack should contain only one view controller") + XCTAssertEqual(navigationController.topViewController, viewController1, "topViewController should be viewController1") } } @@ -149,23 +161,27 @@ private extension ApplicationPasswordDisabledViewModelTests { static let image = UIImage.errorImage static let errorMessage = NSLocalizedString( - "It seems that your site %@ has Application Password disabled. Please enable it to use the WooCommerce app.", + "applicationPasswordDisabled.errorMessage", + value: "It seems that your site %1$@ has Application Password disabled. Please enable it to use the WooCommerce app.", comment: "An error message displayed when the user tries to log in to the app with site credentials but has application password disabled. " + "Reads like: It seems that your site google.com has Application Password disabled. " + "Please enable it to use the WooCommerce app." ) static let secondaryButtonTitle = NSLocalizedString( - "Log In With Another Account", + "applicationPasswordDisabled.secondaryButtonTitle", + value: "Log In With Another Account", comment: "Action button that will restart the login flow." + "Presented when the user tries to log in to the app with site credentials but has application password disabled." ) static let auxiliaryButtonTitle = NSLocalizedString( - "What is Application Password?", + "applicationPasswordDisabled.auxiliaryButtonTitle", + value: "What is Application Password?", comment: "Button that will navigate to a web page explaining Application Password" ) static let primaryButtonTitle = NSLocalizedString( - "Log in with WordPress.com", - comment: "Button that will navigate to the authentication flow with WP.com" + "applicationPasswordDisabled.retry.buttonTitle", + value: "Retry", + comment: "Button to retry fetching application password authorization if application password is disabled" ) } } From 43e803d772a2fabb197dfbedaffbf26ddfdea872 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 15:53:02 +0100 Subject: [PATCH 05/19] clean: removed TODO comment because the track already happen somewhere else --- .../ApplicationPasswordDisabledViewModel.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index 973e117acd3..1b721b91bc0 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -37,7 +37,6 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { let secondaryButtonTitle = Localization.secondaryButtonTitle func viewDidLoad(_ viewController: UIViewController?) { - // TODO: add tracks if necessary } // Navigates back to the third last view controller in the stack if possible, From 965c4d3329d20b377c6d691091aa0d31056c6947 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 15:57:39 +0100 Subject: [PATCH 06/19] update: release-notes 21.7 --- RELEASE-NOTES.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 40c59954774..b287a93f392 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,5 +1,10 @@ *** PLEASE FOLLOW THIS FORMAT: [] [] *** Use [*****] to indicate smoke tests of all critical flows should be run on the final IPA before release (e.g. major library or OS update). + +21.7 +----- +- [*] Better error messages if Application Password login is disabled on user's website. + 21.6 ----- - [*] Payments: improve retry handling when card reader updates fail due to low battery [https://github.com/woocommerce/woocommerce-ios/pull/14990] From 58faf5ec22d5c80022aac6ccd507c0ec70163642 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 31 Jan 2025 16:21:42 +0100 Subject: [PATCH 07/19] update: added PR url in 21.7 release notes --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index b287a93f392..b3d2520e86a 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -3,7 +3,7 @@ 21.7 ----- -- [*] Better error messages if Application Password login is disabled on user's website. +- [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031] 21.6 ----- From 3f40c59fc1383b7881b655fb533c1c3aa81507dc Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 3 Feb 2025 14:51:49 +0100 Subject: [PATCH 08/19] fix: remove unused code --- ...ApplicationPasswordAuthorizationWebViewController.swift | 7 ------- 1 file changed, 7 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index 376e5463e54..e96e56ec760 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -281,13 +281,6 @@ extension ApplicationPasswordAuthorizationWebViewController: WKNavigationDelegat } } -extension ApplicationPasswordAuthorizationWebViewController: UIAdaptivePresentationControllerDelegate { - func presentationControllerDidDismiss(_ presentationController: UIPresentationController) { - // This will be called when the error UI is dismissed - navigationController?.dismiss(animated: true) - } -} - private extension ApplicationPasswordAuthorizationWebViewController { enum Constants { static let successURL = "woocommerce://application-password" From 9b480ba48fc8c2f0c04aac77c43c512fb103c82e Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Mon, 3 Feb 2025 15:51:19 +0100 Subject: [PATCH 09/19] Add navigation to previous view controller in application password flow - Modify `ApplicationPasswordAuthorizationWebViewController.swift` to include a reference to the previous view controller for navigation purposes. - Update `AuthenticationManager.swift` to pass the previous view controller to the web view used for application password authorization. - Enhance `ApplicationPasswordDisabledViewModel.swift` to store the previous view controller and use it for navigation. - Adjust `PostSiteCredentialLoginChecker.swift` to capture and utilize the last view controller before the application password flow. - Update tests in `ApplicationPasswordDisabledViewModelTests.swift` to include the previous view controller in the view model initialization. --- ...sswordAuthorizationWebViewController.swift | 22 ++++++++----- .../AuthenticationManager.swift | 5 +-- ...ApplicationPasswordDisabledViewModel.swift | 13 +++++--- .../PostSiteCredentialLoginChecker.swift | 9 +++--- ...cationPasswordDisabledViewModelTests.swift | 31 +++++++++++++------ 5 files changed, 52 insertions(+), 28 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index e96e56ec760..41807185150 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -34,6 +34,9 @@ final class ApplicationPasswordAuthorizationWebViewController: UIViewController return indicator }() + /// The view controller that was presenting the application password flow. + private weak var previousViewController: UIViewController? + /// WP Core requires that the UUID has lowercased letters. private let appID = UUID().uuidString.lowercased() @@ -50,9 +53,11 @@ final class ApplicationPasswordAuthorizationWebViewController: UIViewController private var authorizationURL: String? init(viewModel: ApplicationPasswordAuthorizationViewModel, + previousViewController: UIViewController?, analytics: Analytics = ServiceLocator.analytics, onSuccess: @escaping (ApplicationPassword, UINavigationController?) -> Void) { self.viewModel = viewModel + self.previousViewController = previousViewController self.onSuccess = onSuccess self.analytics = analytics super.init(nibName: nil, bundle: nil) @@ -159,12 +164,12 @@ private extension ApplicationPasswordAuthorizationWebViewController { analytics.track(.applicationPasswordAuthorizationURLNotAvailable) let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL) - // When the error view controller is popped, also pop the web view + // When the error view controller is popped, navigate to previous VC errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem( image: UIImage(systemName: "chevron.backward"), style: .plain, target: self, - action: #selector(popBothControllers) + action: #selector(navigateToPreviousViewController) ) // Push instead of present @@ -184,10 +189,13 @@ private extension ApplicationPasswordAuthorizationWebViewController { } } - @objc private func popBothControllers() { - // Pop back two view controllers to remove both error and web view - navigationController?.popViewController(animated: false) - navigationController?.popViewController(animated: true) + /// Pops to the previous view controller (if provided) or pops one level otherwise. + @objc private func navigateToPreviousViewController() { + if let previous = previousViewController, let nav = navigationController { + nav.popToViewController(previous, animated: true) + } else { + navigationController?.popViewController(animated: true) + } } func loadAuthorizationPage(url: URL) { @@ -244,7 +252,7 @@ private extension ApplicationPasswordAuthorizationWebViewController { /// with application password disabled. /// func applicationPasswordDisabledUI(for siteURL: String) -> UIViewController { - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL, previousViewController: previousViewController) return ULErrorViewController(viewModel: viewModel) } } diff --git a/WooCommerce/Classes/Authentication/AuthenticationManager.swift b/WooCommerce/Classes/Authentication/AuthenticationManager.swift index 80bb757593e..66acd517b15 100644 --- a/WooCommerce/Classes/Authentication/AuthenticationManager.swift +++ b/WooCommerce/Classes/Authentication/AuthenticationManager.swift @@ -685,9 +685,10 @@ private extension AuthenticationManager { /// Web view to authorize application password for a given site. /// - func applicationPasswordWebView(for siteURL: String) -> UIViewController { + func applicationPasswordWebView(for siteURL: String, previousVC: UIViewController?) -> UIViewController { let viewModel = ApplicationPasswordAuthorizationViewModel(siteURL: siteURL) let controller = ApplicationPasswordAuthorizationWebViewController(viewModel: viewModel, + previousViewController: previousVC, onSuccess: { [weak self] applicationPassword, navigationController in guard let navigationController else { DDLogInfo("⚠️ No navigation controller found") @@ -806,7 +807,7 @@ private extension AuthenticationManager { /// Presents app password site login using a web view. /// private func presentApplicationPasswordWebView(for siteURL: String, in viewController: UIViewController) { - let webViewController = applicationPasswordWebView(for: siteURL) + let webViewController = applicationPasswordWebView(for: siteURL, previousVC: viewController) viewController.navigationController?.pushViewController(webViewController, animated: true) } } diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index 1b721b91bc0..499219375a0 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -6,8 +6,10 @@ import WordPressAuthenticator /// struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { init(siteURL: String, + previousViewController: UIViewController?, authentication: Authentication = ServiceLocator.authenticationManager) { self.siteURL = siteURL + self.previousViewController = previousViewController self.authentication = authentication } @@ -15,6 +17,9 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { let authentication: Authentication let image: UIImage = .errorImage + // The VC that was showing before the application password flow. This is used to navigate back without guesswork. + let previousViewController: UIViewController? + var text: NSAttributedString { let font: UIFont = .body let boldFont: UIFont = font.bold @@ -39,13 +44,11 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { func viewDidLoad(_ viewController: UIViewController?) { } - // Navigates back to the third last view controller in the stack if possible, - // otherwise it navigates to the root view controller. + // Pop to the previous VC func didTapPrimaryButton(in viewController: UIViewController?) { guard let navigationController = viewController?.navigationController else { return } - let viewControllers = navigationController.viewControllers - if viewControllers.count >= 3 { - navigationController.popToViewController(viewControllers[viewControllers.count - 3], animated: true) + if let previousViewController = previousViewController { + navigationController.popToViewController(previousViewController, animated: true) } else { navigationController.popToRootViewController(animated: true) } diff --git a/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift b/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift index d7477ebea64..185fdf4d4dc 100644 --- a/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift +++ b/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift @@ -56,8 +56,9 @@ private extension PostSiteCredentialLoginChecker { analytics.track(event: .ApplicationPassword.applicationPasswordGenerationFailed(scenario: .generation, error: error)) switch error { case ApplicationPasswordUseCaseError.applicationPasswordsDisabled: - // show application password disabled error - let errorUI = applicationPasswordDisabledUI(for: siteURL) + // show application password disabled error, and catch the last view controller that was showing before the Application Password flow. + let previousViewController = navigationController.viewControllers.dropLast().last + let errorUI = applicationPasswordDisabledUI(for: siteURL, previousViewController: previousViewController) navigationController.show(errorUI, sender: nil) case ApplicationPasswordUseCaseError.unauthorizedRequest: showAlert(message: Localization.unauthorizedForAppPassword, in: navigationController) @@ -179,8 +180,8 @@ private extension PostSiteCredentialLoginChecker { /// The error screen to be displayed when the user tries to log in with site credentials /// with application password disabled. /// - func applicationPasswordDisabledUI(for siteURL: String) -> UIViewController { - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL) + func applicationPasswordDisabledUI(for siteURL: String, previousViewController: UIViewController?) -> UIViewController { + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL, previousViewController: previousViewController) return ULErrorViewController(viewModel: viewModel) } } diff --git a/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift b/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift index 121b244d19a..8745fe22794 100644 --- a/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/Authentication/ApplicationPasswordDisabledViewModelTests.swift @@ -28,7 +28,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_image() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let image = viewModel.image @@ -39,7 +40,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_error_message() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) let expectation = Expectations.errorMessage.replacingOccurrences(of: "%1$@", with: "test.com") // When @@ -51,7 +53,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_visibility_for_auxiliary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let isHidden = viewModel.isAuxiliaryButtonHidden @@ -62,7 +65,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_title_for_auxiliary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let auxiliaryButtonTitle = viewModel.auxiliaryButtonTitle @@ -73,7 +77,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_visibility_for_primary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let isHidden = viewModel.isPrimaryButtonHidden @@ -84,7 +89,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_title_for_primary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let primaryButtonTitle = viewModel.primaryButtonTitle @@ -95,7 +101,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_visibility_for_secondary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let isHidden = viewModel.isSecondaryButtonHidden @@ -106,7 +113,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_viewmodel_provides_expected_title_for_secondary_button() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When let secondaryButtonTitle = viewModel.secondaryButtonTitle @@ -117,7 +125,8 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_didTapAuxiliaryButton_presents_a_web_view() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: nil) // When viewModel.didTapAuxiliaryButton(in: navigationController) @@ -131,7 +140,6 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { func test_didTapPrimaryButton_navigates_to_correct_view_controller() { // Given - let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL) let viewController1 = UIViewController() let viewController2 = UIViewController() let viewController3 = UIViewController() @@ -140,6 +148,9 @@ final class ApplicationPasswordDisabledViewModelTests: XCTestCase { window.rootViewController = navigationController window.makeKeyAndVisible() + let viewModel = ApplicationPasswordDisabledViewModel(siteURL: testURL, + previousViewController: viewController1) + XCTAssertEqual(viewController3.navigationController, navigationController, "viewController3 should be part of the navigationController's stack") // When From 6467c96138c9c1bd8d3d66e8be82f2eaf2df7571 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 4 Feb 2025 11:56:53 +0100 Subject: [PATCH 10/19] Update WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift Co-authored-by: Huong Do --- .../ApplicationPasswordAuthorizationWebViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index 41807185150..8a995b585b1 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -191,7 +191,7 @@ private extension ApplicationPasswordAuthorizationWebViewController { /// Pops to the previous view controller (if provided) or pops one level otherwise. @objc private func navigateToPreviousViewController() { - if let previous = previousViewController, let nav = navigationController { + if let previousViewController, let navigationController { nav.popToViewController(previous, animated: true) } else { navigationController?.popViewController(animated: true) From 71d68eeaf83510408bd2500b13863c5e4d6f5de2 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 4 Feb 2025 12:00:16 +0100 Subject: [PATCH 11/19] Update WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift Co-authored-by: Huong Do --- .../ApplicationPasswordDisabledViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index 499219375a0..fb46d81ceb9 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -47,7 +47,7 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { // Pop to the previous VC func didTapPrimaryButton(in viewController: UIViewController?) { guard let navigationController = viewController?.navigationController else { return } - if let previousViewController = previousViewController { + if let previousViewController { navigationController.popToViewController(previousViewController, animated: true) } else { navigationController.popToRootViewController(animated: true) From ec50237500644b52dc2bfb687999f2e1ead3ed4b Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 4 Feb 2025 12:15:44 +0100 Subject: [PATCH 12/19] fix: naming --- .../ApplicationPasswordAuthorizationWebViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index 8a995b585b1..f2d7c7a632a 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -192,7 +192,7 @@ private extension ApplicationPasswordAuthorizationWebViewController { /// Pops to the previous view controller (if provided) or pops one level otherwise. @objc private func navigateToPreviousViewController() { if let previousViewController, let navigationController { - nav.popToViewController(previous, animated: true) + navigationController.popToViewController(previousViewController, animated: true) } else { navigationController?.popViewController(animated: true) } From 6bdb62db7e1266caa2dd63dc920b502d2ad83f4b Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 4 Feb 2025 12:30:47 +0100 Subject: [PATCH 13/19] Add previousViewController parameter for navigation - Update `AuthenticationManager.swift` to include `previousViewController` parameter in `checkSiteCredentialLogin` function - Modify `PostSiteCredentialLoginChecker.swift` to accept `previousViewController` in the initializer and utilize it in error handling --- .../Classes/Authentication/AuthenticationManager.swift | 8 +++++--- .../Authentication/PostSiteCredentialLoginChecker.swift | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/WooCommerce/Classes/Authentication/AuthenticationManager.swift b/WooCommerce/Classes/Authentication/AuthenticationManager.swift index 66acd517b15..b8a7d2162e1 100644 --- a/WooCommerce/Classes/Authentication/AuthenticationManager.swift +++ b/WooCommerce/Classes/Authentication/AuthenticationManager.swift @@ -755,13 +755,15 @@ private extension AuthenticationManager { ) else { return assertionFailure("⛔️ Error creating application password use case") } - checkSiteCredentialLogin(to: siteURL, with: useCase, in: navigationController) + checkSiteCredentialLogin(to: siteURL, with: useCase, in: navigationController, previousViewController: nil) } func checkSiteCredentialLogin(to siteURL: String, with useCase: ApplicationPasswordUseCase, - in navigationController: UINavigationController) { - let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase) + in navigationController: UINavigationController, + previousViewController: UIViewController? = nil) { + let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase, + previousViewController: previousViewController) checker.checkEligibility(for: siteURL, from: navigationController) { [weak self] in guard let self else { return } // Tracking `signedIn` after the user logged in using site creds & application password is created diff --git a/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift b/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift index 185fdf4d4dc..734aec61257 100644 --- a/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift +++ b/WooCommerce/Classes/Authentication/PostSiteCredentialLoginChecker.swift @@ -14,15 +14,18 @@ final class PostSiteCredentialLoginChecker { private let applicationPasswordUseCase: ApplicationPasswordUseCase private let roleEligibilityUseCase: RoleEligibilityUseCaseProtocol private let analytics: Analytics + private let previousViewController: UIViewController? init(applicationPasswordUseCase: ApplicationPasswordUseCase, roleEligibilityUseCase: RoleEligibilityUseCaseProtocol = RoleEligibilityUseCase(stores: ServiceLocator.stores), stores: StoresManager = ServiceLocator.stores, - analytics: Analytics = ServiceLocator.analytics) { + analytics: Analytics = ServiceLocator.analytics, + previousViewController: UIViewController?) { self.applicationPasswordUseCase = applicationPasswordUseCase self.roleEligibilityUseCase = roleEligibilityUseCase self.stores = stores self.analytics = analytics + self.previousViewController = previousViewController } /// Checks whether the user is eligible to use the app. @@ -56,8 +59,7 @@ private extension PostSiteCredentialLoginChecker { analytics.track(event: .ApplicationPassword.applicationPasswordGenerationFailed(scenario: .generation, error: error)) switch error { case ApplicationPasswordUseCaseError.applicationPasswordsDisabled: - // show application password disabled error, and catch the last view controller that was showing before the Application Password flow. - let previousViewController = navigationController.viewControllers.dropLast().last + // show application password disabled error, and use the previous view controller. let errorUI = applicationPasswordDisabledUI(for: siteURL, previousViewController: previousViewController) navigationController.show(errorUI, sender: nil) case ApplicationPasswordUseCaseError.unauthorizedRequest: From fc9335d9188623d783964d81bbac12e6cec00785 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Tue, 4 Feb 2025 13:46:28 +0100 Subject: [PATCH 14/19] Add previousViewController parameter to PostSiteCredentialLoginCheckerTests: update `PostSiteCredentialLoginChecker` instantiation in `PostSiteCredentialLoginCheckerTests.swift` to include `previousViewController: nil` across multiple test cases. --- .../PostSiteCredentialLoginCheckerTests.swift | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift b/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift index 19779ff612b..771cec23fa8 100644 --- a/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift +++ b/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift @@ -32,7 +32,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { func test_application_password_disabled_error_is_displayed_when_application_password_is_disabled() { // Given let useCase = MockApplicationPasswordUseCase(mockGenerationError: ApplicationPasswordUseCaseError.applicationPasswordsDisabled) - let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase) + let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase, + previousViewController: nil) var isSuccess = false // When @@ -51,7 +52,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { func test_error_alert_is_displayed_when_application_password_cannot_be_fetched() { // Given let useCase = MockApplicationPasswordUseCase(mockGenerationError: NetworkError.timeout()) - let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase) + let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase, + previousViewController: nil) var isSuccess = false // When @@ -75,7 +77,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { let errorInfo = StorageEligibilityErrorInfo(name: "Billie Jean", roles: ["skater", "writer"]) roleCheckUseCase.errorToReturn = .insufficientRole(info: errorInfo) let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: appPasswordUseCase, - roleEligibilityUseCase: roleCheckUseCase) + roleEligibilityUseCase: roleCheckUseCase, + previousViewController: nil) var isSuccess = false // When @@ -97,7 +100,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { let roleCheckUseCase = MockRoleEligibilityUseCase() roleCheckUseCase.errorToReturn = .unknown(error: NetworkError.timeout()) let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: appPasswordUseCase, - roleEligibilityUseCase: roleCheckUseCase) + roleEligibilityUseCase: roleCheckUseCase, + previousViewController: nil) var isSuccess = false // When @@ -119,7 +123,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { let roleCheckUseCase = MockRoleEligibilityUseCase() let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: appPasswordUseCase, roleEligibilityUseCase: roleCheckUseCase, - stores: stores) + stores: stores, + previousViewController: nil) var isSuccess = false // When @@ -148,7 +153,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { let roleCheckUseCase = MockRoleEligibilityUseCase() let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: appPasswordUseCase, roleEligibilityUseCase: roleCheckUseCase, - stores: stores) + stores: stores, + previousViewController: nil) var isSuccess = false // When @@ -179,7 +185,8 @@ final class PostSiteCredentialLoginCheckerTests: XCTestCase { let roleCheckUseCase = MockRoleEligibilityUseCase() let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: appPasswordUseCase, roleEligibilityUseCase: roleCheckUseCase, - stores: stores) + stores: stores, + previousViewController: nil) var isSuccess = false // When From daee4c025445ffe171640cab9b898392f2388b28 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 5 Feb 2025 15:35:38 +0100 Subject: [PATCH 15/19] feat: remove weak from `ApplicationPasswordAuthorizationWebViewController` --- .../ApplicationPasswordAuthorizationWebViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index f2d7c7a632a..dffc9f69c64 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -35,7 +35,7 @@ final class ApplicationPasswordAuthorizationWebViewController: UIViewController }() /// The view controller that was presenting the application password flow. - private weak var previousViewController: UIViewController? + private var previousViewController: UIViewController? /// WP Core requires that the UUID has lowercased letters. private let appID = UUID().uuidString.lowercased() From dc68ebc0d4c12394f9d50fe13fc28b47976f7512 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 5 Feb 2025 16:31:13 +0100 Subject: [PATCH 16/19] update: navigation logic in `ApplicationPasswordDisabledViewModel`: change navigationController method from `popToRootViewController` to `popViewController` --- .../ApplicationPasswordDisabledViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift index fb46d81ceb9..f48ab6e9ad0 100644 --- a/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift +++ b/WooCommerce/Classes/Authentication/Navigation Exceptions/ApplicationPasswordDisabledViewModel.swift @@ -50,7 +50,7 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel { if let previousViewController { navigationController.popToViewController(previousViewController, animated: true) } else { - navigationController.popToRootViewController(animated: true) + navigationController.popViewController(animated: true) } } From 9fafc530cd0f85712d6e925be4c41ab80cca27a2 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 7 Feb 2025 00:34:48 +0100 Subject: [PATCH 17/19] Refactor error handling in `ApplicationPasswordAuthorizationWebViewController` and display errorUI instead of error alert --- ...sswordAuthorizationWebViewController.swift | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index dffc9f69c64..8297d759ff0 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -162,28 +162,14 @@ private extension ApplicationPasswordAuthorizationWebViewController { guard let url = try await viewModel.fetchAuthURL() else { DDLogError("⛔️ No authorization URL found for application passwords") analytics.track(.applicationPasswordAuthorizationURLNotAvailable) - - let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL) - // When the error view controller is popped, navigate to previous VC - errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem( - image: UIImage(systemName: "chevron.backward"), - style: .plain, - target: self, - action: #selector(navigateToPreviousViewController) - ) - - // Push instead of present - navigationController?.pushViewController(errorUI, animated: true) - + navigateToErrorUI() return } loadAuthorizationPage(url: url) } catch { DDLogError("⛔️ Error fetching authorization URL for application passwords \(error)") analytics.track(.applicationPasswordAuthorizationURLFetchFailed, withError: error) - showErrorAlert(message: Localization.errorFetchingAuthURL, onRetry: { [weak self] in - self?.fetchAuthorizationURL() - }) + navigateToErrorUI() } activityIndicator.stopAnimating() } @@ -198,6 +184,19 @@ private extension ApplicationPasswordAuthorizationWebViewController { } } + private func navigateToErrorUI() { + let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL) + // When the error view controller is popped, navigate to previous VC + errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem( + image: UIImage(systemName: "chevron.backward"), + style: .plain, + target: self, + action: #selector(navigateToPreviousViewController) + ) + // Push instead of present + navigationController?.pushViewController(errorUI, animated: true) + } + func loadAuthorizationPage(url: URL) { let parameters: [URLQueryItem] = [ URLQueryItem(name: Constants.Query.appName, value: appName), @@ -317,9 +316,5 @@ private extension ApplicationPasswordAuthorizationWebViewController { comment: "Error message displayed when application password authorization fails " + "during login due to the feature being disabled on the input site." ) - static let errorFetchingAuthURL = NSLocalizedString( - "Failed to fetch the authorization URL for application passwords. Please try again.", - comment: "Error message displayed when failed to fetch application password authorization URL during login" - ) } } From b8b44e1883b3b15eb20ee0143cef38f6cdb1ba58 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 14 Feb 2025 12:48:41 +0100 Subject: [PATCH 18/19] update: moved release notes to v21.8 --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index aa28d3ee3bd..ff733757065 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -11,11 +11,11 @@ - [*] Background image upload: Fix missing error notice in iPhones [https://github.com/woocommerce/woocommerce-ios/pull/15117] - [*] Background image upload: Show a notice when the user leaves product details while uploads are pending [https://github.com/woocommerce/woocommerce-ios/pull/15134] - [*] Filters applied in product selector no longer affect the main product list screen. [https://github.com/woocommerce/woocommerce-ios/pull/14764] +- [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031] 21.7 ----- - [**] Fixed an issue with the WordPress Media Library not loading due to a decoding error in media details sizes. [https://github.com/woocommerce/woocommerce-ios/pull/15056] -- [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031] - [*] Now, usernames and emails in text fields across multiple login views are no longer capitalized. [https://github.com/woocommerce/woocommerce-ios/pull/15002] - [*] Product Details: Display cover tag on the first product image [https://github.com/woocommerce/woocommerce-ios/pull/15041] - [*] Payments: Update learn more links to open Stripe-specific docs when using that gateway [https://github.com/woocommerce/woocommerce-ios/pull/15035] From 9a84317839b6a40bbb14d433f570b6d8a8a9ad22 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Fri, 14 Feb 2025 15:59:29 +0100 Subject: [PATCH 19/19] - Change `RequestAuthenticatorError` enum to public in `RequestAuthenticator.swift` - Import `RequestAuthenticatorError` in `ApplicationPasswordAuthorizationWebViewController.swift` - Update error navigation method name to `navigateToApplicationPasswordDisabledUI` in `ApplicationPasswordAuthorizationWebViewController.swift` - Modify error handling logic to differentiate between `applicationPasswordNotAvailable` and other errors in `ApplicationPasswordAuthorizationWebViewController.swift` --- .../ApplicationPassword/RequestAuthenticator.swift | 2 +- ...ationPasswordAuthorizationWebViewController.swift | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Networking/Networking/ApplicationPassword/RequestAuthenticator.swift b/Networking/Networking/ApplicationPassword/RequestAuthenticator.swift index 8cf26bac1b5..6935c5435da 100644 --- a/Networking/Networking/ApplicationPassword/RequestAuthenticator.swift +++ b/Networking/Networking/ApplicationPassword/RequestAuthenticator.swift @@ -1,6 +1,6 @@ import Foundation -enum RequestAuthenticatorError: Error { +public enum RequestAuthenticatorError: Error { case applicationPasswordUseCaseNotAvailable case applicationPasswordNotAvailable } diff --git a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift index 8297d759ff0..6ac5f99d389 100644 --- a/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift +++ b/WooCommerce/Classes/Authentication/Application Password/ApplicationPasswordAuthorizationWebViewController.swift @@ -3,6 +3,7 @@ import UIKit import WebKit import protocol WooFoundation.Analytics import struct Networking.ApplicationPassword +import enum Networking.RequestAuthenticatorError /// View with embedded web view to authorize application password for a site. /// @@ -162,14 +163,19 @@ private extension ApplicationPasswordAuthorizationWebViewController { guard let url = try await viewModel.fetchAuthURL() else { DDLogError("⛔️ No authorization URL found for application passwords") analytics.track(.applicationPasswordAuthorizationURLNotAvailable) - navigateToErrorUI() + navigateToApplicationPasswordDisabledUI() return } loadAuthorizationPage(url: url) } catch { DDLogError("⛔️ Error fetching authorization URL for application passwords \(error)") analytics.track(.applicationPasswordAuthorizationURLFetchFailed, withError: error) - navigateToErrorUI() + if let authError = error as? Networking.RequestAuthenticatorError, + authError == .applicationPasswordNotAvailable { + navigateToApplicationPasswordDisabledUI() + } else { + showErrorAlert(message: Localization.authorizationRejected) + } } activityIndicator.stopAnimating() } @@ -184,7 +190,7 @@ private extension ApplicationPasswordAuthorizationWebViewController { } } - private func navigateToErrorUI() { + private func navigateToApplicationPasswordDisabledUI() { let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL) // When the error view controller is popped, navigate to previous VC errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem(