Skip to content

[Shipping Labels] Delete redundant hstariff class validation logic #15990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 8, 2025
Merged

[Shipping Labels] Delete redundant hstariff class validation logic #15990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b60095f
Delete item tariff class validation
RafaelKayumov Jul 24, 2025
0e87e56
Delete `HSTariffNumberTotalValue` in favor of reactive `totalValue`
RafaelKayumov Jul 24, 2025
b3b97d5
Update tests
RafaelKayumov Jul 24, 2025
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
39 changes: 14 additions & 25 deletions ...Shipping Create Shipping Labels/WooShipping Customs/WooShippingCustomsFormViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,34 +120,28 @@ private extension WooShippingCustomsFormViewModel {
}
.assign(to: &$isMissingITN)

let hsTariffNumberTotalValueDictionary = $itemsViewModels
.map { childViewModels in
childViewModels.map { $0.$hsTariffNumberTotalValue.eraseToAnyPublisher() }
}
.flatMap { childPublishers in
childPublishers.combineLatest()
}
.map { values in
var hsTariffNumberTotalValueDictionary: [String: Decimal] = [:]
for (hsTariffNumber, totalValuePerItem) in values.compacted() {
hsTariffNumberTotalValueDictionary[hsTariffNumber, default: 0] += totalValuePerItem
}
return hsTariffNumberTotalValueDictionary
}
let totalItemValue = $itemsViewModels
.map { childViewModels in
childViewModels.map { $0.$totalValue.eraseToAnyPublisher() }
}
.flatMap { childPublishers in
childPublishers.combineLatest()
}
.map { values in
return values.reduce(0) { partialResult, value in
return partialResult + value
}
}

