fix: remove tax fields from cart queries unless its prepare for completion #280

kieran-osgood-shopify · 2025-02-24T12:29:32Z

What changes are you making?

There is a graphql schema issue (currently being tracked elsewhere, issue # TBT) where the total tax amount has been marked as deprecated on cart, but this should only be true for non cartCompletion requests

This PR adds an additional fragment specifically for cart completion that accesses this, and separates it from the original cart object for the sake of clarity (and safety, as the Buy sdk will throw if it you access a property that wasn't queried)

How to test

Open apple pay and ensure the requests that happen on open (cartBuyerIdentityUpdate, cartSelectedDeliveryOptionsUpdate ) do not request the totalTax amount, but the cartPrepareForCompletion after each does

Before you merge

Important

I've added tests to support my implementation
I have read and agree with the Contribution Guidelines.
I have read and agree with the Code of Conduct.
I've updated the README.

Checklist for releasing a new version

I have bumped the version number in the podspec file.
I have bumped the version number in the README.
I have added a Changelog entry.

Tip

See the Contributing documentation for instructions on how to publish a new version of the library.

…etion

Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.swift

kieran-osgood-shopify · 2025-02-27T20:01:01Z

Samples/MobileBuyIntegration/MobileBuyIntegration/ApplePay/ApplePayHandler.swift


+        do {


this ensures prepare is called even if updating the shipping options fails, which happens regularly at the moment due to a 500 server error atm

kieran-osgood-shopify · 2025-02-27T20:01:28Z

Samples/MobileBuyIntegration/MobileBuyIntegration/ApplePay/ApplePayHandler.swift

-            let shippingContact = payment.shippingContact,
-            payment.shippingContact?.postalAddress?.isoCountryCode == "US"
+            let shippingContact = payment.shippingContact
+//            payment.shippingContact?.postalAddress?.isoCountryCode == "US"


disabled this us country code restriction that only applies to tiagos store, will remove this going forwards i think

Want to remove it now?

kieran-osgood-shopify · 2025-02-27T20:02:05Z

Samples/MobileBuyIntegration/MobileBuyIntegration/ApplePay/ApplePayHandler.swift

-             * update the buyerIdentity with the shippingContact.email
-             */
+            guard
+                let emailAddress = shippingContact.emailAddress,


email guard moved in to vaulted state check as we require it via apple payment so its guaranteed to be set outside of this flow

kieran-osgood-shopify · 2025-02-27T20:03:31Z

Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.swift

@@ -345,7 +351,7 @@ class CartManager: ObservableObject {

        let mutation = Storefront.buildMutation(inContext: CartManager.ContextDirective) {
            $0.cartPaymentUpdate(cartId: cartId, payment: paymentInput) {
-                $0.cart { $0.cartManagerFragment() }
+                $0.cart { $0.cartPrepareForCompletionFragment() }


this change adds tax pulling for the payment update which is still not deprecated

kieran-osgood-shopify · 2025-02-27T20:04:53Z

I've just pushed some changes whilst debugging issues with american addresses, so converted back to draft.
Also adding a note here that the totalTaxAmount needs to be removed from cartManagerFragment before merging

markmur · 2025-06-18T09:49:24Z

Samples/MobileBuyIntegration/MobileBuyIntegration/ApplePay/ApplePayHandler.swift

@@ -146,7 +145,7 @@ extension ApplePayHandler: PKPaymentAuthorizationControllerDelegate {
        didSelectShippingContact contact: PKContact
    ) async -> PKPaymentRequestShippingContactUpdate {
        do {
-            _ = try await CartManager.shared.performBuyerIdentityUpdate(
+            _ = try await CartManager.shared.performCartBuyerIdentityUpdate(


Let's add @discardableResult to these methods to avoid the _ = everywhere

markmur · 2025-06-18T09:49:55Z

Samples/MobileBuyIntegration/MobileBuyIntegration/ApplePay/ApplePayHandler.swift

-            let shippingContact = payment.shippingContact,
-            payment.shippingContact?.postalAddress?.isoCountryCode == "US"
+            let shippingContact = payment.shippingContact
+//            payment.shippingContact?.postalAddress?.isoCountryCode == "US"


Want to remove it now?

Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.swift

markmur · 2025-06-18T09:52:12Z

Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.swift

+                 * Note - `totalTaxAmount` will not be available on self.cart, only access local value
+                 */


I'm not 100% following the "only access local value" part of the comment here

Yeah I can see it may only really makes sense in context with the comment on self.totalTaxAmount
I've modified the comment, and the variable name to make it more obvious

markmur · 2025-06-18T09:52:43Z

Samples/MobileBuyIntegration/MobileBuyIntegration/Info.plist

+	<key>ITSAppUsesNonExemptEncryption</key>
+	<false/>


Why is this needed?

I'll re-submit this as a separate PR, wasn't related to tax fields at all

It just stops app store from re-confirming this in the questionnaire when you submit to testflight

…swift Co-authored-by: Mark Murray <2034704+markmur@users.noreply.github.com>

fix: remove tax fields from cart queries unless its prepare for compl…

4d9d99a

…etion

kieran-osgood-shopify requested a review from a team as a code owner February 24, 2025 12:29

kieran-osgood-shopify added 4 commits February 24, 2025 12:30

refactor: inline guard statement

cfffb45

fix: lint issue with var names

086d7d3

chore: add doc comments on why we are using deprecated field

c6822f8

refactor: ensure summary items include tax from correct fields

bd0dd6f

kieran-osgood-shopify commented Feb 24, 2025

View reviewed changes

Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.swift Outdated Show resolved Hide resolved

kieran-osgood-shopify added 2 commits February 24, 2025 12:44

chore: add comments on why tax is separated from cart state

f9281af

refactor: undo cartResult rename

332608a

kieran-osgood-shopify force-pushed the kieran-osgood/fix/deprecated-tax-fields branch from ab14282 to 332608a Compare February 24, 2025 12:46

kieran-osgood-shopify added 4 commits February 27, 2025 19:58

debug: ensure tax is pulled and improve taxc name

25543dc

add code signing identitys

3f8b629

fix: non-exempt encryption prompt on testflight

04b6f8e

rename buyerIdentityUpdate to cartBuyerIdentityUpdate

55c0b98

kieran-osgood-shopify commented Feb 27, 2025

View reviewed changes

kieran-osgood-shopify marked this pull request as draft February 27, 2025 20:04

Merge branch 'main' into kieran-osgood/fix/deprecated-tax-fields

cea7874

kieran-osgood-shopify marked this pull request as ready for review April 7, 2025 14:28

Merge branch 'main' into kieran-osgood/fix/deprecated-tax-fields

5bf3035

markmur reviewed Jun 18, 2025

View reviewed changes

kieran-osgood-shopify and others added 5 commits June 20, 2025 14:26

Update Samples/MobileBuyIntegration/MobileBuyIntegration/CartManager.…

b82f649

…swift Co-authored-by: Mark Murray <2034704+markmur@users.noreply.github.com>

fix: remove unnecessary info plist value

4023855

refactor: mark discardableResult on fields we dont expect to be handled

4a22f56

refactor: improving doc comment and naming on cart with pending terms

6efdb8b

Amend comment

91ad2c3

kieran-osgood-shopify force-pushed the kieran-osgood/fix/deprecated-tax-fields branch from 2df72da to 91ad2c3 Compare June 20, 2025 13:42

fix: missed discardableResult

0f64d27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: remove tax fields from cart queries unless its prepare for completion #280

fix: remove tax fields from cart queries unless its prepare for completion #280

Uh oh!

kieran-osgood-shopify commented Feb 24, 2025

Uh oh!

Uh oh!

kieran-osgood-shopify Feb 27, 2025

Uh oh!

kieran-osgood-shopify Feb 27, 2025

Uh oh!

markmur Jun 18, 2025

Uh oh!

kieran-osgood-shopify Feb 27, 2025

Uh oh!

kieran-osgood-shopify Feb 27, 2025

Uh oh!

kieran-osgood-shopify commented Feb 27, 2025

Uh oh!

markmur Jun 18, 2025

Uh oh!

markmur Jun 18, 2025

Uh oh!

Uh oh!

markmur Jun 18, 2025

Uh oh!

kieran-osgood-shopify Jun 20, 2025

Uh oh!

markmur Jun 18, 2025

Uh oh!

kieran-osgood-shopify Jun 20, 2025

Uh oh!

Uh oh!

		* Note - `totalTaxAmount` will not be available on self.cart, only access local value
		*/

fix: remove tax fields from cart queries unless its prepare for completion #280

Are you sure you want to change the base?

fix: remove tax fields from cart queries unless its prepare for completion #280

Uh oh!

Conversation

kieran-osgood-shopify commented Feb 24, 2025

What changes are you making?

How to test

Before you merge

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kieran-osgood-shopify commented Feb 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!