fix(admin): enforce carrier rules on shipment options in settings and orders - #381
fix(admin): enforce carrier rules on shipment options in settings and orders#381FreekVR wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Enforces carrier capability rules (requires/excludes/isRequired) for shipment options across the admin shipment-option forms, and forwards the cart’s calculated shipment options to the checkout delivery-options widget so it can lock/derive dependent options consistently.
Changes:
- Introduces a centralized
useShipmentOptionsStatemodule that resolves option visibility/locking/forced values via a pureresolveOptionStates()and exposes that state to field hooks. - Refactors admin shipment-option fields and capability auto-clear logic to use the new option-state + shared shipment-query snapshot reader.
- Adds
cartShipmentOptionsto checkout context/types and threads it into the checkout delivery-options configuration (store + update flow), plus adds tri-state v-model adaptation for plugin toggle components.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/common/src/types/php-pdk.types.ts | Adds cartShipmentOptions to the checkout context type. |
| libs/checkout-delivery-options/src/utils/updateContext.ts | Passes cartShipmentOptions into the delivery-options configuration during context refresh. |
| libs/checkout-delivery-options/src/types/store.types.ts | Documents configuration includes cart shipment options. |
| libs/checkout-delivery-options/src/store/createDeliveryOptionsStore.ts | Seeds delivery-options configuration with cartShipmentOptions from checkout context. |
| apps/admin/src/forms/shipmentOptions/useShipmentOptionsState.visibility.spec.ts | Integration test ensuring option visibility stays reactive with real form-builder hooks. |
| apps/admin/src/forms/shipmentOptions/useShipmentOptionsState.ts | New option-state module: resolves supported/readOnly/forced values and writes forced values into the form. |
| apps/admin/src/forms/shipmentOptions/useShipmentOptionsState.spec.ts | Unit + reactive tests for resolver and watcher behavior (requires/excludes/required, refetch windows, carrier switches). |
| apps/admin/src/forms/shipmentOptions/useCapabilitiesAutoClear.ts | Refactors auto-clear to reuse shared shipment snapshot logic and shared optionFieldName. |
| apps/admin/src/forms/shipmentOptions/readShipmentSnapshot.ts | Extracts shipment-query state reading into a shared helper. |
| apps/admin/src/forms/shipmentOptions/fields/createShipmentOptionField.ts | Simplifies option field hooks to read resolved state from getOptionState() (no per-field enforcement). |
| apps/admin/src/forms/shipmentOptions/fields/createShipmentOptionField.spec.ts | Updates tests to mock option-state rather than carrier capability helpers. |
| apps/admin/src/forms/shipmentOptions/fields/createInsuranceField.ts | Updates docs/formatting; insurance still builds on generic option-field behavior. |
| apps/admin/src/forms/shipmentOptions/field.ts | Adds optionFieldName() helper for consistent option field naming. |
| apps/admin/src/forms/shipmentOptions/createShipmentOptionsForm.ts | Wires capability behavior via useShipmentOptionsState + existing auto-clear and proxy queries. |
| apps/admin/src/forms/shipmentOptions/createShipmentOptionsForm.spec.ts | Mocks useShipmentOptionsState to keep form-creation tests isolated. |
| apps/admin/src/forms/helpers/withTriStateModel.ts | New component wrapper to coerce boolean v-model emits into tri-state ints for form state. |
| apps/admin/src/forms/helpers/withTriStateModel.spec.ts | Tests tri-state coercion and verifies strict $eq afterUpdate rules fire correctly in forms. |
| apps/admin/src/forms/helpers/useFormCapabilities.ts | Removes hasShipmentOption (option availability now comes from option-state module). |
| apps/admin/src/forms/helpers/useFormCapabilities.spec.ts | Removes tests for hasShipmentOption accordingly. |
| apps/admin/src/forms/helpers/triStateFieldIsEnabled.ts | Extracts and exports triStateValueIsEnabled() as the single definition of “enabled” semantics. |
| apps/admin/src/forms/helpers/resolveFormComponent.ts | Wraps selected components with withTriStateModel when resolving form components. |
| apps/admin/src/forms/helpers/index.ts | Exports withTriStateModel. |
| apps/admin/src/data/components.ts | Adds triStateModelComponentNames list controlling which components get the tri-state wrapper. |
Comments suppressed due to low confidence (1)
apps/admin/src/forms/shipmentOptions/useShipmentOptionsState.ts:214
formStatesentries are added for each form but never removed. Add anonScopeDisposecleanup so closing/unmounting the form doesn't leave stale entries in the global registry (and so a reopened form with the same name can’t read old state).
formStates.set(form.name, states.value);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Pass **all** the shipment options from the cart, so that the delivery options can render excludes/requires | ||
| // Ex. "age check" is not selectable in the DO but it makes signature/onlyRecipient mandatory. | ||
| cartShipmentOptions: context.cartShipmentOptions, | ||
| }, |
There was a problem hiding this comment.
I've tried to check the flow locally in my IDE, and I think, in addition to above, changes to cartShipmentOptions do not seem to reach the listener? updateConfigOrAddress only checks whether either one of those (config or address) is changed. When cartShipmentOptions is changed, no update event is fired. Maybe I'm missing something though.
There was a problem hiding this comment.
Good catch, fixed in 2ed45b9. The known map is now kept when the fresh context doesn't carry the key, and still replaced wholesale when it does. Added a spec covering both cases.
There was a problem hiding this comment.
@FreekVR not sure if you've looked at my comment, but this fixes the fallback case but not the listener case I meant, right?
There was a problem hiding this comment.
Sorry, didnt see your comment, I think it came in after I already fixed what copilot reported. So that part is fixed, but I'll have a look at the reactivity for incoming changes.
There was a problem hiding this comment.
Fixed in c2bb58d: a change in cartShipmentOptions now counts as a config change, so the update event goes out and the delivery options pick it up. Added a spec for it (and one for the fallback case).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
libs/checkout-delivery-options/src/types/store.types.ts:12
DeliveryOptionsStoreState.configurationis typed asInputDeliveryOptionsConfiguration, but the store now sets/readsconfiguration.cartShipmentOptions(seecreateDeliveryOptionsStoreandupdateContext). Unless the upstream@myparcel-dev/delivery-optionstype already includes this field, this will break typechecking. Consider widening the local store configuration type with an intersection so this repo stays green until the dependency update lands.
/**
* The configuration passed to the delivery options library, including the cart's active
* shipment options from the checkout context.
*/
configuration: InputDeliveryOptionsConfiguration;
b12eaff to
f80a362
Compare
f80a362 to
70f06b7
Compare
70f06b7 to
abe83c8
Compare
abe83c8 to
91cc472
Compare
91cc472 to
b74084e
Compare
Plugin toggle components emit booleans, while the rules the PHP side attaches to settings fields compare against tri-state ints (1/0/-1) with strict equality. Rules like "enabling 18+ also enables signature and only recipient" therefore silently never ran. Form components with a toggle-like model are now wrapped in an adapter that converts boolean emits to tri-state ints at the component boundary, so form state, conditions, submitted values and stored settings all use the same representation. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Orders that already have an option like 18+ enabled (from default export settings) now show their required options (signature, only recipient) as on and locked as soon as the shipment form opens. Previously the rules only ran when the option was toggled by hand, and the lock could silently drop on re-render. All shipment-option state (availability, locks, forced values) is now decided in one module, useShipmentOptionsState: a pure resolver holds every rule and a single watcher writes forced values through so they persist on the order. Field factories and other consumers only read the resolved state. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slims the option state to the four fields consumers actually read, gives the module one top-down layout, and moves shared pieces to one place: the option field name helper, the tri-state enabled rule, and the removal of the now-unused hasShipmentOption resolver. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The checkout context's new optional cartShipmentOptions key (per carrier, camelCase option name → boolean) now reaches the delivery options widget: the boot configuration includes it, and every context refresh replaces it with the freshly calculated map. The widget uses it to show options the cart already decided on (like the ones age check requires) as checked and locked. The typecheck job fails at updateContext until @myparcel-dev/delivery-options ships cartShipmentOptions in its input type; the dependency bump following the delivery-options release turns it green. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…liably The option-state module registered its per-form states in a WeakMap keyed by the form instance. vue-form-builder evaluates each field's visibleWhen / disabledWhen / readOnlyWhen hook once while the form is being defined — before that registration — and a WeakMap read creates no reactive subscription, so the hooks never re-ran: every carrier's options rendered on every order and nothing ever locked. The registry is now a reactive Map keyed by form name; the first evaluation subscribes even while the form isn't registered yet. Adds a regression test that drives real vue-form-builder elements in the production order (construct, register, recompute) and asserts visibility lands on the elements and follows availability changes. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s them A failed or partial checkout-context fetch falls back to an empty context. The cart shipment options were assigned unconditionally on refresh, so that fallback wiped them and unlocked forced options mid-checkout, while config and strings survived the same situation through merging. The known map is now kept whenever the fresh context does not carry the key; when it does, it still replaces the map wholesale. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The option-state registry is keyed by form name and lived forever, growing with every order opened in a session and briefly serving the previous session's states when a modal reopens. Entries are now removed when the form's scope disposes, matching how wireProxyCapabilities cleans up its queries. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions change The update event only went out when the config or the address changed, so a context refresh that only brought different cart shipment options never reached the delivery options: an option the cart started or stopped forcing stayed as it was until something else changed. A change in that key now counts as a config change. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Toggles report their value to the form as a tri-state int, which is what the form, the rules and the stored settings all use. The int was also handed back down to the component, so plugin toggles — plain checkboxes that expect a boolean — logged a prop type warning on every render and every click. The conversion now works both ways for those components, while components that understand tri-state themselves keep receiving the int. Resolves INT-1596 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b74084e to
6a02503
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/admin/src/forms/shipmentOptions/useShipmentOptionsState.ts:240
availabilityOptionsis always read fromgetCarrierCapabilitiesForShipment(form), even whenselectionMatchesFormis false (i.e. right after a carrier switch while the debounced selection + shipment query still point at the previous carrier). In that windowgetCarrierCapabilitiesForShipment()can returnundefined(shipment query issuccessbut doesn’t contain the newly selected carrier), which makesresolveOptionStates()mark every option as unsupported and briefly hides/disables all fields. Use order-level/dynamic availability when the debounced selection doesn’t match the current form carrier, and only read shipment-scoped availability when the selection matches.
// The shipment response only applies while the form still shows the carrier it was
// fetched for; right after a carrier switch the (debounced) query still holds the old
// carrier's data — the old carrier's locks are dropped immediately.
selectionMatchesForm:
Boolean(shipmentQuery) && form.getValue(FIELD_CARRIER) === shipmentQuery?.selection.value.carrier,
availabilityOptions: capabilities.getCarrierCapabilitiesForShipment(form)?.options,
entries: readEntries(form, allOptionKeys),
Enforces carrier capability rules on shipment options across the admin, and passes the cart's shipment options to the checkout widget.
When a merchant enables an option like age check (18+), the carrier's capability rules say other options (signature, only recipient) must be on as well. The admin now applies those rules everywhere: the carrier settings screen writes the forced values (toggles emit TriState ints, so the PHP-defined rules actually fire), the order form shows only the chosen carrier's options and locks forced ones at render, and the checkout context passes the cart's calculated shipment options to the delivery options widget so it can lock them for the consumer too.
One module per form now owns all option state (useShipmentOptionsState): availability, locking and forced values resolve in one pure function, and the field hooks only read the result. This also fixes a bug where every carrier's options rendered on every order: the hook registry wasn't reactive, so the resolved visibility never reached the fields.
Note: the typecheck job fails on updateContext.ts until @myparcel-dev/delivery-options ships cartShipmentOptions in its input type. Merge order: the delivery-options PR for INT-1596 first, then bump the dependency here.
Merge order for INT-1596:
Fixes INT-1596
Subtask: INT-1731
🤖 Generated with Claude Code