Skip to content

Commit a1acb44

Browse files
itsmeichigoitsmeichigo
authored
Merge pull request #8478 from woocommerce/feat/8453-role-eligibility
REST API: Check role eligibility after authentication without WPCom
2 parents 8a659f0 + 892cad8 commit a1acb44

File tree

15 files changed

+181
-77
lines changed

15 files changed

+181
-77
lines changed

‎Networking/Networking.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,7 @@
721721
DEC51AF92769A212009F3DF4 /* SystemStatus+Settings.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEC51AF82769A212009F3DF4 /* SystemStatus+Settings.swift */; };
722722
DEC51AFB2769C66B009F3DF4 /* SystemStatusMapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEC51AFA2769C66B009F3DF4 /* SystemStatusMapperTests.swift */; };
723723
DEC51B02276AFB35009F3DF4 /* SystemStatus+DropinMustUsePlugin.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEC51B01276AFB34009F3DF4 /* SystemStatus+DropinMustUsePlugin.swift */; };
724+
DEF13C5029629EEA0024A02B /* user-complete-without-data.json in Resources */ = {isa = PBXBuildFile; fileRef = DEF13C4F29629EEA0024A02B /* user-complete-without-data.json */; };
724725
DEFBA74E29485A7600C35BA9 /* RESTRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEFBA74D29485A7600C35BA9 /* RESTRequest.swift */; };
725726
DEFBA7542949CE6600C35BA9 /* RequestProcessor.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEFBA7532949CE6600C35BA9 /* RequestProcessor.swift */; };
726727
DEFBA7562949D17400C35BA9 /* DefaultRequestAuthenticatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEFBA7552949D17300C35BA9 /* DefaultRequestAuthenticatorTests.swift */; };
@@ -1513,6 +1514,7 @@
15131514
DEC51AF82769A212009F3DF4 /* SystemStatus+Settings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "SystemStatus+Settings.swift"; sourceTree = "<group>"; };
15141515
DEC51AFA2769C66B009F3DF4 /* SystemStatusMapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SystemStatusMapperTests.swift; sourceTree = "<group>"; };
15151516
DEC51B01276AFB34009F3DF4 /* SystemStatus+DropinMustUsePlugin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "SystemStatus+DropinMustUsePlugin.swift"; sourceTree = "<group>"; };
1517+
DEF13C4F29629EEA0024A02B /* user-complete-without-data.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "user-complete-without-data.json"; sourceTree = "<group>"; };
15161518
DEFBA74D29485A7600C35BA9 /* RESTRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RESTRequest.swift; sourceTree = "<group>"; };
15171519
DEFBA7532949CE6600C35BA9 /* RequestProcessor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RequestProcessor.swift; sourceTree = "<group>"; };
15181520
DEFBA7552949D17300C35BA9 /* DefaultRequestAuthenticatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultRequestAuthenticatorTests.swift; sourceTree = "<group>"; };
@@ -2296,6 +2298,7 @@
22962298
74ABA1C6213F19FD00FFAD30 /* top-performers-month.json */,
22972299
74ABA1C7213F19FE00FFAD30 /* top-performers-year.json */,
22982300
FE28F6E726842D57004465C7 /* user-complete.json */,
2301+
DEF13C4F29629EEA0024A02B /* user-complete-without-data.json */,
22992302
7495AACE225D366D00801A89 /* variation-as-product.json */,
23002303
02A26F192744F5FC008E4EDB /* wc-site-settings-partial.json */,
23012304
D800DA0D25EFEC21001E13CE /* wcpay-connection-token.json */,
@@ -3087,6 +3090,7 @@
30873090
CC80E3F92948C8BC00D5FF45 /* site-visits-quarter.json in Resources */,
30883091
020D07C223D858BB00FD9580 /* media-upload.json in Resources */,
30893092
31A451D127863A2E00FE81AA /* stripe-account-rejected-other.json in Resources */,
3093+
DEF13C5029629EEA0024A02B /* user-complete-without-data.json in Resources */,
30903094
74ABA1CA213F19FE00FFAD30 /* top-performers-year.json in Resources */,
30913095
31A451CF27863A2E00FE81AA /* stripe-account-rejected-terms-of-service.json in Resources */,
30923096
DE2E8EA9295416C9002E4B14 /* wordpress-site-info.json in Resources */,

