Skip to content

Commit 910c01a

Browse files
jaclyncjaclync
authored
Merge pull request #5466 from woocommerce/feat/5364-jcp-workaround
Jetpack CP: workaround to call `wp/v2/settings` and `wc/v3/settings` for JCP sites for accurate site info
2 parents 5922f52 + 15a5aed commit 910c01a

File tree

6 files changed

+341
-15
lines changed

6 files changed

+341
-15
lines changed

‎Networking/Networking/Remote/AccountRemote.swift‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
11
import Combine
22
import Foundation
33

4+
/// Protocol for `AccountRemote` mainly used for mocking.
5+
///
6+
/// The required methods are intentionally incomplete. Feel free to add the other ones.
7+
///
8+
public protocol AccountRemoteProtocol {
9+
func loadAccount(completion: @escaping (Result<Account, Error>) -> Void)
10+
func loadAccountSettings(for userID: Int64, completion: @escaping (Result<AccountSettings, Error>) -> Void)
11+
func updateAccountSettings(for userID: Int64, tracksOptOut: Bool, completion: @escaping (Result<AccountSettings, Error>) -> Void)
12+
func loadSites() -> AnyPublisher<Result<[Site], Error>, Never>
13+
func checkIfWooCommerceIsActive(for siteID: Int64) -> AnyPublisher<Result<Bool, Error>, Never>
14+
func fetchWordPressSiteSettings(for siteID: Int64) -> AnyPublisher<Result<WordPressSiteSettings, Error>, Never>
15+
func loadSitePlan(for siteID: Int64, completion: @escaping (Result<SitePlan, Error>) -> Void)
16+
}
17+
418
/// Account: Remote Endpoints
519
///
6-
public class AccountRemote: Remote {
20+
public class AccountRemote: Remote, AccountRemoteProtocol {
721

822
/// Loads the Account Details associated with the Credential's authToken.
923
///

‎Yosemite/Yosemite.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
029BA557255E0CD4006171FD /* ShippingLabelStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 029BA556255E0CD4006171FD /* ShippingLabelStore.swift */; };
5959
029BA55B255E0D39006171FD /* ShippingLabelAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 029BA55A255E0D39006171FD /* ShippingLabelAction.swift */; };
6060
02A098242480D0D8002F8C7A /* MockCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02A098232480D0D8002F8C7A /* MockCrashLogger.swift */; };
61+
02A26F1E2744FE97008E4EDB /* MockAccountRemote.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02A26F1D2744FE97008E4EDB /* MockAccountRemote.swift */; };
6162
02BA23C222EEEABC009539E7 /* AvailabilityStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C122EEEABC009539E7 /* AvailabilityStore.swift */; };
6263
02BA23C422EEEB3B009539E7 /* AvailabilityAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C322EEEB3B009539E7 /* AvailabilityAction.swift */; };
6364
02BA23C622EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C522EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift */; };
@@ -437,6 +438,7 @@
437438
029BA556255E0CD4006171FD /* ShippingLabelStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelStore.swift; sourceTree = "<group>"; };
438439
029BA55A255E0D39006171FD /* ShippingLabelAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShippingLabelAction.swift; sourceTree = "<group>"; };
439440
02A098232480D0D8002F8C7A /* MockCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockCrashLogger.swift; sourceTree = "<group>"; };
441+
02A26F1D2744FE97008E4EDB /* MockAccountRemote.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockAccountRemote.swift; sourceTree = "<group>"; };
440442
02BA23C122EEEABC009539E7 /* AvailabilityStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AvailabilityStore.swift; sourceTree = "<group>"; };
441443
02BA23C322EEEB3B009539E7 /* AvailabilityAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AvailabilityAction.swift; sourceTree = "<group>"; };
442444
02BA23C522EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsV4AvailabilityStoreTests.swift; sourceTree = "<group>"; };
@@ -1077,6 +1079,7 @@
10771079
020C908024C7D71D001E2BEB /* MockProductVariationsRemote.swift */,
10781080
02E7FFD82562234F00C53030 /* MockShippingLabelRemote.swift */,
10791081
D4CBAE5F26D440FA00BBE6D1 /* MockAnnouncementsRemote.swift */,
1082+
02A26F1D2744FE97008E4EDB /* MockAccountRemote.swift */,
10801083
);
10811084
path = Remote;
10821085
sourceTree = "<group>";
@@ -1990,6 +1993,7 @@
19901993
02A098242480D0D8002F8C7A /* MockCrashLogger.swift in Sources */,
19911994
02FF055F23D985710058E6E7 /* URL+MediaTests.swift in Sources */,
19921995
D87F27DB25E7E8EA006EC8C9 /* MockCardReader.swift in Sources */,
1996+
02A26F1E2744FE97008E4EDB /* MockAccountRemote.swift in Sources */,
19931997
02124DAC24318D6B00980D74 /* Media+MediaTypeTests.swift in Sources */,
19941998
025CA2D0238F54E800B05C81 /* ProductShippingClassStoreTests.swift in Sources */,
19951999
74A7688E20D45ED400F9D437 /* OrderStoreTests.swift in Sources */,

