-
Notifications
You must be signed in to change notification settings - Fork 20
Feat(Shopify): headless account #1493
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
base: main
Are you sure you want to change the base?
Conversation
Tagging OptionsShould a new tag be published when this PR is merged?
|
WalkthroughAdds Shopify customer address CRUD actions, a password reset and user-update action, a paginated orders loader, corresponding GraphQL operations and generated types, and wires these into the manifest. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
shopify/actions/address/updateAddress.ts (1)
9-60: Action wiring and mutation variables look correct; consider reusing a shared address shapeThe token guard, destructuring, and
storefront.query<CustomerAddressUpdatePayload, MutationCustomerAddressUpdateArgs>call all look correct and consistent with theUpdateAddressmutation.Given
createAddressandupdateAddressshare the same address fields (address/country/province/city/zip), consider extracting a sharedAddressProps/MailingAddressPropstype and reusing it across actions to keep them in sync if you add fields likeaddress2,firstName, orphonelater.shopify/actions/address/createAddress.ts (1)
9-48: Create‑address logic is sound; consider consolidating address propsThe action correctly:
- Guards on
customerAccessToken.- Maps
address→address1and forwardscountry/province/city/zip.- Uses
storefront.query<CustomerAddressCreatePayload, MutationCustomerAddressCreateArgs>with the expected variables.As with
updateAddress, you might want a sharedAddressProps/MailingAddressPropstype reused between create/update to avoid the two drifting if you add more address fields later.shopify/utils/storefront/queries.ts (2)
458-475: Consider adding pagination and more address fields.The query hardcodes
first: 10and lacks pagination support. Customers with more than 10 addresses won't see all of them. Additionally, common address fields likeaddress2,firstName,lastName,phone, andcompanyare missing, which may limit the usefulness of the returned data.Consider this enhancement:
export const FetchCustomerAddresses = { - query: gql`query FetchCustomerAddresses($customerAccessToken: String!) { + query: gql`query FetchCustomerAddresses($customerAccessToken: String!, $first: Int = 10, $after: String) { customer(customerAccessToken: $customerAccessToken) { - addresses(first: 10) { + addresses(first: $first, after: $after) { edges { node { address1 + address2 city + company country + firstName id + lastName + phone province zip } } + pageInfo { + hasNextPage + endCursor + } } } }`, };
648-666: Minor inconsistency: missinguserErrorsfield.For consistency with
UpdateAddressandDeleteAddress, consider addinguserErrorsto the response.customerAddressCreate( customerAccessToken: $customerAccessToken, address: $address ) { customerAddress { id } customerUserErrors { code message } + userErrors { + message + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
shopify/actions/address/createAddress.ts(1 hunks)shopify/actions/address/deleteAddress.ts(1 hunks)shopify/actions/address/setDefaultAddress.ts(1 hunks)shopify/actions/address/updateAddress.ts(1 hunks)shopify/actions/user/sendPasswordResetEmail.ts(1 hunks)shopify/actions/user/updateUser.ts(1 hunks)shopify/loaders/orders/list.ts(1 hunks)shopify/manifest.gen.ts(1 hunks)shopify/utils/storefront/queries.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
shopify/actions/address/deleteAddress.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (2)
CustomerAddressDeletePayload(3200-3210)MutationCustomerAddressDeleteArgs(5343-5346)shopify/utils/storefront/queries.ts (1)
DeleteAddress(717-738)
shopify/actions/address/updateAddress.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (1)
CustomerAddressUpdatePayload(3213-3223)shopify/utils/storefront/queries.ts (1)
UpdateAddress(668-693)
shopify/actions/user/updateUser.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (1)
CustomerUpdatePayload(3376-3392)shopify/utils/storefront/queries.ts (1)
UpdateCustomerInfo(570-593)
shopify/loaders/orders/list.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (1)
QueryRootLocationsArgs(6669-6677)shopify/utils/storefront/queries.ts (1)
OrdersByCustomer(477-533)
shopify/actions/user/sendPasswordResetEmail.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (2)
CustomerRecoverPayload(3306-3314)MutationCustomerRecoverArgs(5371-5373)shopify/utils/storefront/queries.ts (1)
SendPasswordResetEmail(634-646)
shopify/actions/address/createAddress.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (1)
CustomerAddressCreatePayload(3187-3197)shopify/utils/storefront/queries.ts (1)
CreateAddress(648-666)
🔇 Additional comments (11)
shopify/actions/address/deleteAddress.ts (1)
9-41: Delete action is correctly wired to the mutation
addressId→idmapping, token guard, andstorefront.query<CustomerAddressDeletePayload, MutationCustomerAddressDeleteArgs>usage all look correct and consistent withDeleteAddress. No changes needed here.shopify/actions/user/sendPasswordResetEmail.ts (1)
8-32: Password‑reset action is correctly implementedThe action cleanly forwards
{ email }intoSendPasswordResetEmailand returns the typedCustomerRecoverPayload, letting the caller inspectcustomerUserErrors/userErrors. No changes needed here.shopify/actions/address/setDefaultAddress.ts (1)
9-41: Default‑address mutation wiring looks goodToken handling,
addressIdmapping, and the typedstorefront.query<CustomerDefaultAddressUpdatePayload, MutationCustomerDefaultAddressUpdateArgs>call all look consistent with theSetDefaultAddressmutation and the other address actions. No changes required.shopify/manifest.gen.ts (1)
5-57: Generated manifest wiring for new actions/loaders looks consistentThe new imports and entries for:
- address actions (
createAddress,deleteAddress,setDefaultAddress,updateAddress),- cart/order/user actions, and
- the
orders/listloader plusproxy,shop, anduserloadersall look coherent and match the file paths used elsewhere in the PR. As this file is generator‑owned, just ensure it’s regenerated (not hand‑edited) whenever actions or loaders move.
shopify/actions/user/updateUser.ts (1)
9-43: Update customer mutation looks correct; tie Props to the generated input type for future‑proofingThe action is correctly implemented: token handling,
UpdateCustomerInfousage, and the typedstorefront.query<CustomerUpdatePayload, MutationCustomerUpdateArgs>call all align with the GraphQL mutation.To avoid drift if you later expand the payload, consider defining:
import type { CustomerUpdateInput } from "../../utils/storefront/storefront.graphql.gen.ts"; interface Props extends CustomerUpdateInput { // Optionally narrow to the subset you expose in the UI }and then using
customer: propsinstead of a separatePropsshape. That keeps this action automatically in sync with Shopify'sCustomerUpdateInput.shopify/utils/storefront/queries.ts (6)
477-533: LGTM!Well-structured query with proper pagination support, relevant order fields, and
reverse: truefor showing newest orders first. The structure follows existing patterns in the file.
570-593: LGTM!The mutation follows Shopify's standard patterns and includes both
customerUserErrorsanduserErrorsfor comprehensive error handling.
634-646: LGTM!Clean implementation of the password reset flow using Shopify's
customerRecovermutation with proper error handling.
668-693: LGTM!Complete mutation with proper error handling including both
customerUserErrorsanduserErrors.
695-715: LGTM!The mutation correctly returns the new default address ID for confirmation.
717-738: LGTM!Well-implemented delete mutation returning
deletedCustomerAddressIdfor client-side state management, with comprehensive error handling.
| const { count = 12, pageOffset = 1 } = props; | ||
| const pageParam = searchParams.get("page") | ||
| ? Number(searchParams.get("page")) - pageOffset | ||
| : 0; | ||
|
|
||
| const page = props.page ?? pageParam; | ||
| const startCursor = props.startCursor ?? searchParams.get("startCursor") ?? | ||
| ""; | ||
| const endCursor = props.endCursor ?? searchParams.get("endCursor") ?? ""; | ||
|
|
||
| const variables = { | ||
| customerAccessToken, | ||
| ...(startCursor && { after: startCursor, first: count }), | ||
| ...(endCursor && { before: endCursor, last: count }), | ||
| ...(!startCursor && !endCursor && { first: count }), | ||
| }; | ||
|
|
||
| const data = await storefront.query< | ||
| QueryRoot, | ||
| QueryRootCustomerArgs & QueryRootLocationsArgs | ||
| >({ | ||
| ...OrdersByCustomer, | ||
| variables, | ||
| }); | ||
|
|
||
| const orders = data.customer?.orders?.nodes ?? []; | ||
| const pageInfo = data.customer?.orders?.pageInfo; | ||
|
|
||
| if (!pageInfo) { | ||
| throw new Error("Missing pageInfo from Shopify response"); | ||
| } | ||
|
|
||
| const nextPage = new URLSearchParams(searchParams); | ||
| const previousPage = new URLSearchParams(searchParams); | ||
|
|
||
| if (pageInfo.hasNextPage) { | ||
| nextPage.set("page", (page + pageOffset + 1).toString()); | ||
| nextPage.set("startCursor", pageInfo.endCursor ?? ""); | ||
| nextPage.delete("endCursor"); | ||
| } | ||
|
|
||
| if (pageInfo.hasPreviousPage) { | ||
| previousPage.set("page", (page + pageOffset - 1).toString()); | ||
| previousPage.set("endCursor", pageInfo.startCursor ?? ""); | ||
| previousPage.delete("startCursor"); | ||
| } | ||
|
|
||
| const currentPage = Math.max(1, page + pageOffset); | ||
|
|
||
| return { | ||
| orders, | ||
| pageInfo: { | ||
| nextPage: pageInfo.hasNextPage ? `?${nextPage}` : undefined, | ||
| previousPage: pageInfo.hasPreviousPage ? `?${previousPage}` : undefined, | ||
| currentPage, | ||
| records: data.customer?.orders.totalCount ?? 0, | ||
| recordPerPage: count, | ||
| }, |
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.
Previous‑page pagination is inconsistent with the GraphQL query; also harden page parsing
The loader is close, but two things are worth fixing:
- Backward pagination doesn’t currently match the
OrdersByCustomerquery
-
The query definition you’re using only declares
$customerAccessToken,$first, and$after, and passes them intoorders(first: $first, after: $after, reverse: true). -
The loader, however, builds variables with
beforeandlastwhen going backwards:const variables = { customerAccessToken, ...(startCursor && { after: startCursor, first: count }), ...(endCursor && { before: endCursor, last: count }), ...(!startCursor && !endCursor && { first: count }), };
-
Because
$before/$lastare not declared or used in the query, a “previous page” request will not actually page backwards based onendCursor, and may even fall back to Shopify’s defaults.
To make previous‑page links behave correctly, you should either:
-
extend
OrdersByCustomerto accept and forward$before/$lastto theordersconnection, and keep the current variable construction, e.g.:// shopify/utils/storefront/queries.ts (conceptual) query OrdersByCustomer( $customerAccessToken: String!, $first: Int, $after: String, $last: Int, $before: String ) { customer(customerAccessToken: $customerAccessToken) { orders( first: $first, after: $after, last: $last, before: $before, reverse: true ) { ... } } }
and keep the loader’s
variablesas-is (ensuring you never send both forward and backward args at the same time), or -
if you want only forward pagination for now, drop the
before/lastbranch andpreviousPagecomputation until the query supports them, so the UI doesn’t expose a non‑working prev link.
Given the PR description mentions a customer order list with pagination, aligning the query and loader for real backward pagination is preferable.
- Guard against invalid
pagequery parameters
If page is non‑numeric, Number(searchParams.get("page")) becomes NaN, which then propagates into currentPage and the page value written into nextPage/previousPage.
You can make this more robust with something like:
- const pageParam = searchParams.get("page")
- ? Number(searchParams.get("page")) - pageOffset
- : 0;
+ const rawPage = searchParams.get("page");
+ const parsedPage = rawPage ? Number(rawPage) : 0;
+ const safePage = Number.isNaN(parsedPage) ? 0 : parsedPage;
+ const pageParam = safePage - pageOffset;This keeps currentPage and the generated links sane even if someone passes a bad page value.
🤖 Prompt for AI Agents
In shopify/loaders/orders/list.ts around lines 72 to 129, the loader builds
backward-pagination variables (before/last) that the OrdersByCustomer GraphQL
query does not declare/use, and it does not robustly handle non-numeric page
query params; update to either align the query with before/last or remove the
backward branch, and validate page parsing. Fix: extend the OrdersByCustomer
query to accept $before and $last and forward them into orders (ensuring you
never set both forward and backward args simultaneously) so previousPage links
work, and/or if you choose not to implement backward paging yet remove the
before/last variable construction and previousPage URL generation; additionally,
defensively parse the page param by coercing Number(searchParams.get("page")) to
a safe integer with a fallback (e.g., isNaN -> 0 or default page value) before
using it to compute currentPage and link pages.
What is this Contribution About?
Implementing Headless Account for Shopify on Deco.cx
Description
This PR adds Headless Account functionality for Shopify on Deco.cx, enabling user account management via API without relying on Shopify’s default interface.
Completed Tasks
New actions:
Create Address)Set Default Address)Update Address)Delete Address)Update Customer Info)Send Password Reset Email)New Loader:
Get Addresses)List Customer Orders)Loom
https://www.loom.com/share/2b07a3b412674d7d8adef2fb004b2b5e
Reference
Shopify Storefront GraphQL API - Postman
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.