From 4e375c22033bc69f28d9360de15d09c1de57b069 Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Wed, 13 Jul 2022 08:41:03 +0200 Subject: [PATCH] feat(medusa): Prevent default channel from being deleted (#1835) **What** Prevent the default channel from being deleted Fixes CORE-317 --- .../api/__tests__/admin/sales-channels.js | 24 +++ .../factories/simple-sales-channel-factory.ts | 7 +- integration-tests/api/yarn.lock | 175 +++++++++++++++++- .../src/api/middlewares/error-handler.ts | 2 +- .../medusa/src/services/__mocks__/store.js | 16 +- .../src/services/__tests__/sales-channel.ts | 77 +++++--- packages/medusa/src/services/sales-channel.ts | 13 +- packages/medusa/src/services/store.ts | 15 +- 8 files changed, 272 insertions(+), 57 deletions(-) diff --git a/integration-tests/api/__tests__/admin/sales-channels.js b/integration-tests/api/__tests__/admin/sales-channels.js index 5cc9d220a5..df6dcc5551 100644 --- a/integration-tests/api/__tests__/admin/sales-channels.js +++ b/integration-tests/api/__tests__/admin/sales-channels.js @@ -188,6 +188,15 @@ describe("sales channels", () => { name: "test name", description: "test description", }) + + await simpleSalesChannelFactory(dbConnection, { + name: "Default channel", + id: "test-channel", + }) + + await dbConnection.manager.query( + `UPDATE store SET default_sales_channel_id = 'test-channel'` + ) } catch (e) { console.error(e) } @@ -197,6 +206,7 @@ describe("sales channels", () => { const db = useDb() await db.teardown() }) + it("should delete the requested sales channel", async () => { const api = useApi() @@ -286,6 +296,20 @@ describe("sales channels", () => { expect(deletedSalesChannel.id).toEqual(salesChannel.id) expect(deletedSalesChannel.deleted_at).not.toEqual(null) }) + + it("should throw if we attempt to delete default channel", async () => { + const api = useApi() + expect.assertions(2) + + const res = await api + .delete(`/admin/sales-channels/test-channel`, adminReqConfig) + .catch((err) => err) + + expect(res.response.status).toEqual(400) + expect(res.response.data.message).toEqual( + "You cannot delete the default sales channel" + ) + }) }) describe("GET /admin/orders/:id", () => { diff --git a/integration-tests/api/factories/simple-sales-channel-factory.ts b/integration-tests/api/factories/simple-sales-channel-factory.ts index 52c259503a..d623f7f07b 100644 --- a/integration-tests/api/factories/simple-sales-channel-factory.ts +++ b/integration-tests/api/factories/simple-sales-channel-factory.ts @@ -1,11 +1,12 @@ -import { Connection } from "typeorm" -import faker from "faker" import { SalesChannel } from "@medusajs/medusa" +import faker from "faker" +import { Connection } from "typeorm" export type SalesChannelFactoryData = { name?: string description?: string is_disabled?: boolean + id?: string } export const simpleSalesChannelFactory = async ( @@ -20,7 +21,7 @@ export const simpleSalesChannelFactory = async ( const manager = connection.manager const salesChannel = manager.create(SalesChannel, { - id: `simple-id-${Math.random() * 1000}`, + id: data.id ?? `simple-id-${Math.random() * 1000}`, name: data.name || faker.name.firstName(), description: data.description || faker.name.lastName(), is_disabled: diff --git a/integration-tests/api/yarn.lock b/integration-tests/api/yarn.lock index f11e7d6f54..58b0c2f298 100644 --- a/integration-tests/api/yarn.lock +++ b/integration-tests/api/yarn.lock @@ -1,6 +1,3 @@ -# This file is generated by running "yarn install" inside your project. -# Manual changes might be lost - proceed with caution! - __metadata: version: 6 cacheKey: 8c0 @@ -1825,6 +1822,46 @@ __metadata: languageName: node linkType: hard +"@medusajs/medusa-cli@npm:1.3.1-dev-1657570841696": + version: 1.3.1-dev-1657570841696 + resolution: "@medusajs/medusa-cli@npm:1.3.1-dev-1657570841696" + dependencies: + "@babel/polyfill": ^7.8.7 + "@babel/runtime": ^7.9.6 + "@hapi/joi": ^16.1.8 + axios: ^0.21.1 + chalk: ^4.0.0 + configstore: 5.0.1 + core-js: ^3.6.5 + dotenv: ^8.2.0 + execa: ^5.1.1 + fs-exists-cached: ^1.0.0 + fs-extra: ^10.0.0 + hosted-git-info: ^4.0.2 + inquirer: ^8.0.0 + is-valid-path: ^0.1.1 + joi-objectid: ^3.0.1 + meant: ^1.0.1 + medusa-core-utils: 1.1.31-dev-1657570841696 + medusa-telemetry: 0.0.11-dev-1657570841696 + netrc-parser: ^3.1.6 + open: ^8.0.6 + ora: ^5.4.1 + pg-god: ^1.0.11 + prompts: ^2.4.1 + regenerator-runtime: ^0.13.5 + resolve-cwd: ^3.0.0 + stack-trace: ^0.0.10 + ulid: ^2.3.0 + url: ^0.11.0 + winston: ^3.3.3 + yargs: ^15.3.1 + bin: + medusa: cli.js + checksum: 5053ef0c1637be0830bb02c410d3734d030e361b05e5af06ef1a981e8a320206f0d4e050504f9b5e976af95fbabcb346bb6abfe7ccfbe52ecf09afa47e9268d7 + languageName: node + linkType: hard + "@medusajs/medusa-cli@npm:1.3.1-dev-1657640917765": version: 1.3.1-dev-1657640917765 resolution: "@medusajs/medusa-cli@npm:1.3.1-dev-1657640917765" @@ -1865,6 +1902,64 @@ __metadata: languageName: node linkType: hard +"@medusajs/medusa@npm:1.3.3-dev-1657570841696": + version: 1.3.3-dev-1657570841696 + resolution: "@medusajs/medusa@npm:1.3.3-dev-1657570841696" + dependencies: + "@hapi/joi": ^16.1.8 + "@medusajs/medusa-cli": 1.3.1-dev-1657570841696 + "@types/lodash": ^4.14.168 + awilix: ^4.2.3 + body-parser: ^1.19.0 + bull: ^3.12.1 + chokidar: ^3.4.2 + class-transformer: ^0.5.1 + class-validator: ^0.13.1 + connect-redis: ^5.0.0 + cookie-parser: ^1.4.4 + core-js: ^3.6.5 + cors: ^2.8.5 + cross-spawn: ^7.0.3 + express: ^4.17.1 + express-session: ^1.17.1 + fs-exists-cached: ^1.0.0 + glob: ^7.1.6 + ioredis: ^4.17.3 + ioredis-mock: ^5.6.0 + iso8601-duration: ^1.3.0 + joi: ^17.3.0 + joi-objectid: ^3.0.1 + jsonwebtoken: ^8.5.1 + medusa-core-utils: 1.1.31-dev-1657570841696 + medusa-test-utils: 1.1.37-dev-1657570841696 + morgan: ^1.9.1 + multer: ^1.4.2 + node-schedule: ^2.1.0 + papaparse: ^5.3.2 + passport: ^0.4.0 + passport-http-bearer: ^1.0.1 + passport-jwt: ^4.0.0 + passport-local: ^1.0.0 + pg: ^8.5.1 + randomatic: ^3.1.1 + redis: ^3.0.2 + reflect-metadata: ^0.1.13 + request-ip: ^2.1.3 + resolve-cwd: ^3.0.0 + scrypt-kdf: ^2.0.1 + sqlite3: ^5.0.2 + ulid: ^2.3.0 + uuid: ^8.3.1 + winston: ^3.2.1 + peerDependencies: + medusa-interfaces: 1.x + typeorm: 0.2.x + bin: + medusa: cli.js + checksum: b11b24ae6e83ea497777af333586f9989e7511140636e17a4d46612f56c7a6f33a7a6c7f6ae613d846c31882c74e30c05dc6b432e000956b22eac9fdeb1f63e1 + languageName: node + linkType: hard + "@medusajs/medusa@npm:1.3.4-dev-1657640917765": version: 1.3.4-dev-1657640917765 resolution: "@medusajs/medusa@npm:1.3.4-dev-1657640917765" @@ -2491,11 +2586,11 @@ __metadata: "@babel/cli": ^7.12.10 "@babel/core": ^7.12.10 "@babel/node": ^7.12.10 - "@medusajs/medusa": 1.3.4-dev-1657640917765 - babel-preset-medusa-package: 1.1.19-dev-1657640917765 + "@medusajs/medusa": 1.3.3-dev-1657570841696 + babel-preset-medusa-package: 1.1.19-dev-1657570841696 faker: ^5.5.3 jest: ^26.6.3 - medusa-interfaces: 1.3.1-dev-1657640917765 + medusa-interfaces: 1.3.1-dev-1657570841696 typeorm: ^0.2.31 languageName: unknown linkType: soft @@ -2802,6 +2897,26 @@ __metadata: languageName: node linkType: hard +"babel-preset-medusa-package@npm:1.1.19-dev-1657570841696": + version: 1.1.19-dev-1657570841696 + resolution: "babel-preset-medusa-package@npm:1.1.19-dev-1657570841696" + dependencies: + "@babel/plugin-proposal-class-properties": ^7.12.1 + "@babel/plugin-proposal-decorators": ^7.12.1 + "@babel/plugin-proposal-optional-chaining": ^7.14.2 + "@babel/plugin-transform-classes": ^7.12.1 + "@babel/plugin-transform-instanceof": ^7.12.1 + "@babel/plugin-transform-runtime": ^7.12.1 + "@babel/preset-env": ^7.12.7 + "@babel/preset-typescript": ^7.16.0 + babel-plugin-transform-typescript-metadata: ^0.3.1 + core-js: ^3.7.0 + peerDependencies: + "@babel/core": ^7.11.6 + checksum: 60802948b9e278ea047f3c51d3a2aa903599408f2cd38f79dcf75219163055100475cb564ef11189afa3564ad3cdecea7a1bd571f26a4b4450e914772102cb06 + languageName: node + linkType: hard + "babel-preset-medusa-package@npm:1.1.19-dev-1657640917765": version: 1.1.19-dev-1657640917765 resolution: "babel-preset-medusa-package@npm:1.1.19-dev-1657640917765" @@ -6951,6 +7066,16 @@ __metadata: languageName: node linkType: hard +"medusa-core-utils@npm:1.1.31-dev-1657570841696": + version: 1.1.31-dev-1657570841696 + resolution: "medusa-core-utils@npm:1.1.31-dev-1657570841696" + dependencies: + joi: ^17.3.0 + joi-objectid: ^3.0.1 + checksum: 537715c197a15390a644f62db975391454aa56c5c9f41c070059dd167c84982256d1a1b37ecd4cc5abaae784ec0337ee1cab4694797d2946273b882e94d94c1c + languageName: node + linkType: hard + "medusa-core-utils@npm:1.1.31-dev-1657640917765": version: 1.1.31-dev-1657640917765 resolution: "medusa-core-utils@npm:1.1.31-dev-1657640917765" @@ -6961,6 +7086,16 @@ __metadata: languageName: node linkType: hard +"medusa-interfaces@npm:1.3.1-dev-1657570841696": + version: 1.3.1-dev-1657570841696 + resolution: "medusa-interfaces@npm:1.3.1-dev-1657570841696" + peerDependencies: + medusa-core-utils: ^1.1.31 + typeorm: 0.x + checksum: e226125007cb40c7015c46d4d081efeaaea9ce96277093052af02c7c3603366c36e3bb06192d5fb164a4bf344f7e785201e5f063c00d966f878f8b2ccb12e183 + languageName: node + linkType: hard + "medusa-interfaces@npm:1.3.1-dev-1657640917765": version: 1.3.1-dev-1657640917765 resolution: "medusa-interfaces@npm:1.3.1-dev-1657640917765" @@ -6971,6 +7106,23 @@ __metadata: languageName: node linkType: hard +"medusa-telemetry@npm:0.0.11-dev-1657570841696": + version: 0.0.11-dev-1657570841696 + resolution: "medusa-telemetry@npm:0.0.11-dev-1657570841696" + dependencies: + axios: ^0.21.1 + axios-retry: ^3.1.9 + boxen: ^5.0.1 + ci-info: ^3.2.0 + configstore: 5.0.1 + global: ^4.4.0 + is-docker: ^2.2.1 + remove-trailing-slash: ^0.1.1 + uuid: ^8.3.2 + checksum: ee9223fbd54b4bf7a036710aa56de98afa6977877255dcee5f919575343c804efa6655cf69f5b68c876f16bba3bfe027bb556158542aa04bfa49ebe9179f4c6c + languageName: node + linkType: hard + "medusa-telemetry@npm:0.0.11-dev-1657640917765": version: 0.0.11-dev-1657640917765 resolution: "medusa-telemetry@npm:0.0.11-dev-1657640917765" @@ -6988,6 +7140,17 @@ __metadata: languageName: node linkType: hard +"medusa-test-utils@npm:1.1.37-dev-1657570841696": + version: 1.1.37-dev-1657570841696 + resolution: "medusa-test-utils@npm:1.1.37-dev-1657570841696" + dependencies: + "@babel/plugin-transform-classes": ^7.9.5 + medusa-core-utils: 1.1.31-dev-1657570841696 + randomatic: ^3.1.1 + checksum: adc49e171186b1850c8d5aea54913aad6f87eb8a7944da5bdf900582759864db5e93ad1afbfb75c920a900bdce9f3000878a39f72ad3cf3b8609e80b9f208ffd + languageName: node + linkType: hard + "medusa-test-utils@npm:1.1.37-dev-1657640917765": version: 1.1.37-dev-1657640917765 resolution: "medusa-test-utils@npm:1.1.37-dev-1657640917765" diff --git a/packages/medusa/src/api/middlewares/error-handler.ts b/packages/medusa/src/api/middlewares/error-handler.ts index 37496b09ad..b4407a382f 100644 --- a/packages/medusa/src/api/middlewares/error-handler.ts +++ b/packages/medusa/src/api/middlewares/error-handler.ts @@ -1,5 +1,5 @@ -import { MedusaError } from "medusa-core-utils" import { NextFunction, Request, Response } from "express" +import { MedusaError } from "medusa-core-utils" import { Logger } from "../../types/global" const QUERY_RUNNER_RELEASED = "QueryRunnerAlreadyReleasedError" diff --git a/packages/medusa/src/services/__mocks__/store.js b/packages/medusa/src/services/__mocks__/store.js index cf58e44349..39fa727ed0 100644 --- a/packages/medusa/src/services/__mocks__/store.js +++ b/packages/medusa/src/services/__mocks__/store.js @@ -1,28 +1,26 @@ -import { IdMap } from "medusa-test-utils" - export const store = { - _id: IdMap.getId("store"), + id: "test-store", name: "Test store", currencies: ["DKK", "SEK", "GBP"], } export const StoreServiceMock = { - withTransaction: function () { + withTransaction: function() { return this }, - create: jest.fn().mockImplementation(data => { + create: jest.fn().mockImplementation((data) => { return Promise.resolve(data) }), - addCurrency: jest.fn().mockImplementation(data => { + addCurrency: jest.fn().mockImplementation((data) => { return Promise.resolve() }), - removeCurrency: jest.fn().mockImplementation(data => { + removeCurrency: jest.fn().mockImplementation((data) => { return Promise.resolve() }), - update: jest.fn().mockImplementation(data => { + update: jest.fn().mockImplementation((data) => { return Promise.resolve() }), - retrieve: jest.fn().mockImplementation(data => { + retrieve: jest.fn().mockImplementation((data) => { return Promise.resolve(store) }), } diff --git a/packages/medusa/src/services/__tests__/sales-channel.ts b/packages/medusa/src/services/__tests__/sales-channel.ts index bfc7bf6f4d..3a8016da75 100644 --- a/packages/medusa/src/services/__tests__/sales-channel.ts +++ b/packages/medusa/src/services/__tests__/sales-channel.ts @@ -1,10 +1,10 @@ import { IdMap, MockManager, MockRepository } from "medusa-test-utils" -import SalesChannelService from "../sales-channel" -import { EventBusServiceMock } from "../__mocks__/event-bus" -import { EventBusService, StoreService } from "../index" import { FindConditions, FindOneOptions } from "typeorm" import { SalesChannel } from "../../models" -import { store, StoreServiceMock } from "../__mocks__/store"; +import { EventBusService, StoreService } from "../index" +import SalesChannelService from "../sales-channel" +import { EventBusServiceMock } from "../__mocks__/event-bus" +import { store, StoreServiceMock } from "../__mocks__/store" describe("SalesChannelService", () => { const salesChannelData = { @@ -29,10 +29,11 @@ describe("SalesChannelService", () => { } ), create: jest.fn().mockImplementation((data) => data), - save: (salesChannel) => Promise.resolve({ - id: IdMap.getId("sales_channel_1"), - ...salesChannel - }), + save: (salesChannel) => + Promise.resolve({ + id: IdMap.getId("sales_channel_1"), + ...salesChannel, + }), softRemove: jest.fn().mockImplementation((id: string): any => { return Promise.resolve() }), @@ -43,7 +44,7 @@ describe("SalesChannelService", () => { manager: MockManager, eventBusService: EventBusServiceMock as unknown as EventBusService, salesChannelRepository: salesChannelRepositoryMock, - storeService: StoreServiceMock as unknown as StoreService + storeService: StoreServiceMock as unknown as StoreService, }) beforeEach(() => { @@ -75,10 +76,10 @@ describe("SalesChannelService", () => { default_sales_channel: { id: IdMap.getId("sales_channel_1"), ...salesChannelData, - } + }, }) - }) - } as any + }), + } as any, }) const salesChannel = await localSalesChannelService.createDefault() @@ -97,7 +98,7 @@ describe("SalesChannelService", () => { manager: MockManager, eventBusService: EventBusServiceMock as unknown as EventBusService, salesChannelRepository: salesChannelRepositoryMock, - storeService: StoreServiceMock as unknown as StoreService + storeService: StoreServiceMock as unknown as StoreService, }) beforeEach(() => { @@ -127,7 +128,7 @@ describe("SalesChannelService", () => { manager: MockManager, eventBusService: EventBusServiceMock as unknown as EventBusService, salesChannelRepository: salesChannelRepositoryMock, - storeService: StoreServiceMock as unknown as StoreService + storeService: StoreServiceMock as unknown as StoreService, }) const update = { @@ -162,35 +163,53 @@ describe("SalesChannelService", () => { manager: MockManager, eventBusService: EventBusServiceMock as unknown as EventBusService, salesChannelRepository: salesChannelRepositoryMock, - storeService: StoreServiceMock as unknown as StoreService + storeService: { + ...StoreServiceMock, + retrieve: jest.fn().mockImplementation(() => { + return Promise.resolve({ + ...store, + default_sales_channel_id: "default_channel", + default_sales_channel: { + id: "default_channel", + ...salesChannelData, + }, + }) + }), + } as any, }) beforeEach(() => { jest.clearAllMocks() }) - it('should soft remove a sales channel', async () => { + it("should soft remove a sales channel", async () => { const res = await salesChannelService.delete( IdMap.getId("sales_channel_1") ) expect(res).toBeUndefined() - expect(salesChannelRepositoryMock.softRemove) - .toHaveBeenCalledTimes(1) - expect(salesChannelRepositoryMock.softRemove) - .toHaveBeenLastCalledWith({ - id: IdMap.getId("sales_channel_1"), - ...salesChannelData - }) + expect(salesChannelRepositoryMock.softRemove).toHaveBeenCalledTimes(1) + expect(salesChannelRepositoryMock.softRemove).toHaveBeenLastCalledWith({ + id: IdMap.getId("sales_channel_1"), + ...salesChannelData, + }) - expect(EventBusServiceMock.emit) - .toHaveBeenCalledTimes(1) - expect(EventBusServiceMock.emit) - .toHaveBeenLastCalledWith( - SalesChannelService.Events.DELETED, - { "id": IdMap.getId("sales_channel_1") } + expect(EventBusServiceMock.emit).toHaveBeenCalledTimes(1) + expect(EventBusServiceMock.emit).toHaveBeenLastCalledWith( + SalesChannelService.Events.DELETED, + { id: IdMap.getId("sales_channel_1") } + ) + }) + + it("should fail if delete of the default channel is attempted", async () => { + try { + await salesChannelService.delete("default_channel") + } catch (error) { + expect(error.message).toEqual( + "You cannot delete the default sales channel" ) + } }) }) }) diff --git a/packages/medusa/src/services/sales-channel.ts b/packages/medusa/src/services/sales-channel.ts index b358c0f24b..e0457e1fe9 100644 --- a/packages/medusa/src/services/sales-channel.ts +++ b/packages/medusa/src/services/sales-channel.ts @@ -9,8 +9,8 @@ import { CreateSalesChannelInput, UpdateSalesChannelInput, } from "../types/sales-channels" -import EventBusService from "./event-bus" import { buildQuery } from "../utils" +import EventBusService from "./event-bus" import StoreService from "./store" type InjectedDependencies = { @@ -165,6 +165,17 @@ class SalesChannelService extends TransactionBaseService { return } + const store = await this.storeService_.retrieve({ + select: ["default_sales_channel_id"], + }) + + if (salesChannel.id === store?.default_sales_channel_id) { + throw new MedusaError( + MedusaError.Types.NOT_ALLOWED, + "You cannot delete the default sales channel" + ) + } + await salesChannelRepo.softRemove(salesChannel) await this.eventBusService_ diff --git a/packages/medusa/src/services/store.ts b/packages/medusa/src/services/store.ts index b8eb4d118c..b6cc337a6b 100644 --- a/packages/medusa/src/services/store.ts +++ b/packages/medusa/src/services/store.ts @@ -1,15 +1,14 @@ import { MedusaError } from "medusa-core-utils" -import { currencies, Currency } from "../utils/currencies" import { EntityManager } from "typeorm" -import { StoreRepository } from "../repositories/store" -import { CurrencyRepository } from "../repositories/currency" -import EventBusService from "./event-bus" -import { Store } from "../models" -import { AdminPostStoreReq } from "../api/routes/admin/store" -import { FindConfig } from "../types/common" import { TransactionBaseService } from "../interfaces" -import { buildQuery, setMetadata } from "../utils" +import { Store } from "../models" +import { CurrencyRepository } from "../repositories/currency" +import { StoreRepository } from "../repositories/store" +import { FindConfig } from "../types/common" import { UpdateStoreInput } from "../types/store" +import { buildQuery, setMetadata } from "../utils" +import { currencies, Currency } from "../utils/currencies" +import EventBusService from "./event-bus" type InjectedDependencies = { manager: EntityManager