Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions .changeset/fix-visitor-consent-conditional.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@shopify/hydrogen': patch
'@shopify/hydrogen-react': patch
---

Fix cart operations failing on stores without `VisitorConsent` type

Cart operations (like `cart.setMetafields()`) were unconditionally including the `visitorConsent` parameter in GraphQL operations, even when not being used. This caused failures on stores whose Storefront API schema doesn't include the `VisitorConsent` type (older API versions or certain store configurations).

The `visitorConsent` parameter is now only included in cart GraphQL operations when explicitly provided. This restores compatibility with stores that don't support the `VisitorConsent` type while preserving the feature for users who need it.
56 changes: 56 additions & 0 deletions packages/hydrogen-react/src/cart-queries.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import {describe, it, expect} from 'vitest';
import {
CartLineAdd,
CartCreate,
CartLineRemove,
CartLineUpdate,
CartNoteUpdate,
CartBuyerIdentityUpdate,
CartAttributesUpdate,
CartDiscountCodesUpdate,
CartQuery,
} from './cart-queries';

const MOCK_FRAGMENT = '...MockFragment';

describe('cart-queries', () => {
describe('visitorConsent conditional inclusion', () => {
const queries = [
{name: 'CartLineAdd', fn: CartLineAdd},
{name: 'CartCreate', fn: CartCreate},
{name: 'CartLineRemove', fn: CartLineRemove},
{name: 'CartLineUpdate', fn: CartLineUpdate},
{name: 'CartNoteUpdate', fn: CartNoteUpdate},
{name: 'CartBuyerIdentityUpdate', fn: CartBuyerIdentityUpdate},
{name: 'CartAttributesUpdate', fn: CartAttributesUpdate},
{name: 'CartDiscountCodesUpdate', fn: CartDiscountCodesUpdate},
{name: 'CartQuery', fn: CartQuery},
];

describe.each(queries)('$name', ({fn}) => {
it('should not include visitorConsent by default', () => {
const query = fn(MOCK_FRAGMENT);

expect(query).toContain('@inContext');
expect(query).not.toContain('visitorConsent');
expect(query).not.toContain('VisitorConsent');
});

it('should not include visitorConsent when explicitly disabled', () => {
const query = fn(MOCK_FRAGMENT, {includeVisitorConsent: false});

expect(query).toContain('@inContext');
expect(query).not.toContain('visitorConsent');
expect(query).not.toContain('VisitorConsent');
});

it('should include visitorConsent when explicitly enabled', () => {
const query = fn(MOCK_FRAGMENT, {includeVisitorConsent: true});

expect(query).toContain('@inContext');
expect(query).toContain('$visitorConsent: VisitorConsent');
expect(query).toContain('visitorConsent: $visitorConsent');
});
});
});
});
150 changes: 72 additions & 78 deletions packages/hydrogen-react/src/cart-queries.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,41 @@
export const CartLineAdd = (cartFragment: string): string => /* GraphQL */ `
type CartQueryOptions = {
includeVisitorConsent?: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what was the previous behaviour before this param existed? we should default to whatever was before, right?

is there ever a world where we would want people not to include visitor consent? and should we even allow that?

these are genuine questions, im wondering if we could just hardcode it as true and include it every time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The default behaviour was that it wasn't included. We added the feature to include it unconditionally based on an API change (#3408). That PR states: "Most Hydrogen storefronts do NOT need this feature. If you're using Hydrogen's analytics provider, or Shopify's Customer Privacy API, then consent is already handled automatically." So we're defaulting to the original behaviour in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

excellent! thanks for the comprehensive reply

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thank you for prompting me to double check!

};

function getInContextVariables(includeVisitorConsent: boolean): string {
const base = `$country: CountryCode = ZZ
$language: LanguageCode`;

return includeVisitorConsent
? `${base}
$visitorConsent: VisitorConsent`
: base;
}

function getInContextDirective(includeVisitorConsent: boolean): string {
return includeVisitorConsent
? `@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
)`
: `@inContext(
country: $country
language: $language
)`;
}

export const CartLineAdd = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartLineAdd(
$cartId: ID!
$lines: [CartLineInput!]!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartLinesAdd(cartId: $cartId, lines: $lines) {
cart {
...CartFragment
Expand All @@ -22,19 +46,16 @@ export const CartLineAdd = (cartFragment: string): string => /* GraphQL */ `
${cartFragment}
`;

export const CartCreate = (cartFragment: string): string => /* GraphQL */ `
export const CartCreate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartCreate(
$input: CartInput!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartCreate(input: $input) {
cart {
...CartFragment
Expand All @@ -45,20 +66,17 @@ export const CartCreate = (cartFragment: string): string => /* GraphQL */ `
${cartFragment}
`;

export const CartLineRemove = (cartFragment: string): string => /* GraphQL */ `
export const CartLineRemove = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartLineRemove(
$cartId: ID!
$lines: [ID!]!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartLinesRemove(cartId: $cartId, lineIds: $lines) {
cart {
...CartFragment
Expand All @@ -69,20 +87,17 @@ export const CartLineRemove = (cartFragment: string): string => /* GraphQL */ `
${cartFragment}
`;

export const CartLineUpdate = (cartFragment: string): string => /* GraphQL */ `
export const CartLineUpdate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartLineUpdate(
$cartId: ID!
$lines: [CartLineUpdateInput!]!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartLinesUpdate(cartId: $cartId, lines: $lines) {
cart {
...CartFragment
Expand All @@ -93,20 +108,17 @@ export const CartLineUpdate = (cartFragment: string): string => /* GraphQL */ `
${cartFragment}
`;

export const CartNoteUpdate = (cartFragment: string): string => /* GraphQL */ `
export const CartNoteUpdate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartNoteUpdate(
$cartId: ID!
$note: String!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartNoteUpdate(cartId: $cartId, note: $note) {
cart {
...CartFragment
Expand All @@ -119,20 +131,15 @@ export const CartNoteUpdate = (cartFragment: string): string => /* GraphQL */ `

export const CartBuyerIdentityUpdate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartBuyerIdentityUpdate(
$cartId: ID!
$buyerIdentity: CartBuyerIdentityInput!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartBuyerIdentityUpdate(cartId: $cartId, buyerIdentity: $buyerIdentity) {
cart {
...CartFragment
Expand All @@ -145,20 +152,15 @@ export const CartBuyerIdentityUpdate = (

export const CartAttributesUpdate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartAttributesUpdate(
$attributes: [AttributeInput!]!
$cartId: ID!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartAttributesUpdate(attributes: $attributes, cartId: $cartId) {
cart {
...CartFragment
Expand All @@ -171,20 +173,15 @@ export const CartAttributesUpdate = (

export const CartDiscountCodesUpdate = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
mutation CartDiscountCodesUpdate(
$cartId: ID!
$discountCodes: [String!]!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cartDiscountCodesUpdate(cartId: $cartId, discountCodes: $discountCodes) {
cart {
...CartFragment
Expand All @@ -195,19 +192,16 @@ export const CartDiscountCodesUpdate = (
${cartFragment}
`;

export const CartQuery = (cartFragment: string): string => /* GraphQL */ `
export const CartQuery = (
cartFragment: string,
options: CartQueryOptions = {},
): string => /* GraphQL */ `
query CartQuery(
$id: ID!
$numCartLines: Int = 250
$country: CountryCode = ZZ
$language: LanguageCode
$visitorConsent: VisitorConsent
${getInContextVariables(options.includeVisitorConsent ?? false)}
)
@inContext(
country: $country
language: $language
visitorConsent: $visitorConsent
) {
${getInContextDirective(options.includeVisitorConsent ?? false)} {
cart(id: $id) {
...CartFragment
}
Expand Down
Loading
Loading