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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public class OrdersRemote: Remote {
/// - createdVia: If given, limit response to orders created via the specified source (e.g. "pos-rest-api" for Point of Sale).
/// - pageNumber: Number of page that should be retrieved.
/// - pageSize: Number of Orders to be retrieved per page.
/// - completion: Closure to be executed upon completion.
/// - Returns: Array of orders.
/// - Throws: Network or parsing errors.
///
public func loadAllOrders(for siteID: Int64,
statuses: [String]? = nil,
Expand All @@ -36,8 +37,7 @@ public class OrdersRemote: Remote {
productID: Int64? = nil,
createdVia: String? = nil,
pageNumber: Int = Defaults.pageNumber,
pageSize: Int = Defaults.pageSize,
completion: @escaping (Result<[Order], Error>) -> Void) {
pageSize: Int = Defaults.pageSize) async throws -> [Order] {
let utcDateFormatter = DateFormatter.Defaults.iso8601

let statusesString: String? = statuses?.isEmpty == true ? Defaults.statusAny : statuses?.joined(separator: ",")
Expand Down Expand Up @@ -84,7 +84,7 @@ public class OrdersRemote: Remote {
availableAsRESTRequest: true)
let mapper = OrderListMapper(siteID: siteID)

enqueue(request, mapper: mapper, completion: completion)
return try await enqueue(request, mapper: mapper)
}

/// Retrieves a specific `Order`
Expand Down Expand Up @@ -138,13 +138,13 @@ public class OrdersRemote: Remote {
/// - keyword: Search string that should be matched by the orders.
/// - pageNumber: Number of page that should be retrieved.
/// - pageSize: Number of Orders to be retrieved per page.
/// - completion: Closure to be executed upon completion.
/// - Returns: Array of orders matching the search criteria.
/// - Throws: Network or parsing errors.
///
public func searchOrders(for siteID: Int64,
keyword: String,
pageNumber: Int = Defaults.pageNumber,
pageSize: Int = Defaults.pageSize,
completion: @escaping ([Order]?, Error?) -> Void) {
pageSize: Int = Defaults.pageSize) async throws -> [Order] {
let parameters = [
ParameterKeys.keyword: keyword,
ParameterKeys.page: String(pageNumber),
Expand All @@ -162,7 +162,7 @@ public class OrdersRemote: Remote {
availableAsRESTRequest: true)
let mapper = OrderListMapper(siteID: siteID)

enqueue(request, mapper: mapper, completion: completion)
return try await enqueue(request, mapper: mapper)
}

/// Creates an order using the specified fields of a given order
Expand Down
78 changes: 40 additions & 38 deletions Modules/Sources/Yosemite/Stores/OrderStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,14 @@ private extension OrderStore {
/// Searches all of the orders that contain a given Keyword.
///
func searchOrders(siteID: Int64, keyword: String, pageNumber: Int, pageSize: Int, onCompletion: @escaping (Error?) -> Void) {
remote.searchOrders(for: siteID, keyword: keyword, pageNumber: pageNumber, pageSize: pageSize) { [weak self] (orders, error) in
guard let orders = orders else {
Task { @MainActor in
do {
let orders = try await remote.searchOrders(for: siteID, keyword: keyword, pageNumber: pageNumber, pageSize: pageSize)
upsertSearchResultsInBackground(for: siteID, keyword: keyword, readOnlyOrders: orders) {
onCompletion(nil)
}
} catch {
onCompletion(error)
return
}

self?.upsertSearchResultsInBackground(for: siteID, keyword: keyword, readOnlyOrders: orders) {
onCompletion(nil)
}
}
}
Expand Down Expand Up @@ -155,18 +155,19 @@ private extension OrderStore {

let pageNumber = OrdersRemote.Defaults.pageNumber
let startTime = Date()
self.remote.loadAllOrders(for: siteID,
statuses: statuses,
after: after,
before: before,
modifiedAfter: modifiedAfter,
customerID: customerID,
productID: productID,
createdVia: createdVia,
pageNumber: pageNumber,
pageSize: pageSize) { [weak self] result in
switch result {
case .success(let orders):
Task { @MainActor [weak self] in
do {
let orders = try await self?.remote.loadAllOrders(for: siteID,
statuses: statuses,
after: after,
before: before,
modifiedAfter: modifiedAfter,
customerID: customerID,
productID: productID,
createdVia: createdVia,
pageNumber: pageNumber,
pageSize: pageSize) ?? []
let result: Result<[Order], Error> = .success(orders)
switch writeStrategy {
case .doNotSave:
onCompletion(Date().timeIntervalSince(startTime), result)
Expand All @@ -176,7 +177,8 @@ private extension OrderStore {
onCompletion(Date().timeIntervalSince(startTime), result)
}
}
case .failure:
} catch {
let result: Result<[Order], Error> = .failure(error)
onCompletion(Date().timeIntervalSince(startTime), result)
}
}
Expand All @@ -196,22 +198,22 @@ private extension OrderStore {
pageSize: Int,
onCompletion: @escaping (TimeInterval, Error?) -> Void) {
let startTime = Date()
remote.loadAllOrders(for: siteID,
statuses: statuses,
after: after,
before: before,
modifiedAfter: modifiedAfter,
customerID: customerID,
productID: productID,
createdVia: createdVia,
pageNumber: pageNumber,
pageSize: pageSize) { [weak self] result in
switch result {
case .success(let orders):
self?.upsertStoredOrdersInBackground(readOnlyOrders: orders) {
Task { @MainActor in
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The alignment of these props seems to be off!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the alignment in 8377aba.

do {
let orders = try await remote.loadAllOrders(for: siteID,
statuses: statuses,
after: after,
before: before,
modifiedAfter: modifiedAfter,
customerID: customerID,
productID: productID,
createdVia: createdVia,
pageNumber: pageNumber,
pageSize: pageSize)
upsertStoredOrdersInBackground(readOnlyOrders: orders) {
onCompletion(Date().timeIntervalSince(startTime), nil)
}
case .failure(let error):
} catch {
onCompletion(Date().timeIntervalSince(startTime), error)
}
}
Expand All @@ -228,11 +230,11 @@ private extension OrderStore {
}

// If there are no locally stored orders, then check remote.
remote.loadAllOrders(for: siteID, pageNumber: Default.firstPageNumber, pageSize: 1) { result in
switch result {
case .success(let orders):
Task { @MainActor in
do {
let orders = try await remote.loadAllOrders(for: siteID, pageNumber: Default.firstPageNumber, pageSize: 1)
onCompletion(.success(orders.isEmpty == false))
case .failure(let error):
} catch {
onCompletion(.failure(error))
}
}
Expand Down
85 changes: 30 additions & 55 deletions Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,57 +60,42 @@ final class OrdersRemoteTests: XCTestCase {

/// Verifies that loadAllOrders properly parses the `orders-load-all` sample response.
///
func testLoadAllOrdersProperlyReturnsParsedOrders() throws {
func testLoadAllOrdersProperlyReturnsParsedOrders() async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Since we're updating the test signature we can update the test name to use snake case as well 🐍 , ideally with when/then too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, updated in aa00e4c.

// Given
let remote = OrdersRemote(network: network)

network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")

// When
var result: Result<[Order], Error>?
waitForExpectation { expectation in
remote.loadAllOrders(for: sampleSiteID) { aResult in
result = aResult
expectation.fulfill()
}
}
let orders = try await remote.loadAllOrders(for: sampleSiteID)

// Then
let orders = try XCTUnwrap(result?.get())
XCTAssert(orders.count == 4)
}

/// Verifies that loadAllOrders properly relays Networking Layer errors.
///
func testLoadAllOrdersProperlyRelaysNetworkingErrors() throws {
func testLoadAllOrdersProperlyRelaysNetworkingErrors() async throws {
// Given
let remote = OrdersRemote(network: network)

// When
var result: Result<[Order], Error>?
waitForExpectation { expectation in
remote.loadAllOrders(for: sampleSiteID) { aResult in
result = aResult
expectation.fulfill()
}
// When & Then
do {
_ = try await remote.loadAllOrders(for: sampleSiteID)
XCTFail("Expected error to be thrown")
} catch {
XCTAssertEqual(error as? NetworkError, .notFound(response: nil))
}

// Then
XCTAssertTrue(try XCTUnwrap(result).isFailure)
}

func test_loadAllOrders_includes_modifiedAfter_parameter_when_provided() {
func test_loadAllOrders_includes_modifiedAfter_parameter_when_provided() async throws {
// Given
let remote = OrdersRemote(network: network)
let modifiedAfter = Date()
network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")

// When
_ = waitFor { promise in
remote.loadAllOrders(for: self.sampleSiteID, modifiedAfter: modifiedAfter) { result in
promise(result)
}
}
_ = try await remote.loadAllOrders(for: sampleSiteID, modifiedAfter: modifiedAfter)

// Then
guard let queryParameters = network.queryParameters else {
Expand All @@ -123,18 +108,14 @@ final class OrdersRemoteTests: XCTestCase {
XCTAssertTrue(queryParameters.contains(expectedParam), "Expected to have param: \(expectedParam)")
}

func test_loadAllOrders_includes_customer_parameter_when_provided() {
func test_loadAllOrders_includes_customer_parameter_when_provided() async throws {
// Given
let remote = OrdersRemote(network: network)
network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
let expectedCustomerID: Int64 = 123

// When
_ = waitFor { promise in
remote.loadAllOrders(for: self.sampleSiteID, customerID: expectedCustomerID) { result in
promise(result)
}
}
_ = try await remote.loadAllOrders(for: sampleSiteID, customerID: expectedCustomerID)

// Then
guard let queryParameters = network.queryParameters else {
Expand All @@ -146,18 +127,14 @@ final class OrdersRemoteTests: XCTestCase {
XCTAssertTrue(queryParameters.contains(expectedParam), "Expected to have param: \(expectedParam)")
}

func test_loadAllOrders_includes_product_parameter_when_provided() {
func test_loadAllOrders_includes_product_parameter_when_provided() async throws {
// Given
let remote = OrdersRemote(network: network)
network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
let expectedProductID: Int64 = 13

// When
_ = waitFor { promise in
remote.loadAllOrders(for: self.sampleSiteID, productID: expectedProductID) { result in
promise(result)
}
}
_ = try await remote.loadAllOrders(for: sampleSiteID, productID: expectedProductID)

// Then
guard let queryParameters = network.queryParameters else {
Expand Down Expand Up @@ -243,35 +220,33 @@ final class OrdersRemoteTests: XCTestCase {

/// Verifies that searchOrders properly parses the `orders-load-all` sample response.
///
func testSearchOrdersProperlyReturnsParsedOrders() {
func test_searchOrders_properly_returns_parsed_orders() async throws {
// Given
let remote = OrdersRemote(network: network)
let expectation = self.expectation(description: "Load All Orders")

network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")

remote.searchOrders(for: sampleSiteID, keyword: String()) { (orders, error) in
XCTAssertNil(error)
XCTAssertNotNil(orders)
XCTAssert(orders!.count == 4)
expectation.fulfill()
}
// When
let orders = try await remote.searchOrders(for: sampleSiteID, keyword: String())

wait(for: [expectation], timeout: Constants.expectationTimeout)
// Then
XCTAssert(orders.count == 4)
}

/// Verifies that searchOrders properly relays Networking Layer errors.
///
func testSearchOrdersProperlyRelaysNetworkingErrors() {
func test_searchOrders_properly_relays_networking_error() async throws {
// Given
let remote = OrdersRemote(network: network)
let expectation = self.expectation(description: "Load All Orders")

remote.searchOrders(for: sampleSiteID, keyword: String()) { (orders, error) in
XCTAssertNil(orders)
XCTAssertNotNil(error)
expectation.fulfill()
do {
// When
_ = try await remote.searchOrders(for: sampleSiteID, keyword: String())
XCTFail("Expected error to be thrown")
} catch {
// Then
XCTAssertEqual(error as? NetworkError, .notFound(response: nil))
}

wait(for: [expectation], timeout: Constants.expectationTimeout)
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,13 @@ final class ConnectivityToolViewModel {
/// Test fetching the site orders by actually fetching orders.
///
func testFetchingOrders() async -> ConnectivityToolCard.State {
await withCheckedContinuation { continuation in
orderRemote?.loadAllOrders(for: siteID) { [weak self] result in
guard let self else { return }

switch result {
case .success:
DDLogInfo("Connectivity Tool: ✅ Site Orders")
case .failure(let error):
DDLogError("Connectivity Tool: ❌ Site Orders\n\(error)")
}

let state = self.stateForSiteResult(result)
continuation.resume(returning: state)
}
do {
_ = try await orderRemote?.loadAllOrders(for: siteID)
DDLogInfo("Connectivity Tool: ✅ Site Orders")
return stateForSiteResult(Result<[Order], Error>.success([]))
} catch {
DDLogError("Connectivity Tool: ❌ Site Orders\n\(error)")
return stateForSiteResult(Result<[Order], Error>.failure(error))
}
}

Expand Down
8 changes: 2 additions & 6 deletions WooCommerce/Woo Watch App/Orders/OrdersDataService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,9 @@ final class OrdersDataService {
ordersRemote = OrdersRemote(network: network)
}

/// Async wrapper that fetches orders for a store ID.
/// Fetches orders for a store ID.
///
func loadAllOrders(for storeID: Int64, pageNumber: Int, pageSize: Int) async throws -> [Order] {
try await withCheckedThrowingContinuation { continuation in
ordersRemote.loadAllOrders(for: storeID, pageNumber: pageNumber, pageSize: pageSize) { result in
continuation.resume(with: result)
}
}
try await ordersRemote.loadAllOrders(for: storeID, pageNumber: pageNumber, pageSize: pageSize)
}
}
Loading