fix: adjustments based on seb's feedback

- remove eager from shipping_option relationship in `models/custom-shipping-options.ts`
- remove custom_shipping_options relation from cart model
This commit is contained in:
zakariaelas
2021-10-13 17:06:35 +01:00
parent 3d088c351b
commit dba1d5bb69
19 changed files with 424 additions and 139 deletions
@@ -29,7 +29,6 @@ describe("GET /store/shipping-options", () => {
"items",
"items.variant",
"items.variant.product",
"custom_shipping_options",
],
}
)
@@ -37,13 +37,7 @@ export default async (req, res) => {
const cart = await cartService.retrieve(value.cart_id, {
select: ["subtotal"],
relations: [
"region",
"items",
"items.variant",
"items.variant.product",
"custom_shipping_options",
],
relations: ["region", "items", "items.variant", "items.variant.product"],
})
const options = await shippingProfileService.fetchCartOptions(cart)
-9
View File
@@ -44,8 +44,6 @@
* $ref: "#/components/schemas/payment_session"
* payment:
* $ref: "#/components/schemas/payment"
* custom_shipping_options:
* $ref: "#/components/schemas/custom_shipping_option"
* shipping_methods:
* type: array
* items:
@@ -221,13 +219,6 @@ export class Cart {
@JoinColumn({ name: "payment_id" })
payment: Payment
@OneToMany(
() => CustomShippingOption,
method => method.cart,
{ cascade: ["insert"] }
)
custom_shipping_options: CustomShippingOption[]
@OneToMany(
() => ShippingMethod,
method => method.cart,
@@ -29,7 +29,7 @@ export class CustomShippingOption {
@Column()
shipping_option_id: string;
@ManyToOne(() => ShippingOption, { eager: true })
@ManyToOne(() => ShippingOption)
@JoinColumn({ name: "shipping_option_id" })
shipping_option: ShippingOption
@@ -0,0 +1,5 @@
import { EntityRepository, Repository } from "typeorm"
import { CustomShippingOption } from './../models/custom-shipping-option';
@EntityRepository(CustomShippingOption)
export class CustomShippingOptionRepository extends Repository<CustomShippingOption> {}
+30 -36
View File
@@ -1316,13 +1316,10 @@ describe("CartService", () => {
let cartService = new CartService({})
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.findCustomShippingOption(cart, "test-so")
const cartCSO = [
{ id: "cso-test", shipping_option_id: "test-so", price: 20 },
]
const result = cartService.findCustomShippingOption(cartCSO, "test-so")
expect(result).toEqual({
id: "cso-test",
@@ -1332,25 +1329,20 @@ describe("CartService", () => {
})
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: [],
}
const result = cartService.findCustomShippingOption(cart, "test-so")
const cartCSO = []
const result = cartService.findCustomShippingOption(cartCSO, "test-so")
expect(result).toBeUndefined()
})
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: 500 },
],
}
const cartCSO = [
{ id: "cso-test", shipping_option_id: "test-so", price: 500 },
]
expect(() => {
cartService.findCustomShippingOption(cart, "some-other-so")
cartService.findCustomShippingOption(cartCSO, "some-other-so")
}).toThrow(MedusaError)
})
})
@@ -1373,13 +1365,6 @@ describe("CartService", () => {
profile_id: IdMap.getId(m.profile),
},
})),
custom_shipping_options: (config.custom_shipping_options || []).map(
cso => ({
...cso,
id: IdMap.getId(cso.id),
shipping_option_id: IdMap.getId(cso.shipping_option_id),
})
),
discounts: [],
}
}
@@ -1391,11 +1376,7 @@ describe("CartService", () => {
const cart3 = buildCart("lines", {
items: [{ id: "line", profile: "profile1" }],
})
const cartWithCustomSO = buildCart("cart-with-custom-so", {
custom_shipping_options: [
{ id: "cso-test", shipping_option_id: "test-so" },
],
})
const cartWithCustomSO = buildCart("cart-with-custom-so")
const cartRepository = MockRepository({
findOneWithRelations: (rels, q) => {
@@ -1432,6 +1413,20 @@ describe("CartService", () => {
},
}
const customShippingOptionService = {
list: jest.fn().mockImplementation(({ cart_id }) => {
if (cart_id === IdMap.getId("cart-with-custom-so")) {
return [
{
id: "cso-test",
shipping_profile_id: "test-so",
cart_id: IdMap.getId("cart-with-custom-so"),
},
]
}
}),
}
const cartService = new CartService({
manager: MockManager,
totalsService,
@@ -1439,6 +1434,7 @@ describe("CartService", () => {
shippingOptionService,
lineItemService,
eventBusService,
customShippingOptionService,
})
beforeEach(() => {
@@ -1538,11 +1534,9 @@ describe("CartService", () => {
cartService.findCustomShippingOption = jest
.fn()
.mockImplementation(cart => {
if (cart.id === IdMap.getId("cart-with-custom-so")) {
return {
price: 0,
}
.mockImplementation(cartCustomShippingOptions => {
return {
price: 0,
}
})
@@ -0,0 +1,130 @@
import CustomShippingOptionService from "../custom-shipping-option"
import { MockManager, MockRepository, IdMap } from "medusa-test-utils"
describe("CustomShippingOptionService", () => {
describe("list", () => {
const customShippingOptionRepository = MockRepository({
find: q => {
return Promise.resolve([
{
id: "cso-test",
shipping_option_id: "test-so",
price: 0,
cart_id: "test-cso-cart",
},
])
},
})
const customShippingOptionService = new CustomShippingOptionService({
manager: MockManager,
customShippingOptionRepository,
})
beforeAll(async () => {
jest.clearAllMocks()
})
it("calls customShippingOptionRepository find method", async () => {
await customShippingOptionService.list(
{ cart_id: "test-cso-cart" },
{
relations: ["shipping_option"],
}
)
expect(customShippingOptionRepository.find).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.find).toHaveBeenCalledWith({
where: {
cart_id: "test-cso-cart",
},
relations: ["shipping_option"],
})
})
})
describe("retrieve", () => {
const customShippingOptionRepository = MockRepository({
findOne: q => {
if (q.where.id === "cso-test") {
return Promise.resolve({
id: "cso-test",
shipping_option_id: "test-so",
price: 0,
cart_id: "test-cso-cart",
})
}
},
})
const customShippingOptionService = new CustomShippingOptionService({
manager: MockManager,
customShippingOptionRepository,
})
beforeAll(async () => {
jest.clearAllMocks()
})
it("calls customShippingOptionRepository findOne method", async () => {
await customShippingOptionService.retrieve("cso-test", {
relations: ["shipping_option", "cart"],
})
expect(customShippingOptionRepository.findOne).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.findOne).toHaveBeenCalledWith({
where: { id: "cso-test" },
relations: ["shipping_option", "cart"],
})
})
it("fails when custom shipping option is not found", async () => {
expect(customShippingOptionService.retrieve("bad-cso")).rejects.toThrow(
`Custom shipping option with id: bad-cso was not found.`
)
})
})
describe("create", () => {
const customShippingOptionRepository = MockRepository({
create: jest
.fn()
.mockImplementation(f => Promise.resolve({ id: "test-cso", ...f })),
save: jest.fn().mockImplementation(f => Promise.resolve(f)),
})
const customShippingOptionService = new CustomShippingOptionService({
manager: MockManager,
customShippingOptionRepository,
})
beforeAll(async () => {
jest.clearAllMocks()
})
it("calls customShippingOptionRepository create method", async () => {
const customShippingOption = {
cart_id: "test-cso-cart",
shipping_option_id: "test-so",
price: 30,
}
await customShippingOptionService.create(customShippingOption)
expect(customShippingOptionRepository.create).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.create).toHaveBeenCalledWith({
cart_id: "test-cso-cart",
shipping_option_id: "test-so",
price: 30,
metadata: {},
})
expect(customShippingOptionRepository.save).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.save).toHaveBeenCalledWith({
id: "test-cso",
cart_id: "test-cso-cart",
shipping_option_id: "test-so",
price: 30,
metadata: {},
})
})
})
})
@@ -192,43 +192,53 @@ describe("ShippingProfileService", () => {
},
}
const customShippingOptionService = {
list: jest.fn().mockImplementation(({ cart_id }, config) => {
if (cart_id === "cso-cart") {
return Promise.resolve([
{
id: "cso_1",
cart_id: "cso-cart",
shipping_option: {
id: "test-option",
amount: 200,
name: "Test option",
},
price: 0,
},
])
}
return Promise.resolve([])
}),
}
const profileService = new ShippingProfileService({
manager: MockManager,
shippingProfileRepository: profRepo,
shippingOptionService,
customShippingOptionService,
})
beforeEach(() => {
jest.clearAllMocks()
})
it("given a swap cart with custom shipping options, should return correct custom shipping options ", async () => {
it("given a cart with custom shipping options, should return correct custom shipping options ", async () => {
const cart = {
id: "swap-cart",
id: "cso-cart",
type: "swap",
custom_shipping_options: [
{
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: "test-option1", amount: 10 }),
expect.objectContaining({ id: "test-option2", amount: 0 }),
expect.objectContaining({
id: "test-option",
amount: 0,
name: "Test option",
}),
])
})
it("given correct options when cart has no custom shipping options, should return normal shipping options", async () => {
it("given a cart with no custom shipping options, should return normal shipping options", async () => {
const cart = {
items: [
{
@@ -170,6 +170,14 @@ describe("SwapService", () => {
findOneWithRelations: () => Promise.resolve(existing),
})
const customShippingOptionService = {
create: jest.fn().mockReturnValue(Promise.resolve({ id: "cso-test" })),
update: jest.fn().mockReturnValue(Promise.resolve()),
withTransaction: function() {
return this
},
}
const lineItemService = {
create: jest.fn().mockImplementation(d => Promise.resolve(d)),
update: jest.fn().mockImplementation(d => Promise.resolve(d)),
@@ -185,6 +193,7 @@ describe("SwapService", () => {
swapRepository: swapRepo,
cartService,
lineItemService,
customShippingOptionService,
})
it("finds swap and calls return create cart", async () => {
@@ -216,9 +225,6 @@ describe("SwapService", () => {
discounts: testOrder.discounts,
region_id: testOrder.region_id,
customer_id: testOrder.customer_id,
custom_shipping_options: [
{ shipping_option_id: "test-option", price: 10 },
],
type: "swap",
metadata: {
swap_id: IdMap.getId("test-swap"),
+19 -8
View File
@@ -31,6 +31,7 @@ class CartService extends BaseService {
addressRepository,
paymentSessionRepository,
inventoryService,
customShippingOptionService,
}) {
super()
@@ -84,6 +85,9 @@ class CartService extends BaseService {
/** @private @const {InventoryService} */
this.inventoryService_ = inventoryService
/** @private @const {CustomShippingOptionService} */
this.customShippingOptionService_ = customShippingOptionService
}
withTransaction(transactionManager) {
@@ -109,6 +113,7 @@ class CartService extends BaseService {
addressRepository: this.addressRepository_,
giftCardService: this.giftCardService_,
inventoryService: this.inventoryService_,
customShippingOptionService: this.customShippingOptionService_,
})
cloned.transactionManager_ = transactionManager
@@ -1317,11 +1322,17 @@ class CartService extends BaseService {
"items.variant",
"payment_sessions",
"items.variant.product",
"custom_shipping_options",
],
})
let customShippingOption = this.findCustomShippingOption(cart, optionId)
let cartCustomShippingOptions = await this.customShippingOptionService_.list(
{ cart_id: cart.id }
)
let customShippingOption = this.findCustomShippingOption(
cartCustomShippingOptions,
optionId
)
const { shipping_methods } = cart
@@ -1374,17 +1385,17 @@ class CartService extends BaseService {
}
/**
* Finds the cart's custom shipping option based on the passed option id.
* Finds the cart's custom shipping options 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} option - id of the normal or custom shipping option to add as valid method
* @param {Object} cartCustomShippingOptions - the cart's custom shipping options
* @param {string} option - id of the normal or custom shipping option to find in the cartCustomShippingOptions
* @returns {CustomShippingOption | undefined}
*/
findCustomShippingOption(cart, optionId) {
let customOption = cart.custom_shipping_options?.find(
findCustomShippingOption(cartCustomShippingOptions, optionId) {
let customOption = cartCustomShippingOptions?.find(
cso => cso.shipping_option_id === optionId
)
const hasCustomOptions = cart.custom_shipping_options?.length
const hasCustomOptions = cartCustomShippingOptions?.length
if (hasCustomOptions && !customOption) {
throw new MedusaError(
@@ -0,0 +1,112 @@
import { MedusaError } from "medusa-core-utils"
import { BaseService } from "medusa-interfaces"
import _ from "lodash"
class CustomShippingOptionService extends BaseService {
constructor({ manager, customShippingOptionRepository }) {
super()
/** @private @const {EntityManager} */
this.manager_ = manager
/** @private @const {customShippingOptionRepository} */
this.customShippingOptionRepository_ = customShippingOptionRepository
}
/**
* Sets the service's manager to a given transaction manager
* @param {EntityManager} manager - the manager to use
* @return {CustomShippingOptionService} a cloned CustomShippingOption service
*/
withTransaction(manager) {
if (!manager) {
return this
}
const cloned = new CustomShippingOptionService({
manager,
customShippingOptionRepository: this.customShippingOptionRepository_,
})
cloned.transactionManager_ = manager
return cloned
}
/**
* Retrieves a specific shipping option.
* @param {string} id - the id of the custom shipping option to retrieve.
* @param {*} config - any options needed to query for the result.
* @returns {Promise} which resolves to the requested custom shipping option.
*/
async retrieve(id, config = {}) {
const customShippingOptionRepo = this.manager_.getCustomRepository(
this.customShippingOptionRepository_
)
const validatedId = this.validateId_(id)
const query = this.buildQuery_({ id: validatedId }, config)
const customShippingOption = await customShippingOptionRepo.findOne(query)
if (!customShippingOption) {
throw new MedusaError(
MedusaError.Types.NOT_FOUND,
`Custom shipping option with id: ${id} was not found.`
)
}
return customShippingOption
}
/** Fetches all custom shipping options related to the given selector
* @param {Object} selector - the query object for find
* @param {Object} config - the configuration used to find the objects. contains relations, skip, and take.
* @return {Promise<CustomShippingOption[]>} custom shipping options matching the query
*/
async list(
selector,
config = {
skip: 0,
take: 50,
relations: [],
}
) {
const customShippingOptionRepo = this.manager_.getCustomRepository(
this.customShippingOptionRepository_
)
const query = this.buildQuery_(selector, config)
return customShippingOptionRepo.find(query)
}
/**
* Creates a custom shipping option associated with a given author
* @param {object} data - the custom shipping option to create
* @param {*} config - any configurations if needed, including meta data
* @returns {Promise} resolves to the creation result
*/
async create(data, config = { metadata: {} }) {
const { metadata } = config
const { cart_id, shipping_option_id, price } = data
return this.atomicPhase_(async manager => {
const customShippingOptionRepo = manager.getCustomRepository(
this.customShippingOptionRepository_
)
const customShippingOption = await customShippingOptionRepo.create({
cart_id,
shipping_option_id,
price,
metadata,
})
const result = await customShippingOptionRepo.save(customShippingOption)
return result
})
}
}
export default CustomShippingOptionService
@@ -14,6 +14,7 @@ class ShippingProfileService extends BaseService {
productService,
productRepository,
shippingOptionService,
customShippingOptionService,
}) {
super()
@@ -31,6 +32,9 @@ class ShippingProfileService extends BaseService {
/** @private @const {ShippingOptionService} */
this.shippingOptionService_ = shippingOptionService
/** @private @const {CustomShippingOptionService} */
this.customShippingOptionService_ = customShippingOptionService
}
withTransaction(transactionManager) {
@@ -43,6 +47,7 @@ class ShippingProfileService extends BaseService {
shippingProfileRepository: this.shippingProfileRepository_,
productService: this.productService_,
shippingOptionService: this.shippingOptionService_,
customShippingOptionService: this.customShippingOptionService_,
})
cloned.transactionManager_ = transactionManager
@@ -413,8 +418,15 @@ class ShippingProfileService extends BaseService {
* @return {[ShippingOption]} a list of the available shipping options
*/
async fetchCartOptions(cart) {
if (cart.custom_shipping_options?.length) {
return cart.custom_shipping_options.map(cso => ({
const customShippingOptions = await this.customShippingOptionService_.list(
{
cart_id: cart.id,
},
{ relations: ["shipping_option"] }
)
if (customShippingOptions?.length) {
return customShippingOptions.map(cso => ({
...cso.shipping_option,
amount: cso.price,
}))
+15 -4
View File
@@ -32,6 +32,7 @@ class SwapService extends BaseService {
fulfillmentService,
orderService,
inventoryService,
customShippingOptionService,
}) {
super()
@@ -70,6 +71,9 @@ class SwapService extends BaseService {
/** @private @const {EventBusService} */
this.eventBus_ = eventBusService
/** @private @const {CustomShippingOptionService} */
this.customShippingOptionService_ = customShippingOptionService
}
withTransaction(transactionManager) {
@@ -90,6 +94,7 @@ class SwapService extends BaseService {
orderService: this.orderService_,
inventoryService: this.inventoryService_,
fulfillmentService: this.fulfillmentService_,
customShippingOptionService: this.customShippingOptionService_,
})
cloned.transactionManager_ = transactionManager
@@ -568,16 +573,22 @@ class SwapService extends BaseService {
region_id: order.region_id,
customer_id: order.customer_id,
type: "swap",
custom_shipping_options: customShippingOptions.map(so => ({
price: so.price,
shipping_option_id: so.option_id,
})),
metadata: {
swap_id: swap.id,
parent_order_id: order.id,
},
})
for (const customShippingOption of customShippingOptions) {
await this.customShippingOptionService_
.withTransaction(manager)
.create({
cart_id: cart.id,
shipping_option_id: customShippingOption.option_id,
price: customShippingOption.price,
})
}
for (const item of swap.additional_items) {
await this.lineItemService_.withTransaction(manager).update(item.id, {
cart_id: cart.id,