Skip to content

refactor: defer handling of prepareForCompletion violations to submission #545

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
kieran-osgood-shopify merged 1 commit into from
Mar 27, 2026
Merged

refactor: defer handling of prepareForCompletion violations to submission #545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 6 additions & 13 deletions ...ets/ApplePay/ApplePayAuthorizationDelegate/ApplePayAuthorizationDelegate+Controller.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
// Taxes may become pending again fail to resolve despite updating within the didUpdatePaymentMethod
// So we retry one time to see if the error clears on retry
_ = try await Task.retrying(priority: nil, maxRetryCount: 1) {
try await self.controller.storefront.cartPaymentUpdate(

Check warning on line 236 in Sources/ShopifyAcceleratedCheckouts/Wallets/ApplePay/ApplePayAuthorizationDelegate/ApplePayAuthorizationDelegate+Controller.swift

View workflow job for this annotation

GitHub Actions / call-workflow-passing-data / test 

capture of 'self' with non-Sendable type 'ApplePayAuthorizationDelegate' in a '@sendable' closure

Check warning on line 236 in Sources/ShopifyAcceleratedCheckouts/Wallets/ApplePay/ApplePayAuthorizationDelegate/ApplePayAuthorizationDelegate+Controller.swift

View workflow job for this annotation

GitHub Actions / call-workflow-passing-data / test 

capture of 'self' with non-Sendable type 'ApplePayAuthorizationDelegate' in a '@sendable' closure
id: cartID,
totalAmount: totalAmount,
applePayPayment: applePayPayment
Expand Down Expand Up @@ -302,14 +302,10 @@
)
}

