Skip to content

Commit 6cf213b

Browse files
committed
Simplify refund result handling
1 parent 4d081eb commit 6cf213b

File tree

8 files changed

+34
-90
lines changed

8 files changed

+34
-90
lines changed

Networking/Networking.xcodeproj/project.pbxproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,6 @@
10061006
DE02ABB32B563E61008E0AC4 /* blaze-payment-info.json in Resources */ = {isa = PBXBuildFile; fileRef = DE02ABB22B563E61008E0AC4 /* blaze-payment-info.json */; };
10071007
DE02ABB52B563E96008E0AC4 /* BlazePaymentInfoMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = DE02ABB42B563E96008E0AC4 /* BlazePaymentInfoMapper.swift */; };
10081008
DE02ABB72B563F3A008E0AC4 /* BlazePaymentInfoMapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DE02ABB62B563F3A008E0AC4 /* BlazePaymentInfoMapperTests.swift */; };
1009-
DE1D3E7B2DD1FBC100565CF9 /* WooShippingLabelRefundMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = DE1D3E7A2DD1FBC100565CF9 /* WooShippingLabelRefundMapper.swift */; };
10101009
DE1D3E7D2DD1FDF100565CF9 /* wooshipping-address-validation-error.json in Resources */ = {isa = PBXBuildFile; fileRef = DE1D3E7C2DD1FDF100565CF9 /* wooshipping-address-validation-error.json */; };
10111010
DE1D3E7F2DD200E900565CF9 /* wooshipping-label-refund-error.json in Resources */ = {isa = PBXBuildFile; fileRef = DE1D3E7E2DD200E900565CF9 /* wooshipping-label-refund-error.json */; };
10121011
DE2004552BF5BD1A00660A72 /* ProductStock.swift in Sources */ = {isa = PBXBuildFile; fileRef = DE2004542BF5BD1A00660A72 /* ProductStock.swift */; };
@@ -2255,7 +2254,6 @@
22552254
DE02ABB42B563E96008E0AC4 /* BlazePaymentInfoMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlazePaymentInfoMapper.swift; sourceTree = "<group>"; };
22562255
DE02ABB62B563F3A008E0AC4 /* BlazePaymentInfoMapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlazePaymentInfoMapperTests.swift; sourceTree = "<group>"; };
22572256
DE1D3E782DD1ED3100565CF9 /* shipping-label-refund-status-error.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "shipping-label-refund-status-error.json"; sourceTree = "<group>"; };
2258-
DE1D3E7A2DD1FBC100565CF9 /* WooShippingLabelRefundMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WooShippingLabelRefundMapper.swift; sourceTree = "<group>"; };
22592257
DE1D3E7C2DD1FDF100565CF9 /* wooshipping-address-validation-error.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "wooshipping-address-validation-error.json"; sourceTree = "<group>"; };
22602258
DE1D3E7E2DD200E900565CF9 /* wooshipping-label-refund-error.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "wooshipping-label-refund-error.json"; sourceTree = "<group>"; };
22612259
DE2004542BF5BD1A00660A72 /* ProductStock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductStock.swift; sourceTree = "<group>"; };
@@ -3847,7 +3845,6 @@
38473845
EEBBDD852D9440BD008D6CE5 /* WooShippingUpdateShipmentMapper.swift */,
38483846
CE90E99D2CEFCB100068D852 /* WooShippingStatusMapper.swift */,
38493847
EE7E80B72D84F18700E6FF5B /* WooShippingConfigMapper.swift */,
3850-
DE1D3E7A2DD1FBC100565CF9 /* WooShippingLabelRefundMapper.swift */,
38513848
CE606D8E2BE39426001CB424 /* ShippingMethodMapper.swift */,
38523849
FE28F6E326842848004465C7 /* UserMapper.swift */,
38533850
077F39D326A58DE700ABEADC /* SystemStatusMapper.swift */,
@@ -5462,7 +5459,6 @@
54625459
24F98C522502E79800F49B68 /* FeatureFlagRemote.swift in Sources */,
54635460
74A1D26D21189DFF00931DFA /* SiteVisitStatsMapper.swift in Sources */,
54645461
45152809257A7C6E0076B03C /* ProductAttributesRemote.swift in Sources */,
5465-
DE1D3E7B2DD1FBC100565CF9 /* WooShippingLabelRefundMapper.swift in Sources */,
54665462
EECDBEDE296845F400293C4E /* RESTRequestConvertible.swift in Sources */,
54675463
EE7E80B82D84F19100E6FF5B /* WooShippingConfigMapper.swift in Sources */,
54685464
EE839C262ABEF9C900049545 /* AIProductMapper.swift in Sources */,

Networking/Networking/Mapper/WooShippingLabelRefundMapper.swift