$internationalTransactionNumber.combineLatest(
$itemsViewModels,
hsTariffNumberTotalValueDictionary,
totalItemValue,
$destinationCountryCode)
.map { input -> ITNValidationError? in
let (itn, items, hsTariffNumberTotalValueDictionary, countryCode) = input
let (itn, totalItemValue, countryCode) = input
guard itn.isEmpty else {
return ITNNumberValidator.isValid(itn) ? nil : .invalidFormat
}

let totalItemValue = items.reduce(0, { sum, item in
sum + item.totalValue
})
if totalItemValue > Constants.minimumValueForRequiredITN {
return .missingForTotalShipmentValue
}
Expand All @@ -160,11 +154,6 @@ private extension WooShippingCustomsFormViewModel {
return .missingForRequiredDestination
}

for (_, value) in hsTariffNumberTotalValueDictionary {
if value > Constants.minimumValueForRequiredITN {
return .missingForTariffClass
}
}
return nil
}
.assign(to: &$itnValidationError)
Expand Down
39 changes: 14 additions & 25 deletions ...Shipping Create Shipping Labels/WooShipping Customs/WooShippingCustomsItemViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ final class WooShippingCustomsItemViewModel: ObservableObject {
@Published var hsTariffNumber: String = ""
@Published var valuePerUnit: String = ""
@Published var weightPerUnit: String = ""
// Useful to determine externally if the shipping requires an ITN
@Published var hsTariffNumberTotalValue: (String, Decimal)?

@Published private var originCountryCode: String?

Expand All @@ -31,13 +29,7 @@ final class WooShippingCustomsItemViewModel: ObservableObject {

@Published private(set) var countries: [Country] = []

var totalValue: Decimal {
guard currencySymbol == "$",
let valuePerUnitDecimal = Decimal(string: valuePerUnit) else {
return 0
}
return valuePerUnitDecimal * itemQuantity
}
@Published private(set) var totalValue: Decimal = 0

/// View model for selecting a country from a list.
var countrySelectorVM: CountrySelectorViewModel {
Expand Down Expand Up @@ -104,7 +96,7 @@ final class WooShippingCustomsItemViewModel: ObservableObject {

combineToPreselectCountry()
combineRequiredInformationIsEntered()
combineHSTariffNumberTotalValue()
combineTotalItemValue()
}
}

Expand Down Expand Up @@ -162,22 +154,19 @@ private extension WooShippingCustomsItemViewModel {
.store(in: &cancellables)
}

func combineHSTariffNumberTotalValue() {
Publishers.CombineLatest($valuePerUnit, $hsTariffNumber)
.sink { [weak self] valuePerUnit, hsTariffNumber in
guard let self = self else { return }

guard self.currencySymbol == "$",
let valuePerUnitDecimal = Decimal(string: valuePerUnit),
hsTariffNumber.isNotEmpty,
isValidTariffNumber else {
self.hsTariffNumberTotalValue = nil
return
}

self.hsTariffNumberTotalValue = (hsTariffNumber, valuePerUnitDecimal * itemQuantity)
func combineTotalItemValue() {
$valuePerUnit.map { [weak self] valuePerUnit in
guard
let self,
currencySymbol == "$",
let valuePerUnitDecimal = Decimal(string: valuePerUnit)
else {
return 0
}
.store(in: &cancellables)

return valuePerUnitDecimal * itemQuantity
}
.assign(to: &$totalValue)
}

/// Specifically introduced to check for a `0` value
Expand Down
49 changes: 7 additions & 42 deletions ...ing Create Shipping Labels/WooShipping Customs/WooShippingCustomsFormViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ final class WooShippingCustomsFormViewModelTests: XCTestCase {
}
}

func test_itnValidationError_when_item_view_models_hsTariffNumberTotalValue_is_nil() {
func test_itnValidationError_when_item_view_models_total_value_exceeds_threshold() {
// Given
viewModel = WooShippingCustomsFormViewModel(order: Order.fake().copy(currency: "USD"),
shipment: sampleShipment,
Expand All @@ -173,45 +173,6 @@ final class WooShippingCustomsFormViewModelTests: XCTestCase {
XCTAssertEqual(viewModel.itnValidationError, .missingForTotalShipmentValue)
}

func test_itnValidationError_when_item_view_models_hsTariffNumberTotalValue_is_less_than_2500() {
// Given
viewModel = WooShippingCustomsFormViewModel(order: Order.fake(),
shipment: sampleShipment,
onFormReady: { _ in })

// When
viewModel.itemsViewModels.first?.hsTariffNumberTotalValue = ("123456", 1000)

// Then
XCTAssertNil(viewModel.itnValidationError)
}

func test_itnValidationError_when_item_view_models_hsTariffNumberTotalValue_is_more_than_2500() {
// Given
viewModel = WooShippingCustomsFormViewModel(order: Order.fake(),
shipment: sampleShipment,
onFormReady: { _ in })

// When
viewModel.itemsViewModels[0].requiredInformationIsEntered = true
viewModel.itemsViewModels[0].hsTariffNumberTotalValue = ("123456", 1000)
viewModel.itemsViewModels[1].hsTariffNumberTotalValue = ("123456", 2000)
viewModel.itemsViewModels[1].requiredInformationIsEntered = true
viewModel.internationalTransactionNumber = ""
viewModel.updateDestinationCountry(code: "CA") // ITN is not required for Canada

// Then
XCTAssertTrue(viewModel.requiredInformationIsEntered)
XCTAssertNil(viewModel.itnValidationError)

// When
viewModel.updateDestinationCountry(code: "UK")

// Then
XCTAssertFalse(viewModel.requiredInformationIsEntered)
XCTAssertEqual(viewModel.itnValidationError, .missingForTariffClass)
}

func test_itnValidationError_when_destination_country_requires_ITN() {
// Given
let requiredDestinations = ["IR", "SY", "KP", "CU", "SD"]
Expand Down Expand Up @@ -481,12 +442,16 @@ final class WooShippingCustomsFormViewModelTests: XCTestCase {

private extension WooShippingCustomsFormViewModelTests {
var sampleShipment: Shipment {
return sampleShipment()
}

func sampleShipment(_ manualValuePerUnit: Double? = nil) -> Shipment {
let item1 = ShippingLabelPackageItem(productOrVariationID: 1,
orderItemID: 123,
name: "Shirt",
weight: 0.5,
quantity: 2,
value: 9.99,
value: manualValuePerUnit ?? 9.99,
dimensions: ProductDimensions.fake(),
attributes: [],
imageURL: nil)
Expand All @@ -495,7 +460,7 @@ private extension WooShippingCustomsFormViewModelTests {
name: "Pants",
weight: 0.5,
quantity: 1,
value: 11,
value: manualValuePerUnit ?? 11,
dimensions: ProductDimensions.fake(),
attributes: [],
imageURL: nil)
Expand Down
61 changes: 0 additions & 61 deletions ...ing Create Shipping Labels/WooShipping Customs/WooShippingCustomsItemViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,67 +66,6 @@ final class WooShippingCustomsItemViewModelTests: XCTestCase {
XCTAssertFalse(viewModel.isValidWeight, "isValidWeight should be false when the weight is zero")
}

func test_hsTariffNumberTotalValue_when_currency_symbol_is_not_$_returns_nil() {
// Given
viewModel = WooShippingCustomsItemViewModel(itemName: "Test",
itemProductID: 22,
itemQuantity: 1,
itemValue: 0,
itemWeight: 0,
currencySymbol: "$")

// Then
XCTAssertNil(viewModel.hsTariffNumberTotalValue)
}

func test_hsTariffNumberTotalValue_when_currency_symbol_is_$_but_hsTariffNumber_is_empty_returns_nil() {
//Given
viewModel = WooShippingCustomsItemViewModel(itemName: "Test",
itemProductID: 22,
itemQuantity: 1,
itemValue: 0,
itemWeight: 0,
currencySymbol: "$")

// When
viewModel.valuePerUnit = "1000"
viewModel.hsTariffNumber = ""

// Then
XCTAssertNil(viewModel.hsTariffNumberTotalValue)
}

func test_hsTariffNumberTotalValue_when_currency_symbol_is_$_but_invalid_hs_tariff_number_returns_nil() {
// Given
viewModel = WooShippingCustomsItemViewModel(itemName: "Test",
itemProductID: 22,
itemQuantity: 1,
itemValue: 0,
itemWeight: 0,
currencySymbol: "$")
viewModel.hsTariffNumber = "123"
viewModel.valuePerUnit = "1000"

// Then
XCTAssertNil(viewModel.hsTariffNumberTotalValue)
}

func test_hsTariffNumberTotalValue_when_currency_symbol_is_$_and_value_is_more_than_2500_and_valid_hs_tariff_number_returns_values() {
// Given
viewModel = WooShippingCustomsItemViewModel(itemName: "Test",
itemProductID: 22,
itemQuantity: 2,
itemValue: 0,
itemWeight: 0,
currencySymbol: "$")
viewModel.valuePerUnit = "3000"
viewModel.hsTariffNumber = "123456"

// Then
XCTAssertEqual(viewModel.hsTariffNumberTotalValue?.0, viewModel.hsTariffNumber)
XCTAssertEqual(viewModel.hsTariffNumberTotalValue?.1, Decimal(string: viewModel.valuePerUnit)! * 2)
}

func test_isNumberValid_whenGivenValidTariffNumbers_shouldReturnTrue() {
XCTAssertTrue(HSTariffNumberValidator.isNumberValid("123456"))
XCTAssertTrue(HSTariffNumberValidator.isNumberValid("12.34.56"))
Expand Down
Loading