fix: more adjustments

add: composite unique constraint for shipping_option_id and cart_id
fix: fetchCartOptions to format custom shipping options like normal shipping options
fix: addShippingMethod should throw when custom shipping options is not empty and no optionId corresponds to custom shipping options

integration-tests/api/__tests__/store/cart.js

@@ -5,6 +5,7 @@ const {
   GiftCard,
   Cart,
   CustomShippingOption,
+  ShippingOption,
 } = require("@medusajs/medusa")
 
 const setupServer = require("../../../helpers/setup-server")
@@ -466,11 +467,6 @@ describe("/store/carts", () => {
         })
 
         cartWithCustomSo = await manager.save(_cart)
-
-        await manager.insert(CustomShippingOption, {
-          id: "orphan-cso",
-          price: 0,
-        })
       } catch (err) {
         console.log(err)
       }
@@ -497,8 +493,9 @@ describe("/store/carts", () => {
       expect(cartWithShippingMethod.status).toEqual(200)
     })
 
-    it("given a cart with custom options and a custom option id already belonging to said cart, then it should add a shipping method based on the given custom shipping option", async () => {
-      const customOptionId = cartWithCustomSo.custom_shipping_options[0].id
+    it("given a cart with custom options and a shipping option already belonging to said cart, then it should add a shipping method based on the given custom shipping option", async () => {
+      const shippingOptionId =
+        cartWithCustomSo.custom_shipping_options[0].shipping_option_id
 
       const api = useApi()
 
@@ -506,7 +503,7 @@ describe("/store/carts", () => {
         .post(
           "/store/carts/test-cart-with-cso/shipping-methods",
           {
-            option_id: customOptionId,
+            option_id: shippingOptionId,
           },
           { withCredentials: true }
         )
@@ -515,27 +512,28 @@ describe("/store/carts", () => {
       expect(
         cartWithCustomShippingMethod.data.cart.shipping_methods
       ).toContainEqual(
-        expect.objectContaining({ shipping_option_id: "test-option", price: 5 })
+        expect.objectContaining({
+          shipping_option_id: shippingOptionId,
+          price: 5,
+        })
       )
       expect(cartWithCustomShippingMethod.status).toEqual(200)
     })
 
-    it("given a cart with custom options and a custom option id not belonging to said cart, then it should throw a shipping option not found error", async () => {
+    it("given a cart with custom options and an option id not corresponding to any custom shipping option, then it should throw an invalid error", async () => {
       const api = useApi()
 
       try {
         await api.post(
           "/store/carts/test-cart-with-cso/shipping-methods",
           {
-            option_id: "orphan-cso",
+            option_id: "orphan-so",
           },
           { withCredentials: true }
         )
       } catch (err) {
-        expect(err.response.status).toEqual(404)
-        expect(err.response.data.message).toEqual(
-          "Shipping Option with orphan-cso was not found"
-        )
+        expect(err.response.status).toEqual(400)
+        expect(err.response.data.message).toEqual("Wrong shipping option")
       }
     })
 

integration-tests/api/__tests__/store/shipping-options.js

@@ -160,7 +160,7 @@ describe("/store/shipping-options", () => {
       )
     })
 
-    it("given a swap cart, when user retrieves its shipping options, then should return a list of custom shipping options", async () => {
+    it("given a cart with custom shipping options, when user retrieves its shipping options, then should return the list of custom shipping options", async () => {
       const api = useApi()
 
       const response = await api
@@ -173,8 +173,9 @@ describe("/store/shipping-options", () => {
       expect(response.data.shipping_options).toEqual(
         expect.arrayContaining([
           expect.objectContaining({
-            shipping_option_id: "test-option",
-            price: 0,
+            id: "test-option",
+            amount: 0,
+            name: "test-option",
           }),
         ])
       )

packages/medusa/src/migrations/1633614437919-add_custom_shipping_options.ts

@@ -1,10 +1,10 @@
 import {MigrationInterface, QueryRunner} from "typeorm";
 
-export class addCustomShippingOptions1633522106578 implements MigrationInterface {
-    name = 'addCustomShippingOptions1633522106578'
+export class addCustomShippingOptions1633614437919 implements MigrationInterface {
+    name = 'addCustomShippingOptions1633614437919'
 
     public async up(queryRunner: QueryRunner): Promise<void> {
-        await queryRunner.query(`CREATE TABLE "custom_shipping_option" ("id" character varying NOT NULL, "price" integer NOT NULL, "shipping_option_id" character varying NOT NULL, "cart_id" character varying, "created_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updated_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "deleted_at" TIMESTAMP WITH TIME ZONE, "metadata" jsonb, CONSTRAINT "PK_8dfcb5c1172c29eec4a728420cc" PRIMARY KEY ("id"))`);
+        await queryRunner.query(`CREATE TABLE "custom_shipping_option" ("id" character varying NOT NULL, "price" integer NOT NULL, "shipping_option_id" character varying NOT NULL, "cart_id" character varying, "created_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updated_at" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "deleted_at" TIMESTAMP WITH TIME ZONE, "metadata" jsonb, CONSTRAINT "UQ_0f838b122a9a01d921aa1cdb669" UNIQUE ("shipping_option_id", "cart_id"), CONSTRAINT "PK_8dfcb5c1172c29eec4a728420cc" PRIMARY KEY ("id"))`);
         await queryRunner.query(`CREATE INDEX "IDX_44090cb11b06174cbcc667e91c" ON "custom_shipping_option" ("shipping_option_id") `);
         await queryRunner.query(`CREATE INDEX "IDX_93caeb1bb70d37c1d36d6701a7" ON "custom_shipping_option" ("cart_id") `);
         await queryRunner.query(`ALTER TYPE "cart_type_enum" RENAME TO "cart_type_enum_old"`);

packages/medusa/src/models/custom-shipping-option.ts

@@ -7,6 +7,7 @@ import {
   JoinColumn,
   ManyToOne,
   PrimaryColumn,
+  Unique,
   UpdateDateColumn
 } from "typeorm";
 import { ulid } from "ulid";
@@ -16,6 +17,7 @@ import { ShippingOption } from "./shipping-option";
 
 
 @Entity()
+@Unique(['shipping_option_id', 'cart_id'])
 export class CustomShippingOption {
   @PrimaryColumn()
   id: string

packages/medusa/src/services/__tests__/cart.js

@@ -1308,65 +1308,50 @@ describe("CartService", () => {
     })
   })
 
-  describe("extractShippingOptionIdAndPrice", () => {
+  describe("findCustomShippingOption", () => {
     beforeEach(() => {
       jest.clearAllMocks()
     })
 
     let cartService = new CartService({})
 
-    it("given a cart with custom shipping options and a custom shipping option id, then it should return a normal shipping option id corresponding to the custom shipping option id and a customPrice", async () => {
+    it("given a cart with custom shipping options and a shipping option id corresponding to a custom shipping option, then it should return a custom shipping option", async () => {
       const cart = {
         id: "cart-with-so",
         custom_shipping_options: [
           { id: "cso-test", shipping_option_id: "test-so", price: 20 },
         ],
       }
-      const result = cartService.extractShippingOptionIdAndPrice(
-        cart,
-        "cso-test"
-      )
+      const result = cartService.findCustomShippingOption(cart, "test-so")
 
       expect(result).toEqual({
-        optionId: "test-so",
-        customPrice: { price: 20 },
+        id: "cso-test",
+        shipping_option_id: "test-so",
+        price: 20,
       })
     })
 
-    it("given a cart with custom shipping options and a normal shipping option id, then it should return a normal shipping option id and empty customPrice", async () => {
+    it("given a cart with empty custom shipping options and shipping option id, then it should return undefined", async () => {
       const cart = {
         id: "cart-with-so",
-        custom_shipping_options: [
-          { id: "cso-test", shipping_option_id: "test-so", price: 20 },
-        ],
+        custom_shipping_options: [],
       }
-      const result = cartService.extractShippingOptionIdAndPrice(
-        cart,
-        "test-so"
-      )
+      const result = cartService.findCustomShippingOption(cart, "test-so")
 
-      expect(result).toEqual({
-        optionId: "test-so",
-        customPrice: {},
-      })
+      expect(result).toBeUndefined()
     })
 
-    it("given a cart with custom shipping options and a custom shipping option id that does not belong to the cart, then it should return the custom shipping option id and empty customPrice", async () => {
+    it("given a cart with custom shipping options and a shipping option id that does not belong to the cart, then it should throw an invalid error", async () => {
       const cart = {
         id: "cart-with-so",
         custom_shipping_options: [
-          { id: "cso-test", shipping_option_id: "test-so", price: 20 },
+          { id: "cso-test", shipping_option_id: "test-so", price: 500 },
         ],
       }
-      const result = cartService.extractShippingOptionIdAndPrice(
-        cart,
-        "cso-test-2"
-      )
 
-      expect(result).toEqual({
-        optionId: "cso-test-2",
-        customPrice: {},
-      })
+      expect(() => {
+        cartService.findCustomShippingOption(cart, "some-other-so")
+      }).toThrow(MedusaError)
     })
   })
 
@@ -1545,27 +1530,25 @@ describe("CartService", () => {
       })
     })
 
-    it("adds a shipping method from a custom shipping option and custom price", async () => {
+    it("successfully adds a shipping method from a custom shipping option and custom price", async () => {
       const data = {
         id: "test",
         extra: "yes",
       }
 
-      cartService.extractShippingOptionIdAndPrice = jest
+      cartService.findCustomShippingOption = jest
         .fn()
-        .mockImplementation((cart, optionId) => {
+        .mockImplementation(cart => {
           if (cart.id === IdMap.getId("cart-with-custom-so")) {
             return {
-              optionId: IdMap.getId("test-so"),
-              customPrice: { price: 0 },
+              price: 0,
             }
           }
-          return { optionId, customPrice: {} }
         })
 
       await cartService.addShippingMethod(
         IdMap.getId("cart-with-custom-so"),
-        IdMap.getId("cso-test"),
+        IdMap.getId("test-so"),
         data
       )
       expect(shippingOptionService.createShippingMethod).toHaveBeenCalledWith(

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

@@ -207,14 +207,24 @@ describe("ShippingProfileService", () => {
         id: "swap-cart",
         type: "swap",
         custom_shipping_options: [
-          { option_id: "test-option1", id: "cso-option1", price: 10 },
-          { option_id: "test-option2", id: "cso-option2", price: 0 },
+          {
+            shipping_option_id: "test-option1",
+            id: "cso-option1",
+            shipping_option: { id: "test-option1" },
+            price: 10,
+          },
+          {
+            shipping_option_id: "test-option2",
+            id: "cso-option2",
+            shipping_option: { id: "test-option2" },
+            price: 0,
+          },
         ],
       }
 
       await expect(profileService.fetchCartOptions(cart)).resolves.toEqual([
-        expect.objectContaining({ id: "cso-option1" }),
-        expect.objectContaining({ id: "cso-option2" }),
+        expect.objectContaining({ id: "test-option1", amount: 10 }),
+        expect.objectContaining({ id: "test-option2", amount: 0 }),
       ])
     })
 

packages/medusa/src/services/cart.js

@@ -1305,7 +1305,7 @@ class CartService extends BaseService {
    * @param {Object} data - the fulmillment data for the method
    * @return {Promise} the result of the update operation
    */
-  async addShippingMethod(cartId, optionIdOrCustomOptionId, data) {
+  async addShippingMethod(cartId, optionId, data) {
     return this.atomicPhase_(async manager => {
       const cart = await this.retrieve(cartId, {
         select: ["subtotal"],
@@ -1321,19 +1321,19 @@ class CartService extends BaseService {
         ],
       })
 
-      let { optionId, customPrice } = this.extractShippingOptionIdAndPrice(
-        cart,
-        optionIdOrCustomOptionId
-      )
+      let customShippingOption = this.findCustomShippingOption(cart, optionId)
 
       const { shipping_methods } = cart
 
+      const shippingMethodConfig = customShippingOption
+        ? { cart, price: customShippingOption.price }
+        : {
+            cart,
+          }
+
       const newMethod = await this.shippingOptionService_
         .withTransaction(manager)
-        .createShippingMethod(optionId, data, {
-          cart,
-          ...customPrice,
-        })
+        .createShippingMethod(optionId, data, shippingMethodConfig)
 
       const methods = [newMethod]
       if (shipping_methods.length) {
@@ -1374,32 +1374,26 @@ class CartService extends BaseService {
   }
 
   /**
-   * Adds the corresponding shipping method either from a normal or custom option to the list of shipping methods associated with
-   * the cart.
+   * Finds the cart's custom shipping option based on the passed option id.
+   * throws if custom options is not empty and no shipping option corresponds to optionId
    * @param {Object} cart - the cart object
-   * @param {string} optionIdOrCustomOptionId - id of the normal or custom shipping option to add as valid method
-   * @returns {{ optionId: string; customPrice: { price: number; } | {};}}
+   * @param {string} option - id of the normal or custom shipping option to add as valid method
+   * @returns {CustomShippingOption | undefined}
    */
-  extractShippingOptionIdAndPrice(cart, optionIdOrCustomOptionId) {
-    if (
-      cart.custom_shipping_options &&
-      cart.custom_shipping_options.length > 0
-    ) {
-      const customOption = cart.custom_shipping_options.find(
-        cso => cso.id === optionIdOrCustomOptionId
+  findCustomShippingOption(cart, optionId) {
+    let customOption = cart.custom_shipping_options?.find(
+      cso => cso.shipping_option_id === optionId
+    )
+    const hasCustomOptions = cart.custom_shipping_options?.length
+
+    if (hasCustomOptions && !customOption) {
+      throw new MedusaError(
+        MedusaError.Types.INVALID_DATA,
+        "Wrong shipping option"
       )
-      if (customOption) {
-        return {
-          optionId: customOption.shipping_option_id,
-          customPrice: { price: customOption.price },
-        }
-      }
     }
 
-    return {
-      optionId: optionIdOrCustomOptionId,
-      customPrice: {},
-    }
+    return customOption
   }
 
   /**

packages/medusa/src/services/shipping-profile.js

@@ -410,13 +410,15 @@ class ShippingProfileService extends BaseService {
    * Finds all the shipping profiles that cover the products in a cart, and
    * validates all options that are available for the cart.
    * @param {Cart} cart - the cart object to find shipping options for
-   * @return {[ShippingOptions]} a list of the available shipping options
+   * @return {[ShippingOption]} a list of the available shipping options
    */
   async fetchCartOptions(cart) {
-    const customShippingOptions = cart.custom_shipping_options
-
-    if (customShippingOptions && customShippingOptions.length > 0)
-      return customShippingOptions
+    if (cart.custom_shipping_options?.length) {
+      return cart.custom_shipping_options.map(cso => ({
+        ...cso.shipping_option,
+        amount: cso.price,
+      }))
+    }
 
     const profileIds = this.getProfilesInCart_(cart)