feat(core): Merge custom fields on updating order lines #3673
Conversation
Fixes #3391. BREAKING CHANGE: when using the `adjustOrderLine` mutation and passing custom fields, the values you pass will now be _merged_ with any existing custom field values. Previously, the custom fields object would completely _replace_ any existing values. Although this is more of a bug fix (the old behaviour is usually undesirable), it is listed as a breaking change in the unlikely case that someone relies on the existing behaviour. In this case, you'll need to explicitly set `null` values for any custom fields that you want to "remove".
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a merge strategy for updating Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ShopAPI
participant OrderService
participant DB
Client->>ShopAPI: adjustOrderLine(orderLineId, customFields)
ShopAPI->>OrderService: adjustOrderLines(orderLineId, customFields)
OrderService->>DB: Fetch existing OrderLine
OrderService->>OrderService: Merge customFields (overwrite, unset, preserve)
OrderService->>DB: Update OrderLine with merged customFields
OrderService->>ShopAPI: Return updated OrderLine
ShopAPI->>Client: Response with updated customFields
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/docs/guides/developer-guide/custom-fields/index.md (1)
1350-1382: Fix list style consistency.The unordered lists in this section use dashes while the rest of the document uses asterisks.
Apply this diff to maintain consistency:
-- Shop API: [addItemToOrder](/reference/graphql-api/shop/mutations#additemtoorder) will have a 3rd input argument, `customFields`, which allows custom field values to be set when adding an item to the order. -- Shop API: [adjustOrderLine](/reference/graphql-api/shop/mutations#additemtoorder) will have a 3rd input argument, `customFields`, which allows custom field values to be updated. -- Admin API: the equivalent mutations for manipulating draft orders and for modifying and order will also have inputs to allow custom field values to be set. +* Shop API: [addItemToOrder](/reference/graphql-api/shop/mutations#additemtoorder) will have a 3rd input argument, `customFields`, which allows custom field values to be set when adding an item to the order. +* Shop API: [adjustOrderLine](/reference/graphql-api/shop/mutations#additemtoorder) will have a 3rd input argument, `customFields`, which allows custom field values to be updated. +* Admin API: the equivalent mutations for manipulating draft orders and for modifying and order will also have inputs to allow custom field values to be set.-- Admin API: [modifyOrder](/reference/graphql-api/admin/mutations#modifyorder) will have a `customFields` field on the input object. +* Admin API: [modifyOrder](/reference/graphql-api/admin/mutations#modifyorder) will have a `customFields` field on the input object.-- Shop API: [eligibleShippingMethods](/reference/graphql-api/shop/queries#eligibleshippingmethods) will have public custom fields available on the result. +* Shop API: [eligibleShippingMethods](/reference/graphql-api/shop/queries#eligibleshippingmethods) will have public custom fields available on the result.-- Shop API: [eligiblePaymentMethods](/reference/graphql-api/shop/queries#eligiblepaymentmethods) will have public custom fields available on the result. +* Shop API: [eligiblePaymentMethods](/reference/graphql-api/shop/queries#eligiblepaymentmethods) will have public custom fields available on the result.-- Shop API: [registerCustomerAccount](/reference/graphql-api/shop/mutations#registercustomeraccount) will have have a `customFields` field on the input - object. +* Shop API: [registerCustomerAccount](/reference/graphql-api/shop/mutations#registercustomeraccount) will have have a `customFields` field on the input + object.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/docs/guides/developer-guide/custom-fields/index.md(16 hunks)packages/core/e2e/custom-fields.e2e-spec.ts(2 hunks)packages/core/e2e/order-line-custom-fields.e2e-spec.ts(1 hunks)packages/core/src/service/services/order.service.ts(1 hunks)scripts/codegen/generate-graphql-types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
scripts/codegen/generate-graphql-types.ts (1)
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-01T07:36:12.107Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Do not use explicit typings or generic parameters with useQuery or useMutation; rely on type inference from the graphql function.
packages/core/e2e/custom-fields.e2e-spec.ts (2)
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-01T07:36:12.107Z
Learning: Applies to packages/dashboard/src/**/*.{js,jsx,ts,tsx} : When creating useQuery calls, follow the documented pattern: import { api } from '@/graphql/api.js', import { graphql } from '@/graphql/graphql.js', and use useQuery from '@tanstack/react-query' with the graphql document and api.query.
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-01T07:36:12.107Z
Learning: Applies to packages/dashboard/src/**/*.{js,jsx,ts,tsx} : When creating useMutation calls, follow the documented pattern: import { api } from '@/graphql/api.js', import { graphql } from '@/graphql/graphql.js', and use useMutation from '@tanstack/react-query' with the graphql document and api.mutate.
docs/docs/guides/developer-guide/custom-fields/index.md (1)
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-01T07:36:12.107Z
Learning: Applies to packages/dashboard/src/**/*.{js,jsx,ts,tsx} : For forms, prefer using the FormFieldWrapper component (the custom wrapper around Shadcn react-hook-form components) over raw Shadcn components.
🪛 markdownlint-cli2 (0.17.2)
docs/docs/guides/developer-guide/custom-fields/index.md
1350-1350: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1351-1351: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1352-1352: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1362-1362: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1368-1368: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1374-1374: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
1380-1380: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: unit tests (22.x)
- GitHub Check: codegen / codegen
- GitHub Check: unit tests (24.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (20.x)
- GitHub Check: build (22.x)
- GitHub Check: build (24.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
packages/core/src/service/services/order.service.ts (1)
784-804: Excellent implementation of the custom fields merge strategy!The implementation correctly addresses the PR objective by merging custom fields instead of replacing them entirely. The logic properly handles:
- Preserving existing custom field values when not explicitly changed
- Allowing explicit
nullvalues to unset fields- Ignoring
undefinedvalues to maintain existing data- Using the merged fields for relation updates
The code is well-documented and follows a clear, logical flow.
scripts/codegen/generate-graphql-types.ts (1)
37-37: LGTM! Correct exclusion of the new test file.Adding the new
order-line-custom-fields.e2e-specto the ignore list is appropriate since end-to-end test specification files should not be included in the GraphQL type generation process.packages/core/e2e/custom-fields.e2e-spec.ts (1)
7-7: LGTM! Clean formatting improvements.The import consolidation and formatting fixes improve code consistency.
Also applies to: 10-10, 194-195
docs/docs/guides/developer-guide/custom-fields/index.md (2)
21-21: LGTM! Formatting improvements enhance readability.The consistent spacing around commas and braces improves code readability throughout the documentation.
Also applies to: 64-72, 91-101, 114-122, 194-195, 354-358, 383-386, 560-563, 634-636, 702-704, 965-967, 1195-1208, 1216-1218, 1264-1270
1342-1382: Excellent documentation of special API behaviors!This new section clearly explains the automatic API extensions that occur when custom fields are defined on specific entities. The information about OrderLine custom fields directly supports the PR's merging functionality.
packages/core/e2e/order-line-custom-fields.e2e-spec.ts (1)
1-373: Excellent test coverage for the new merging behavior!This comprehensive test suite thoroughly validates the custom fields merging functionality:
- Tests all custom field types (string, int, boolean, nullable, relation)
- Validates partial updates preserve existing values
- Confirms explicit null unsets fields
- Covers edge cases with empty objects
- Ensures proper test isolation with beforeEach cleanup
The tests clearly demonstrate the breaking change behavior where users must now explicitly set fields to
nullto remove them.
Description
Fixes #3391
When you have custom fields on the
OrderLineentity, and use theadjustOrderLinemutation to update the values, Vendure will now merge those values with any existing values that already are set on the given OrderLine.Breaking changes
Although this is more of a bug fix (the old behaviour is usually undesirable),
it is listed as a breaking change in the unlikely case that someone relies on
the existing behaviour. In this case, you'll need to explicitly set
nullvalues for any custom fields that you want to "remove".
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit
Documentation
Bug Fixes
Tests