‎Yosemite/Yosemite/Model/Storage/Site+ReadOnlyConvertible.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ extension Storage.Site: ReadOnlyConvertible {
1414
tagline = site.description
1515
url = site.url
1616
// plan = site.plan // We're not assigning the plan here because it's not sent on the intial API request.
17-
// TODO: 5364 - update `isJetpackThePluginInstalled`
18-
// TODO: 5364 - update `isJetpackConnected`
17+
isJetpackThePluginInstalled = site.isJetpackThePluginInstalled
18+
isJetpackConnected = site.isJetpackConnected
1919
isWooCommerceActive = NSNumber(booleanLiteral: site.isWooCommerceActive)
2020
isWordPressStore = NSNumber(booleanLiteral: site.isWordPressStore)
2121
timezone = site.timezone
@@ -30,8 +30,8 @@ extension Storage.Site: ReadOnlyConvertible {
3030
description: tagline ?? "",
3131
url: url ?? "",
3232
plan: plan ?? "",
33-
isJetpackThePluginInstalled: true, // TODO: 5364 - persist in storage
34-
isJetpackConnected: true, // TODO: 5364 - persist in storage
33+
isJetpackThePluginInstalled: isJetpackThePluginInstalled,
34+
isJetpackConnected: isJetpackConnected,
3535
isWooCommerceActive: isWooCommerceActive?.boolValue ?? false,
3636
isWordPressStore: isWordPressStore?.boolValue ?? false,
3737
timezone: timezone ?? "",

‎Yosemite/Yosemite/Stores/AccountStore.swift‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import Storage
77
// MARK: - AccountStore
88
//
99
public class AccountStore: Store {
10-
private let remote: AccountRemote
10+
private let remote: AccountRemoteProtocol
1111
private var cancellables = Set<AnyCancellable>()
1212

1313
/// Shared private StorageType for use during synchronizeSites and synchronizeSitePlan processes
@@ -21,7 +21,7 @@ public class AccountStore: Store {
2121
super.init(dispatcher: dispatcher, storageManager: storageManager, network: network)
2222
}
2323

24-
public init(dispatcher: Dispatcher, storageManager: StorageManagerType, network: Network, remote: AccountRemote) {
24+
public init(dispatcher: Dispatcher, storageManager: StorageManagerType, network: Network, remote: AccountRemoteProtocol) {
2525
self.remote = remote
2626
super.init(dispatcher: dispatcher, storageManager: storageManager, network: network)
2727
}
@@ -110,6 +110,43 @@ private extension AccountStore {
110110
///
111111
func synchronizeSites(selectedSiteID: Int64?, onCompletion: @escaping (Result<Void, Error>) -> Void) {
112112
remote.loadSites()
113+
.flatMap { result -> AnyPublisher<Result<[Site], Error>, Never> in
114+
switch result {
115+
case .success(let sites):
116+
return sites.publisher.flatMap { [weak self] site -> AnyPublisher<Site, Never> in
117+
let sitePublisher = Just<Site>(site).eraseToAnyPublisher()
118+
guard let self = self else {
119+
return sitePublisher
120+
}
121+
122+
// If a site is connected to Jetpack via Jetpack Connection Package, some information about the site is unavailable or inaccurate
123+
// in `me/sites` endpoint made in `AccountRemote.loadSites`.
124+
// As a workaround, we need to make 2 other API requests (ref p91TBi-6lK-p2):
125+
// - Check if WooCommerce plugin is active via `wc/v3/settings` endpoint
126+
// - Fetch site metadata like the site name, description, and URL via `wp/v2/settings` endpoint
127+
if site.isJetpackCPConnected {
128+
let wcAvailabilityPublisher = self.remote.checkIfWooCommerceIsActive(for: site.siteID)
129+
let wpSiteSettingsPublisher = self.remote.fetchWordPressSiteSettings(for: site.siteID)
130+
return Publishers.Zip3(sitePublisher, wcAvailabilityPublisher, wpSiteSettingsPublisher)
131+
.map {
132+
(site, isWooCommerceActiveResult, wpSiteSettingsResult) -> Site in
133+
var site = site
134+
if case let .success(isWooCommerceActive) = isWooCommerceActiveResult {
135+
site = site.copy(isWooCommerceActive: isWooCommerceActive)
136+
}
137+
if case let .success(wpSiteSettings) = wpSiteSettingsResult {
138+
site = site.copy(name: wpSiteSettings.name, description: wpSiteSettings.description, url: wpSiteSettings.url)
139+
}
140+
return site
141+
}.eraseToAnyPublisher()
142+
} else {
143+
return sitePublisher
144+
}
145+
}.collect().map { .success($0) }.eraseToAnyPublisher()
146+
case .failure:
147+
return Just<Result<[Site], Error>>(result).eraseToAnyPublisher()
148+
}
149+
}
113150
.sink { [weak self] result in
114151
switch result {
115152
case .success(let sites):

‎Yosemite/YosemiteTests/Mocks/Networking/Remote/MockAccountRemote.swift‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import Combine
2+
import Foundation
3+
import Networking
4+
5+
import XCTest
6+
7+
/// Mock for `AccountRemote`.
8+
///
9+
final class MockAccountRemote {
10+
/// Returns the value as a publisher when `loadSites` is called.
11+
var loadSitesResult: Result<[Site], Error> = .success([])
12+
13+
/// Returns the requests that have been made to `AccountRemoteProtocol`.
14+
var invocations = [Invocation]()
15+
16+
/// The results to return based on the given site ID in `checkIfWooCommerceIsActive`
17+
private var checkIfWooCommerceIsActiveResultsBySiteID = [Int64: Result<Bool, Error>]()
18+
19+
/// The results to return based on the given site ID in `fetchWordPressSiteSettings`
20+
private var fetchWordPressSiteSettingsResultsBySiteID = [Int64: Result<WordPressSiteSettings, Error>]()
21+
22+
/// Returns the value as a publisher when `checkIfWooCommerceIsActive` is called.
23+
func whenCheckingIfWooCommerceIsActive(siteID: Int64, thenReturn result: Result<Bool, Error>) {
24+
checkIfWooCommerceIsActiveResultsBySiteID[siteID] = result
25+
}
26+
27+
/// Returns the value as a publisher when `fetchWordPressSiteSettings` is called.
28+
func whenFetchingWordPressSiteSettings(siteID: Int64, thenReturn result: Result<WordPressSiteSettings, Error>) {
29+
fetchWordPressSiteSettingsResultsBySiteID[siteID] = result
30+
}
31+
}
32+
33+
extension MockAccountRemote {
34+
enum Invocation: Equatable {
35+
case loadSites
36+
case checkIfWooCommerceIsActive(siteID: Int64)
37+
case fetchWordPressSiteSettings(siteID: Int64)
38+
}
39+
}
40+
41+
// MARK: - AccountRemoteProtocol
42+
43+
extension MockAccountRemote: AccountRemoteProtocol {
44+
func loadAccount(completion: @escaping (Result<Account, Error>) -> Void) {
45+
// no-op
46+
}
47+
48+
func loadAccountSettings(for userID: Int64, completion: @escaping (Result<AccountSettings, Error>) -> Void) {
49+
// no-op
50+
}
51+
52+
func updateAccountSettings(for userID: Int64, tracksOptOut: Bool, completion: @escaping (Result<AccountSettings, Error>) -> Void) {
53+
// no-op
54+
}
55+
56+
func loadSites() -> AnyPublisher<Result<[Site], Error>, Never> {
57+
invocations.append(.loadSites)
58+
return Just<Result<[Site], Error>>(loadSitesResult).eraseToAnyPublisher()
59+
}
60+
61+
func checkIfWooCommerceIsActive(for siteID: Int64) -> AnyPublisher<Result<Bool, Error>, Never> {
62+
invocations.append(.checkIfWooCommerceIsActive(siteID: siteID))
63+
if let result = checkIfWooCommerceIsActiveResultsBySiteID[siteID] {
64+
return Just<Result<Bool, Error>>(result).eraseToAnyPublisher()
65+
} else {
66+
XCTFail("\(String(describing: self)) Could not find result for site ID: \(siteID)")
67+
return Empty<Result<Bool, Error>, Never>().eraseToAnyPublisher()
68+
}
69+
}
70+
71+
func fetchWordPressSiteSettings(for siteID: Int64) -> AnyPublisher<Result<WordPressSiteSettings, Error>, Never> {
72+
invocations.append(.fetchWordPressSiteSettings(siteID: siteID))
73+
if let result = fetchWordPressSiteSettingsResultsBySiteID[siteID] {
74+
return Just<Result<WordPressSiteSettings, Error>>(result).eraseToAnyPublisher()
75+
} else {
76+
XCTFail("\(String(describing: self)) Could not find result for site ID: \(siteID)")
77+
return Empty<Result<WordPressSiteSettings, Error>, Never>().eraseToAnyPublisher()
78+
}
79+
}
80+
81+
func loadSitePlan(for siteID: Int64, completion: @escaping (Result<SitePlan, Error>) -> Void) {
82+
// no-op
83+
}
84+
}

0 commit comments

Comments
 (0)