feat(types,core-flows): Allow tax providers to return metadata for cart/order storage#14605
feat(types,core-flows): Allow tax providers to return metadata for cart/order storage#14605rjstanley wants to merge 5 commits intomedusajs:developfrom
Conversation
🦋 Changeset detectedLatest commit: d4663ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 76 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@rjstanley is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
82b609c to
c7288d6
Compare
|
@claude review this |
Pull Request Review - #14605SummaryThis PR adds a valuable feature allowing tax providers to return metadata alongside tax lines, which is automatically stored on carts/orders. The implementation is well-designed and maintains backward compatibility. ✅ Strengths1. Clean Architecture
2. Backward Compatibility
3. Implementation Quality
|
…rt/order storage This change enables tax providers to return metadata alongside tax lines, which workflows automatically store on the cart or order. This is useful for tax providers that need to store calculation references (e.g., calculation_id) for later use during order completion. Changes: - Add TaxLinesResult type that providers can return instead of just tax lines array - Update ITaxProvider.getTaxLines return type to accept either array or TaxLinesResult - Update getItemTaxLinesStep to handle TaxLinesResult and pass sourceMetadata through - Update updateTaxLinesWorkflow to store sourceMetadata on cart via updateCartsStep - Update upsertTaxLinesWorkflow to store sourceMetadata on cart via updateCartsStep - Update updateOrderTaxLinesWorkflow to store sourceMetadata on order via updateOrdersStep
- Update return types on ITaxModuleService, TaxModuleService, and TaxProviderService to include TaxLinesResult union type - Add documentation for metadata collision behavior (shipping takes precedence over item metadata) - Enhance JSDoc for sourceMetadata with best practices: namespacing, merge behavior, and security considerations - Add integration tests for TaxLinesResult with mock provider that returns sourceMetadata - Add backward compatibility test verifying array return format
6c73739 to
b8296d7
Compare
- Add updateCartTaxMetadataStep that fetches current cart metadata from DB before merging sourceMetadata to prevent data loss - Includes compensation logic to restore previous metadata on failure - Update update-tax-lines and upsert-tax-lines workflows to use new step - Fix mock provider identifier to use underscore (mock_metadata)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| }, | ||
| }, | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Order metadata update uses stale data risking loss
Medium Severity
The order metadata update uses order.metadata from the workflow context (fetched at workflow start), but for carts a dedicated updateCartTaxMetadataStep was created that explicitly fetches fresh metadata from the database immediately before merging. If another process updates the order's metadata between the initial order query and this transform step, those changes are overwritten and lost. This inconsistency with the cart implementation could cause silent data loss during race conditions.
There was a problem hiding this comment.
This concern doesn't apply to the order workflow. The key difference is:
Carts: The workflow accepts either cart_id OR cart as input. When input.cart is provided directly, it might not have the metadata field populated, causing certain data loss. That's why we created updateCartTaxMetadataStep.
Orders: The workflow always fetches the order from the database (lines 201-206), and metadata is included in the fields list (lines 23, 76). So order.metadata is always fresh from the DB at workflow start.
The theoretical race condition (another process updating metadata between fetch and update) applies to many read-then-write patterns and isn't specific to this implementation.
|
@NicolasGorga could you please review again? 😄 |
Summary
This PR enables tax providers to return metadata alongside tax lines, which workflows automatically store on the cart or order. This addresses a common limitation where tax providers need to store calculation references (e.g.,
calculation_id) for later use during order completion (e.g., committing tax transactions).Use case: Tax providers like TaxJar, Avalara, and Numeral often perform a "calculation" during checkout that returns a
calculation_id. During order completion, thiscalculation_idis used to "commit" the transaction with the tax service. Previously, providers had to use raw SQL to store this ID on the cart since they lacked access to the Cart module from within theITaxProviderinterface.Changes
TaxLinesResulttype that providers can return instead of just a tax lines arrayITaxProvider.getTaxLinesreturn type to accept either array orTaxLinesResultgetItemTaxLinesStepto handleTaxLinesResultand passsourceMetadatathroughupdateTaxLinesWorkflowto storesourceMetadataon cart viaupdateCartsStepupsertTaxLinesWorkflowto storesourceMetadataon cart viaupdateCartsStepupdateOrderTaxLinesWorkflowto storesourceMetadataon order viaupdateOrdersStepExample usage in a tax provider
The workflow automatically merges this metadata into the cart/order's metadata field.
Backward compatibility
This change is fully backward compatible:
TaxLinesResulttype is optional and only used when providers need to store metadataTest plan
TaxLinesResultwithsourceMetadataproperly stores metadata on cartTaxLinesResultwithsourceMetadataproperly stores metadata on orderNote
Enables providers to return metadata with tax calculations and have it automatically stored on carts/orders.
TaxLinesResulttype and updatesITaxProvider.getTaxLines/tax module service to return either an array orTaxLinesResultget-item-tax-linesstep to detectTaxLinesResult, aggregatesourceMetadata, and pass it throughupdate-cart-tax-metadatastep (with compensation) and wires it intoupdate-tax-linesandupsert-tax-linescart workflows; order workflow updates metadata viaupdateOrdersStepmetadatain affected queries to support merge semanticsWritten by Cursor Bugbot for commit d4663ae. This will update automatically on new commits. Configure here.