Throw on flat rate shipping options with no amount

packages/medusa/src/services/__tests__/shipping-option.js

@@ -1,8 +1,7 @@
-import _ from "lodash"
-import { IdMap, MockRepository, MockManager } from "medusa-test-utils"
+import { IdMap, MockManager, MockRepository } from "medusa-test-utils"
+import TaxInclusivePricingFeatureFlag from "../../loaders/feature-flags/tax-inclusive-pricing"
+import { FlagRouter } from "../../utils/flag-router"
 import ShippingOptionService from "../shipping-option"
-import { FlagRouter } from "../../utils/flag-router";
-import TaxInclusivePricingFeatureFlag from "../../loaders/feature-flags/tax-inclusive-pricing";
 
 describe("ShippingOptionService", () => {
   describe("retrieve", () => {
@@ -36,13 +35,20 @@ describe("ShippingOptionService", () => {
               provider_id: "no_calc",
             })
           case IdMap.getId("validId"):
+            return Promise.resolve({
+              provider_id: "provider",
+              amount: 100,
+              data: {
+                provider_data: "true",
+              },
+            })
+          case "flat-rate-no-amount":
             return Promise.resolve({
               provider_id: "provider",
               data: {
                 provider_data: "true",
               },
             })
-
           default:
             return Promise.resolve({})
         }
@@ -137,7 +143,7 @@ describe("ShippingOptionService", () => {
     it("sets flat rate price", async () => {
       await optionService.update(IdMap.getId("validId"), {
         price_type: "flat_rate",
-        amount: 100,
+        amount: 200,
       })
 
       expect(shippingOptionRepository.save).toHaveBeenCalledTimes(1)
@@ -147,21 +153,32 @@ describe("ShippingOptionService", () => {
           provider_data: "true",
         },
         price_type: "flat_rate",
-        amount: 100,
+        amount: 200,
       })
     })
 
+    it("throws on flat rate but no amount", async () => {
+      try {
+        await optionService.update(IdMap.getId("flat-rate-no-amount"), {
+          price_type: "flat_rate",
+          amount: 100,
+        })
+      } catch (error) {
+        expect(error.message).toEqual(
+          "Flat rate shipping options must have an amount"
+        )
+      }
+    })
+
     it("sets calculated price", async () => {
       await optionService.update(IdMap.getId("validId"), {
         price_type: "calculated",
       })
 
       expect(fulfillmentProviderService.canCalculate).toHaveBeenCalledTimes(1)
       expect(fulfillmentProviderService.canCalculate).toHaveBeenCalledWith({
-        amount: null,
         data: { provider_data: "true" },
         provider_id: "provider",
-        price_type: "calculated",
       })
 
       expect(shippingOptionRepository.save).toHaveBeenCalledTimes(1)
@@ -645,19 +662,19 @@ describe("ShippingOptionService", () => {
       totalsService,
       fulfillmentProviderService: providerService,
       featureFlagRouter: new FlagRouter({
-        [TaxInclusivePricingFeatureFlag.key]: true
+        [TaxInclusivePricingFeatureFlag.key]: true,
       }),
     })
 
     beforeEach(() => {
       jest.clearAllMocks()
     })
 
-    it("should create a shipping method that also includes the taxes",  async () => {
+    it("should create a shipping method that also includes the taxes", async () => {
       await optionService.createShippingMethod("random_id", {}, { price: 10 })
       expect(shippingMethodRepository.save).toHaveBeenCalledWith(
         expect.objectContaining({
-          includes_tax: true
+          includes_tax: true,
         })
       )
     })

packages/medusa/src/services/fulfillment-provider.ts

@@ -8,7 +8,6 @@ import {
   FulfillmentProvider,
   LineItem,
   Order,
-  Return,
   ShippingMethod,
   ShippingOption,
 } from "../models"
@@ -29,6 +28,11 @@ type FulfillmentProviderContainer = MedusaContainer & {
   [key in `${FulfillmentProviderKey}`]: typeof BaseFulfillmentService
 }
 
+type CalculateOptionPriceInput = {
+  provider_id: string
+  data: Record<string, unknown>
+}
+
 /**
  * Helps retrive fulfillment providers
  */
@@ -119,7 +123,7 @@ class FulfillmentProviderService extends TransactionBaseService {
     ) as unknown as Record<string, unknown>
   }
 
-  async canCalculate(option: ShippingOption): Promise<boolean> {
+  async canCalculate(option: CalculateOptionPriceInput): Promise<boolean> {
     const provider = this.retrieveProvider(option.provider_id)
     return provider.canCalculate(option.data) as unknown as boolean
   }

packages/medusa/src/services/shipping-option.ts

@@ -1,6 +1,7 @@
 import { MedusaError } from "medusa-core-utils"
 import { DeepPartial, EntityManager } from "typeorm"
 import { TransactionBaseService } from "../interfaces"
+import TaxInclusivePricingFeatureFlag from "../loaders/feature-flags/tax-inclusive-pricing"
 import {
   Cart,
   Order,
@@ -15,15 +16,14 @@ import { ShippingOptionRequirementRepository } from "../repositories/shipping-op
 import { ExtendedFindConfig, FindConfig, Selector } from "../types/common"
 import {
   CreateShippingMethodDto,
+  CreateShippingOptionInput,
   ShippingMethodUpdate,
   UpdateShippingOptionInput,
-  CreateShippingOptionInput,
 } from "../types/shipping-options"
 import { buildQuery, isDefined, setMetadata } from "../utils"
+import { FlagRouter } from "../utils/flag-router"
 import FulfillmentProviderService from "./fulfillment-provider"
 import RegionService from "./region"
-import { FlagRouter } from "../utils/flag-router"
-import TaxInclusivePricingFeatureFlag from "../loaders/feature-flags/tax-inclusive-pricing"
 
 type InjectedDependencies = {
   manager: EntityManager
@@ -409,6 +409,8 @@ class ShippingOptionService extends TransactionBaseService {
       const optionRepo = manager.getCustomRepository(this.optionRepository_)
       const option = optionRepo.create(data as DeepPartial<ShippingOption>)
 
+      option.price_type = await this.validatePriceType_(data.price_type, option)
+
       const region = await this.regionService_
         .withTransaction(manager)
         .retrieve(option.region_id, {
@@ -426,9 +428,10 @@ class ShippingOptionService extends TransactionBaseService {
         )
       }
 
-      option.price_type = await this.validatePriceType_(data.price_type, option)
       option.amount =
-        data.price_type === "calculated" ? null : data.amount ?? null
+        data.price_type === ShippingOptionPriceType.CALCULATED
+          ? null
+          : data.amount ?? null
 
       if (
         this.featureFlagRouter_.isFeatureEnabled(
@@ -496,16 +499,20 @@ class ShippingOptionService extends TransactionBaseService {
   ): Promise<ShippingOptionPriceType> {
     if (
       !priceType ||
-      (priceType !== "flat_rate" && priceType !== "calculated")
+      (priceType !== ShippingOptionPriceType.FLAT_RATE &&
+        priceType !== ShippingOptionPriceType.CALCULATED)
     ) {
       throw new MedusaError(
         MedusaError.Types.INVALID_DATA,
         "The price must be of type flat_rate or calculated"
       )
     }
 
-    if (priceType === "calculated") {
-      const canCalculate = await this.providerService_.canCalculate(option)
+    if (priceType === ShippingOptionPriceType.CALCULATED) {
+      const canCalculate = await this.providerService_.canCalculate({
+        provider_id: option.provider_id,
+        data: option.data,
+      })
       if (!canCalculate) {
         throw new MedusaError(
           MedusaError.Types.INVALID_DATA,
@@ -514,6 +521,13 @@ class ShippingOptionService extends TransactionBaseService {
       }
     }
 
+    if (priceType === ShippingOptionPriceType.FLAT_RATE && !option.amount) {
+      throw new MedusaError(
+        MedusaError.Types.INVALID_DATA,
+        "Flat rate shipping options must have an amount"
+      )
+    }
+
     return priceType
   }
 
@@ -597,20 +611,21 @@ class ShippingOptionService extends TransactionBaseService {
         option.requirements = acc
       }
 
+      if (isDefined(update.amount)) {
+        option.amount = update.amount
+      }
+
       if (isDefined(update.price_type)) {
         option.price_type = await this.validatePriceType_(
           update.price_type,
           option
         )
-        if (update.price_type === "calculated") {
+
+        if (update.price_type === ShippingOptionPriceType.CALCULATED) {
           option.amount = null
         }
       }
 
-      if (isDefined(update.amount) && option.price_type !== "calculated") {
-        option.amount = update.amount
-      }
-
       if (isDefined(update.name)) {
         option.name = update.name
       }