// `prepareCartForCompletion` filters 'non-terminal' violations
// See: `ErrorHandler.filterApplePayResolvableViolations`
//
// Before `didAuthorizePayment`, Apple Pay redacts PII, these are expected to be resolved by submit
// After `didAuthorizePayment`, PII is available, but we continue filtering in `prepareForCompletion` so
// `cartSubmitForCompletion` can return all violations at once
// This gives the user the best chance to fix everything in a single pass.
//
/// `prepareCartForCompletion` ignores all violations — submit handles error surfacing.
/// Before `didAuthorizePayment`, Apple Pay redacts PII so most violations are expected.
/// We rely on `cartSubmitForCompletion` to return all violations at once,
/// giving the user the best chance to fix everything in a single pass.
@discardableResult
func prepareCartForCompletion(id: GraphQLScalars.ID) async throws -> StorefrontAPI.Cart? {
do {
Expand All @@ -320,11 +316,8 @@
if case let .response(_, _, .cartPrepareForCompletion(payload)) = error,
case let .notReady(notReady) = payload.result
{
let actionableErrors = ErrorHandler.filterApplePayResolvableViolations(errors: notReady.errors)
if actionableErrors.isEmpty {
try setCart(to: notReady.cart)
return notReady.cart
}
try setCart(to: notReady.cart)
return notReady.cart
}
throw error
}
Expand Down
70 changes: 5 additions & 65 deletions ...opifyAcceleratedCheckouts/Wallets/ApplePay/ErrorHandler/ErrorHandler_CartCompletion.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ extension ErrorHandler {

private static func map(
payload: StorefrontAPI.CartPrepareForCompletionPayload,
shippingCountry: String?,
requiredContactFields: Set<PKContactField>?
shippingCountry _: String?,
Copy link
Copy Markdown
Contributor Author

@kieran-osgood-shopify kieran-osgood-shopify Mar 27, 2026

Choose a reason for hiding this comment

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

I'm purposefully not fully undoing this path entirely as I know this will be useful again when we improve the UX to surface these errors agin

requiredContactFields _: Set<PKContactField>?
) -> PaymentSheetAction {
guard let result = payload.result else { return PaymentSheetAction.interrupt(reason: .other) }
switch result {
Expand All @@ -56,22 +56,10 @@ extension ErrorHandler {
return PaymentSheetAction.interrupt(reason: .cartNotReady)
}
let allCodes = notReady.errors.map { $0.rawCode }.joined(separator: ", ")
let actionableErrors = filterApplePayResolvableViolations(errors: notReady.errors)
if actionableErrors.isEmpty {
ShopifyAcceleratedCheckouts.logger.debug(
"ErrorHandler: prepare: all violations are Apple Pay resolvable, continuing flow. Filtered: [\(allCodes)]"
)
return PaymentSheetAction.continueFlow
}
let actionableCodes = actionableErrors.map { $0.rawCode }.joined(separator: ", ")
ShopifyAcceleratedCheckouts.logger.error(
"ErrorHandler: prepare: actionable violations found: [\(actionableCodes)]. All violations: [\(allCodes)]"
ShopifyAcceleratedCheckouts.logger.debug(
"ErrorHandler: prepare: ignoring violations, deferring to submit. Codes: [\(allCodes)]"
)
let filteredErrors = filterGenericViolations(errors: actionableErrors)
let actions = filteredErrors.map {
getErrorAction(error: $0, shippingCountry: shippingCountry, checkoutURL: nil, requiredContactFields: requiredContactFields)
}
return getHighestPriorityAction(actions: actions)
return PaymentSheetAction.continueFlow
case .throttled:
return PaymentSheetAction.interrupt(reason: .cartThrottled)
case .ready:
Expand Down Expand Up @@ -494,54 +482,6 @@ extension ErrorHandler {

// MARK: - Helpers

private static let applePayResolvableViolationCodes: Set<StorefrontAPI.CartCompletionErrorCode> = [
.deliveryAddress1Invalid,
.deliveryAddress1Required,
.deliveryAddress1TooLong,
.deliveryAddress2Invalid,
.deliveryAddress2Required,
.deliveryAddress2TooLong,
.deliveryAddressRequired,
.deliveryCityInvalid,
.deliveryCityRequired,
.deliveryCityTooLong,
.deliveryCompanyInvalid,
.deliveryCompanyRequired,
.deliveryCompanyTooLong,
.deliveryCountryRequired,
.deliveryFirstNameInvalid,
.deliveryFirstNameRequired,
.deliveryFirstNameTooLong,
.deliveryInvalidPostalCodeForCountry,
.deliveryInvalidPostalCodeForZone,
.deliveryLastNameInvalid,
.deliveryLastNameRequired,
.deliveryLastNameTooLong,
.deliveryOptionsPhoneNumberInvalid,
.deliveryOptionsPhoneNumberRequired,
.deliveryPhoneNumberInvalid,
.deliveryPhoneNumberRequired,
.deliveryPostalCodeInvalid,
.deliveryPostalCodeRequired,
.deliveryZoneNotFound,
.deliveryZoneRequiredForCountry,
.buyerIdentityEmailIsInvalid,
.buyerIdentityEmailRequired,
.buyerIdentityPhoneIsInvalid,

// When default address is non-serviable supported then sheet immediately closes
// During #583 - These may be remapped to .showError during completion stage
.deliveryNoDeliveryAvailableForMerchandiseLine, .merchandiseNotApplicable
]

static func filterApplePayResolvableViolations(
errors: [StorefrontAPI.CartCompletionError]
) -> [StorefrontAPI.CartCompletionError] {
errors.filter { error in
!applePayResolvableViolationCodes.contains(error.code)
}
}

private static func filterGenericViolations(errors: [StorefrontAPI.CartCompletionError]) -> [StorefrontAPI.CartCompletionError] {
if errors.count == 1, errors.first?.code == .paymentsUnacceptablePaymentAmount {
return errors
Expand Down
144 changes: 8 additions & 136 deletions ...koutsTests/Wallets/ApplePay/ErrorHandler/ErrorHandler_CartPrepareForCompletionTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,34 +105,13 @@ class ErrorHandler_CartPrepareForCompletionTests: XCTestCase {
}
}

// MARK: - Apple Pay Resolvable Violation Filtering
// MARK: - All prepare violations are ignored (deferred to submit)

func testMap_whenNotReadyWithOnlyApplePayResolvableErrors_returnsContinueFlow() {
func testMap_whenNotReadyWithErrors_returnsContinueFlow() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryFirstNameRequired, message: "Enter a first name"),
.init(code: .deliveryLastNameRequired, message: "Enter a last name"),
.init(code: .deliveryAddress1Required, message: "Enter an address"),
.init(code: .deliveryPhoneNumberRequired, message: "Enter a phone number")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
userErrors: []
)

let result = ErrorHandler.map(stage: .prepare(payload), shippingCountry: nil)

switch result {
case .continueFlow:
break
default:
XCTFail("Expected continueFlow when all errors are Apple Pay resolvable, got: \(result)")
}
}

func testMap_whenNotReadyWithBuyerIdentityErrors_returnsContinueFlow() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .buyerIdentityEmailRequired, message: "Email required"),
.init(code: .buyerIdentityEmailIsInvalid, message: "Email invalid")
.init(code: .merchandiseOutOfStock, message: "Item out of stock"),
.init(code: .taxesMustBeDefined, message: "Tax error")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
Expand All @@ -145,87 +124,11 @@ class ErrorHandler_CartPrepareForCompletionTests: XCTestCase {
case .continueFlow:
break
default:
XCTFail("Expected continueFlow when all errors are BUYER_IDENTITY_*, got: \(result)")
}
}

func testMap_whenNotReadyWithMixOfApplePayResolvableAndActionableError_returnsActionForActionableError() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryFirstNameRequired, message: "Enter a first name"),
.init(code: .merchandiseOutOfStock, message: "Item out of stock")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
userErrors: []
)

let result = ErrorHandler.map(stage: .prepare(payload), shippingCountry: nil)

switch result {
case let .interrupt(reason, _):
XCTAssertEqual(reason, .outOfStock)
default:
XCTFail("Expected interrupt with .outOfStock for actionable error")
}
}

