Skip to content

Commit 62ec3c5

Browse files
staskusstaskus
committed
Clarify customerID vs userID naming in CustomerStore
1 parent b2c680e commit 62ec3c5

File tree

11 files changed

+52
-110
lines changed

11 files changed

+52
-110
lines changed

‎Modules/Sources/Networking/Remote/CustomerRemote.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ public class CustomerRemote: Remote {
44
/// Retrieves a `Customer`
55
///
66
/// - Parameters:
7-
/// - customerID: ID of the registered WordPress user (customer) that will be retrieved.
7+
/// - userID: ID of the registered WordPress user (customer) that will be retrieved.
88
/// - siteID: Site for which we'll fetch the customer.
99
/// - completion: Closure to be executed upon completion.
1010
///
11-
public func retrieveCustomer(for siteID: Int64, with customerID: Int64, completion: @escaping (Result<Customer, Error>) -> Void) {
12-
let path = "customers/\(customerID)"
11+
public func retrieveCustomer(for siteID: Int64, with userID: Int64, completion: @escaping (Result<Customer, Error>) -> Void) {
12+
let path = "customers/\(userID)"
1313
let request = JetpackRequest(wooApiVersion: .mark3,
1414
method: .get,
1515
siteID: siteID,

‎Modules/Sources/Storage/Tools/StorageType+Extensions.swift‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -767,22 +767,22 @@ public extension StorageType {
767767

768768
// MARK: - Customers
769769

770-
/// Returns a single Customer given a `siteID` and `customerID`
771-
///
772-
func loadCustomer(siteID: Int64, customerID: Int64) -> Customer? {
773-
let predicate = \Customer.siteID == siteID && \Customer.customerID == customerID
774-
return firstObject(ofType: Customer.self, matching: predicate)
775-
}
776-
777770
func loadAllCustomers(siteID: Int64) -> [Customer] {
778771
let predicate = \Customer.siteID == siteID
779772
return allObjects(ofType: Customer.self, matching: predicate, sortedBy: [])
780773
}
781774

775+
/// Returns stored Customers given a `siteID` matching `userIDs`
776+
///
777+
func loadCustomers(siteID: Int64, matchingUserIDs userIDs: [Int64]) -> [Customer] {
778+
let predicate = NSPredicate(format: "siteID == %lld && userID in %@", siteID, userIDs)
779+
return allObjects(ofType: Customer.self, matching: predicate, sortedBy: [])
780+
}
781+
782782
/// Returns stored Customers given a `siteID` matching `customerIDs`
783783
///
784-
func loadCustomers(siteID: Int64, matching customerIDs: [Int64]) -> [Customer] {
785-
let predicate = NSPredicate(format: "siteID == %lld && customerID in %@", siteID, customerIDs)
784+
func loadCustomers(siteID: Int64, matchingCustomerIDs userIDs: [Int64]) -> [Customer] {
785+
let predicate = NSPredicate(format: "siteID == %lld && customerID in %@", siteID, userIDs)
786786
return allObjects(ofType: Customer.self, matching: predicate, sortedBy: [])
787787
}
788788

‎Modules/Sources/Yosemite/Actions/CustomerAction.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,14 @@ public enum CustomerAction: Action {
8585
/// Retrieves a single Customer from a site
8686
///
8787
///- `siteID`: The site for which customers should be fetched.
88-
///- `customerID`: ID of the Customer to be fetched.
88+
///- `customerID`: ID of the registered WordPress user (customer) that will be retrieved.
89+
8990
///- `onCompletion`: Invoked when the operation finishes.
9091
/// - `result.success(Customer)`: The Customer object
9192
/// - `result.failure(Error)`: Error fetching Customer
9293
case retrieveCustomer(
9394
siteID: Int64,
94-
customerID: Int64,
95+
userID: Int64,
9596
onCompletion: (Result<Customer, Error>) -> Void)
9697

9798

‎Modules/Sources/Yosemite/Model/Storage/Customer+ReadOnlyConvertible.swift‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,12 @@ import Storage
77
///
88
///
99
public protocol StorageCustomerConvertible {
10-
var loadingID: Int64 { get }
10+
var userID: Int64 { get }
11+
var customerID: Int64 { get }
1112
}
1213

13-
extension Yosemite.Customer: StorageCustomerConvertible {
14-
public var loadingID: Int64 { customerID }
15-
}
16-
17-
extension Yosemite.WCAnalyticsCustomer: StorageCustomerConvertible {
18-
public var loadingID: Int64 { customerID }
19-
}
14+
extension Yosemite.Customer: StorageCustomerConvertible {}
15+
extension Yosemite.WCAnalyticsCustomer: StorageCustomerConvertible {}
2016

2117
// MARK: - Storage.Customer: ReadOnlyConvertible
2218
//

‎Modules/Sources/Yosemite/Stores/CustomerStore.swift‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public final class CustomerStore: Store {
6666
onCompletion: onCompletion)
6767
case let .searchWCAnalyticsCustomers(siteID, pageNumber, pageSize, keyword, filter, onCompletion):
6868
searchWCAnalyticsCustomers(for: siteID, pageNumber: pageNumber, pageSize: pageSize, keyword: keyword, filter: filter, onCompletion: onCompletion)
69-
case .retrieveCustomer(siteID: let siteID, customerID: let customerID, onCompletion: let onCompletion):
70-
retrieveCustomer(for: siteID, with: customerID, onCompletion: onCompletion)
69+
case .retrieveCustomer(siteID: let siteID, userID: let userID, onCompletion: let onCompletion):
70+
retrieveCustomer(for: siteID, with: userID, onCompletion: onCompletion)
7171
case let .synchronizeLightCustomersData(siteID, pageNumber, pageSize, orderby, order, filterEmpty, onCompletion):
7272
synchronizeLightCustomersData(siteID: siteID,
7373
pageNumber: pageNumber,
@@ -174,14 +174,14 @@ public final class CustomerStore: Store {
174174
///
175175
/// - Parameters:
176176
/// - siteID: The site for which customers should be fetched.
177-
/// - customerID: ID of the Customer to be fetched.
177+
/// - customerID: ID of the registered WordPress user (customer) that will be retrieved.
178178
/// - onCompletion: Invoked when the operation finishes. Will upsert the Customer to Storage, or return an Error.
179179
///
180180
func retrieveCustomer(
181181
for siteID: Int64,
182-
with customerID: Int64,
182+
with userID: Int64,
183183
onCompletion: @escaping (Result<Customer, Error>) -> Void) {
184-
customerRemote.retrieveCustomer(for: siteID, with: customerID) { [weak self] result in
184+
customerRemote.retrieveCustomer(for: siteID, with: userID) { [weak self] result in
185185
guard let self else { return }
186186
switch result {
187187
case .success(let customer):
@@ -267,7 +267,7 @@ public final class CustomerStore: Store {
267267
let group = DispatchGroup()
268268
for result in searchResults {
269269
// At the moment, we're not searching through non-registered customers
270-
// As we only search by customer ID, calls to /wc/v3/customers/0 will always fail
270+
// As we only search by user ID, calls to /wc/v3/customers/0 will always fail
271271
// https://github.com/woocommerce/woocommerce-ios/issues/7741
272272
if result.userID == 0 {
273273
continue
@@ -282,7 +282,7 @@ public final class CustomerStore: Store {
282282
}
283283

284284
group.notify(queue: .main) {
285-
self.upsertSearchCustomerResult(
285+
self.upsertRegisteredSearchCustomerResult(
286286
siteID: siteID,
287287
keyword: keyword,
288288
readOnlyCustomers: customers,
@@ -297,21 +297,23 @@ public final class CustomerStore: Store {
297297
// MARK: Storage operations
298298
private extension CustomerStore {
299299
/// Inserts or updates CustomerSearchResults in Storage
300+
/// Only used in CommandSearchUICommand when .betterCustomerSelectionInOrder feature flag is disabled
301+
/// Likely could be removed
300302
///
301-
private func upsertSearchCustomerResult(siteID: Int64,
302-
keyword: String,
303-
readOnlyCustomers: [Networking.Customer],
304-
onCompletion: @escaping () -> Void) {
303+
private func upsertRegisteredSearchCustomerResult(siteID: Int64,
304+
keyword: String,
305+
readOnlyCustomers: [Networking.Customer],
306+
onCompletion: @escaping () -> Void) {
305307
storageManager.performAndSave({ storage in
306308
let storedSearchResult = storage.loadCustomerSearchResult(siteID: siteID, keyword: keyword) ??
307309
storage.insertNewObject(ofType: Storage.CustomerSearchResult.self)
308310

309311
storedSearchResult.siteID = siteID
310312
storedSearchResult.keyword = keyword
311313

312-
let storedCustomers = storage.loadCustomers(siteID: siteID, matching: readOnlyCustomers.map { $0.customerID })
314+
let storedCustomers = storage.loadCustomers(siteID: siteID, matchingUserIDs: readOnlyCustomers.map { $0.userID })
313315
for result in readOnlyCustomers {
314-
if let storedCustomer = storedCustomers.first(where: { $0.customerID == result.customerID }) {
316+
if let storedCustomer = storedCustomers.first(where: { $0.userID == result.userID }) {
315317
storedSearchResult.addToCustomers(storedCustomer)
316318
}
317319
}
@@ -328,7 +330,7 @@ private extension CustomerStore {
328330
storage.deleteCustomers(siteID: siteID)
329331
}
330332

331-
let storedCustomers = storage.loadCustomers(siteID: siteID, matching: readOnlyCustomers.map { $0.loadingID })
333+
let storedCustomers = storage.loadCustomers(siteID: siteID, matchingCustomerIDs: readOnlyCustomers.map { $0.customerID })
332334
let storedSearchResult: CustomerSearchResult? = {
333335
guard let keyword else {
334336
return nil
@@ -395,11 +397,11 @@ private extension CustomerStore {
395397
storedSearchResult: Storage.CustomerSearchResult?,
396398
in storage: StorageType) {
397399
let storageCustomer: Storage.Customer = {
398-
// If the specific customerID for that siteID already exists, return it
399-
// If doesn't or the user is unregistered (loadingID == 0), insert a new one in Storage
400+
// If the specific userId for that siteID already exists, return it
401+
// If doesn't or the user is unregistered (userId == 0), insert a new one in Storage
400402
// Since we reset the customers everytime we request them, there's no risk of having duplicated unregistered customers
401-
if readOnlyCustomer.loadingID != 0,
402-
let storedCustomer = storedCustomers.first(where: { $0.customerID == readOnlyCustomer.loadingID }) {
403+
if readOnlyCustomer.userID != 0,
404+
let storedCustomer = storedCustomers.first(where: { $0.userID == readOnlyCustomer.userID }) {
403405
return storedCustomer
404406
} else {
405407
return storage.insertNewObject(ofType: Storage.Customer.self)

‎Modules/Tests/NetworkingTests/Mapper/CustomerMapperTests.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ class CustomerMapperTests: XCTestCase {
4444

4545
// Then
4646
XCTAssertNotNil(customer)
47-
XCTAssertEqual(customer.customerID, 25)
47+
XCTAssertEqual(customer.userID, 25)
48+
XCTAssertEqual(customer.customerID, 0)
4849
XCTAssertEqual(customer.email, "[email protected]")
4950
XCTAssertEqual(customer.firstName, "John")
5051
XCTAssertEqual(customer.lastName, "Doe")
@@ -76,7 +77,8 @@ class CustomerMapperTests: XCTestCase {
7677

7778
// Then
7879
XCTAssertNotNil(customer)
79-
XCTAssertEqual(customer.customerID, 25)
80+
XCTAssertEqual(customer.userID, 25)
81+
XCTAssertEqual(customer.customerID, 0)
8082
XCTAssertEqual(customer.email, "[email protected]")
8183
XCTAssertEqual(customer.firstName, "John")
8284
XCTAssertEqual(customer.lastName, "Doe")

‎Modules/Tests/StorageTests/Tools/StorageTypeExtensionsTests.swift‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -175,41 +175,6 @@ final class StorageTypeExtensionsTests: XCTestCase {
175175
XCTAssertEqual(coupon, storedCoupon)
176176
}
177177

178-
func test_loadCustomer_by_siteID_and_customerID() throws {
179-
// Given
180-
let customerID: Int64 = 123
181-
let customer = storage.insertNewObject(ofType: Customer.self)
182-
customer.siteID = sampleSiteID
183-
customer.customerID = customerID
184-
185-
// When
186-
let storedCustomer = try XCTUnwrap(storage.loadCustomer(siteID: sampleSiteID, customerID: customerID))
187-
188-
// Then
189-
XCTAssertEqual(customer, storedCustomer)
190-
}
191-
192-
func test_loadCustomers_by_siteID_and_customerIDs() {
193-
// Given
194-
let customer1 = storage.insertNewObject(ofType: Customer.self)
195-
customer1.siteID = sampleSiteID
196-
customer1.customerID = 1
197-
198-
let customer2 = storage.insertNewObject(ofType: Customer.self)
199-
customer2.siteID = sampleSiteID
200-
customer2.customerID = 2
201-
202-
let customer3 = storage.insertNewObject(ofType: Customer.self)
203-
customer3.siteID = sampleSiteID
204-
customer3.customerID = 3
205-
206-
// When
207-
let results = storage.loadCustomers(siteID: sampleSiteID, matching: [1, 3])
208-
209-
// Then
210-
XCTAssertEqual(Set(results), Set([customer1, customer3]))
211-
}
212-
213178
func test_loadCustomerSearchResult_by_siteID_and_keyword() throws {
214179
// Given
215180
let keyword: String = "some keyword"

‎Modules/Tests/YosemiteTests/Stores/CustomerStoreTests.swift‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ final class CustomerStoreTests: XCTestCase {
1313
private var searchRemote: WCAnalyticsCustomerRemote!
1414
private var store: CustomerStore!
1515
private let dummySiteID: Int64 = 12345
16-
private let dummyCustomerID: Int64 = 25
16+
private let dummyUserID: Int64 = 25
1717
private let dummyKeyword: String = "John"
1818

1919
override func setUp() {
@@ -39,7 +39,7 @@ final class CustomerStoreTests: XCTestCase {
3939

4040
// When
4141
let result: Result<Networking.Customer, Error> = waitFor { promise in
42-
let action = CustomerAction.retrieveCustomer(siteID: self.dummySiteID, customerID: self.dummyCustomerID) { result in
42+
let action = CustomerAction.retrieveCustomer(siteID: self.dummySiteID, userID: self.dummyUserID) { result in
4343
promise(result)
4444
}
4545
self.store.onAction(action)
@@ -48,7 +48,7 @@ final class CustomerStoreTests: XCTestCase {
4848
// Then
4949
XCTAssertTrue(result.isSuccess)
5050
let customer = try result.get()
51-
XCTAssertEqual(customer.customerID, 25)
51+
XCTAssertEqual(customer.userID, 25)
5252
XCTAssertEqual(customer.firstName, "John")
5353
XCTAssertEqual(customer.lastName, "Doe")
5454
XCTAssertEqual(customer.email, "[email protected]")
@@ -80,7 +80,7 @@ final class CustomerStoreTests: XCTestCase {
8080

8181
// When
8282
let result: Result<Networking.Customer, Error> = waitFor { promise in
83-
let action = CustomerAction.retrieveCustomer(siteID: self.dummySiteID, customerID: self.dummyCustomerID) { result in
83+
let action = CustomerAction.retrieveCustomer(siteID: self.dummySiteID, userID: self.dummyUserID) { result in
8484
promise(result)
8585
}
8686
self.store.onAction(action)
@@ -200,30 +200,6 @@ final class CustomerStoreTests: XCTestCase {
200200
XCTAssertTrue(storedCustomerSearchResults?.customers?.allSatisfy { $0.firstName?.contains(dummyKeyword) == true } ?? false )
201201
}
202202

203-
func test_retrieveCustomer_upserts_the_returned_Customer() {
204-
// Given
205-
network.simulateResponse(requestUrlSuffix: "customers/25", filename: "customer")
206-
XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Customer.self), 0)
207-
208-
// When
209-
let result: Result<Networking.Customer, Error> = waitFor { promise in
210-
let action = CustomerAction.retrieveCustomer(siteID: self.dummySiteID, customerID: self.dummyCustomerID) { result in
211-
promise(result)
212-
}
213-
self.store.onAction(action)
214-
}
215-
216-
// Then
217-
XCTAssertTrue(result.isSuccess)
218-
XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Customer.self), 1)
219-
220-
let storedCustomer = viewStorage.loadCustomer(siteID: dummySiteID, customerID: dummyCustomerID)
221-
XCTAssertNotNil(storedCustomer)
222-
XCTAssertEqual(storedCustomer?.siteID, dummySiteID)
223-
XCTAssertEqual(storedCustomer?.customerID, dummyCustomerID)
224-
XCTAssertEqual(storedCustomer?.firstName, "John")
225-
}
226-
227203
func test_searchCustomers_returns_no_customers_when_customer_is_not_registered() throws {
228204
// Given
229205
network.simulateResponse(requestUrlSuffix: "customers", filename: "wc-analytics-customers")

‎WooCommerce/Classes/ViewRelated/Customers/CustomerDetailViewModel.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ extension CustomerDetailViewModel {
319319

320320
// Don't show loading state if we already have customer billing or shipping data to display
321321
updateStateIfNeeded(to: .loading)
322-
let action = CustomerAction.retrieveCustomer(siteID: siteID, customerID: userID) { [weak self] result in
322+
let action = CustomerAction.retrieveCustomer(siteID: siteID, userID: userID) { [weak self] result in
323323
guard let self else { return }
324324
switch result {
325325
case .success:

‎WooCommerce/Classes/ViewRelated/Orders/Order Creation/CustomerSection/CustomerSelectorViewModel.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ final class CustomerSelectorViewModel {
5252
return
5353
}
5454
// Get the full data about that customer using WordPress user ID
55-
stores.dispatch(CustomerAction.retrieveCustomer(siteID: siteID, customerID: customer.userID, onCompletion: { [weak self] result in
55+
stores.dispatch(CustomerAction.retrieveCustomer(siteID: siteID, userID: customer.userID, onCompletion: { [weak self] result in
5656
switch result {
5757
case .success(let customer):
5858
self?.onCustomerSelected(customer)

0 commit comments

Comments
 (0)