Skip to content

fix(fulfillment): respect cancellation guard overrides - #16745

Open
kudala-bharani wants to merge 1 commit into
medusajs:developfrom
kudala-bharani:fix/fulfillment-cancellation-override
Open

fix(fulfillment): respect cancellation guard overrides#16745
kudala-bharani wants to merge 1 commit into
medusajs:developfrom
kudala-bharani:fix/fulfillment-cancellation-override

Conversation

@kudala-bharani

Copy link
Copy Markdown

Summary

What — Make cancelFulfillment respect a subclass's canCancelFulfillmentOrThrow override.

Why — The hardcoded FulfillmentModuleService reference always invokes the base guard, even when a custom module overrides it. Fixes #16744.

How — Resolve the guard through the service's runtime constructor. The base class still rejects shipped and delivered fulfillments; no default cancellation rules change. Includes a patch changeset.

Testing

  • Added five unit cases using the actual module service and decorators: default shipped/delivered rejections, subclass overrides permitting each state, and a custom override rejecting cancellation.
  • Before the fix, the three override cases failed as expected. After the fix, all 32 fulfillment unit tests pass (yarn workspace @medusajs/fulfillment test --runInBand).
  • yarn workspace @medusajs/fulfillment build passes. The dependency build also passed for all 13 required packages.
  • Prettier and both standard and Medusa-specific ESLint checks pass for the changed files.
  • Database integration tests were not run locally because PostgreSQL is not installed. Existing integration coverage for ordinary cancellation, idempotency, and the default shipped/delivered restrictions is unchanged.

Examples

A custom fulfillment module overriding canCancelFulfillmentOrThrow can now enforce its own cancellation policy without reimplementing cancelFulfillment and its provider, persistence, and event-handling logic.

Checklist

  • I have added a patch changeset for this PR.
  • The changes are covered by relevant tests.
  • I have verified the code works as intended locally.
  • I have linked the related issue.

@kudala-bharani
kudala-bharani requested a review from a team as a code owner September 7, 2026 02:01
@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 05eab92

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 83 packages
Name Type
@medusajs/fulfillment Patch
@medusajs/medusa Patch
@medusajs/test-utils Patch
@medusajs/loyalty-plugin Patch
@medusajs/medusa-oas-cli Patch
integration-tests-http Patch
@medusajs/analytics Patch
@medusajs/api-key Patch
@medusajs/auth Patch
@medusajs/caching Patch
@medusajs/cart Patch
@medusajs/currency Patch
@medusajs/customer Patch
@medusajs/file Patch
@medusajs/index Patch
@medusajs/inventory Patch
@medusajs/link-modules Patch
@medusajs/locking Patch
@medusajs/notification Patch
@medusajs/order Patch
@medusajs/payment Patch
@medusajs/pricing Patch
@medusajs/product Patch
@medusajs/promotion Patch
@medusajs/rbac Patch
@medusajs/region Patch
@medusajs/sales-channel Patch
@medusajs/search Patch
@medusajs/settings Patch
@medusajs/stock-location Patch
@medusajs/store Patch
@medusajs/tax Patch
@medusajs/translation Patch
@medusajs/user Patch
@medusajs/workflow-engine-inmemory Patch
@medusajs/workflow-engine-redis Patch
@medusajs/search-postgres Patch
@medusajs/draft-order Patch
@medusajs/oas-github-ci Patch
@medusajs/cache-inmemory Patch
@medusajs/cache-redis Patch
@medusajs/event-bus-local Patch
@medusajs/event-bus-redis Patch
@medusajs/analytics-local Patch
@medusajs/analytics-posthog Patch
@medusajs/auth-emailpass Patch
@medusajs/auth-github Patch
@medusajs/auth-google Patch
@medusajs/auth-oidc Patch
@medusajs/caching-redis Patch
@medusajs/file-local Patch
@medusajs/file-s3 Patch
@medusajs/fulfillment-manual Patch
@medusajs/locking-postgres Patch
@medusajs/locking-redis Patch
@medusajs/notification-local Patch
@medusajs/notification-sendgrid Patch
@medusajs/payment-stripe Patch
@medusajs/core-flows Patch
@medusajs/framework Patch
@medusajs/instantsearch-adapter Patch
@medusajs/js-sdk Patch
@medusajs/modules-sdk Patch
@medusajs/orchestration Patch
@medusajs/query Patch
@medusajs/types Patch
@medusajs/utils Patch
@medusajs/workflows-sdk Patch
create-medusa-app Patch
@medusajs/http-types-generator Patch
@medusajs/cli Patch
@medusajs/deps Patch
@medusajs/eslint-plugin Patch
@medusajs/telemetry Patch
@medusajs/admin-bundler Patch
@medusajs/admin-sdk Patch
@medusajs/admin-shared Patch
@medusajs/admin-vite-plugin Patch
@medusajs/dashboard Patch
@medusajs/icons Patch
@medusajs/toolbox Patch
@medusajs/ui-preset Patch
@medusajs/ui Patch

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

@medusa-os-bot

medusa-os-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for the contribution! Initial automated review looks good.

This PR fixes a real bug in FulfillmentModuleService.cancelFulfillment: the cancellation guard (canCancelFulfillmentOrThrow) was always called via the hardcoded FulfillmentModuleService class reference, so any subclass override was silently bypassed. The fix replaces the hardcoded reference with this.constructor as typeof FulfillmentModuleService, which is the standard JavaScript pattern for polymorphic dispatch on a static method. The PR is well-scoped: - Linked to a valid, open issue (#16744, labelled good first issue / type: bug). - PR template is fully filled out (What/Why/How/Testing sections). - Patch changeset included with the correct fix(fulfillment): … format and @medusajs/fulfillment: patch bump type. - Five focused unit tests cover: default shipped/delivered rejections (unchanged default behaviour), subclass override permitting each state, and a subclass enforcing a custom restriction. - No security, performance, or correctness concerns. Minor non-blocking note: the local variable Service in fulfillment-module-service.ts uses PascalCase. Medusa's conventions require local variables to be camelCase; a name like ServiceClass would be more conventional. This does not affect correctness.

Triggered by: new PR opened

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FulfillmentModuleService.cancelFulfillment calls the shipped/delivered guard via a hardcoded class reference, breaking subclass overrides

1 participant