Skip to content
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
merged 26 commits into from
Mar 12, 2025
Merged

Checkout API to Cart API migration #1000

merged 26 commits into from
Mar 12, 2025

Conversation

kdaviduik
Copy link
Contributor

@kdaviduik kdaviduik commented Jan 16, 2025

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 API

WHAT is this pull request doing?

Replaces the Checkout API dependency from all .checkout interface methods such as fetch, addLines, removeLines etc. All existing checkout methods now use Cart API equivalents under the hood.

Checklist

  • Added tests
  • Added or updated documentation
  • Validated buy-button-js operation with this SDK

@kdaviduik kdaviduik marked this pull request as draft January 16, 2025 01:08
@kdaviduik kdaviduik force-pushed the sdk-v3 branch 2 times, most recently from ef4ee14 to f27cb16 Compare January 20, 2025 18:18
@chadwackerman
Copy link

This is late and stores are breaking because of the ineptitude and disorganization here. GET IT DONE.

@kdaviduik kdaviduik force-pushed the sdk-v3 branch 3 times, most recently from 0d35ea1 to 68b660b Compare March 4, 2025 18:59
kdaviduik and others added 11 commits March 4, 2025 17:46
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>
@kdaviduik kdaviduik force-pushed the sdk-v3 branch 2 times, most recently from 78d16fa to 4068572 Compare March 4, 2025 22:48
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.
kdaviduik and others added 3 commits March 11, 2025 12:08
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).
@kdaviduik kdaviduik requested a review from juanpprieto March 12, 2025 23:46
@kdaviduik kdaviduik marked this pull request as ready for review March 12, 2025 23:46
Copy link
Contributor

@juanpprieto juanpprieto left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀🚀🚀🚀🚀🚀

@kdaviduik kdaviduik merged commit 426bf04 into main Mar 12, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants