-
Notifications
You must be signed in to change notification settings - Fork 264
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.
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
Checkout API to Cart API migration #1000
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ef4ee14
to
f27cb16
Compare
This is late and stores are breaking because of the ineptitude and disorganization here. GET IT DONE. |
0d35ea1
to
68b660b
Compare
Co-authored-by: Juan P. Prieto <jp@calltheguys.co>
Before these tests weren't actually testing anything useful - the API response was mocked, and then the test ensured that the mock returned the return value we mocked. Now these tests execute actual storefront API calls, and are now useful integration tests that we can trust.
The goal of this commit is to isolate the non-test, non-doc code changes to be reviewed. This commit contains: - Changes to the GQL operations used in the checkout resource - Input and payload mapping to match the previous inputs/payloads as closely as possible This commit does NOT contain: - Unit tests - Integration tests - Migration guide - Docs updates Co-authored-by: Juan P. Prieto <jp@calltheguys.co>
In cart-payload-mapper-test, these are just unit tests. All of the other tests make actual SF API calls and verify that the response returned from the SDK is what we expect. Our goal here is to ensure that when a given checkout method is called with SDK v3, the response is the same as what previous versions of the SDK returned (aside from known/documented discrepancies between the cart/checkout APIs). In the tests, the "expected" values are all values that we got when we ran these operations with the previous SDK version. The "actual" values are what v3 of the SDK returns for the same operation. Co-authored-by: Juan P. Prieto <jp@calltheguys.co>
This property is added on by the GraphQL JS client. `hasPreviousPage` was coming out as true and `hasNextPage` was coming out as false, but since we fetch all pages and give them back as a single array, those values are misleading, so now we're hard-coding them both to false.
As a sanity check during the development phase, we were throwing an error if we were aware of a discount code but it wasn't appearing in the output of our discount mapping. Upon investigating further, we determined that this is expected behaviour for any discount codes that are applied but have `applicable: false`. Therefore, we are updating our sanity check to only throw an error if the discount code IS applied but somehow didn't get mapped. These sanity checks that throw errors will be removed before release.
The assertions were slightly off, since the item is $70 rather than $73.5.
We throw some errors as part of the cart payload mapping. This ensures they'll be returned in the way the user is expecting.
Co-authored-by: Juan P. Prieto <jp@calltheguys.co>
78d16fa
to
4068572
Compare
We got feedback that files like index.js weren't appearing in our v3.0.0-rc.0 version in npm. Adding this explicitly to the shipit config should hopefully resolve the issue.
A shop testing our v3 RC reported that the subtotalPrice field was always being rendered in their shop as $0. We realized this is because that shop expected the currency amounts to be strings (like what the SF API/JS Buy SDK v2 return), but we were returning some currency amounts as numbers. This change ensures that all currency fields we return should be of type string rather than type number, which should resolve the bug for that shop. Normalize all currency fields in the checkout payload to contain the same number of decimal places that would be returned by the storefront API. In the storefront API, currency amounts are returned as a string that contains 1 decimal place (if the 2nd decimal place is 0) or else 2 decimal places. In our mapping functions, we are typically converting to strings with 2 decimal places. In case any clients of the JS Buy SDK are relying only a single decimal place being returned in some cases, we want to normalize the decimal places.
We want to ensure that the types are correct, in addition to the values. We don't want `24` and `'24.0'` to be considered "equal". Therefore, we've updated to using stricter equality checks in our tests.
Because this was missing, lineItemsSubtotalPrice was always null
There was a mistake originally, as cart's total amount includes any gift cards that have been applied, but checkout's total price should reflect the price before gift cards have been applied. Additionally, I've added more integration tests to ensure that all the expected fields are being returned (not null).
…al and cleanup SFAPI client guide
juanpprieto
approved these changes
Mar 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀🚀🚀🚀🚀🚀🚀🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Public SDK deprecation notice
WHY are these changes introduced?
The main goal of this (final) version of the SDK is to extend the longevity of its
.checkout
interface by replacing it with an equivalent interface based on the Cart APIWHAT is this pull request doing?
Replaces the
Checkout API
dependency from all.checkout
interface methods such asfetch
,addLines
,removeLines
etc. All existing checkout methods now use Cart API equivalents under the hood.Checklist