Lines changed: 0 additions & 58 deletions
This file was deleted.

Networking/Networking/Remote/WooShippingRemote.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public protocol WooShippingRemoteProtocol {
6868
func refundShippingLabel(siteID: Int64,
6969
orderID: Int64,
7070
shippingLabelID: Int64,
71-
completion: @escaping (Result<ShippingLabel, Error>) -> Void)
71+
completion: @escaping (Result<ShippingLabelRefund, Error>) -> Void)
7272
}
7373

7474
/// Shipping Labels Remote Endpoints for the WooShipping Plugin.
@@ -484,14 +484,14 @@ public final class WooShippingRemote: Remote, WooShippingRemoteProtocol {
484484
public func refundShippingLabel(siteID: Int64,
485485
orderID: Int64,
486486
shippingLabelID: Int64,
487-
completion: @escaping (Result<ShippingLabel, Error>) -> Void) {
487+
completion: @escaping (Result<ShippingLabelRefund, Error>) -> Void) {
488488
let path = Path.refundLabel(orderID: orderID, labelID: shippingLabelID)
489489
let request = JetpackRequest(wooApiVersion: .wooShipping,
490490
method: .post,
491491
siteID: siteID,
492492
path: path,
493493
availableAsRESTRequest: true)
494-
let mapper = WooShippingLabelRefundMapper(siteID: siteID, orderID: orderID)
494+
let mapper = ShippingLabelRefundMapper()
495495
enqueue(request, mapper: mapper, completion: completion)
496496
}
497497
}

