From cbe2b7f687923aecdf34db31726c048ef1f8db21 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Thu, 11 Aug 2022 22:18:11 +0200 Subject: [PATCH] chore(medusa): Remove intepestive services re instanciation in loop (#2036) * chore(medusa): Renove intepestive services re instanciation in loop * test(medusa): Fix missing deps * fix(medusa): Missing await --- .../medusa/src/services/__tests__/order.js | 2 + .../src/services/__tests__/price-list.js | 2 + packages/medusa/src/services/cart.ts | 20 ++- packages/medusa/src/services/claim.ts | 155 +++++++++--------- packages/medusa/src/services/draft-order.ts | 64 ++++---- packages/medusa/src/services/fulfillment.ts | 6 +- packages/medusa/src/services/order.ts | 65 ++++---- packages/medusa/src/services/price-list.ts | 5 +- packages/medusa/src/services/product.ts | 10 +- packages/medusa/src/services/return.js | 15 +- .../medusa/src/services/shipping-profile.ts | 13 +- packages/medusa/src/services/swap.js | 76 +++++---- 12 files changed, 240 insertions(+), 193 deletions(-) diff --git a/packages/medusa/src/services/__tests__/order.js b/packages/medusa/src/services/__tests__/order.js index 77d4a9d86b..6198601231 100644 --- a/packages/medusa/src/services/__tests__/order.js +++ b/packages/medusa/src/services/__tests__/order.js @@ -1,6 +1,7 @@ import { IdMap, MockManager, MockRepository } from "medusa-test-utils" import OrderService from "../order" import { InventoryServiceMock } from "../__mocks__/inventory" +import { LineItemServiceMock } from "../__mocks__/line-item"; describe("OrderService", () => { const totalsService = { @@ -520,6 +521,7 @@ describe("OrderService", () => { manager: MockManager, orderRepository: orderRepo, eventBusService, + lineItemService: LineItemServiceMock }) beforeEach(async () => { diff --git a/packages/medusa/src/services/__tests__/price-list.js b/packages/medusa/src/services/__tests__/price-list.js index d794530f4f..588085e003 100644 --- a/packages/medusa/src/services/__tests__/price-list.js +++ b/packages/medusa/src/services/__tests__/price-list.js @@ -2,6 +2,7 @@ import { MedusaError } from "medusa-core-utils" import { IdMap, MockManager, MockRepository } from "medusa-test-utils" import PriceListService from "../price-list" import { MoneyAmountRepository } from "../../repositories/money-amount" +import { RegionServiceMock } from "../__mocks__/region"; const priceListRepository = MockRepository({ findOne: (q) => { @@ -129,6 +130,7 @@ describe("PriceListService", () => { customerGroupService, priceListRepository, moneyAmountRepository: updateRelatedMoneyAmountRepository, + regionService: RegionServiceMock }) it("update only existing price lists and related money amount", async () => { diff --git a/packages/medusa/src/services/cart.ts b/packages/medusa/src/services/cart.ts index 96f2c06610..ed5e142631 100644 --- a/packages/medusa/src/services/cart.ts +++ b/packages/medusa/src/services/cart.ts @@ -1736,14 +1736,17 @@ class CartService extends TransactionBaseService { const methods = [newShippingMethod] if (shipping_methods?.length) { + const shippingOptionServiceTx = + this.shippingOptionService_.withTransaction(transactionManager) + for (const shippingMethod of shipping_methods) { if ( shippingMethod.shipping_option.profile_id === newShippingMethod.shipping_option.profile_id ) { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .deleteShippingMethods(shippingMethod) + await shippingOptionServiceTx.deleteShippingMethods( + shippingMethod + ) } else { methods.push(shippingMethod) } @@ -1751,13 +1754,14 @@ class CartService extends TransactionBaseService { } if (cart.items?.length) { + const lineItemServiceTx = + this.lineItemService_.withTransaction(transactionManager) + await Promise.all( cart.items.map(async (item) => { - return this.lineItemService_ - .withTransaction(transactionManager) - .update(item.id, { - has_shipping: this.validateLineItemShipping_(methods, item), - }) + return lineItemServiceTx.update(item.id, { + has_shipping: this.validateLineItemShipping_(methods, item), + }) }) ) } diff --git a/packages/medusa/src/services/claim.ts b/packages/medusa/src/services/claim.ts index b68ac78209..1253d8acb9 100644 --- a/packages/medusa/src/services/claim.ts +++ b/packages/medusa/src/services/claim.ts @@ -150,32 +150,29 @@ export default class ClaimService extends TransactionBaseService< } if (shipping_methods) { + const shippingOptionServiceTx = + this.shippingOptionService_.withTransaction(transactionManager) + for (const m of claim.shipping_methods) { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .updateShippingMethod(m.id, { - claim_order_id: null, - }) + await shippingOptionServiceTx.updateShippingMethod(m.id, { + claim_order_id: null, + }) } for (const method of shipping_methods) { if (method.id) { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .updateShippingMethod(method.id, { - claim_order_id: claim.id, - }) + await shippingOptionServiceTx.updateShippingMethod(method.id, { + claim_order_id: claim.id, + }) } else { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .createShippingMethod( - method.option_id as string, - (method as any).data, - { - claim_order_id: claim.id, - price: method.price, - } - ) + await shippingOptionServiceTx.createShippingMethod( + method.option_id as string, + (method as any).data, + { + claim_order_id: claim.id, + price: method.price, + } + ) } } } @@ -186,11 +183,12 @@ export default class ClaimService extends TransactionBaseService< } if (claim_items) { + const claimItemServiceTx = + this.claimItemService_.withTransaction(transactionManager) + for (const i of claim_items) { if (i.id) { - await this.claimItemService_ - .withTransaction(transactionManager) - .update(i.id, i) + await claimItemServiceTx.update(i.id, i) } } } @@ -235,12 +233,13 @@ export default class ClaimService extends TransactionBaseService< ...rest } = data + const lineItemServiceTx = + this.lineItemService_.withTransaction(transactionManager) + for (const item of claim_items) { - const line = await this.lineItemService_ - .withTransaction(transactionManager) - .retrieve(item.item_id, { - relations: ["order", "swap", "claim_order", "tax_lines"], - }) + const line = await lineItemServiceTx.retrieve(item.item_id, { + relations: ["order", "swap", "claim_order", "tax_lines"], + }) if ( line.order?.canceled_at || @@ -337,24 +336,31 @@ export default class ClaimService extends TransactionBaseService< let newItems: LineItem[] = [] if (isDefined(additional_items)) { + const inventoryServiceTx = + this.inventoryService_.withTransaction(transactionManager) + for (const item of additional_items) { - await this.inventoryService_ - .withTransaction(transactionManager) - .confirmInventory(item.variant_id, item.quantity) + await inventoryServiceTx.confirmInventory( + item.variant_id, + item.quantity + ) } newItems = await Promise.all( additional_items.map((i) => - this.lineItemService_ - .withTransaction(transactionManager) - .generate(i.variant_id, order.region_id, i.quantity) + lineItemServiceTx.generate( + i.variant_id, + order.region_id, + i.quantity + ) ) ) for (const newItem of newItems) { - await this.inventoryService_ - .withTransaction(transactionManager) - .adjustInventory(newItem.variant_id, -newItem.quantity) + await inventoryServiceTx.adjustInventory( + newItem.variant_id, + -newItem.quantity + ) } } @@ -378,46 +384,44 @@ export default class ClaimService extends TransactionBaseService< if (result.additional_items && result.additional_items.length) { const calcContext = this.totalsService_.getCalculationContext(order) - const lineItems = await this.lineItemService_ - .withTransaction(transactionManager) - .list({ - id: result.additional_items.map((i) => i.id), - }) + const lineItems = await lineItemServiceTx.list({ + id: result.additional_items.map((i) => i.id), + }) await this.taxProviderService_ .withTransaction(transactionManager) .createTaxLines(lineItems, calcContext) } if (shipping_methods) { + const shippingOptionServiceTx = + this.shippingOptionService_.withTransaction(transactionManager) + for (const method of shipping_methods) { if (method.id) { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .updateShippingMethod(method.id, { - claim_order_id: result.id, - }) + await shippingOptionServiceTx.updateShippingMethod(method.id, { + claim_order_id: result.id, + }) } else { - await this.shippingOptionService_ - .withTransaction(transactionManager) - .createShippingMethod( - method.option_id as string, - (method as any).data, - { - claim_order_id: result.id, - price: method.price, - } - ) + await shippingOptionServiceTx.createShippingMethod( + method.option_id as string, + (method as any).data, + { + claim_order_id: result.id, + price: method.price, + } + ) } } } + const claimItemServiceTx = + this.claimItemService_.withTransaction(transactionManager) + for (const ci of claim_items) { - await this.claimItemService_ - .withTransaction(transactionManager) - .create({ - ...ci, - claim_order_id: result.id, - }) + await claimItemServiceTx.create({ + ...ci, + claim_order_id: result.id, + }) } if (return_shipping) { @@ -584,14 +588,14 @@ export default class ClaimService extends TransactionBaseService< ) const claimOrder = await claimRepo.save(claim) + const eventBusTx = this.eventBus_.withTransaction(transactionManager) + for (const fulfillment of fulfillments) { - await this.eventBus_ - .withTransaction(transactionManager) - .emit(ClaimService.Events.FULFILLMENT_CREATED, { - id: id, - fulfillment_id: fulfillment.id, - no_notification: claim.no_notification, - }) + await eventBusTx.emit(ClaimService.Events.FULFILLMENT_CREATED, { + id: id, + fulfillment_id: fulfillment.id, + no_notification: claim.no_notification, + }) } return claimOrder @@ -708,6 +712,9 @@ export default class ClaimService extends TransactionBaseService< claim.fulfillment_status = ClaimFulfillmentStatus.SHIPPED + const lineItemServiceTx = + this.lineItemService_.withTransaction(transactionManager) + for (const additionalItem of claim.additional_items) { const shipped = shipment.items.find( (si) => si.item_id === additionalItem.id @@ -715,11 +722,9 @@ export default class ClaimService extends TransactionBaseService< if (shipped) { const shippedQty = (additionalItem.shipped_quantity || 0) + shipped.quantity - await this.lineItemService_ - .withTransaction(transactionManager) - .update(additionalItem.id, { - shipped_quantity: shippedQty, - }) + await lineItemServiceTx.update(additionalItem.id, { + shipped_quantity: shippedQty, + }) if (shippedQty !== additionalItem.quantity) { claim.fulfillment_status = diff --git a/packages/medusa/src/services/draft-order.ts b/packages/medusa/src/services/draft-order.ts index cffb387a0b..e519aab001 100644 --- a/packages/medusa/src/services/draft-order.ts +++ b/packages/medusa/src/services/draft-order.ts @@ -266,20 +266,22 @@ class DraftOrderService extends TransactionBaseService { const { shipping_methods, no_notification_order, items, ...rawCart } = data + const cartServiceTx = + this.cartService_.withTransaction(transactionManager) + if (rawCart.discounts) { const { discounts } = rawCart rawCart.discounts = [] for (const { code } of discounts) { - await this.cartService_ - .withTransaction(transactionManager) - .applyDiscount(rawCart as Cart, code) + await cartServiceTx.applyDiscount(rawCart as Cart, code) } } - const createdCart = await this.cartService_ - .withTransaction(transactionManager) - .create({ type: CartType.DRAFT_ORDER, ...rawCart }) + const createdCart = await cartServiceTx.create({ + type: CartType.DRAFT_ORDER, + ...rawCart, + }) const draftOrder = draftOrderRepo.create({ cart_id: createdCart.id, @@ -293,22 +295,26 @@ class DraftOrderService extends TransactionBaseService { id: result.id, }) + const lineItemServiceTx = + this.lineItemService_.withTransaction(transactionManager) + for (const item of items) { if (item.variant_id) { - const line = await this.lineItemService_ - .withTransaction(transactionManager) - .generate(item.variant_id, data.region_id, item.quantity, { + const line = await lineItemServiceTx.generate( + item.variant_id, + data.region_id, + item.quantity, + { metadata: item?.metadata || {}, unit_price: item.unit_price, cart: createdCart, - }) + } + ) - await this.lineItemService_ - .withTransaction(transactionManager) - .create({ - ...line, - cart_id: createdCart.id, - }) + await lineItemServiceTx.create({ + ...line, + cart_id: createdCart.id, + }) } else { let price if (typeof item.unit_price === `undefined` || item.unit_price < 0) { @@ -318,23 +324,23 @@ class DraftOrderService extends TransactionBaseService { } // custom line items can be added to a draft order - await this.lineItemService_ - .withTransaction(transactionManager) - .create({ - cart_id: createdCart.id, - has_shipping: true, - title: item.title || "Custom item", - allow_discounts: false, - unit_price: price, - quantity: item.quantity, - }) + await lineItemServiceTx.create({ + cart_id: createdCart.id, + has_shipping: true, + title: item.title || "Custom item", + allow_discounts: false, + unit_price: price, + quantity: item.quantity, + }) } } for (const method of shipping_methods) { - await this.cartService_ - .withTransaction(transactionManager) - .addShippingMethod(createdCart.id, method.option_id, method.data) + await cartServiceTx.addShippingMethod( + createdCart.id, + method.option_id, + method.data + ) } return result diff --git a/packages/medusa/src/services/fulfillment.ts b/packages/medusa/src/services/fulfillment.ts index 9a116b595f..89a97b770d 100644 --- a/packages/medusa/src/services/fulfillment.ts +++ b/packages/medusa/src/services/fulfillment.ts @@ -274,12 +274,12 @@ class FulfillmentService extends TransactionBaseService { fulfillment.canceled_at = new Date() - const lineItemService = this.lineItemService_.withTransaction(manager) + const lineItemServiceTx = this.lineItemService_.withTransaction(manager) for (const fItem of fulfillment.items) { - const item = await lineItemService.retrieve(fItem.item_id) + const item = await lineItemServiceTx.retrieve(fItem.item_id) const fulfilledQuantity = item.fulfilled_quantity - fItem.quantity - await lineItemService.update(item.id, { + await lineItemServiceTx.update(item.id, { fulfilled_quantity: fulfilledQuantity, }) } diff --git a/packages/medusa/src/services/order.ts b/packages/medusa/src/services/order.ts index f99105e0bc..93115a1b9a 100644 --- a/packages/medusa/src/services/order.ts +++ b/packages/medusa/src/services/order.ts @@ -479,10 +479,10 @@ class OrderService extends TransactionBaseService { */ async createFromCart(cartId: string): Promise { return await this.atomicPhase_(async (manager) => { - const cartService = this.cartService_.withTransaction(manager) - const inventoryService = this.inventoryService_.withTransaction(manager) + const cartServiceTx = this.cartService_.withTransaction(manager) + const inventoryServiceTx = this.inventoryService_.withTransaction(manager) - const cart = await cartService.retrieve(cartId, { + const cart = await cartServiceTx.retrieve(cartId, { select: ["subtotal", "total"], relations: [ "region", @@ -506,7 +506,7 @@ class OrderService extends TransactionBaseService { for (const item of cart.items) { try { - await inventoryService.confirmInventory( + await inventoryServiceTx.confirmInventory( item.variant_id, item.quantity ) @@ -516,7 +516,7 @@ class OrderService extends TransactionBaseService { .withTransaction(manager) .cancelPayment(payment) } - await cartService.update(cart.id, { payment_authorized_at: null }) + await cartServiceTx.update(cart.id, { payment_authorized_at: null }) throw err } } @@ -618,13 +618,16 @@ class OrderService extends TransactionBaseService { .updateShippingMethod(method.id, { order_id: result.id }) } - const lineItemService = this.lineItemService_.withTransaction(manager) + const lineItemServiceTx = this.lineItemService_.withTransaction(manager) for (const item of cart.items) { - await lineItemService.update(item.id, { order_id: result.id }) + await lineItemServiceTx.update(item.id, { order_id: result.id }) } for (const item of cart.items) { - await inventoryService.adjustInventory(item.variant_id, -item.quantity) + await inventoryServiceTx.adjustInventory( + item.variant_id, + -item.quantity + ) } await this.eventBus_ @@ -634,7 +637,7 @@ class OrderService extends TransactionBaseService { no_notification: result.no_notification, }) - await cartService.update(cart.id, { completed_at: new Date() }) + await cartServiceTx.update(cart.id, { completed_at: new Date() }) return result }) @@ -697,6 +700,8 @@ class OrderService extends TransactionBaseService { no_notification: evaluatedNoNotification, }) + const lineItemServiceTx = this.lineItemService_.withTransaction(manager) + order.fulfillment_status = FulfillmentStatus.SHIPPED for (const item of order.items) { const shipped = shipmentRes.items.find((si) => si.item_id === item.id) @@ -706,7 +711,7 @@ class OrderService extends TransactionBaseService { order.fulfillment_status = FulfillmentStatus.PARTIALLY_SHIPPED } - await this.lineItemService_.withTransaction(manager).update(item.id, { + await lineItemServiceTx.update(item.id, { shipped_quantity: shippedQty, }) } else { @@ -839,6 +844,9 @@ class OrderService extends TransactionBaseService { .withTransaction(manager) .createShippingMethod(optionId, data ?? {}, { order, ...config }) + const shippingOptionServiceTx = + this.shippingOptionService_.withTransaction(manager) + const methods = [newMethod] if (shipping_methods.length) { for (const sm of shipping_methods) { @@ -846,9 +854,7 @@ class OrderService extends TransactionBaseService { sm.shipping_option.profile_id === newMethod.shipping_option.profile_id ) { - await this.shippingOptionService_ - .withTransaction(manager) - .deleteShippingMethods(sm) + await shippingOptionServiceTx.deleteShippingMethods(sm) } else { methods.push(sm) } @@ -927,9 +933,10 @@ class OrderService extends TransactionBaseService { order.no_notification = no_notification ?? false } + const lineItemServiceTx = this.lineItemService_.withTransaction(manager) if (update.items) { for (const item of items as LineItem[]) { - await this.lineItemService_.withTransaction(manager).create({ + await lineItemServiceTx.create({ ...item, order_id: orderId, }) @@ -1004,16 +1011,15 @@ class OrderService extends TransactionBaseService { throwErrorIf(order.swaps, notCanceled, "swaps") throwErrorIf(order.claims, notCanceled, "claims") + const inventoryServiceTx = this.inventoryService_.withTransaction(manager) for (const item of order.items) { - await this.inventoryService_ - .withTransaction(manager) - .adjustInventory(item.variant_id, item.quantity) + await inventoryServiceTx.adjustInventory(item.variant_id, item.quantity) } + const paymentProviderServiceTx = + this.paymentProviderService_.withTransaction(manager) for (const p of order.payments) { - await this.paymentProviderService_ - .withTransaction(manager) - .cancelPayment(p) + await paymentProviderServiceTx.cancelPayment(p) } order.status = OrderStatus.CANCELED @@ -1051,11 +1057,13 @@ class OrderService extends TransactionBaseService { ) } + const paymentProviderServiceTx = + this.paymentProviderService_.withTransaction(manager) + const payments: Payment[] = [] for (const p of order.payments) { if (p.captured_at === null) { - const result = await this.paymentProviderService_ - .withTransaction(manager) + const result = await paymentProviderServiceTx .capturePayment(p) .catch((err) => { this.eventBus_ @@ -1251,14 +1259,13 @@ class OrderService extends TransactionBaseService { const evaluatedNoNotification = no_notification !== undefined ? no_notification : order.no_notification + const eventBusTx = this.eventBus_.withTransaction(manager) for (const fulfillment of fulfillments) { - await this.eventBus_ - .withTransaction(manager) - .emit(OrderService.Events.FULFILLMENT_CREATED, { - id: orderId, - fulfillment_id: fulfillment.id, - no_notification: evaluatedNoNotification, - }) + await eventBusTx.emit(OrderService.Events.FULFILLMENT_CREATED, { + id: orderId, + fulfillment_id: fulfillment.id, + no_notification: evaluatedNoNotification, + }) } return result diff --git a/packages/medusa/src/services/price-list.ts b/packages/medusa/src/services/price-list.ts index bac06de9c3..f005c0c1fb 100644 --- a/packages/medusa/src/services/price-list.ts +++ b/packages/medusa/src/services/price-list.ts @@ -476,11 +476,10 @@ class PriceListService extends TransactionBaseService { >(prices: T[]): Promise { const prices_: typeof prices = [] + const regionServiceTx = this.regionService_.withTransaction(this.manager_) for (const p of prices) { if (p.region_id) { - const region = await this.regionService_ - .withTransaction(this.manager_) - .retrieve(p.region_id) + const region = await regionServiceTx.retrieve(p.region_id) p.currency_code = region.currency_code } diff --git a/packages/medusa/src/services/product.ts b/packages/medusa/src/services/product.ts index 3369949387..7346a4579d 100644 --- a/packages/medusa/src/services/product.ts +++ b/packages/medusa/src/services/product.ts @@ -658,10 +658,14 @@ class ProductService extends TransactionBaseService< await productOptionRepo.save(option) + const productVariantServiceTx = + this.productVariantService_.withTransaction(manager) for (const variant of product.variants) { - this.productVariantService_ - .withTransaction(manager) - .addOptionValue(variant.id, option.id, "Default Value") + await productVariantServiceTx.addOptionValue( + variant.id, + option.id, + "Default Value" + ) } const result = await this.retrieve(productId) diff --git a/packages/medusa/src/services/return.js b/packages/medusa/src/services/return.js index 5de7d227a8..24ca142212 100644 --- a/packages/medusa/src/services/return.js +++ b/packages/medusa/src/services/return.js @@ -625,22 +625,23 @@ class ReturnService extends BaseService { const result = await returnRepository.save(updateObj) + const lineItemServiceTx = this.lineItemService_.withTransaction(manager) for (const i of returnObj.items) { - const lineItem = await this.lineItemService_ - .withTransaction(manager) - .retrieve(i.item_id) + const lineItem = await lineItemServiceTx.retrieve(i.item_id) const returnedQuantity = (lineItem.returned_quantity || 0) + i.quantity - await this.lineItemService_.withTransaction(manager).update(i.item_id, { + await lineItemServiceTx.update(i.item_id, { returned_quantity: returnedQuantity, }) } + const inventoryServiceTx = this.inventoryService_.withTransaction(manager) for (const line of newLines) { const orderItem = order.items.find((i) => i.id === line.item_id) if (orderItem) { - await this.inventoryService_ - .withTransaction(manager) - .adjustInventory(orderItem.variant_id, line.received_quantity) + await inventoryServiceTx.adjustInventory( + orderItem.variant_id, + line.received_quantity + ) } } diff --git a/packages/medusa/src/services/shipping-profile.ts b/packages/medusa/src/services/shipping-profile.ts index 5d4a8cb15c..4ee1b8e3ba 100644 --- a/packages/medusa/src/services/shipping-profile.ts +++ b/packages/medusa/src/services/shipping-profile.ts @@ -297,20 +297,21 @@ class ShippingProfileService extends TransactionBaseService si.item_id === i.id) if (shipped) { const shippedQty = (i.shipped_quantity || 0) + shipped.quantity - await this.lineItemService_.withTransaction(manager).update(i.id, { + await lineItemServiceTx.update(i.id, { shipped_quantity: shippedQty, })