feat(medusa): Validate LineItems in SalesChannel (#1871)

* wip: validate line item SC

* fix: repository type, remove relation, use sc id, check if cart has associated sc

* feat: setup tests and seeder, change entity retrieval in cart service method

* feat: remove repo usage and method, use Adrien's method from product service to check sc association, add test cases, add seeder entities, accept flag for validating sc on the endpoint

* feat: add a unit test to ensure validation method is called if flag is passed

* feat: allow `validate_sales_channels` flag in other relevant endpoints

* fix: typo

* fix: flag rename

* fix: correct FF in test suites

* fix: address PR feedback

* fix: change error message

* feat: remove query params, guard with FF, refactor

* feat: guard validation in the service

* refactor: rename validation method

* refactor: reorganise tests

* wip: cleanup test file

* wip: revert cart seeder changes use factories

* fix: remove seeder, update mocks

* fix: method name

* fix: units, validate by default

* git: resolve merge conflicts

* refactor: separate line item sales chanel units

Co-authored-by: fPolic <frane@medusajs.com>
This commit is contained in:
Frane PolićandfPolic authored and GitHub committed 2022-07-27 21:39:06 +02:00
1 parent 204dd23a39
commit 3fbe8d7d08
9 files changed
+387 -28

No files matched your search

@@ -0,0 +1,144 @@
const path = require("path")
const { useDb } = require("../../../helpers/use-db")
const { useApi } = require("../../../helpers/use-api")
const { simpleCartFactory, simpleProductFactory } = require("../../factories")
const startServerWithEnvironment =
require("../../../helpers/start-server-with-environment").default
jest.setTimeout(30000)
describe("Line Item - Sales Channel", () => {
let dbConnection
let medusaProcess
const doAfterEach = async () => {
const db = useDb()
return await db.teardown()
}
beforeAll(async () => {
const cwd = path.resolve(path.join(__dirname, "..", ".."))
try {
const [process, connection] = await startServerWithEnvironment({
cwd,
env: { MEDUSA_FF_SALES_CHANNELS: true },
verbose: false,
})
dbConnection = connection
medusaProcess = process
} catch (error) {
console.log(error)
}
})
afterAll(async () => {
const db = useDb()
await db.shutdown()
medusaProcess.kill()
})
beforeEach(async () => {
try {
await simpleProductFactory(dbConnection, {
id: "test-product-in-sales-channel",
title: "test product belonging to a channel",
sales_channels: [
{
id: "main-sales-channel",
name: "Main sales channel",
description: "Main sales channel",
is_disabled: false,
},
],
variants: [
{
id: "test-variant-sales-channel",
title: "test variant in sales channel",
product_id: "test-product-in-sales-channel",
inventory_quantity: 1000,
prices: [
{
currency: "usd",
amount: 59,
},
],
},
],
})
await simpleProductFactory(dbConnection, {
id: "test-product-no-sales-channel",
variants: [
{
id: "test-variant-no-sales-channel",
},
],
})
await simpleCartFactory(dbConnection, {
id: "test-cart-with-sales-channel",
sales_channel: {
id: "main-sales-channel",
},
})
} catch (err) {
console.log(err)
throw err
}
})
afterEach(async () => {
await doAfterEach()
})
describe("Adding line item with associated sales channel to a cart", () => {
it("adding line item to a cart with associated sales channel returns 400", async () => {
const api = useApi()
const response = await api
.post(
"/store/carts/test-cart-with-sales-channel/line-items",
{
variant_id: "test-variant-no-sales-channel", // variant's product doesn't belong to a sales channel
quantity: 1,
},
{ withCredentials: true }
)
.catch((err) => err.response)
expect(response.status).toEqual(400)
expect(response.data.type).toEqual("invalid_data")
})
it("adding line item successfully if product and cart belong to the same sales channel", async () => {
const api = useApi()
const response = await api
.post(
"/store/carts/test-cart-with-sales-channel/line-items",
{
variant_id: "test-variant-sales-channel",
quantity: 1,
},
{ withCredentials: true }
)
.catch((err) => console.log(err))
expect(response.status).toEqual(200)
expect(response.data.cart).toMatchObject({
id: "test-cart-with-sales-channel",
items: [
expect.objectContaining({
cart_id: "test-cart-with-sales-channel",
description: "test variant in sales channel",
title: "test product belonging to a channel",
variant_id: "test-variant-sales-channel",
}),
],
sales_channel_id: "main-sales-channel",
})
})
})
})
+26 -10
View File
@@ -1,13 +1,29 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. # This file is generated by running "yarn install" inside your project.
# yarn lockfile v1 # Manual changes might be lost - proceed with caution!
__metadata:
version: 6
cacheKey: 8c0
"@faker-js/faker@^5.5.3": "@faker-js/faker@npm:^5.5.3":
version "5.5.3" version: 5.5.3
resolved "https://registry.yarnpkg.com/@faker-js/faker/-/faker-5.5.3.tgz#18e3af6b8eae7984072bbeb0c0858474d7c4cefe" resolution: "@faker-js/faker@npm:5.5.3"
integrity sha512-R11tGE6yIFwqpaIqcfkcg7AICXzFg14+5h5v0TfF/9+RMDL6jhzCy/pxHVOfbALGdtVYdt6JdR21tuxEgl34dw== checksum: 3f7fbf0b0cfe23c7750ab79b123be8f845e5f376ec28bf43b7b017983b6fc3a9dc22543c4eea52e30cc119699c0f47f62a2c02e9eae9b6a20b75955e9c3eb887
languageName: node
linkType: hard
dotenv@^10.0.0: "dotenv@npm:^10.0.0":
version "10.0.0" version: 10.0.0
resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-10.0.0.tgz#3d4227b8fb95f81096cdd2b66653fb2c7085ba81" resolution: "dotenv@npm:10.0.0"
integrity sha512-rlBi9d8jpv9Sf1klPjNfFAuWDjKLwTIJJ/VxtoTwIR6hnZxcEOQCZg2oIL3MWBYw5GpUDKOEnND7LXTbIpQ03Q== checksum: 2d8d4ba64bfaff7931402aa5e8cbb8eba0acbc99fe9ae442300199af021079eafa7171ce90e150821a5cb3d74f0057721fbe7ec201a6044b68c8a7615f8c123f
languageName: node
linkType: hard
"integration-tests@workspace:.":
version: 0.0.0-use.local
resolution: "integration-tests@workspace:."
dependencies:
"@faker-js/faker": ^5.5.3
dotenv: ^10.0.0
languageName: unknown
linkType: soft
@@ -1,4 +1,10 @@
import { IsInt, IsObject, IsOptional, IsString } from "class-validator" import {
IsBoolean,
IsInt,
IsObject,
IsOptional,
IsString,
} from "class-validator"
import { MedusaError } from "medusa-core-utils" import { MedusaError } from "medusa-core-utils"
import { EntityManager } from "typeorm" import { EntityManager } from "typeorm"
import { import {
@@ -12,6 +18,7 @@ import {
LineItemService, LineItemService,
} from "../../../../services" } from "../../../../services"
import { validator } from "../../../../utils/validator" import { validator } from "../../../../utils/validator"
import { FlagRouter } from "../../../../utils/flag-router"
/** /**
* @oas [post] /draft-orders/{id}/line-items * @oas [post] /draft-orders/{id}/line-items
* operationId: "PostDraftOrdersDraftOrderLineItems" * operationId: "PostDraftOrdersDraftOrderLineItems"
@@ -92,7 +99,7 @@ export default async (req, res) => {
await cartService await cartService
.withTransaction(manager) .withTransaction(manager)
.addLineItem(draftOrder.cart_id, line) .addLineItem(draftOrder.cart_id, line, { validateSalesChannels: false })
} else { } else {
// custom line items can be added to a draft order // custom line items can be added to a draft order
await lineItemService.withTransaction(manager).create({ await lineItemService.withTransaction(manager).create({
@@ -1,6 +1,7 @@
import { Type } from "class-transformer" import { Type } from "class-transformer"
import { import {
IsArray, IsArray,
IsBoolean,
IsInt, IsInt,
IsNotEmpty, IsNotEmpty,
IsOptional, IsOptional,
@@ -16,6 +17,7 @@ import { CartService, LineItemService, RegionService } from "../../../../service
import { decorateLineItemsWithTotals } from "./decorate-line-items-with-totals" import { decorateLineItemsWithTotals } from "./decorate-line-items-with-totals"
import SalesChannelFeatureFlag from "../../../../loaders/feature-flags/sales-channels"; import SalesChannelFeatureFlag from "../../../../loaders/feature-flags/sales-channels";
import { FeatureFlagDecorators } from "../../../../utils/feature-flag-decorators"; import { FeatureFlagDecorators } from "../../../../utils/feature-flag-decorators";
import { FlagRouter } from "../../../../utils/flag-router"
/** /**
* @oas [post] /carts * @oas [post] /carts
@@ -77,6 +79,7 @@ export default async (req, res) => {
const cartService: CartService = req.scope.resolve("cartService") const cartService: CartService = req.scope.resolve("cartService")
const regionService: RegionService = req.scope.resolve("regionService") const regionService: RegionService = req.scope.resolve("regionService")
const entityManager: EntityManager = req.scope.resolve("manager") const entityManager: EntityManager = req.scope.resolve("manager")
const featureFlagRouter: FlagRouter = req.scope.resolve("featureFlagRouter")
await entityManager.transaction(async (manager) => { await entityManager.transaction(async (manager) => {
let regionId: string let regionId: string
@@ -116,7 +119,10 @@ export default async (req, res) => {
}) })
await cartService await cartService
.withTransaction(manager) .withTransaction(manager)
.addLineItem(cart.id, lineItem) .addLineItem(cart.id, lineItem, {
validateSalesChannels:
featureFlagRouter.isFeatureEnabled("sales_channels"),
})
}) })
) )
} }
@@ -4,6 +4,7 @@ import { defaultStoreCartFields, defaultStoreCartRelations } from "."
import { CartService, LineItemService } from "../../../../services" import { CartService, LineItemService } from "../../../../services"
import { validator } from "../../../../utils/validator" import { validator } from "../../../../utils/validator"
import { decorateLineItemsWithTotals } from "./decorate-line-items-with-totals" import { decorateLineItemsWithTotals } from "./decorate-line-items-with-totals"
import { FlagRouter } from "../../../../utils/flag-router"
/** /**
* @oas [post] /carts/{id}/line-items * @oas [post] /carts/{id}/line-items
@@ -34,10 +35,12 @@ export default async (req, res) => {
const customerId = req.user?.customer_id const customerId = req.user?.customer_id
const validated = await validator(StorePostCartsCartLineItemsReq, req.body) const validated = await validator(StorePostCartsCartLineItemsReq, req.body)
const manager: EntityManager = req.scope.resolve("manager")
const lineItemService: LineItemService = req.scope.resolve("lineItemService") const lineItemService: LineItemService = req.scope.resolve("lineItemService")
const cartService: CartService = req.scope.resolve("cartService") const cartService: CartService = req.scope.resolve("cartService")
const manager: EntityManager = req.scope.resolve("manager")
const featureFlagRouter: FlagRouter = req.scope.resolve("featureFlagRouter")
await manager.transaction(async (m) => { await manager.transaction(async (m) => {
const txCartService = cartService.withTransaction(m) const txCartService = cartService.withTransaction(m)
const cart = await txCartService.retrieve(id) const cart = await txCartService.retrieve(id)
@@ -48,7 +51,11 @@ export default async (req, res) => {
customer_id: customerId || cart.customer_id, customer_id: customerId || cart.customer_id,
metadata: validated.metadata, metadata: validated.metadata,
}) })
await txCartService.addLineItem(id, line)
await txCartService.addLineItem(id, line, {
validateSalesChannels:
featureFlagRouter.isFeatureEnabled("sales_channels"),
})
const updated = await txCartService.retrieve(id, { const updated = await txCartService.retrieve(id, {
relations: ["payment_sessions"], relations: ["payment_sessions"],
@@ -1,12 +1,20 @@
import { Brackets, DeleteResult, EntityRepository, In, Repository } from "typeorm" import {
Brackets,
DeleteResult,
EntityRepository,
In,
Repository,
} from "typeorm"
import { SalesChannel } from "../models" import { SalesChannel } from "../models"
import { ExtendedFindConfig, Selector } from "../types/common"; import { ExtendedFindConfig, Selector } from "../types/common"
@EntityRepository(SalesChannel) @EntityRepository(SalesChannel)
export class SalesChannelRepository extends Repository<SalesChannel> { export class SalesChannelRepository extends Repository<SalesChannel> {
public async getFreeTextSearchResultsAndCount( public async getFreeTextSearchResultsAndCount(
q: string, q: string,
options: ExtendedFindConfig<SalesChannel, Selector<SalesChannel>> = { where: {} }, options: ExtendedFindConfig<SalesChannel, Selector<SalesChannel>> = {
where: {},
}
): Promise<[SalesChannel[], number]> { ): Promise<[SalesChannel[], number]> {
const options_ = { ...options } const options_ = { ...options }
delete options_?.where?.name delete options_?.where?.name
@@ -17,8 +25,9 @@ export class SalesChannelRepository extends Repository<SalesChannel> {
.where(options_.where) .where(options_.where)
.andWhere( .andWhere(
new Brackets((qb) => { new Brackets((qb) => {
qb.where(`sales_channel.description ILIKE :q`, { q: `%${q}%` }) qb.where(`sales_channel.description ILIKE :q`, {
.orWhere(`sales_channel.name ILIKE :q`, { q: `%${q}%` }) q: `%${q}%`,
}).orWhere(`sales_channel.name ILIKE :q`, { q: `%${q}%` })
}) })
) )
.skip(options.skip) .skip(options.skip)
@@ -2,7 +2,7 @@ import { IdMap } from "medusa-test-utils"
import { MedusaError } from "medusa-core-utils" import { MedusaError } from "medusa-core-utils"
export const LineItemAdjustmentServiceMock = { export const LineItemAdjustmentServiceMock = {
withTransaction: function() { withTransaction: function () {
return this return this
}, },
create: jest.fn().mockImplementation((data) => { create: jest.fn().mockImplementation((data) => {
+123 -1
View File
@@ -4,7 +4,7 @@ import { IdMap, MockManager, MockRepository } from "medusa-test-utils"
import CartService from "../cart" import CartService from "../cart"
import { InventoryServiceMock } from "../__mocks__/inventory" import { InventoryServiceMock } from "../__mocks__/inventory"
import { LineItemAdjustmentServiceMock } from "../__mocks__/line-item-adjustment" import { LineItemAdjustmentServiceMock } from "../__mocks__/line-item-adjustment"
import { FlagRouter } from "../../utils/flag-router"; import { FlagRouter } from "../../utils/flag-router"
const eventBusService = { const eventBusService = {
emit: jest.fn(), emit: jest.fn(),
@@ -354,6 +354,13 @@ describe("CartService", () => {
}, },
} }
const productVariantService = {
retrieve: jest.fn(),
withTransaction: function () {
return this
},
}
const inventoryService = { const inventoryService = {
...InventoryServiceMock, ...InventoryServiceMock,
confirmInventory: jest.fn().mockImplementation((variantId, _quantity) => { confirmInventory: jest.fn().mockImplementation((variantId, _quantity) => {
@@ -397,6 +404,7 @@ describe("CartService", () => {
}) })
}, },
}) })
const cartService = new CartService({ const cartService = new CartService({
manager: MockManager, manager: MockManager,
totalsService, totalsService,
@@ -406,6 +414,7 @@ describe("CartService", () => {
eventBusService, eventBusService,
shippingOptionService, shippingOptionService,
inventoryService, inventoryService,
productVariantService,
lineItemAdjustmentService: LineItemAdjustmentServiceMock, lineItemAdjustmentService: LineItemAdjustmentServiceMock,
featureFlagRouter: new FlagRouter({}), featureFlagRouter: new FlagRouter({}),
}) })
@@ -562,6 +571,119 @@ describe("CartService", () => {
}) })
}) })
describe("addLineItem w. SalesChannel", () => {
const lineItemService = {
update: jest.fn(),
create: jest.fn(),
withTransaction: function () {
return this
},
}
const shippingOptionService = {
deleteShippingMethods: jest.fn(),
withTransaction: function () {
return this
},
}
const productVariantService = {
retrieve: jest.fn(),
withTransaction: function () {
return this
},
}
const inventoryService = {
...InventoryServiceMock,
confirmInventory: jest.fn().mockImplementation((variantId, _quantity) => {
if (variantId !== IdMap.getId("cannot-cover")) {
return true
} else {
throw new MedusaError(
MedusaError.Types.NOT_ALLOWED,
`Variant with id: ${variantId} does not have the required inventory`
)
}
}),
}
const cartRepository = MockRepository({
findOneWithRelations: (rels, q) => {
if (q.where.id === IdMap.getId("cartWithLine")) {
return Promise.resolve({
id: IdMap.getId("cartWithLine"),
items: [
{
id: IdMap.getId("merger"),
title: "will merge",
variant_id: IdMap.getId("existing"),
should_merge: true,
quantity: 1,
},
],
})
}
return Promise.resolve({
id: IdMap.getId("emptyCart"),
shipping_methods: [
{
shipping_option: {
profile_id: IdMap.getId("testProfile"),
},
},
],
items: [],
})
},
})
const cartService = new CartService({
manager: MockManager,
totalsService,
cartRepository,
lineItemService,
lineItemRepository: MockRepository(),
eventBusService,
shippingOptionService,
inventoryService,
productVariantService,
lineItemAdjustmentService: LineItemAdjustmentServiceMock,
featureFlagRouter: new FlagRouter({ sales_channels: true }),
})
beforeEach(() => {
jest.clearAllMocks()
})
it("validates if cart and variant's product belong to the same sales channel if flag is passed", async () => {
const validateSpy = jest
.spyOn(cartService, "validateLineItem")
.mockImplementation(() => Promise.resolve(true))
const lineItem = {
title: "New Line",
description: "This is a new line",
thumbnail: "test-img-yeah.com/thumb",
variant_id: IdMap.getId("can-cover"),
unit_price: 123,
quantity: 10,
}
await cartService.addLineItem(IdMap.getId("cartWithLine"), lineItem, {
validateSalesChannels: false,
})
expect(cartService.validateLineItem).not.toHaveBeenCalled()
await cartService.addLineItem(IdMap.getId("cartWithLine"), lineItem)
expect(cartService.validateLineItem).toHaveBeenCalledTimes(1)
validateSpy.mockClear()
})
})
describe("removeLineItem", () => { describe("removeLineItem", () => {
const lineItemService = { const lineItemService = {
delete: jest.fn(), delete: jest.fn(),
+53 -5
View File
@@ -12,7 +12,6 @@ import {
Discount, Discount,
LineItem, LineItem,
ShippingMethod, ShippingMethod,
User,
SalesChannel, SalesChannel,
} from "../models" } from "../models"
import { AddressRepository } from "../repositories/address" import { AddressRepository } from "../repositories/address"
@@ -45,8 +44,8 @@ import TaxProviderService from "./tax-provider"
import TotalsService from "./totals" import TotalsService from "./totals"
import SalesChannelFeatureFlag from "../loaders/feature-flags/sales-channels" import SalesChannelFeatureFlag from "../loaders/feature-flags/sales-channels"
import { FlagRouter } from "../utils/flag-router" import { FlagRouter } from "../utils/flag-router"
import SalesChannelService from "./sales-channel"
import StoreService from "./store" import StoreService from "./store"
import { SalesChannelService } from "./index"
type InjectedDependencies = { type InjectedDependencies = {
manager: EntityManager manager: EntityManager
@@ -101,7 +100,6 @@ class CartService extends TransactionBaseService<CartService> {
protected readonly eventBus_: EventBusService protected readonly eventBus_: EventBusService
protected readonly productVariantService_: ProductVariantService protected readonly productVariantService_: ProductVariantService
protected readonly productService_: ProductService protected readonly productService_: ProductService
protected readonly featureFlagRouter_: FlagRouter
protected readonly storeService_: StoreService protected readonly storeService_: StoreService
protected readonly salesChannelService_: SalesChannelService protected readonly salesChannelService_: SalesChannelService
protected readonly regionService_: RegionService protected readonly regionService_: RegionService
@@ -117,6 +115,7 @@ class CartService extends TransactionBaseService<CartService> {
protected readonly customShippingOptionService_: CustomShippingOptionService protected readonly customShippingOptionService_: CustomShippingOptionService
protected readonly priceSelectionStrategy_: IPriceSelectionStrategy protected readonly priceSelectionStrategy_: IPriceSelectionStrategy
protected readonly lineItemAdjustmentService_: LineItemAdjustmentService protected readonly lineItemAdjustmentService_: LineItemAdjustmentService
protected readonly featureFlagRouter_: FlagRouter
constructor({ constructor({
manager, manager,
@@ -540,7 +539,7 @@ class CartService extends TransactionBaseService<CartService> {
* shipping methods. * shipping methods.
* @param shippingMethods - the set of shipping methods to check from * @param shippingMethods - the set of shipping methods to check from
* @param lineItem - the line item * @param lineItem - the line item
* @return boolean representing wheter shipping method is validated * @return boolean representing whether shipping method is validated
*/ */
protected validateLineItemShipping_( protected validateLineItemShipping_(
shippingMethods: ShippingMethod[], shippingMethods: ShippingMethod[],
@@ -566,13 +565,49 @@ class CartService extends TransactionBaseService<CartService> {
return false return false
} }
/**
* Check if line item's variant belongs to the cart's sales channel.
*
* @param cart - the cart for the line item
* @param lineItem - the line item being added
* @return a boolean indicating validation result
*/
protected async validateLineItem(
cart: Cart,
lineItem: LineItem
): Promise<boolean> {
if (!cart.sales_channel_id) {
return true
}
const lineItemVariant = await this.productVariantService_
.withTransaction(this.manager_)
.retrieve(lineItem.variant_id)
return !!(
await this.productService_
.withTransaction(this.manager_)
.filterProductsBySalesChannel(
[lineItemVariant.product_id],
cart.sales_channel_id
)
).length
}
/** /**
* Adds a line item to the cart. * Adds a line item to the cart.
* @param cartId - the id of the cart that we will add to * @param cartId - the id of the cart that we will add to
* @param lineItem - the line item to add. * @param lineItem - the line item to add.
* @param config
* validateSalesChannels - should check if product belongs to the same sales chanel as cart
* (if cart has associated sales channel)
* @return the result of the update operation * @return the result of the update operation
*/ */
async addLineItem(cartId: string, lineItem: LineItem): Promise<Cart> { async addLineItem(
cartId: string,
lineItem: LineItem,
config = { validateSalesChannels: true }
): Promise<Cart> {
return await this.atomicPhase_( return await this.atomicPhase_(
async (transactionManager: EntityManager) => { async (transactionManager: EntityManager) => {
const cart = await this.retrieve(cartId, { const cart = await this.retrieve(cartId, {
@@ -588,6 +623,17 @@ class CartService extends TransactionBaseService<CartService> {
], ],
}) })
if (this.featureFlagRouter_.isFeatureEnabled("sales_channels")) {
if (config.validateSalesChannels) {
if (!(await this.validateLineItem(cart, lineItem))) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`The product "${lineItem.title}" must belongs to the sales channel on which the cart has been created.`
)
}
}
}
let currentItem: LineItem | undefined let currentItem: LineItem | undefined
if (lineItem.should_merge) { if (lineItem.should_merge) {
currentItem = cart.items.find((item) => { currentItem = cart.items.find((item) => {
@@ -942,8 +988,10 @@ class CartService extends TransactionBaseService<CartService> {
/** /**
* Remove the cart line item that does not belongs to the newly assigned sales channel * Remove the cart line item that does not belongs to the newly assigned sales channel
*
* @param cart - The cart being updated * @param cart - The cart being updated
* @param newSalesChannelId - The new sales channel being assigned to the cart * @param newSalesChannelId - The new sales channel being assigned to the cart
* @return void
* @protected * @protected
*/ */
protected async onSalesChannelChange( protected async onSalesChannelChange(