fix(rest/nodejs): quote one fulfillment group per method - #181
Open
yingzhehu-TT wants to merge 1 commit into
Open
fix(rest/nodejs): quote one fulfillment group per method#181yingzhehu-TT wants to merge 1 commit into
yingzhehu-TT wants to merge 1 commit into
Conversation
damaz91
approved these changes
Aug 13, 2026
This file contains hidden or 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
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.
Summary
Quote one fulfillment group per method, per
fulfillment.md, so a method themerchant cannot quote options for reports its line items with an empty option
list rather than no group at all.
Motivation
fulfillment.md"Business Response Behavior" is a MUST:recalculateTotalsonly ever created that group insidemethod.type === "shipping" && method.selected_destination_id && method.destinations,and then only when the destination resolved to a country. Every other method —
any non-shipping type, or a shipping method whose destination the merchant does
not serve — was returned with
groups: [].That is also a correctness bug, not just a schema deviation. The completion gate
reads
method.groups?.every((group) => group.selected_option_id), and[].every(...)istrue, so such a checkout completed with no option everselected, no fulfillment charged, and an order whose
fulfillment.expectationswas empty — the merchant had nothing telling them what to ship or where.
This is the same vacuous-truth hazard #155 fixed one level up, where the comment
already reads "an empty methods array must not satisfy the gate via
[].every(...) === true". The nested array had no equivalent guard.The Python reference server was never affected:
checkout_service.pygates onif method.groups:, and an empty list is falsy in Python. This change brings theNode sample in line on that case.
Scope
One behaviour change, in
recalculateTotals. The completion gate is deliberatelynot touched: once every method carries a group, the gate's existing
selected_option_idcheck is what refuses completion, and hardening it furtherwould add an unreachable branch. The
if (!method.groups || …)block inside thequoting path becomes dead and is folded into the assignment it guarded.
line_item_idsis seeded with?? []becausefulfillment_group.jsonlists itas required — a repaired group should not itself be schema-invalid.
Known consequence, stated deliberately: this mock merchant quotes options for
shipping only, so a
pickupmethod now returns a group withoptions: []andstays uncompletable. Refusing is the correct outcome — silently completing was
the bug — but it means the sample advertises a
pickupmethod it cannot serve.Quoting real pickup options is a new merchant capability rather than part of this
fix; happy to add it here if maintainers prefer.
Validation
npm test134 passed / 0 failed (133 before).tsc --noEmitexit 0.prettier --checkclean.The new test fails against
mainwithexpected: 1, actual: 0— no group isproduced — and passes with the change.
Totals were diffed across create / option-selection / completion, pickup,
multi-method, unknown buyer, no destination, digital-only, empty methods, and a
PUT without fulfillment: no
totalsvalue changes. The only responsedifference is
groups: []becoming a single group carrying the method's lineitems.