func testMap_whenNotReadyWithMultipleActionableErrors_returnsHighestPriorityAction() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryFirstNameRequired, message: "Apple Pay resolvable - filtered"),
.init(code: .merchandiseOutOfStock, message: "Out of stock"),
.init(code: .taxesMustBeDefined, message: "Tax error")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
userErrors: []
)

let result = ErrorHandler.map(stage: .prepare(payload), shippingCountry: nil)

switch result {
case let .interrupt(reason, _):
XCTAssertEqual(reason, .outOfStock, "outOfStock should take priority over unhandled tax error")
default:
XCTFail("Expected interrupt for multiple actionable errors")
XCTFail("Expected continueFlow — all prepare violations are deferred to submit, got: \(result)")
}
}

func testMap_whenNotReadyWithMultipleDeceleration_returnsHighestPriorityInterrupt() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .merchandiseNotEnoughStockAvailable, message: "Not enough stock"),
.init(code: .paymentsUnacceptablePaymentAmount, message: "Payment amount issue")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
userErrors: []
)

let result = ErrorHandler.map(stage: .prepare(payload), shippingCountry: nil)

switch result {
case let .interrupt(reason, _):
XCTAssertEqual(reason, .notEnoughStock, "notEnoughStock should win when paymentsUnacceptable is filtered by filterGenericViolations")
default:
XCTFail("Expected interrupt for multiple deceleration errors")
}
}

func testFilterApplePayResolvableViolations_preservesNonResolvableErrors() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryFirstNameRequired, message: ""),
.init(code: .merchandiseOutOfStock, message: ""),
.init(code: .buyerIdentityEmailRequired, message: ""),
.init(code: .paymentsUnacceptablePaymentAmount, message: "")
]

let filtered = ErrorHandler.filterApplePayResolvableViolations(errors: errors)

XCTAssertEqual(filtered.count, 2)
XCTAssertEqual(filtered[0].code, .merchandiseOutOfStock)
XCTAssertEqual(filtered[1].code, .paymentsUnacceptablePaymentAmount)
}

func testFilterApplePayResolvableViolations_preservesUnknownDeliveryCodes() throws {
func testMap_whenNotReadyWithUnknownErrorCodes_returnsContinueFlow() throws {
let json = """
{"code": "DELIVERY_DETAIL_CHANGED", "message": "Delivery details changed"}
"""
Expand All @@ -234,39 +137,8 @@ class ErrorHandler_CartPrepareForCompletionTests: XCTestCase {
XCTAssertEqual(error.code, .unknownValue)
XCTAssertEqual(error.rawCode, "DELIVERY_DETAIL_CHANGED")

let filtered = ErrorHandler.filterApplePayResolvableViolations(errors: [error])
XCTAssertEqual(filtered.count, 1, "Unknown DELIVERY_* codes should not be assumed Apple Pay resolvable")
}

func testFilterApplePayResolvableViolations_filtersDeliveryNoDeliveryAvailableForMerchandiseLine() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryNoDeliveryAvailableForMerchandiseLine, message: "Can't ship to address")
]

let filtered = ErrorHandler.filterApplePayResolvableViolations(errors: errors)

XCTAssertEqual(filtered.count, 0)
}

func testFilterApplePayResolvableViolations_preservesDeliveryNoDeliveryAvailable() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryNoDeliveryAvailable, message: "No delivery available")
]

let filtered = ErrorHandler.filterApplePayResolvableViolations(errors: errors)

XCTAssertEqual(filtered.count, 1)
XCTAssertEqual(filtered[0].code, .deliveryNoDeliveryAvailable)
}

func testMap_whenNotReadyWithResolvableAndDeliveryNoDeliveryForMerchandiseLine_returnsContinueFlow() {
let errors: [StorefrontAPI.CartCompletionError] = [
.init(code: .deliveryLastNameRequired, message: "Enter a last name"),
.init(code: .deliveryAddress1Required, message: "Enter an address"),
.init(code: .deliveryNoDeliveryAvailableForMerchandiseLine, message: "Can't be shipped to your address")
]
let payload = StorefrontAPI.CartPrepareForCompletionPayload(
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: errors)),
result: .notReady(StorefrontAPI.CartStatusNotReady(cart: nil, errors: [error])),
userErrors: []
)

Expand All @@ -276,7 +148,7 @@ class ErrorHandler_CartPrepareForCompletionTests: XCTestCase {
case .continueFlow:
break
default:
XCTFail("Expected continueFlow when all errors are Apple Pay resolvable, got: \(result)")
XCTFail("Expected continueFlow — unknown codes in prepare are deferred to submit, got: \(result)")
}
}
}
Loading