diff --git a/api/src/database/migrations/supplier-email-index.js b/api/src/database/migrations/supplier-email-index.js index 7e20c1bb2..a73c3e5eb 100644 --- a/api/src/database/migrations/supplier-email-index.js +++ b/api/src/database/migrations/supplier-email-index.js @@ -6,7 +6,7 @@ const options = { partialFilterExpression: { email: { $type: 'string', $gt: '' } }, }; -async function migrateContactEmailIndex(db, name, indexOptions) { +async function migrateContactEmailIndex(db, name, indexOptions, keys = { email: 1 }) { if (!(await db.listCollections({ name }, { nameOnly: true }).hasNext())) return; const collection = db.collection(name); const indexes = await collection.listIndexes().toArray(); @@ -14,7 +14,7 @@ async function migrateContactEmailIndex(db, name, indexOptions) { // a new uniqueness rule fit historical data. A duplicate is a deferred // migration, not a reason to take the entire till offline. try { - await collection.createIndex({ email: 1 }, indexOptions); + await collection.createIndex(keys, indexOptions); } catch (error) { if (error?.code !== 11000) throw error; console.warn( @@ -36,7 +36,16 @@ async function migrateContactEmailIndex(db, name, indexOptions) { } } -const migrateSupplierEmailIndex = (db) => migrateContactEmailIndex(db, 'suppliers', options); +const migrateSupplierEmailIndex = (db) => + migrateContactEmailIndex( + db, + 'suppliers', + { + ...options, + name: 'supplier_branch_nonempty_email', + }, + { license: 1, branch_id: 1, email: 1 } + ); const migrateCustomerEmailIndex = (db) => migrateContactEmailIndex(db, 'customers', { ...options, diff --git a/api/src/repositories/supplier.repository.js b/api/src/repositories/supplier.repository.js index 23c5efa64..63cc0e4db 100644 --- a/api/src/repositories/supplier.repository.js +++ b/api/src/repositories/supplier.repository.js @@ -72,11 +72,12 @@ class SupplierRepository extends BaseModel { /** * Find supplier by email */ - async findByEmail(email) { + async findByEmail(email, branchId = BaseModel.currentBranch) { const collection = await this.getCollection(this.collectionName); return await collection.findOne({ email, license: BaseModel.license, + branch_id: branchId, is_deleted: { $ne: true }, }); } @@ -228,6 +229,9 @@ class SupplierRepository extends BaseModel { throw new Error('Supplier not found'); } + const backup = await BaseModel.deletedDocumentBackup(this.collectionName, supplier); + if (!backup?.status) throw new Error(backup?.message || 'Supplier backup failed'); + // Backup to recycle bin collection const recycleCollection = await this.getCollection(this.collectionName + '_recycle_bin'); await recycleCollection.insertOne({ @@ -264,6 +268,13 @@ class SupplierRepository extends BaseModel { return { deletedCount: 0 }; } + // The common recycle record is also the deletion event consumed by sync. + // Keep the legacy backup below for existing restore screens. + for (const supplier of suppliers) { + const backup = await BaseModel.deletedDocumentBackup(this.collectionName, supplier); + if (!backup?.status) throw new Error(backup?.message || 'Supplier backup failed'); + } + // Backup to recycle bin collection const recycleCollection = await this.getCollection(this.collectionName + '_recycle_bin'); const backupDocs = suppliers.map((supplier) => ({ diff --git a/api/src/services/extension-namespace.js b/api/src/services/extension-namespace.js index 6eb6bc38d..f03bd8b6a 100644 --- a/api/src/services/extension-namespace.js +++ b/api/src/services/extension-namespace.js @@ -185,7 +185,29 @@ async function executeNamespace(db, scope, descriptor, actor, input, dependencie ) ) fail('extension_result_invalid', 422); - const sequence = (row.effectSequence || 0) + 1; + // Older sync agents copied item replay fences between installations while + // namespace counters stayed local. Raise the counter once, before claiming + // a NEW command. Never renumber a pending command: some of its effects may + // already have committed. Item receipts and stock remain untouched. + let sequenceFloor = row.effectSequence || 0; + if (row.localStockFenceVersion !== 1) { + const field = `extension_stock_streams.${key._id}.sequence`; + const cursor = db.collection('items').find( + { + license: key.license, + [field]: { $exists: true }, + $or: [{ branch_id: key.branch_id }, { 'branch_access.branch_id': key.branch_id }], + }, + { projection: { [field]: 1 } } + ); + for await (const item of cursor) { + const receiptSequence = item.extension_stock_streams?.[key._id]?.sequence; + if (!Number.isSafeInteger(receiptSequence) || receiptSequence < 0) + fail('extension_stock_receipt_invalid'); + sequenceFloor = Math.max(sequenceFloor, receiptSequence); + } + } + const sequence = sequenceFloor + 1; if (!Number.isSafeInteger(sequence)) fail('extension_sequence_exhausted'); const locked = await namespaces.updateOne( { @@ -200,6 +222,7 @@ async function executeNamespace(db, scope, descriptor, actor, input, dependencie { $set: { effectSequence: sequence, + localStockFenceVersion: 1, pending: { sequence, operationId, diff --git a/api/src/services/supplier.service.js b/api/src/services/supplier.service.js index bc4c88f9a..0c0acdd60 100644 --- a/api/src/services/supplier.service.js +++ b/api/src/services/supplier.service.js @@ -249,7 +249,10 @@ class SupplierService { // Check for duplicate email if (supplierData.email) { - const existingSupplier = await this.repository.findByEmail(supplierData.email); + const existingSupplier = await this.repository.findByEmail( + supplierData.email, + supplierData.branch_id + ); if (existingSupplier) { return { status: false, @@ -297,7 +300,10 @@ class SupplierService { // Check for duplicate email if email is being updated if (updateData.email && updateData.email !== existingSupplier.email) { - const duplicateSupplier = await this.repository.findByEmail(updateData.email); + const duplicateSupplier = await this.repository.findByEmail( + updateData.email, + existingSupplier.branch_id + ); if (duplicateSupplier) { return { status: false, @@ -650,7 +656,10 @@ class SupplierService { address: existingSupplier.address || '', }); } else { - if (supplier.email && (await this.repository.findByEmail(supplier.email))) { + if ( + supplier.email && + (await this.repository.findByEmail(supplier.email, supplier.branch_id)) + ) { validationErrors.push({ name: supplier.name, status: 'Email is already used by another supplier', diff --git a/api/tests/extension-namespace.integration.cjs b/api/tests/extension-namespace.integration.cjs index 4e35626bd..d1d0bd095 100644 --- a/api/tests/extension-namespace.integration.cjs +++ b/api/tests/extension-namespace.integration.cjs @@ -51,6 +51,58 @@ function fixture() { } const noEffect = { executeEffect: async () => ({ applied: true }) }; +test('upgrade advances above imported fences without deleting items or replaying stock', async () => { + const f = fixture(); + const token = require('node:crypto').createHash('sha256') + .update(`${f.scope.license}:${f.scope.branchId}:${f.descriptor.id}`).digest('hex'); + const item = { _id: new ObjectId(), license: f.scope.license, branch_id: f.scope.branchId, + track_inventory: true, item_status: 'regular', available_quantity: 5, + extension_stock_streams: { [token]: { sequence: 330, digest: 'old-receipt', applied: true } } }; + await db.collection('items').insertMany([item, + { ...item, _id: new ObjectId(), license: new ObjectId(), extension_stock_streams: { [token]: { sequence: 999 } } }, + { ...item, _id: new ObjectId(), branch_id: new ObjectId(), extension_stock_streams: { [token]: { sequence: 999 } } }]); + const { applyStockEffect } = require('../src/services/extension-stock-effects'); + const deps = { executeEffect: async context => applyStockEffect(db, f.scope, { + itemId: String(item._id), operationId: context.operationId, deltaMilli: -1000, + stream: { id: f.descriptor.id, sequence: context.sequence } }) }; + const result = await executeNamespace(db, f.scope, f.descriptor, f.actor, f.input, deps); + assert.deepEqual(await executeNamespace(db, f.scope, f.descriptor, f.actor, f.input, deps), result); + const saved = await db.collection('items').findOne({ _id: item._id }); + assert.equal(saved.available_quantity, 4); + assert.equal(saved.extension_stock_streams[token].sequence, 331); + const row = await db.collection('extension_namespaces').findOne({ _id: token }); + assert.equal(row.effectSequence, 331); + assert.equal(row.localStockFenceVersion, 1); + assert.equal(await db.collection('items').countDocuments({ license: f.scope.license }), 2); +}); + +test('upgrade does not renumber a pending effect or discard its receipts', async () => { + const f = fixture(); + await assert.rejects(executeNamespace(db, f.scope, f.descriptor, f.actor, f.input, + { executeEffect: async () => { throw Error('interrupted'); } }), /interrupted/); + const row = await db.collection('extension_namespaces').findOne({ license: f.scope.license }); + await db.collection('extension_namespaces').updateOne({ _id: row._id }, { $unset: { localStockFenceVersion: '' } }); + const item = { _id: new ObjectId(), license: f.scope.license, branch_id: f.scope.branchId, + available_quantity: 4, extension_stock_streams: { [row._id]: { sequence: 330 } } }; + await db.collection('items').insertOne(item); + await recoverNamespace(db, f.scope, f.descriptor, f.actor, { + executeEffect: async context => { assert.equal(context.sequence, 1); return { applied: true }; } }); + assert.deepEqual(await db.collection('items').findOne({ _id: item._id }), item); + assert.equal((await db.collection('extension_namespaces').findOne({ _id: row._id })).effectSequence, 1); +}); + +test('invalid imported fence refuses a new command without changing stock', async () => { + const f = fixture(); + const token = require('node:crypto').createHash('sha256') + .update(`${f.scope.license}:${f.scope.branchId}:${f.descriptor.id}`).digest('hex'); + const item = { _id: new ObjectId(), license: f.scope.license, branch_id: f.scope.branchId, + available_quantity: 5, extension_stock_streams: { [token]: { sequence: 'bad' } } }; + await db.collection('items').insertOne(item); + await assert.rejects(executeNamespace(db, f.scope, f.descriptor, f.actor, f.input, noEffect), /extension_stock_receipt_invalid/); + assert.deepEqual(await db.collection('items').findOne({ _id: item._id }), item); + assert.equal((await db.collection('extension_namespaces').findOne({ _id: token })).pending, undefined); +}); + test('repeated removal and restore retain state, effect receipts and other shops', async () => { const f = fixture(); await executeNamespace(db, f.scope, f.descriptor, f.actor, f.input, noEffect); diff --git a/api/tests/extensions-routes.integration.cjs b/api/tests/extensions-routes.integration.cjs index 2314f1558..a468807eb 100644 --- a/api/tests/extensions-routes.integration.cjs +++ b/api/tests/extensions-routes.integration.cjs @@ -139,7 +139,7 @@ test('signed view and catalogue retain staff, installation and branch boundaries headers: { Authorization: 'Bearer reader' }, }); assert.deepEqual((await list.json()).extensions, [ - { id: descriptor.id, displayName: descriptor.displayName, version: descriptor.version, enabled: true, installed: true }, + { id: descriptor.id, displayName: descriptor.displayName, version: descriptor.version, enabled: true, installed: true, salesWorkspace: null }, ]); assert.equal((await request('/view', { token: 'key' })).status, 403); const view = await (await request('/view', { token: 'reader' })).json(); diff --git a/api/tests/unit/database/optional-email-indexes.test.js b/api/tests/unit/database/optional-email-indexes.test.js index d4df81695..e2081eee7 100644 --- a/api/tests/unit/database/optional-email-indexes.test.js +++ b/api/tests/unit/database/optional-email-indexes.test.js @@ -25,6 +25,32 @@ const makeDb = (index = { name: 'email_1', key: { email: 1 }, unique: true }) => }; describe('migrateOptionalEmailIndexes', () => { + test('replaces the old business-wide supplier constraint only after the branch index exists', async () => { + const { db, collections } = makeDb({ + name: 'supplier_nonempty_email', + key: { email: 1 }, + unique: true, + }); + await migrateOptionalEmailIndexes(db); + const suppliers = collections.get('suppliers'); + expect(suppliers.createIndex).toHaveBeenCalledWith( + { license: 1, branch_id: 1, email: 1 }, + expect.objectContaining({ name: 'supplier_branch_nonempty_email', unique: true }) + ); + expect(suppliers.createIndex.mock.invocationCallOrder[0]).toBeLessThan( + suppliers.dropIndex.mock.invocationCallOrder[0] + ); + }); + + test('retains the old supplier constraint and contacts if branch uniqueness cannot be built', async () => { + const { db, collections } = makeDb(); + collections.get('suppliers').createIndex.mockRejectedValue({ code: 11000 }); + const warning = jest.spyOn(console, 'warn').mockImplementation(() => {}); + await migrateOptionalEmailIndexes(db); + expect(collections.get('suppliers').dropIndex).not.toHaveBeenCalled(); + expect(collections.get('suppliers').updateMany).not.toHaveBeenCalled(); + warning.mockRestore(); + }); test('unsets blank emails and replaces non-sparse unique indexes', async () => { const { db, collections } = makeDb(); await migrateOptionalEmailIndexes(db); @@ -36,7 +62,9 @@ describe('migrateOptionalEmailIndexes', () => { ) { expect(collection.updateMany).not.toHaveBeenCalled(); expect(collection.createIndex).toHaveBeenCalledWith( - { email: 1 }, + collection === collections.get('suppliers') + ? { license: 1, branch_id: 1, email: 1 } + : { email: 1 }, expect.objectContaining({ unique: true, partialFilterExpression: { email: { $type: 'string', $gt: '' } }, diff --git a/api/tests/unit/repositories/supplier.repository.test.js b/api/tests/unit/repositories/supplier.repository.test.js index 502b70cee..edd4ea912 100644 --- a/api/tests/unit/repositories/supplier.repository.test.js +++ b/api/tests/unit/repositories/supplier.repository.test.js @@ -27,6 +27,7 @@ jest.mock('../../../src/models/base.model', () => { } MockBaseModel.prototype.getCollection = jest.fn(); MockBaseModel.simplifyFields = jest.fn((doc) => doc); + MockBaseModel.deletedDocumentBackup = jest.fn().mockResolvedValue({ status: true }); MockBaseModel.license = null; MockBaseModel.currentBranch = null; MockBaseModel.loggedUser = null; @@ -461,6 +462,25 @@ describe('SupplierRepository', () => { // ── bulkSoftDelete ─────────────────────────────────────────────────────────── describe('bulkSoftDelete', () => { + test('publishes a sync deletion record before removing suppliers', async () => { + const doc = { _id: FAKE_ID, branch_id: FAKE_BRANCH, license: FAKE_LICENSE }; + col.find.mockReturnValue(mkChain([doc])); + await repo.bulkSoftDelete([FAKE_ID]); + expect(BaseModel.deletedDocumentBackup).toHaveBeenCalledWith('suppliers', doc); + expect(BaseModel.deletedDocumentBackup.mock.invocationCallOrder[0]).toBeLessThan( + col.deleteMany.mock.invocationCallOrder[0] + ); + }); + + test('does not delete when the sync backup cannot be saved', async () => { + col.find.mockReturnValue(mkChain([{ _id: FAKE_ID }])); + BaseModel.deletedDocumentBackup.mockResolvedValueOnce({ + status: false, + message: 'Backup unavailable', + }); + await expect(repo.bulkSoftDelete([FAKE_ID])).rejects.toThrow('Backup unavailable'); + expect(col.deleteMany).not.toHaveBeenCalled(); + }); test('backs up and deletes multiple suppliers', async () => { const docs = [{ _id: FAKE_ID }, { _id: FAKE_ID_2 }]; col.find.mockReturnValue(mkChain(docs)); diff --git a/frontend/layouts/header.html b/frontend/layouts/header.html index cc6044255..0db58d890 100644 --- a/frontend/layouts/header.html +++ b/frontend/layouts/header.html @@ -121,6 +121,17 @@

Posnic