diff --git a/API.order.md b/API.order.md index b51a891..ef11405 100644 --- a/API.order.md +++ b/API.order.md @@ -104,7 +104,7 @@ const shipment = await store.order.createShipment(googleAuthId, 10); *Source: `src/store/order/delete.js`* -Deletes an order by its id within the user's selected company. +Deletes an order by its id within the user's selected company. Refuses to delete the order if any shipment manifest references it. **Parameters** @@ -120,7 +120,7 @@ Deletes an order by its id within the user's selected company. **Throws** -`Error` — If the order is not found. +`Error` — If the order is not found or has shipment manifests. **Example** diff --git a/API.shipping.md b/API.shipping.md index 3b2c6d4..4bd7021 100644 --- a/API.shipping.md +++ b/API.shipping.md @@ -28,7 +28,7 @@ Creates a shipping (package) for a shipment manifest. The number is allocated fr **Throws** -`Error` — If manifest is not found, carrier is not found, manifest is not a shipment, or input is invalid. +`Error` — If manifest is not found, carrier is not found, manifest is not a shipment, shipment is already shipped, or input is invalid. **Example** diff --git a/src/providers/mssql/queries/logistics.js b/src/providers/mssql/queries/logistics.js index 92221fe..0649598 100644 --- a/src/providers/mssql/queries/logistics.js +++ b/src/providers/mssql/queries/logistics.js @@ -97,12 +97,29 @@ OUTPUT INSERTED.id, INSERTED.manifest_id, INSERTED.[number], INSERTED.tracking_c WHERE id = @id; `; +export const CHECK_SHIPMENT_ALREADY_SHIPPED = ` +SELECT TOP 1 1 +FROM manifest m +WHERE m.id = @manifestId + AND ( + EXISTS ( + SELECT 1 FROM shipping s + WHERE s.manifest_id = m.id AND LOWER(s.[state]) = 'shipped' + ) + OR EXISTS ( + SELECT 1 FROM shipmentsummary ss + WHERE ss.manifest_id = m.id AND LOWER(ss.[state]) IN ('delivered', 'returned') + ) + ); +`; + export const CHECK_ORDER_SHIPPED = ` SELECT TOP 1 s.id FROM shipping s WHERE s.manifest_id = @manifestId AND LOWER(s.[state]) = 'shipped'; `; + export const GET_ORDER_LINES_FOR_SHIPPING = ` SELECT id, sku, name, amount FROM manifestarticle diff --git a/src/providers/mssql/queries/shipment.js b/src/providers/mssql/queries/shipment.js index 8011d67..90b56a1 100644 --- a/src/providers/mssql/queries/shipment.js +++ b/src/providers/mssql/queries/shipment.js @@ -51,7 +51,21 @@ FROM shipping WHERE manifest_id = @id AND LOWER([state]) = 'shipped'; `; +export const CHECK_ORDER_HAS_SHIPMENTS = ` +SELECT TOP 1 m.id +FROM manifest m +LEFT JOIN shipmentsummary ss ON ss.manifest_id = m.id AND ss.company_id = @companyId +WHERE m.company_id = @companyId + AND m.[type] = 'shipment' + AND ( + m.[number] LIKE @pattern + OR ss.order_id = @orderId + OR ss.order_number = @orderNumber + ); +`; + export const DELETE_SHIPMENT = ` DELETE FROM manifest WHERE id = @id AND company_id = @companyId AND [type] = 'shipment'; `; + diff --git a/src/providers/sqlite3/queries/logistics.js b/src/providers/sqlite3/queries/logistics.js index 051313a..30f5932 100644 --- a/src/providers/sqlite3/queries/logistics.js +++ b/src/providers/sqlite3/queries/logistics.js @@ -18,6 +18,7 @@ export const { SHIPPING_DELETE, GET_SHIPPING_BY_ID, SHIPPING_UPDATE, + CHECK_SHIPMENT_ALREADY_SHIPPED, CHECK_ORDER_SHIPPED, GET_ORDER_LINES_FOR_SHIPPING, GET_MANIFEST_HEADER_BY_ID, diff --git a/src/providers/sqlite3/queries/shipment.js b/src/providers/sqlite3/queries/shipment.js index 1f8dd91..904f7a7 100644 --- a/src/providers/sqlite3/queries/shipment.js +++ b/src/providers/sqlite3/queries/shipment.js @@ -11,5 +11,7 @@ export const { GET_SHIPPED_AMOUNTS_BY_ORDER_PATTERN, GET_SHIPMENT_HEADER, CHECK_SHIPMENT_PACKAGES_SHIPPED, + CHECK_ORDER_HAS_SHIPMENTS, DELETE_SHIPMENT, } = queries; + diff --git a/src/store/order/delete.js b/src/store/order/delete.js index 4f5d157..e9c486f 100644 --- a/src/store/order/delete.js +++ b/src/store/order/delete.js @@ -1,11 +1,21 @@ import { DELETE_MANIFEST as MSSQL_DELETE, + GET_MANIFEST_HEADER as MSSQL_GET_HEADER, ORDER_TYPE as MSSQL_ORDER_TYPE, } from '../../providers/mssql/queries/manifestDoc.js'; import { DELETE_MANIFEST as SQLITE_DELETE, + GET_MANIFEST_HEADER as SQLITE_GET_HEADER, ORDER_TYPE as SQLITE_ORDER_TYPE, } from '../../providers/sqlite3/queries/manifestDoc.js'; +import { + CHECK_ORDER_HAS_SHIPMENTS as MSSQL_CHECK_ORDER_HAS_SHIPMENTS, + GET_SHIPMENT_PREFIX as MSSQL_GET_SHIPMENT_PREFIX, +} from '../../providers/mssql/queries/shipment.js'; +import { + CHECK_ORDER_HAS_SHIPMENTS as SQLITE_CHECK_ORDER_HAS_SHIPMENTS, + GET_SHIPMENT_PREFIX as SQLITE_GET_SHIPMENT_PREFIX, +} from '../../providers/sqlite3/queries/shipment.js'; import resolveSelectedCompanyAccess from '../companyuser/resolveSelectedCompanyAccess.js'; import makeRunner from '../runner.js'; @@ -13,16 +23,24 @@ export default function del(provider) { const access = resolveSelectedCompanyAccess(provider); const isSqlite = provider?.dialect === 'sqlite3'; const deleteSql = isSqlite ? SQLITE_DELETE : MSSQL_DELETE; + const getHeaderSql = isSqlite ? SQLITE_GET_HEADER : MSSQL_GET_HEADER; + const getShipmentPrefixSql = isSqlite + ? SQLITE_GET_SHIPMENT_PREFIX + : MSSQL_GET_SHIPMENT_PREFIX; + const checkOrderHasShipmentsSql = isSqlite + ? SQLITE_CHECK_ORDER_HAS_SHIPMENTS + : MSSQL_CHECK_ORDER_HAS_SHIPMENTS; const orderType = isSqlite ? SQLITE_ORDER_TYPE : MSSQL_ORDER_TYPE; /** * Deletes an order by its id within the user's selected company. + * Refuses to delete the order if any shipment manifest references it. * * @param {number} googleAuthId - The id of the Google auth user. * @param {number} id - The id of the order to delete. * @param {object} [tx] - Optional transaction. * @returns {Promise} - * @throws {Error} If the order is not found. + * @throws {Error} If the order is not found or has shipment manifests. * * @example * await store.order.delete(42, 5); @@ -32,18 +50,52 @@ export default function del(provider) { throw new Error('Order not found'); } - const runner = makeRunner(provider, tx); - const { companyId } = await access(googleAuthId, runner); + const run = async (runner) => { + const { companyId } = await access(googleAuthId, runner); - const result = await runner.query(deleteSql, { - id, - companyId, - type: orderType, - }); + const headerRes = await runner.query(getHeaderSql, { + id, + companyId, + type: orderType, + }); + const order = headerRes.rows[0]; + if (!order) { + throw new Error('Order not found'); + } - if (!result.rowsAffected) { - throw new Error('Order not found'); + const prefixRes = await runner.query(getShipmentPrefixSql, { companyId }); + const shipmentPrefix = prefixRes.rows[0]?.prefix?.trim?.() || '-L-'; + const pattern = `${order.number}${shipmentPrefix}%`; + + const checkRes = await runner.query(checkOrderHasShipmentsSql, { + companyId, + orderId: id, + orderNumber: order.number, + pattern, + }); + + if (checkRes.rows?.length > 0) { + throw new Error('Order has shipment manifests'); + } + + const result = await runner.query(deleteSql, { + id, + companyId, + type: orderType, + }); + + if (!result.rowsAffected) { + throw new Error('Order not found'); + } + }; + + if (tx) { + return run(makeRunner(provider, tx)); } + if (typeof provider?.transaction === 'function') { + return provider.transaction(run); + } + return run(makeRunner(provider)); }; } diff --git a/src/store/shipping/create.js b/src/store/shipping/create.js index e462d9f..cc9b899 100644 --- a/src/store/shipping/create.js +++ b/src/store/shipping/create.js @@ -40,7 +40,7 @@ export default function create(provider) { * @param {number} [input.warehouseId] - Optional warehouse ID for stock deduction (defaults to company's first warehouse). * @param {object} [tx] - Optional transaction to run within. * @returns {Promise<{id: number, manifestId: number, number: string, trackingCode: string|null, carrierId: number, carrier: string|null, type: string, state: string}>} - * @throws {Error} If manifest is not found, carrier is not found, manifest is not a shipment, or input is invalid. + * @throws {Error} If manifest is not found, carrier is not found, manifest is not a shipment, shipment is already shipped, or input is invalid. * * @example * await store.shipping.create(googleAuthId, { manifestId: 42, carrierId: 1, type: 'parcel', state: 'shipped', trackingCode: '1Z999' }); @@ -79,13 +79,12 @@ export default function create(provider) { throw new Error('Carrier not found'); } - let alreadyShipped = false; - if (isShipped) { - // Check if shipment was already marked shipped by an existing package - const shippedCheck = await runner.query(q.CHECK_ORDER_SHIPPED, { - manifestId, - }); - alreadyShipped = Boolean(shippedCheck.rows[0]); + // Verify related shipment is not already shipped + const shippedCheck = await runner.query(q.CHECK_SHIPMENT_ALREADY_SHIPPED, { + manifestId, + }); + if (shippedCheck.rows[0]) { + throw new Error('Shipment is already shipped'); } // Allocate shipping number and insert shipping record @@ -108,7 +107,7 @@ export default function create(provider) { state, }; - if (isShipped && !alreadyShipped) { + if (isShipped) { // Load shipment manifest lines const linesRes = await runner.query(q.GET_ORDER_LINES_FOR_SHIPPING, { manifestId, diff --git a/test/shipment.test.js b/test/shipment.test.js index 8ec6ee7..8ff3f1e 100644 --- a/test/shipment.test.js +++ b/test/shipment.test.js @@ -192,6 +192,64 @@ describe('store.shipment', () => { }); }); + describe('shipping.create', () => { + it('allows adding shippings while shipment is unshipped, but refuses once shipped', async () => { + const order = await makeOrder(); + const s = await store.shipment.create(gid, { orderId: order.id, lines: [{ sku: 'SH-A', amount: 5 }] }); + const carrier = await store.carrier.create(gid, { name: `Carrier ${order.number}`, hasPackages: true }); + + // First package: draft -> allowed + const p1 = await store.shipping.create(gid, { + manifestId: s.id, + carrierId: carrier.id, + type: 'parcel', + state: 'draft', + }); + assert.ok(p1.id); + + // Second package while still draft -> allowed + const p2 = await store.shipping.create(gid, { + manifestId: s.id, + carrierId: carrier.id, + type: 'parcel', + state: 'draft', + }); + assert.ok(p2.id); + + // Now mark package as shipped -> shipment is now shipped + await store.shipping.update(gid, p1.id, { state: 'shipped' }); + + // Attempting to add a new shipping to the already shipped shipment must fail + await assert.rejects( + () => store.shipping.create(gid, { + manifestId: s.id, + carrierId: carrier.id, + type: 'parcel', + state: 'draft', + }), + { message: 'Shipment is already shipped' }, + ); + }); + + it('refuses adding shipping if shipment is in delivered state', async () => { + const order = await makeOrder(); + const s = await store.shipment.create(gid, { orderId: order.id, lines: [{ sku: 'SH-A', amount: 2 }] }); + const carrier = await store.carrier.create(gid, { name: `Carrier Deliv ${order.number}`, hasPackages: true }); + + await store.shipment.update(gid, s.id, { state: 'delivered' }); + + await assert.rejects( + () => store.shipping.create(gid, { + manifestId: s.id, + carrierId: carrier.id, + type: 'parcel', + state: 'draft', + }), + { message: 'Shipment is already shipped' }, + ); + }); + }); + describe('update', () => { it('changes the state and validates it', async () => { const order = await makeOrder(); @@ -242,8 +300,24 @@ describe('store.shipment', () => { await assert.rejects(() => store.shipment.delete(gid, null), { message: 'Shipment is required' }); assert.ok(await store.shipment.getById(gid, s.id)); }); + + it('refuses to delete an order manifest when there is a shipment manifest referencing it', async () => { + const order = await makeOrder(); + const s = await store.shipment.create(gid, { orderId: order.id, lines: [{ sku: 'SH-A', amount: 1 }] }); + + await assert.rejects( + () => store.order.delete(gid, order.id), + { message: 'Order has shipment manifests' }, + ); + + // Once all shipment manifests referencing the order are deleted, the order can be deleted + await store.shipment.delete(gid, s.id); + await store.order.delete(gid, order.id); + await assert.rejects(() => store.order.getById(gid, order.id), { message: 'Order not found' }); + }); }); + describe('list / rebuildSummaries', () => { let lt; let o1;