Skip to content

Commit c50ba96

Browse files
itsmeichigoitsmeichigo
authored
Shipping Labels: Better formatted surcharge for additional shipping services (#15826)
2 parents 6d4fdd8 + 4d0db19 commit c50ba96

File tree

5 files changed

+92
-34
lines changed

5 files changed

+92
-34
lines changed

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/ShipmentDetails/WooShippingShipmentDetailsViewModel.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ final class WooShippingShipmentDetailsViewModel: ObservableObject {
174174
///
175175
@MainActor
176176
func refreshPackagesAndShippingRates() async throws {
177-
guard let selectedPackage, let updatedPackage = try await refreshSelectedPackage(from: selectedPackage) else {
177+
guard let selectedRate, let selectedPackage, let updatedPackage = try await refreshSelectedPackage(from: selectedPackage) else {
178178
throw WooShippingLabelPurchaseError.failedToRefreshSelectedPackage
179179
}
180180
self.selectedPackage = updatedPackage
@@ -185,7 +185,7 @@ final class WooShippingShipmentDetailsViewModel: ObservableObject {
185185
hazmatCategory: hazmatCategory,
186186
customsForm: customsForm)
187187

188-
guard let shippingService, let selectedRate else {
188+
guard let shippingService else {
189189
throw WooShippingLabelPurchaseError.failedToRefreshSelectedRate
190190
}
191191
try await withCheckedThrowingContinuation { continuation in

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooShippingServiceCardView.swift‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,19 @@ struct WooShippingServiceCardView: View {
3131
Group {
3232
VStack(alignment: .leading, spacing: 0) {
3333
if let tracking = viewModel.trackingLabel {
34-
HStack(alignment: .firstTextBaseline) {
34+
HStack {
3535
checkmark
3636
Text(tracking)
3737
}
3838
}
3939
if let insurance = viewModel.insuranceLabel {
40-
HStack(alignment: .firstTextBaseline) {
40+
HStack {
4141
checkmark
4242
Text(insurance)
4343
}
4444
}
4545
if let freePickup = viewModel.freePickupLabel {
46-
HStack(alignment: .firstTextBaseline) {
46+
HStack {
4747
checkmark
4848
Text(freePickup)
4949
}
@@ -52,7 +52,7 @@ struct WooShippingServiceCardView: View {
5252
.fixedSize(horizontal: false, vertical: true)
5353

5454
if let signatureRequired = viewModel.signatureRequiredLabel {
55-
HStack(alignment: .firstTextBaseline) {
55+
HStack {
5656
selectionCircle(selected: viewModel.signatureRequirement == .signatureRequired)
5757
Text(signatureRequired)
5858
}
@@ -62,7 +62,7 @@ struct WooShippingServiceCardView: View {
6262
}
6363
}
6464
if let adultSignatureRequired = viewModel.adultSignatureRequiredLabel {
65-
HStack(alignment: .firstTextBaseline) {
65+
HStack {
6666
selectionCircle(selected: viewModel.signatureRequirement == .adultSignatureRequired)
6767
Text(adultSignatureRequired)
6868
}
@@ -72,7 +72,7 @@ struct WooShippingServiceCardView: View {
7272
}
7373
}
7474
if let carbonNeutralLabel = viewModel.carbonNeutralLabel {
75-
HStack(alignment: .firstTextBaseline) {
75+
HStack {
7676
selectionCircle(selected: viewModel.carbonNeutralSelected)
7777
Text(carbonNeutralLabel)
7878
}
@@ -81,24 +81,24 @@ struct WooShippingServiceCardView: View {
8181
viewModel.handleTap(on: .carbonNeutral)
8282
}
8383
}
84-
if let saturdayDeliveryLabel = viewModel.saturdayDeliveryLabel {
85-
HStack(alignment: .firstTextBaseline) {
86-
selectionCircle(selected: viewModel.saturdayDeliverySelected)
87-
Text(saturdayDeliveryLabel)
84+
if let additionalHandlingLabel = viewModel.additionalHandlingLabel {
85+
HStack {
86+
selectionCircle(selected: viewModel.additionalHandlingSelected)
87+
Text(additionalHandlingLabel)
8888
}
8989
.contentShape(Rectangle())
9090
.onTapGesture {
91-
viewModel.handleTap(on: .saturdayDelivery)
91+
viewModel.handleTap(on: .additionalHandling)
9292
}
9393
}
94-
if let additionalHandlingLabel = viewModel.additionalHandlingLabel {
95-
HStack(alignment: .firstTextBaseline) {
96-
selectionCircle(selected: viewModel.additionalHandlingSelected)
97-
Text(additionalHandlingLabel)
94+
if let saturdayDeliveryLabel = viewModel.saturdayDeliveryLabel {
95+
HStack {
96+
selectionCircle(selected: viewModel.saturdayDeliverySelected)
97+
Text(saturdayDeliveryLabel)
9898
}
9999
.contentShape(Rectangle())
100100
.onTapGesture {
101-
viewModel.handleTap(on: .additionalHandling)
101+
viewModel.handleTap(on: .saturdayDelivery)
102102
}
103103
}
104104
}

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/WooShipping Create Shipping Labels/WooShipping Package and Rate Selection/WooShippingServiceCardViewModel.swift‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -168,43 +168,51 @@ final class WooShippingServiceCardViewModel: Identifiable, ObservableObject {
168168
let extras = [trackingLabel, insuranceLabel?.localizedLowercase, freePickupLabel].compactMap { $0 }
169169
let extraInfoLabel = extras.isNotEmpty ? extras.joined(separator: ", ") : nil
170170

171+
func formatSurcharge(serviceRate: Double) -> String {
172+
let amount = Decimal(serviceRate - rate.rate)
173+
let isNegative = amount < 0
174+
let formattedAmount = currencyFormatter.formatAmount(amount, isNegative: isNegative) ?? ""
175+
let prefix = isNegative ? "" : "+"
176+
return prefix + formattedAmount
177+
}
178+
171179
let signatureRequiredLabel: String? = {
172180
guard let signatureRate else {
173181
return nil
174182
}
175-
let amount = currencyFormatter.formatAmount(Decimal(signatureRate.rate - rate.rate)) ?? ""
183+
let amount = formatSurcharge(serviceRate: signatureRate.rate)
176184
return String(format: Localization.signatureRequired, amount)
177185
}()
178186

179187
let adultSignatureRequiredLabel: String? = {
180188
guard let adultSignatureRate else {
181189
return nil
182190
}
183-
let amount = currencyFormatter.formatAmount(Decimal(adultSignatureRate.rate - rate.rate)) ?? ""
191+
let amount = formatSurcharge(serviceRate: adultSignatureRate.rate)
184192
return String(format: Localization.adultSignatureRequired, amount)
185193
}()
186194

187195
let carbonNeutralLabel: String? = {
188196
guard let carbonNeutralRate else {
189197
return nil
190198
}
191-
let amount = currencyFormatter.formatAmount(Decimal(carbonNeutralRate.rate - rate.rate)) ?? ""
199+
let amount = formatSurcharge(serviceRate: carbonNeutralRate.rate)
192200
return String(format: Localization.carbonNeural, amount)
193201
}()
194202

195203
let saturdayDeliveryLabel: String? = {
196204
guard let saturdayDeliveryRate else {
197205
return nil
198206
}
199-
let amount = currencyFormatter.formatAmount(Decimal(saturdayDeliveryRate.rate - rate.rate)) ?? ""
207+
let amount = formatSurcharge(serviceRate: saturdayDeliveryRate.rate)
200208
return String(format: Localization.saturdayDelivery, amount)
201209
}()
202210

203211
let additionalHandlingLabel: String? = {
204212
guard let additionalHandlingRate else {
205213
return nil
206214
}
207-
let amount = currencyFormatter.formatAmount(Decimal(additionalHandlingRate.rate - rate.rate)) ?? ""
215+
let amount = formatSurcharge(serviceRate: additionalHandlingRate.rate)
208216
return String(format: Localization.additionalHandling, amount)
209217
}()
210218

@@ -303,29 +311,29 @@ private extension WooShippingServiceCardViewModel {
303311
static let freePickup = NSLocalizedString("wooShipping.createLabels.shippingService.freePickup",
304312
value: "Free pickup",
305313
comment: "Label when shipping rate includes free pickup in Woo Shipping label creation flow.")
306-
static let signatureRequired = NSLocalizedString("wooShipping.createLabels.shippingService.signatureRequired",
307-
value: "Signature Required (+%1$@)",
314+
static let signatureRequired = NSLocalizedString("wooShipping.createLabels.shippingService.signatureRequiredLabel",
315+
value: "Signature Required (%1$@)",
308316
comment: "Label when shipping rate has option to require a signature in " +
309317
"Woo Shipping label creation flow. Reads like: 'Signature required (+$3.70)'")
310-
static let adultSignatureRequired = NSLocalizedString("wooShipping.createLabels.shippingService.adultSignatureRequired",
311-
value: "Adult Signature Required (+%1$@)",
318+
static let adultSignatureRequired = NSLocalizedString("wooShipping.createLabels.shippingService.adultSignatureRequiredLabel",
319+
value: "Adult Signature Required (%1$@)",
312320
comment: "Label when shipping rate has option to require an adult signature in " +
313321
"Woo Shipping label creation flow. Reads like: 'Adult signature required (+$9.35)'")
314322
static let carbonNeural = NSLocalizedString(
315323
"wooShipping.createLabels.shippingService.carbonNeural",
316-
value: "Carbon Neutral (+%1$@)",
324+
value: "Carbon Neutral (%1$@)",
317325
comment: "Label when shipping rate has option for carbon neutral delivery in " +
318326
"Woo Shipping label creation flow. Reads like: 'Carbon Neutral (+$9.35)'"
319327
)
320328
static let saturdayDelivery = NSLocalizedString(
321329
"wooShipping.createLabels.shippingService.saturdayDelivery",
322-
value: "Saturday Delivery (+%1$@)",
330+
value: "Saturday Delivery (%1$@)",
323331
comment: "Label when shipping rate has option for Saturday delivery in " +
324332
"Woo Shipping label creation flow. Reads like: 'Saturday Delivery (+$9.35)'"
325333
)
326334
static let additionalHandling = NSLocalizedString(
327335
"wooShipping.createLabels.shippingService.additionalHandling",
328-
value: "Additional Handling (+%1$@)",
336+
value: "Additional Handling (%1$@)",
329337
comment: "Label when shipping rate has option for additional handling in " +
330338
"Woo Shipping label creation flow. Reads like: 'Additional Handling (+$9.35)'"
331339
)

‎WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingServiceCardViewModelTests.swift‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,39 @@ final class WooShippingServiceCardViewModelTests: XCTestCase {
8282
XCTAssertEqual(viewModel.additionalHandlingLabel, "Additional Handling (+$3.89)")
8383
}
8484

85+
func test_it_inits_with_expected_values_with_extra_services_negative_surcharge() {
86+
// Given
87+
let viewModel = WooShippingServiceCardViewModel(selected: true,
88+
signatureRequired: true,
89+
carbonNeutralSelected: true,
90+
saturdayDeliverySelected: false,
91+
additionalHandlingSelected: true,
92+
rate: MockShippingLabelCarrierRate.makeRate(rate: 25.95, insurance: "100"),
93+
signatureRate: MockShippingLabelCarrierRate.makeRate(rate: 25.69),
94+
adultSignatureRate: MockShippingLabelCarrierRate.makeRate(rate: 33.05),
95+
carbonNeutralRate: MockShippingLabelCarrierRate.makeRate(rate: 26.15),
96+
saturdayDeliveryRate: MockShippingLabelCarrierRate.makeRate(rate: 25.69),
97+
additionalHandlingRate: MockShippingLabelCarrierRate.makeRate(rate: 38.9),
98+
currencySettings: CurrencySettings())
99+
100+
// Then
101+
XCTAssertEqual(viewModel.selected, true)
102+
XCTAssertEqual(viewModel.signatureRequirement, .signatureRequired)
103+
XCTAssertEqual(viewModel.title, "USPS - Parcel Select Mail")
104+
XCTAssertEqual(viewModel.daysToDeliveryLabel, "2 business days")
105+
XCTAssertEqual(viewModel.rateLabel, "$38.84")
106+
XCTAssertEqual(viewModel.carrierLogo, UIImage(named: "shipping-label-usps-logo"))
107+
XCTAssertEqual(viewModel.trackingLabel, "Tracking")
108+
XCTAssertEqual(viewModel.insuranceLabel, "Insurance (up to $100.00)")
109+
XCTAssertEqual(viewModel.freePickupLabel, "Free pickup")
110+
XCTAssertEqual(viewModel.extraInfoLabel, "Includes tracking, insurance (up to $100.00), free pickup")
111+
XCTAssertEqual(viewModel.signatureRequiredLabel, "Signature Required (-$0.26)")
112+
XCTAssertEqual(viewModel.adultSignatureRequiredLabel, "Adult Signature Required (+$7.10)")
113+
XCTAssertEqual(viewModel.carbonNeutralLabel, "Carbon Neutral (+$0.20)")
114+
XCTAssertEqual(viewModel.saturdayDeliveryLabel, "Saturday Delivery (-$0.26)")
115+
XCTAssertEqual(viewModel.additionalHandlingLabel, "Additional Handling (+$12.95)")
116+
}
117+
85118
func test_insuranceLabel_shows_expected_value_for_non_number_insurance() {
86119
// Given
87120
let viewModel = WooShippingServiceCardViewModel(rate: MockShippingLabelCarrierRate.makeRate(insurance: "limited"))

‎WooCommerce/WooCommerceTests/ViewRelated/Shipping Label/WooShipping Create Shipping Labels/WooShippingShipmentDetailsViewModelTests.swift‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ final class WooShippingShipmentDetailsViewModelTests: XCTestCase {
584584
}
585585

586586
@MainActor
587-
func test_refreshPackagesAndShippingRates_updates_selected_package() async {
587+
func test_refreshPackagesAndShippingRates_updates_selected_package_and_rate() async throws {
588588
// Given
589589
let originAddressSubject = CurrentValueSubject<WooShippingAddress?, Never>(sampleOriginAddress(country: "US", state: "NY"))
590590
let destinationAddressSubject = CurrentValueSubject<WooShippingAddress?, Never>(sampleDestinationAddress(country: "US", state: "CA"))
@@ -598,12 +598,19 @@ final class WooShippingShipmentDetailsViewModelTests: XCTestCase {
598598
stores: stores)
599599
let package = samplePackageData()
600600
viewModel.selectPackage(package)
601-
viewModel.shippingService?.onSelectRate?(sampleSelectedRate())
601+
let rate = ShippingLabelCarrierRate.fake().copy(
602+
title: "Rate",
603+
rate: 20.11,
604+
serviceID: "test_rate"
605+
)
606+
viewModel.shippingService?.onSelectRate?(WooShippingSelectedRate(rate: rate))
602607

603608
// Confidence check
604609
XCTAssertEqual(viewModel.selectedPackage?.id, package.id)
610+
XCTAssertEqual(viewModel.selectedRate?.rate.rate, rate.rate)
605611

606612
// When: package is refreshed
613+
let expectedRate = rate.copy(rate: 22.11)
607614
let updatedPackage = WooShippingCustomPackage(id: "small_flat_box",
608615
name: "custom",
609616
rawType: "box",
@@ -617,15 +624,25 @@ final class WooShippingShipmentDetailsViewModelTests: XCTestCase {
617624
savedPredefinedPackages: [],
618625
allPredefinedOptions: [])))
619626
case let .loadLabelRates(_, _, _, _, packages, completion):
620-
completion(packages, .success([]))
627+
let result = ShippingLabelCarriersAndRates(
628+
packageID: "0",
629+
defaultRates: [expectedRate],
630+
signatureRequired: [expectedRate.copy(rate: 23.33)],
631+
adultSignatureRequired: [expectedRate.copy(rate: 25.78)],
632+
carbonNeutral: [],
633+
saturdayDelivery: [],
634+
additionalHandling: []
635+
)
636+
completion(packages, .success([result]))
621637
default:
622638
break
623639
}
624640
}
625-
try? await viewModel.refreshPackagesAndShippingRates() // ignoring failure in refreshing rate for simplicity
641+
try await viewModel.refreshPackagesAndShippingRates()
626642

627643
// Then
628644
XCTAssertEqual(viewModel.selectedPackage?.name, updatedPackage.name)
645+
XCTAssertEqual(viewModel.selectedRate?.rate.rate, expectedRate.rate)
629646
}
630647

631648
func test_changing_origin_address_resets_selected_rate() throws {

0 commit comments

Comments
 (0)