‎Networking/Networking/Mapper/UserMapper.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ struct UserMapper: Mapper {
1616
.siteID: siteID
1717
]
1818

19-
return try decoder.decode(UserEnvelope.self, from: response).user
19+
do {
20+
return try decoder.decode(UserEnvelope.self, from: response).user
21+
} catch {
22+
return try decoder.decode(User.self, from: response)
23+
}
2024
}
2125
}
2226

‎Networking/Networking/Remote/UserRemote.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public final class UserRemote: Remote {
1414
"context": "edit",
1515
"fields": "id,username,id_wpcom,email,first_name,last_name,nickname,roles"
1616
]
17-
let request = JetpackRequest(wooApiVersion: .none, method: .get, siteID: siteID, path: path, parameters: parameters)
17+
let request = JetpackRequest(wooApiVersion: .none, method: .get, siteID: siteID, path: path, parameters: parameters, availableAsRESTRequest: true)
1818
let mapper = UserMapper(siteID: siteID)
1919

2020
enqueue(request, mapper: mapper, completion: completion)

‎Networking/Networking/Requests/RESTRequest.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ struct RESTRequest: URLRequestConvertible {
4747
/// Returns a URLRequest instance representing the current REST API Request.
4848
///
4949
func asURLRequest() throws -> URLRequest {
50-
let components = [siteURL, Settings.basePath, wooApiVersion.path, path].map { $0.trimSlashes() }
50+
let components = [siteURL, Settings.basePath, wooApiVersion.path, path]
51+
.map { $0.trimSlashes() }
52+
.filter { $0.isEmpty == false }
5153
let url = try components.joined(separator: "/").asURL()
5254
let request = try URLRequest(url: url, method: method)
5355
switch method {

‎Networking/NetworkingTests/Mapper/UserMapperTests.swift‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,24 @@ import XCTest
77
final class UserMapperTests: XCTestCase {
88
private let testSiteID: Int64 = 123
99

10-
func test_User_fields_are_properly_parsed() {
11-
guard let user = mapUserFromMockResponse() else {
10+
func test_User_fields_are_properly_parsed_with_data_envelope() {
11+
guard let user = mapUserFromMockResponseWithDataEnvelope() else {
12+
XCTFail()
13+
return
14+
}
15+
16+
XCTAssertEqual(user.localID, 1)
17+
XCTAssertEqual(user.username, "test-username")
18+
XCTAssertEqual(user.firstName, "Test")
19+
XCTAssertEqual(user.lastName, "User")
20+
XCTAssertEqual(user.email, "[email protected]")
21+
XCTAssertEqual(user.nickname, "User's Nickname")
22+
XCTAssertEqual(user.roles, ["administrator"])
23+
XCTAssertEqual(user.siteID, testSiteID)
24+
}
25+
26+
func test_User_fields_are_properly_parsed_without_data_envelope() {
27+
guard let user = mapUserFromMockResponseWithoutDataEnvelope() else {
1228
XCTFail()
1329
return
1430
}
@@ -25,7 +41,7 @@ final class UserMapperTests: XCTestCase {
2541
}
2642

2743
private extension UserMapperTests {
28-
func mapUserFromMockResponse() -> User? {
44+
func mapUserFromMockResponseWithDataEnvelope() -> User? {
2945
// Note: the JSON content is shortened due to the "fields" parameter
3046
// usage in UserRemote.
3147
guard let response = Loader.contentsOf("user-complete") else {
@@ -34,4 +50,14 @@ private extension UserMapperTests {
3450

3551
return try? UserMapper(siteID: testSiteID).map(response: response)
3652
}
53+
54+
func mapUserFromMockResponseWithoutDataEnvelope() -> User? {
55+
// Note: the JSON content is shortened due to the "fields" parameter
56+
// usage in UserRemote.
57+
guard let response = Loader.contentsOf("user-complete-without-data") else {
58+
return nil
59+
}
60+
61+
return try? UserMapper(siteID: testSiteID).map(response: response)
62+
}
3763
}

‎Networking/NetworkingTests/Responses/user-complete-without-data.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"id": 1,
3+
"username": "test-username",
4+
"first_name": "Test",
5+
"last_name": "User",
6+
"email": "[email protected]",
7+
"nickname": "User's Nickname",
8+
"roles": [
9+
"administrator"
10+
]
11+
}

‎WooCommerce/Classes/Authentication/AuthenticationManager.swift‎

Lines changed: 85 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ class AuthenticationManager: Authentication {
5151
/// Keep strong reference of the use case to check for application password availability if necessary.
5252
private var applicationPasswordUseCase: ApplicationPasswordUseCase?
5353

54+
/// Keep strong reference of the use case to check for role eligibility if necessary.
55+
private lazy var roleEligibilityUseCase: RoleEligibilityUseCase = {
56+
.init(stores: ServiceLocator.stores)
57+
}()
58+
5459
init(storageManager: StorageManagerType = ServiceLocator.storageManager,
5560
featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService,
5661
analytics: Analytics = ServiceLocator.analytics) {
@@ -829,9 +834,12 @@ private extension AuthenticationManager {
829834
with: applicationPasswordUseCase,
830835
in: navigationController) { [weak self] in
831836
guard let self else { return }
832-
// TODO: check for role eligibility & check for Woo
833-
// navigates to home screen immediately with a placeholder store ID
834-
self.startStorePicker(with: WooConstants.placeholderStoreID, in: navigationController)
837+
self.checkRoleEligibility(in: navigationController) { [weak self] in
838+
guard let self else { return }
839+
// TODO: check for Woo
840+
// navigates to home screen immediately with a placeholder store ID
841+
self.startStorePicker(with: WooConstants.placeholderStoreID, in: navigationController)
842+
}
835843
}
836844
}
837845

@@ -854,17 +862,85 @@ private extension AuthenticationManager {
854862
// show generic error
855863
await MainActor.run {
856864
DDLogError("⛔️ Error generating application password: \(error)")
857-
let alert = FancyAlertViewController.makeApplicationPasswordAlert(retryAction: { [weak self] in
858-
self?.checkApplicationPassword(for: siteURL, with: useCase, in: navigationController, onSuccess: onSuccess)
859-
}, restartLoginAction: {
860-
ServiceLocator.stores.deauthenticate()
861-
navigationController.popToRootViewController(animated: true)
862-
})
865+
let alert = FancyAlertViewController.makeSiteCredentialLoginAlert(
866+
message: Localization.applicationPasswordError,
867+
retryAction: { [weak self] in
868+
self?.checkApplicationPassword(for: siteURL, with: useCase, in: navigationController, onSuccess: onSuccess)
869+
},
870+
restartLoginAction: {
871+
ServiceLocator.stores.deauthenticate()
872+
navigationController.popToRootViewController(animated: true)
873+
}
874+
)
875+
navigationController.present(alert, animated: true)
876+
}
877+
}
878+
}
879+
}
880+
881+
/// Checks role eligibility for the logged in user with the site address saved in the credentials.
882+
/// Placeholder store ID is used because we are checking for users logging in with site credentials.
883+
///
884+
func checkRoleEligibility(in navigationController: UINavigationController, onSuccess: @escaping () -> Void) {
885+
roleEligibilityUseCase.checkEligibility(for: WooConstants.placeholderStoreID) { [weak self] result in
886+
guard let self else { return }
887+
switch result {
888+
case .success:
889+
onSuccess()
890+
case .failure(let error):
891+
if case let RoleEligibilityError.insufficientRole(errorInfo) = error {
892+
self.showRoleErrorScreen(for: WooConstants.placeholderStoreID,
893+
errorInfo: errorInfo,
894+
in: navigationController,
895+
onSuccess: onSuccess)
896+
} else {
897+
// show generic error
898+
DDLogError("⛔️ Error checking role eligibility: \(error)")
899+
let alert = FancyAlertViewController.makeSiteCredentialLoginAlert(
900+
message: Localization.roleEligibilityCheckError,
901+
retryAction: { [weak self] in
902+
self?.checkRoleEligibility(in: navigationController, onSuccess: onSuccess)
903+
},
904+
restartLoginAction: {
905+
ServiceLocator.stores.deauthenticate()
906+
navigationController.popToRootViewController(animated: true)
907+
}
908+
)
863909
navigationController.present(alert, animated: true)
864910
}
865911
}
866912
}
867913
}
914+
915+
/// Shows a Role Error page using the provided error information.
916+
///
917+
func showRoleErrorScreen(for siteID: Int64,
918+
errorInfo: StorageEligibilityErrorInfo,
919+
in navigationController: UINavigationController,
920+
onSuccess: @escaping () -> Void) {
921+
let errorViewModel = RoleErrorViewModel(siteID: siteID, title: errorInfo.name, subtitle: errorInfo.humanizedRoles, useCase: self.roleEligibilityUseCase)
922+
let errorViewController = RoleErrorViewController(viewModel: errorViewModel)
923+
924+
errorViewModel.onSuccess = onSuccess
925+
errorViewModel.onDeauthenticationRequest = {
926+
ServiceLocator.stores.deauthenticate()
927+
navigationController.popToRootViewController(animated: true)
928+
}
929+
navigationController.show(errorViewController, sender: self)
930+
}
931+
}
932+
933+
private extension AuthenticationManager {
934+
enum Localization {
935+
static let applicationPasswordError = NSLocalizedString(
936+
"Error fetching application password for your site.",
937+
comment: "Error message displayed when application password cannot be fetched after authentication."
938+
)
939+
static let roleEligibilityCheckError = NSLocalizedString(
940+
"Error fetching user information.",
941+
comment: "Error message displayed when user information cannot be fetched after authentication."
942+
)
943+
}
868944
}
869945

870946
// MARK: - ViewModel Factory

‎WooCommerce/Classes/Authentication/Navigation Exceptions/RoleErrorViewController.swift‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import WordPressAuthenticator
55
/// RoleErrorOutput enables communication from the view model to the view controller.
66
/// Note that it's important for the view model to weakly retain the view controller.
77
protocol RoleErrorOutput: AnyObject {
8+
func updatePrimaryButtonState(loading: Bool)
9+
810
/// Updates title and subtitle label text with latest content.
911
func refreshTitleLabels()
1012

@@ -26,7 +28,7 @@ class RoleErrorViewController: UIViewController {
2628
@IBOutlet weak var descriptionLabel: UILabel!
2729
@IBOutlet weak var linkButton: UIButton!
2830

29-
@IBOutlet weak var primaryActionButton: UIButton!
31+
@IBOutlet weak var primaryActionButton: ButtonActivityIndicator!
3032
@IBOutlet weak var secondaryActionButton: UIButton!
3133

3234
// MARK: Properties
@@ -111,6 +113,7 @@ class RoleErrorViewController: UIViewController {
111113

112114
func configurePrimaryActionButton() {
113115
primaryActionButton.applyPrimaryButtonStyle()
116+
primaryActionButton.indicator.color = .white
114117
primaryActionButton.setTitle(viewModel.primaryButtonTitle, for: .normal)
115118
primaryActionButton.on(.touchUpInside) { [weak self] _ in
116119
self?.viewModel.didTapPrimaryButton()
@@ -170,6 +173,14 @@ class RoleErrorViewController: UIViewController {
170173
// MARK: - RoleErrorOutput
171174

172175
extension RoleErrorViewController: RoleErrorOutput {
176+
func updatePrimaryButtonState(loading: Bool) {
177+
if loading {
178+
primaryActionButton.showActivityIndicator()
179+
} else {
180+
primaryActionButton.hideActivityIndicator()
181+
}
182+
}
183+
173184
func refreshTitleLabels() {
174185
// these are the only components that may change on runtime, e.g. the role changed
175186
// to something else, but still incorrect; or the user updated their display name.

0 commit comments

Comments
 (0)