feat(medusa, inventory): Inventory Management module #2956
Conversation
|
| /** | ||
| * Deletes a reservation item by line item id. | ||
| * @param {string} lineItemId - the id of the line item to delete. | ||
| * @returns {Promise<void>} - an empty promise | ||
| */ | ||
| async deleteByLineItem(lineItemId: string): Promise<void> { | ||
| await this.atomicPhase_(async (manager) => { | ||
| const itemRepository = manager.getRepository(ReservationItem) | ||
|
|
||
| const items = await this.list({ line_item_id: lineItemId }) | ||
|
|
||
| const ops: Promise<unknown>[] = [] | ||
| for (const item of items) { | ||
| ops.push(itemRepository.softRemove({ line_item_id: lineItemId })) | ||
| ops.push( | ||
| this.inventoryLevelService_ | ||
| .withTransaction(manager) | ||
| .adjustReservedQuantity( | ||
| item.inventory_item_id, | ||
| item.location_id, | ||
| item.quantity * -1 | ||
| ) | ||
| ) | ||
| } | ||
| await Promise.all(ops) | ||
| }) | ||
|
|
||
| await this.eventBusService_.emit( | ||
| ReservationItemService.Events.DELETED_BY_LINE_ITEM, | ||
| { | ||
| line_item_id: lineItemId, | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
thought (non-blocking): wonder if the existence of a line_item_id is a bad pattern as it couples the InventoryService with the design of the core.
It could be argued that a cleaner approach would be to have a LineItemReservations table in the core with:
line_item_id: id of a cart/order line itemreservation_item_id: id of a reservation in the inventory system
It is not a big deal for me, but would like to hear your opinion. (cc: @pKorsholm)
There was a problem hiding this comment.
In this case to me makes more sense the inventory service managing reservations and holding the external relation (line_item_id).
Having a LineItemReservation would transfer the responsibility of managing reservations to the line item, which is somewhat duplicating the business logic.
There was a problem hiding this comment.
Makes sense - I was thinking of a scenario where a user integrates a third party IMS tool through our interfaces. With the current implementation, we assume that the third party tool's representation of a reservation/allocation is able to hold a field to represent the line_item_id. This was the assumption I wanted to challenge, but I think it is fair to go on as is and not delve too much on this right now :)
olivermrbl
changed the title
olivermrbl
changed the title
packages/inventory/src/migrations/schema-migrations/1665748086258-inventory_setup.ts
Outdated
Show resolved
Hide resolved
| export const service = InventoryService | ||
| export const migrations = [SchemaMigration] | ||
| export const loaders = [ConnectionLoader] |
There was a problem hiding this comment.
Suggestion(non-blocking): maybe for a different pr, I talked to @adrien2p and he suggested creating a defined shape for module exports.
There was a problem hiding this comment.
I'm thinking we could add it in a follow up pr 😄 no need to do anything about it here
There was a problem hiding this comment.
I agree with that, and we also will probably need to expand it a bit to optionaly integrate the module to the core while loading it.
What:
Module to manage inventory items in multiple locations
FIXES: CORE-921