Networking/NetworkingTests/Remote/WooShippingRemoteTests.swift

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ final class WooShippingRemoteTests: XCTestCase {
724724
network.simulateResponse(requestUrlSuffix: "label/refund/\(sampleOrderID)/\(labelID)", filename: "wooshipping-label-refund-success")
725725

726726
// When
727-
let result: Result<ShippingLabel, Error> = waitFor { promise in
727+
let result: Result<ShippingLabelRefund, Error> = waitFor { promise in
728728
remote.refundShippingLabel(siteID: self.sampleSiteID,
729729
orderID: self.sampleOrderID,
730730
shippingLabelID: labelID) { result in
@@ -733,9 +733,8 @@ final class WooShippingRemoteTests: XCTestCase {
733733
}
734734

735735
// Then
736-
let label = try XCTUnwrap(result.get())
737-
XCTAssertEqual(label.shippingLabelID, 1149)
738-
XCTAssertEqual(label.refund, ShippingLabelRefund(dateRequested: Date(timeIntervalSince1970: 1723147248.000), status: .pending))
736+
let refund = try XCTUnwrap(result.get())
737+
XCTAssertEqual(refund, ShippingLabelRefund(dateRequested: Date(timeIntervalSince1970: 1723147248.000), status: .pending))
739738
}
740739

741740
func test_refundShippingLabel_returns_error_on_failure() throws {
@@ -745,7 +744,7 @@ final class WooShippingRemoteTests: XCTestCase {
745744
network.simulateResponse(requestUrlSuffix: "label/refund/\(sampleOrderID)/\(labelID)", filename: "wooshipping-label-refund-error")
746745

747746
// When
748-
let result: Result<ShippingLabel, Error> = waitFor { promise in
747+
let result: Result<ShippingLabelRefund, Error> = waitFor { promise in
749748
remote.refundShippingLabel(siteID: self.sampleSiteID,
750749
orderID: self.sampleOrderID,
751750
shippingLabelID: labelID) { result in

Networking/NetworkingTests/Responses/wooshipping-label-refund-success.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"success": false,
44
"refund": {
55
"status": "pending",
6-
"request_date": 1723147248369,
6+
"request_date": 1723147248000,
77
"is_manual": false
88
},
99
"label": {

Yosemite/Yosemite/Stores/WooShippingStore.swift

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,10 @@ private extension WooShippingStore {
221221
shippingLabelID: shippingLabel.shippingLabelID) { [weak self] result in
222222
guard let self else { return }
223223
switch result {
224-
case .success(let label):
225-
upsertShippingLabelRefundInBackground(shippingLabel: label) {
226-
completion(.success(label))
224+
case .success(let refund):
225+
upsertShippingLabelRefundInBackground(shippingLabel: shippingLabel,
226+
refund: refund) { updatedLabel in
227+
completion(.success(updatedLabel))
227228
}
228229
case .failure(let error):
229230
completion(.failure(error))
@@ -556,24 +557,31 @@ private extension WooShippingStore {
556557
/// Updates the specified shipping label with the given refund *in a background thread*.
557558
/// `onCompletion` will be called on the main thread!
558559
func upsertShippingLabelRefundInBackground(shippingLabel: ShippingLabel,
559-
onCompletion: @escaping () -> Void) {
560-
storageManager.performAndSave ({ storage in
560+
refund: ShippingLabelRefund,
561+
onCompletion: @escaping (_ updatedLabel: ShippingLabel) -> Void) {
562+
storageManager.performAndSave ({ storage -> ShippingLabel in
561563
let storageShippingLabel = storage.loadShippingLabel(siteID: shippingLabel.siteID,
562564
orderID: shippingLabel.orderID,
563565
shippingLabelID: shippingLabel.shippingLabelID)
564566
guard let storageShippingLabel else {
565-
return
567+
DDLogWarn("⚠️ No shipping label found in storage when updating refund")
568+
return shippingLabel.copy(refund: refund)
566569
}
567570

568-
if let refund = shippingLabel.refund {
569-
let storageRefund = storageShippingLabel.refund ?? storage.insertNewObject(ofType: Storage.ShippingLabelRefund.self)
570-
storageRefund.update(with: refund)
571-
storageShippingLabel.refund = storageRefund
572-
} else {
573-
storageShippingLabel.refund = nil
574-
}
571+
let storageRefund = storageShippingLabel.refund ?? storage.insertNewObject(ofType: Storage.ShippingLabelRefund.self)
572+
storageRefund.update(with: refund)
573+
storageShippingLabel.refund = storageRefund
574+
return storageShippingLabel.toReadOnly()
575575

576-
}, completion: onCompletion, on: .main)
576+
}, completion: { result in
577+
switch result {
578+
case .success(let label):
579+
onCompletion(label)
580+
case .failure(let error):
581+
DDLogError("⛔️ Error upserting shipping label refund: \(error)")
582+
onCompletion(shippingLabel.copy(refund: refund))
583+
}
584+
}, on: .main)
577585
}
578586

579587
/// Updates order's `dateModified` locally

Yosemite/YosemiteTests/Mocks/Networking/Remote/MockWooShippingRemote.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ final class MockWooShippingRemote {
6464
private var updateShipment = [ResultKey: Result<WooShippingShipments, Error>]()
6565

6666
/// The results to return based on the given arguments in `refundShippingLabel`
67-
private var refundShippingLabel = [RefundResultKey: Result<ShippingLabel, Error>]()
67+
private var refundShippingLabel = [RefundResultKey: Result<ShippingLabelRefund, Error>]()
6868

6969
/// Set the value passed to the `completion` block if `createPackage` is called.
7070
func whenCreatePackage(siteID: Int64,
@@ -175,7 +175,7 @@ final class MockWooShippingRemote {
175175
func whenRefundingShippingLabel(siteID: Int64,
176176
orderID: Int64,
177177
shippingLabelID: Int64,
178-
thenReturn result: Result<ShippingLabel, Error>) {
178+
thenReturn result: Result<ShippingLabelRefund, Error>) {
179179
let key = RefundResultKey(siteID: siteID, orderID: orderID, shippingLabelID: shippingLabelID)
180180
refundShippingLabel[key] = result
181181
}
@@ -423,7 +423,7 @@ extension MockWooShippingRemote: WooShippingRemoteProtocol {
423423
func refundShippingLabel(siteID: Int64,
424424
orderID: Int64,
425425
shippingLabelID: Int64,
426-
completion: @escaping (Result<ShippingLabel, Error>) -> Void) {
426+
completion: @escaping (Result<ShippingLabelRefund, Error>) -> Void) {
427427
DispatchQueue.main.async { [weak self] in
428428
guard let self else { return }
429429
let key = RefundResultKey(siteID: siteID, orderID: orderID, shippingLabelID: shippingLabelID)

Yosemite/YosemiteTests/Stores/WooShippingStoreTests.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -957,12 +957,11 @@ final class WooShippingStoreTests: XCTestCase {
957957
let remote = MockWooShippingRemote()
958958
let expectedRefund = Yosemite.ShippingLabelRefund(dateRequested: Date(), status: .pending)
959959
let shippingLabel = MockShippingLabel.emptyLabel().copy(siteID: sampleSiteID, orderID: sampleOrderID, shippingLabelID: 123)
960-
let expectedUpdatedLabel = shippingLabel.copy(refund: expectedRefund)
961960

962961
remote.whenRefundingShippingLabel(siteID: shippingLabel.siteID,
963962
orderID: shippingLabel.orderID,
964963
shippingLabelID: shippingLabel.shippingLabelID,
965-
thenReturn: .success(expectedUpdatedLabel))
964+
thenReturn: .success(expectedRefund))
966965
let store = WooShippingStore(dispatcher: dispatcher, storageManager: storageManager, network: network, remote: remote)
967966

968967
// Inserts a shipping label without a refund.

0 commit comments

Comments
 (0)