fix: variant price update (#1093)

* fix: variant prices update + integration tests

* add: unit tests

* fix: rename variable

* add: integration tests

* fix: integration tests

* fix: test name

* fix: move db logic to repo layer + create upsert method

* fix: linting

integration-tests/api/__tests__/admin/__snapshots__/product.js.snap

@@ -364,7 +364,7 @@ Array [
             "currency_code": "usd",
             "deleted_at": null,
             "id": StringMatching /\\^test-price\\*/,
-            "region_id": null,
+            "region_id": "test-region",
             "sale_amount": null,
             "updated_at": Any<String>,
             "variant_id": StringMatching /\\^test-variant\\*/,

integration-tests/api/__tests__/admin/product.js

@@ -1189,6 +1189,278 @@ describe("/admin/products", () => {
     })
   })
 
+  describe("updates a variant's prices", () => {
+    beforeEach(async () => {
+      try {
+        await productSeeder(dbConnection)
+        await adminSeeder(dbConnection)
+      } catch (err) {
+        console.log(err)
+        throw err
+      }
+    })
+
+    afterEach(async () => {
+      const db = useDb()
+      await db.teardown()
+    })
+
+    it("successfully updates a variant's prices by changing an existing price (currency_code)", async () => {
+      const api = useApi()
+      const data = {
+        prices: [
+          {
+            currency_code: "usd",
+            amount: 1500,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product/variants/test-variant", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data).toEqual({
+        product: expect.objectContaining({
+          id: "test-product",
+          variants: expect.arrayContaining([
+            expect.objectContaining({
+              id: "test-variant",
+              prices: expect.arrayContaining([
+                expect.objectContaining({
+                  amount: 1500,
+                  currency_code: "usd",
+                }),
+              ]),
+            }),
+          ]),
+        }),
+      })
+    })
+
+    it("successfully updates a variant's price by changing an existing price (given a region_id)", async () => {
+      const api = useApi()
+      const data = {
+        prices: [
+          {
+            region_id: "test-region",
+            amount: 1500,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product1/variants/test-variant_3", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data.product).toEqual(
+        expect.objectContaining({
+          variants: expect.arrayContaining([
+            expect.objectContaining({
+              id: "test-variant_3",
+              prices: expect.arrayContaining([
+                expect.objectContaining({
+                  amount: 1500,
+                  currency_code: "usd",
+                  region_id: "test-region",
+                }),
+              ]),
+            }),
+          ]),
+        })
+      )
+    })
+
+    it("successfully updates a variant's prices by adding a new price", async () => {
+      const api = useApi()
+      const data = {
+        title: "Test variant prices",
+        prices: [
+          // usd price coming from the product seeder
+          {
+            currency_code: "usd",
+            amount: 100,
+          },
+          {
+            currency_code: "eur",
+            amount: 4500,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product/variants/test-variant", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data).toEqual({
+        product: expect.objectContaining({
+          id: "test-product",
+          variants: expect.arrayContaining([
+            expect.objectContaining({
+              id: "test-variant",
+              prices: [
+                expect.objectContaining({
+                  amount: 100,
+                  currency_code: "usd",
+                }),
+                expect.objectContaining({
+                  amount: 4500,
+                  currency_code: "eur",
+                }),
+              ],
+            }),
+          ]),
+        }),
+      })
+    })
+
+    it("successfully updates a variant's prices by replacing a price", async () => {
+      const api = useApi()
+      const data = {
+        prices: [
+          {
+            currency_code: "eur",
+            amount: 4500,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product/variants/test-variant", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data.product.variants[0].prices.length).toEqual(
+        data.prices.length
+      )
+      expect(response.data.product.variants[0].prices).toEqual([
+        expect.objectContaining({
+          amount: 4500,
+          currency_code: "eur",
+        }),
+      ])
+    })
+
+    it("successfully updates a variant's prices by deleting a price and adding another price", async () => {
+      const api = useApi()
+      const data = {
+        prices: [
+          {
+            currency_code: "dkk",
+            amount: 8000,
+          },
+          {
+            currency_code: "eur",
+            amount: 900,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product/variants/test-variant", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data.product.variants[0].prices.length).toEqual(
+        data.prices.length
+      )
+      expect(response.data.product.variants[0].prices).toEqual([
+        expect.objectContaining({
+          amount: 8000,
+          currency_code: "dkk",
+        }),
+        expect.objectContaining({
+          amount: 900,
+          currency_code: "eur",
+        }),
+      ])
+    })
+
+    it("successfully updates a variant's prices by updating an existing price (using region_id) and adding another price", async () => {
+      const api = useApi()
+      const data = {
+        prices: [
+          {
+            region_id: "test-region",
+            amount: 8000,
+          },
+          {
+            currency_code: "eur",
+            amount: 900,
+          },
+        ],
+      }
+
+      const response = await api
+        .post("/admin/products/test-product1/variants/test-variant_3", data, {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+
+      expect(response.data.product.variants[1].prices.length).toEqual(
+        data.prices.length
+      )
+
+      expect(response.data.product.variants[1].prices).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            amount: 8000,
+            currency_code: "usd",
+            region_id: "test-region",
+          }),
+          expect.objectContaining({
+            amount: 900,
+            currency_code: "eur",
+          }),
+        ])
+      )
+    })
+  })
+
   describe("testing for soft-deletion + uniqueness on handles, collection and variant properties", () => {
     beforeEach(async () => {
       try {

integration-tests/api/helpers/product-seeder.js

@@ -197,7 +197,14 @@ module.exports = async (connection, data = {}) => {
     ean: "test-ean3",
     upc: "test-upc3",
     product_id: "test-product1",
-    prices: [{ id: "test-price3", currency_code: "usd", amount: 100 }],
+    prices: [
+      {
+        id: "test-price3",
+        region_id: "test-region",
+        currency_code: "usd",
+        amount: 100,
+      },
+    ],
     options: [
       {
         id: "test-variant-option-3",

integration-tests/api/package.json

@@ -8,15 +8,15 @@
     "build": "babel src -d dist --extensions \".ts,.js\""
   },
   "dependencies": {
-    "@medusajs/medusa": "1.1.64-dev-1644230658795",
+    "@medusajs/medusa": "1.1.64-dev-1645441522984",
-    "medusa-interfaces": "1.1.34-dev-1644230658795",
+    "medusa-interfaces": "1.1.34-dev-1645441522984",
     "typeorm": "^0.2.31"
   },
   "devDependencies": {
     "@babel/cli": "^7.12.10",
     "@babel/core": "^7.12.10",
     "@babel/node": "^7.12.10",
-    "babel-preset-medusa-package": "1.1.19-dev-1644230658795",
+    "babel-preset-medusa-package": "1.1.19-dev-1645441522984",
     "jest": "^26.6.3"
   }
 }

integration-tests/api/yarn.lock

@@ -1256,10 +1256,10 @@
     "@types/yargs" "^15.0.0"
     chalk "^4.0.0"
 
-"@medusajs/medusa-cli@1.1.27-dev-1644230658795":
+"@medusajs/medusa-cli@1.1.27-dev-1645441522984":
-  version "1.1.27-dev-1644230658795"
+  version "1.1.27-dev-1645441522984"
-  resolved "http://localhost:4873/@medusajs%2fmedusa-cli/-/medusa-cli-1.1.27-dev-1644230658795.tgz#dc1fed2e68d4f3fa134786d07c3a252aa2f07354"
+  resolved "http://localhost:4873/@medusajs%2fmedusa-cli/-/medusa-cli-1.1.27-dev-1645441522984.tgz#e3a5d8430c98592cce7909e1052fb5e2aec6797f"
-  integrity sha512-m+DqNNdpGO0wubizrPwQoBad0LrGpyut9tdI24U6a0202Dbr5DL+ekW5Lgm8VEwNd/cIiHkI5Trym/hXsnun+w==
+  integrity sha512-NnyH16LliwphtoGHg4phJBHFmtbt8I6tI9gs1ora2J/1gbZZbl5bjn9LG5MGZrCye69OBb1UdTWB+UWROgTBzg==
   dependencies:
     "@babel/polyfill" "^7.8.7"
     "@babel/runtime" "^7.9.6"
@@ -1277,8 +1277,8 @@
     is-valid-path "^0.1.1"
     joi-objectid "^3.0.1"
     meant "^1.0.1"
-    medusa-core-utils "1.1.31-dev-1644230658795"
+    medusa-core-utils "1.1.31-dev-1645441522984"
-    medusa-telemetry "0.0.11-dev-1644230658795"
+    medusa-telemetry "0.0.11-dev-1645441522984"
     netrc-parser "^3.1.6"
     open "^8.0.6"
     ora "^5.4.1"
@@ -1292,13 +1292,13 @@
     winston "^3.3.3"
     yargs "^15.3.1"
 
-"@medusajs/medusa@1.1.64-dev-1644230658795":
+"@medusajs/medusa@1.1.64-dev-1645441522984":
-  version "1.1.64-dev-1644230658795"
+  version "1.1.64-dev-1645441522984"
-  resolved "http://localhost:4873/@medusajs%2fmedusa/-/medusa-1.1.64-dev-1644230658795.tgz#10f957947ce9521bc8fe109737765647b98d3d7e"
+  resolved "http://localhost:4873/@medusajs%2fmedusa/-/medusa-1.1.64-dev-1645441522984.tgz#46dda1904705f2adb15e8aaafe0e1ecdce09763d"
-  integrity sha512-gIuCzyEGT/lXG0yBGOdQ5H9pUg2WTYz/Q5nH8d2sRGDhW2A47cLOJb8VYsqrx6EcSNirW4bygoZgo24gyL5PFg==
+  integrity sha512-oQkdKGRhpa504vBhAmdRZMRxD1HUnXA6l0amaIeHk7OpT8JewMt63f/G6nttXatkHUNcYV0tlRW+mgy1gAyAmA==
   dependencies:
     "@hapi/joi" "^16.1.8"
-    "@medusajs/medusa-cli" "1.1.27-dev-1644230658795"
+    "@medusajs/medusa-cli" "1.1.27-dev-1645441522984"
     "@types/lodash" "^4.14.168"
     awilix "^4.2.3"
     body-parser "^1.19.0"
@@ -1322,8 +1322,8 @@
     joi "^17.3.0"
     joi-objectid "^3.0.1"
     jsonwebtoken "^8.5.1"
-    medusa-core-utils "1.1.31-dev-1644230658795"
+    medusa-core-utils "1.1.31-dev-1645441522984"
-    medusa-test-utils "1.1.37-dev-1644230658795"
+    medusa-test-utils "1.1.37-dev-1645441522984"
     morgan "^1.9.1"
     multer "^1.4.2"
     passport "^0.4.0"
@@ -1947,10 +1947,10 @@ babel-preset-jest@^26.6.2:
     babel-plugin-jest-hoist "^26.6.2"
     babel-preset-current-node-syntax "^1.0.0"
 
-babel-preset-medusa-package@1.1.19-dev-1644230658795:
+babel-preset-medusa-package@1.1.19-dev-1645441522984:
-  version "1.1.19-dev-1644230658795"
+  version "1.1.19-dev-1645441522984"
-  resolved "http://localhost:4873/babel-preset-medusa-package/-/babel-preset-medusa-package-1.1.19-dev-1644230658795.tgz#11b437ba399ed335c2ca8ecd0ced8e2c94557e68"
+  resolved "http://localhost:4873/babel-preset-medusa-package/-/babel-preset-medusa-package-1.1.19-dev-1645441522984.tgz#d7f494e6bcdf97d22e32913809a842d90720ee71"
-  integrity sha512-NSvAyqCQgnkGGAN5/Vs6QHT/KoG8AmPnuvMSooT2QBRB2h/vULVI4UuP7CQb3mfnfjX/gm9Tbch/zHhQJKPRUw==
+  integrity sha512-slmfLD+uwJhvYz2MMwFepjpbBB6K4EaZbKImJJ/Bp4RcZzoc44lCV/e5+9q2HU7hz+aLgZVangFvL9vLXglyzQ==
   dependencies:
     "@babel/plugin-proposal-class-properties" "^7.12.1"
     "@babel/plugin-proposal-decorators" "^7.12.1"
@@ -5135,25 +5135,25 @@ media-typer@0.3.0:
   resolved "https://registry.yarnpkg.com/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748"
   integrity sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=
 
-medusa-core-utils@1.1.31-dev-1644230658795:
+medusa-core-utils@1.1.31-dev-1645441522984:
-  version "1.1.31-dev-1644230658795"
+  version "1.1.31-dev-1645441522984"
-  resolved "http://localhost:4873/medusa-core-utils/-/medusa-core-utils-1.1.31-dev-1644230658795.tgz#fdeb0df1976c0331c3ce975f368429adcc4732f8"
+  resolved "http://localhost:4873/medusa-core-utils/-/medusa-core-utils-1.1.31-dev-1645441522984.tgz#e8942d486689b3fa5b2dcdd6b98cb7f2a46ffc3a"
-  integrity sha512-DdaTYsepwqpJGg2Hk6fa5ODVZZr8vKOwTKuAdlE45+LcildU625/2fdHYI8fUdBuOqKMnZRwWqI06I77xlpHgw==
+  integrity sha512-BRQf/vQoiHbRfEty1vSqcX5rbiQaoeYigiParQMmD3fnRprrKkxh9UjsPzVckiyjLM2QU4NIPcve42HmCJ+iKA==
   dependencies:
     joi "^17.3.0"
     joi-objectid "^3.0.1"
 
-medusa-interfaces@1.1.34-dev-1644230658795:
+medusa-interfaces@1.1.34-dev-1645441522984:
-  version "1.1.34-dev-1644230658795"
+  version "1.1.34-dev-1645441522984"
-  resolved "http://localhost:4873/medusa-interfaces/-/medusa-interfaces-1.1.34-dev-1644230658795.tgz#b719329abe31a337ad4350161e8b45f3dd8af1e7"
+  resolved "http://localhost:4873/medusa-interfaces/-/medusa-interfaces-1.1.34-dev-1645441522984.tgz#81522e69f2416916a4a134263db62f73dc4c00c0"
-  integrity sha512-lKd5QeZi/kEU9yte5HwUafRLWO0F3L2ArgrNkNoGOgA49tOKE5qS+BYqH/hImmGBSducT3ig8Bd2bCn96iEbDA==
+  integrity sha512-1IMOtBJvCwPh4pscOn7Z9c+vOkebadHu8AB9KbwWRM3ziyYgwNMVg8tC90sn20PctDXeQyWhsPfY1IkWj83WXw==
   dependencies:
-    medusa-core-utils "1.1.31-dev-1644230658795"
+    medusa-core-utils "1.1.31-dev-1645441522984"
 
-medusa-telemetry@0.0.11-dev-1644230658795:
+medusa-telemetry@0.0.11-dev-1645441522984:
-  version "0.0.11-dev-1644230658795"
+  version "0.0.11-dev-1645441522984"
-  resolved "http://localhost:4873/medusa-telemetry/-/medusa-telemetry-0.0.11-dev-1644230658795.tgz#2599eefe6440795e73e71a90ca41e63e61101405"
+  resolved "http://localhost:4873/medusa-telemetry/-/medusa-telemetry-0.0.11-dev-1645441522984.tgz#6939f21cbf01015df6d59983cfdfebc4d853d781"
-  integrity sha512-yAi6W7NXqVnCvseow5eLhP8mRh8sV79PGN/otPSmhuT3v82W9aXhIdhdOQVIbxsc7N1P0+3iunh7Qu7uR9cElw==
+  integrity sha512-l2kYVlYYs0tMIy27xCyj1USfh0yE8L9i+c8j0jW6mqcAJ+rVMPrahBs4jQs8TD1ptFtLDgYUNRg4WK1/f6RN8Q==
   dependencies:
     axios "^0.21.1"
     axios-retry "^3.1.9"
@@ -5165,13 +5165,13 @@ medusa-telemetry@0.0.11-dev-1644230658795:
     remove-trailing-slash "^0.1.1"
     uuid "^8.3.2"
 
-medusa-test-utils@1.1.37-dev-1644230658795:
+medusa-test-utils@1.1.37-dev-1645441522984:
-  version "1.1.37-dev-1644230658795"
+  version "1.1.37-dev-1645441522984"
-  resolved "http://localhost:4873/medusa-test-utils/-/medusa-test-utils-1.1.37-dev-1644230658795.tgz#803fcd6b6e7e831a14449af8545edfbe2302af3f"
+  resolved "http://localhost:4873/medusa-test-utils/-/medusa-test-utils-1.1.37-dev-1645441522984.tgz#14e197adaab890e0aa1bbe812d8dd51387e38661"
-  integrity sha512-VTTuHRngkoGCTLNzTE1Z14BfesRRlpmceS5qnicVkPLidY+l20WhJDAWLpE7RmRNQxuOXirJEdHT99cNQrA0yA==
+  integrity sha512-5qfcgPA/usM+n9vUoZluK0rEldaz5movukqDeyPTI1YbhJpIjQAq2f8GkHPIORDg4ppawUYz93d3gchIhe87FA==
   dependencies:
     "@babel/plugin-transform-classes" "^7.9.5"
-    medusa-core-utils "1.1.31-dev-1644230658795"
+    medusa-core-utils "1.1.31-dev-1645441522984"
     randomatic "^3.1.1"
 
 merge-descriptors@1.0.1:

packages/medusa/src/repositories/money-amount.ts

@@ -1,5 +1,58 @@
-import { EntityRepository, Repository } from "typeorm"
+import {
+  Brackets,
+  EntityRepository,
+  In,
+  IsNull,
+  Not,
+  Repository,
+} from "typeorm"
 import { MoneyAmount } from "../models/money-amount"
 
+type Price = Partial<
+  Pick<
+    MoneyAmount,
+    "currency_code" | "region_id" | "sale_amount" | "currency_code"
+  >
+> & { amount: number }
+
 @EntityRepository(MoneyAmount)
-export class MoneyAmountRepository extends Repository<MoneyAmount> { }
+export class MoneyAmountRepository extends Repository<MoneyAmount> {
+  public async findVariantPricesNotIn(variantId: string, prices: Price[]) {
+    const pricesNotInPricesPayload = await this.createQueryBuilder()
+      .where({
+        variant_id: variantId,
+      })
+      .andWhere(
+        new Brackets((qb) => {
+          qb.where({
+            currency_code: Not(In(prices.map((p) => p.currency_code))),
+          }).orWhere({ region_id: Not(In(prices.map((p) => p.region_id))) })
+        })
+      )
+      .getMany()
+    return pricesNotInPricesPayload
+  }
+
+  public async upsertCurrencyPrice(variantId: string, price: Price) {
+    let moneyAmount = await this.findOne({
+      where: {
+        currency_code: price.currency_code,
+        variant_id: variantId,
+        region_id: IsNull(),
+      },
+    })
+
+    if (!moneyAmount) {
+      moneyAmount = this.create({
+        ...price,
+        currency_code: price.currency_code?.toLowerCase(),
+        variant_id: variantId,
+      })
+    } else {
+      moneyAmount.amount = price.amount
+      moneyAmount.sale_amount = price.sale_amount
+    }
+
+    return await this.save(moneyAmount)
+  }
+}

packages/medusa/src/services/__tests__/product-variant.js

@@ -4,7 +4,7 @@ import ProductVariantService from "../product-variant"
 
 const eventBusService = {
   emit: jest.fn(),
-  withTransaction: function () {
+  withTransaction: function() {
     return this
   },
 }
@@ -253,7 +253,7 @@ describe("ProductVariantService", () => {
       .fn()
       .mockReturnValue(() => Promise.resolve())
 
-    productVariantService.setCurrencyPrice = jest
+    productVariantService.updateVariantPrices = jest
       .fn()
       .mockReturnValue(() => Promise.resolve())
 
@@ -381,14 +381,16 @@ describe("ProductVariantService", () => {
         ],
       })
 
-      expect(productVariantService.setCurrencyPrice).toHaveBeenCalledTimes(1)
+      expect(productVariantService.updateVariantPrices).toHaveBeenCalledTimes(1)
-      expect(productVariantService.setCurrencyPrice).toHaveBeenCalledWith(
+      expect(productVariantService.updateVariantPrices).toHaveBeenCalledWith(
         IdMap.getId("ironman"),
-        {
+        [
-          currency_code: "dkk",
+          {
-          amount: 1000,
+            currency_code: "dkk",
-          sale_amount: 750,
+            amount: 1000,
-        }
+            sale_amount: 750,
+          },
+        ]
       )
 
       expect(productVariantRepository.save).toHaveBeenCalledTimes(1)
@@ -416,23 +418,125 @@ describe("ProductVariantService", () => {
     })
   })
 
+  describe("updateVariantPrices", () => {
+    const moneyAmountRepository = MockRepository({
+      remove: () => Promise.resolve(),
+    })
+    const oldPrices = [
+      {
+        currency_code: "dkk",
+        amount: 1000,
+        variant_id: "ironman",
+        region_id: null,
+      },
+    ]
+
+    moneyAmountRepository.findVariantPricesNotIn = jest
+      .fn()
+      .mockImplementation(() => Promise.resolve(oldPrices))
+
+    const productVariantRepository = MockRepository({
+      findOne: (query) => Promise.resolve({ id: IdMap.getId("ironman") }),
+    })
+
+    const productOptionValueRepository = MockRepository({
+      findOne: () =>
+        Promise.resolve({ id: IdMap.getId("some-value"), value: "blue" }),
+    })
+
+    const productVariantService = new ProductVariantService({
+      manager: MockManager,
+      eventBusService,
+      moneyAmountRepository,
+      productVariantRepository,
+      productOptionValueRepository,
+    })
+
+    productVariantService.updateOptionValue = jest
+      .fn()
+      .mockReturnValue(() => Promise.resolve())
+
+    productVariantService.setCurrencyPrice = jest
+      .fn()
+      .mockReturnValue(() => Promise.resolve())
+
+    productVariantService.setRegionPrice = jest
+      .fn()
+      .mockReturnValue(() => Promise.resolve())
+
+    beforeEach(async () => {
+      jest.clearAllMocks()
+    })
+
+    it("successfully removes obsolete prices and calls setCurrencyPrice on new/existing prices", async () => {
+      await productVariantService.updateVariantPrices("ironman", [
+        {
+          currency_code: "usd",
+          amount: 4000,
+        },
+      ])
+
+      expect(
+        moneyAmountRepository.findVariantPricesNotIn
+      ).toHaveBeenCalledTimes(1)
+
+      expect(productVariantService.setCurrencyPrice).toHaveBeenCalledTimes(1)
+      expect(productVariantService.setCurrencyPrice).toHaveBeenCalledWith(
+        "ironman",
+        {
+          currency_code: "usd",
+          amount: 4000,
+        }
+      )
+
+      expect(moneyAmountRepository.remove).toHaveBeenCalledTimes(1)
+      expect(moneyAmountRepository.remove).toHaveBeenCalledWith(oldPrices)
+    })
+
+    it("successfully removes obsolete prices and calls setRegionPrice on new/existing prices", async () => {
+      await productVariantService.updateVariantPrices("ironman", [
+        {
+          region_id: "test-region",
+          amount: 4000,
+          sale_amount: 2000,
+        },
+      ])
+
+      expect(
+        moneyAmountRepository.findVariantPricesNotIn
+      ).toHaveBeenCalledTimes(1)
+
+      expect(productVariantService.setRegionPrice).toHaveBeenCalledTimes(1)
+      expect(productVariantService.setRegionPrice).toHaveBeenCalledWith(
+        "ironman",
+        {
+          region_id: "test-region",
+          amount: 4000,
+          sale_amount: 2000,
+        }
+      )
+
+      expect(moneyAmountRepository.remove).toHaveBeenCalledTimes(1)
+      expect(moneyAmountRepository.remove).toHaveBeenCalledWith(oldPrices)
+    })
+  })
+
   describe("setCurrencyPrice", () => {
     const productVariantRepository = MockRepository({
       findOne: (query) => Promise.resolve({ id: IdMap.getId("ironman") }),
     })
 
-    const moneyAmountRepository = MockRepository({
+    const moneyAmountRepository = MockRepository()
-      findOne: (query) => {
+
-        if (query.where.currency_code === "usd") {
+    moneyAmountRepository.upsertCurrencyPrice = jest
-          return Promise.resolve(undefined)
+      .fn()
-        }
+      .mockImplementation((variantId, price) => {
         return Promise.resolve({
-          id: IdMap.getId("dkk"),
+          id: IdMap.getId("test-amount"),
-          variant_id: IdMap.getId("ironman"),
+          variant_id: IdMap.getId(variantId),
-          currency_code: "dkk",
+          ...price,
         })
-      },
+      })
-    })
 
     const productVariantService = new ProductVariantService({
       manager: MockManager,
@@ -445,50 +549,32 @@ describe("ProductVariantService", () => {
       jest.clearAllMocks()
     })
 
-    it("successfully creates a price if none exist with given currency", async () => {
+    it("calls upsert price with given currency", async () => {
       await productVariantService.setCurrencyPrice(IdMap.getId("ironman"), {
         currency_code: "usd",
         amount: 100,
       })
 
-      expect(moneyAmountRepository.create).toHaveBeenCalledTimes(1)
+      expect(moneyAmountRepository.upsertCurrencyPrice).toHaveBeenCalledTimes(1)
-      expect(moneyAmountRepository.create).toHaveBeenCalledWith({
+      expect(moneyAmountRepository.upsertCurrencyPrice).toHaveBeenCalledWith(
-        variant_id: IdMap.getId("ironman"),
+        IdMap.getId("ironman"),
-        currency_code: "usd",
+        {
-        amount: 100,
+          currency_code: "usd",
-      })
+          amount: 100,
-
+        }
-      expect(moneyAmountRepository.save).toHaveBeenCalledTimes(1)
+      )
-    })
-
-    it("successfully updates a non-regional price if currency exists", async () => {
-      await productVariantService.setCurrencyPrice(IdMap.getId("ironman"), {
-        currency_code: "dkk",
-        amount: 1000,
-      })
-
-      expect(moneyAmountRepository.create).toHaveBeenCalledTimes(0)
-
-      expect(moneyAmountRepository.save).toHaveBeenCalledTimes(1)
-      expect(moneyAmountRepository.save).toHaveBeenCalledWith({
-        variant_id: IdMap.getId("ironman"),
-        id: IdMap.getId("dkk"),
-        currency_code: "dkk",
-        amount: 1000,
-        sale_amount: undefined,
-      })
     })
   })
 
   describe("getRegionPrice", () => {
     const regionService = {
-      retrieve: function () {
+      retrieve: function() {
         return Promise.resolve({
           id: IdMap.getId("california"),
           name: "California",
         })
       },
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }

packages/medusa/src/services/product-variant.ts

@@ -1,12 +1,6 @@
 import { MedusaError } from "medusa-core-utils"
 import { BaseService } from "medusa-interfaces"
-import {
+import { Brackets, EntityManager, ILike, In, SelectQueryBuilder } from "typeorm"
-  Brackets,
-  EntityManager,
-  ILike,
-  IsNull,
-  SelectQueryBuilder,
-} from "typeorm"
 import { MoneyAmount } from "../models/money-amount"
 import { Product } from "../models/product"
 import { ProductOptionValue } from "../models/product-option-value"
@@ -285,17 +279,7 @@ class ProductVariantService extends BaseService {
       const { prices, options, metadata, inventory_quantity, ...rest } = update
 
       if (prices) {
-        for (const price of prices) {
+        await this.updateVariantPrices(variant.id, prices)
-          if (price.region_id) {
-            await this.setRegionPrice(variant.id, {
-              region_id: price.region_id,
-              amount: price.amount,
-              sale_amount: price.sale_amount || undefined,
-            })
-          } else {
-            await this.setCurrencyPrice(variant.id, price)
-          }
-        }
       }
 
       if (options) {
@@ -333,6 +317,37 @@ class ProductVariantService extends BaseService {
     })
   }
 
+  async updateVariantPrices(
+    variantId: string,
+    prices: ProductVariantPrice[]
+  ): Promise<void> {
+    return this.atomicPhase_(async (manager: EntityManager) => {
+      const moneyAmountRepo = manager.getCustomRepository(
+        this.moneyAmountRepository_
+      )
+
+      // get prices to be deleted
+      const obsoletePrices = await moneyAmountRepo.findVariantPricesNotIn(
+        variantId,
+        prices
+      )
+
+      for (const price of prices) {
+        if (price.region_id) {
+          await this.setRegionPrice(variantId, {
+            region_id: price.region_id,
+            amount: price.amount,
+            sale_amount: price.sale_amount || undefined,
+          })
+        } else {
+          await this.setCurrencyPrice(variantId, price)
+        }
+      }
+
+      await moneyAmountRepo.remove(obsoletePrices)
+    })
+  }
+
   /**
    * Sets the default price for the given currency.
    * @param {string} variantId - the id of the variant to set prices for
@@ -348,26 +363,7 @@ class ProductVariantService extends BaseService {
         this.moneyAmountRepository_
       )
 
-      let moneyAmount = await moneyAmountRepo.findOne({
+      return await moneyAmountRepo.upsertCurrencyPrice(variantId, price)
-        where: {
-          currency_code: price.currency_code?.toLowerCase(),
-          variant_id: variantId,
-          region_id: IsNull(),
-        },
-      })
-
-      if (!moneyAmount) {
-        moneyAmount = moneyAmountRepo.create({
-          ...price,
-          currency_code: price.currency_code?.toLowerCase(),
-          variant_id: variantId,
-        })
-      } else {
-        moneyAmount.amount = price.amount
-        moneyAmount.sale_amount = price.sale_amount
-      }
-
-      return await moneyAmountRepo.save(moneyAmount)
     })
   }