From d4e0d80afaf4ecf3b89fa5e99f4547be46ec295e Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 29 Jun 2026 14:44:51 +0530 Subject: [PATCH 01/16] feat(#10706): add bulk operation document model Introduce the @medic/bulk-operations shared library holding the document model that the bulk operation framework shares across delete, move and merge: the log document (medic-logs) read by the polling endpoint, and the per-action documents (medic-sentinel) that Sentinel processes in batches. buildBulkOperation assembles both from per-action operation lists. The per-item params live in a base64 json attachment so advancing an action's cursor does not rewrite the whole list. --- shared-libs/bulk-operations/package.json | 11 +++ shared-libs/bulk-operations/src/index.js | 98 +++++++++++++++++++++ shared-libs/bulk-operations/test/index.js | 100 ++++++++++++++++++++++ 3 files changed, 209 insertions(+) create mode 100644 shared-libs/bulk-operations/package.json create mode 100644 shared-libs/bulk-operations/src/index.js create mode 100644 shared-libs/bulk-operations/test/index.js diff --git a/shared-libs/bulk-operations/package.json b/shared-libs/bulk-operations/package.json new file mode 100644 index 00000000000..24d1de4831d --- /dev/null +++ b/shared-libs/bulk-operations/package.json @@ -0,0 +1,11 @@ +{ + "name": "@medic/bulk-operations", + "version": "1.0.0", + "description": "Shared document model for the CHT-Core bulk operation framework (delete, move, merge)", + "main": "src/index.js", + "scripts": { + "test": "nyc --nycrcPath='../nyc.config.js' mocha ./test" + }, + "author": "", + "license": "Apache-2.0" +} diff --git a/shared-libs/bulk-operations/src/index.js b/shared-libs/bulk-operations/src/index.js new file mode 100644 index 00000000000..d84078cce83 --- /dev/null +++ b/shared-libs/bulk-operations/src/index.js @@ -0,0 +1,98 @@ +const { v7: uuid } = require('uuid'); + +/** + * Document model for the bulk operation framework. + * + * A bulk operation is recorded with two kinds of document: + * - a single LOG document (stored in medic-logs) that tracks overall status and is read by the polling endpoint. + * - one ACTION document per action type (stored in medic-sentinel) that Sentinel processes in batches. + * + * Delete, move and merge all express their work as a set of these actions. + */ + +const LOG_ID_PREFIX = 'bulk-operation:'; +const ACTION_ID_PREFIX = 'bulk-operation-action:'; + +const ACTIONS = { + ARCHIVE: 'archive', + SET_CONTACT: 'set-contact', + DELETE_USER: 'delete-user', +}; + +const STATUSES = { + QUEUED: 'queued', + COMPLETED: 'completed', + FAILED: 'failed', +}; + +// The per-item params for each action live in an attachment rather than inline, so updating the +// action document's cursor as Sentinel works through the batches does not rewrite the whole list. +const OPERATIONS_ATTACHMENT = 'operations'; +const OPERATIONS_CONTENT_TYPE = 'application/json'; + +const getOperationUuid = (operationId) => operationId.slice(LOG_ID_PREFIX.length); + +const generateOperationId = () => `${LOG_ID_PREFIX}${uuid()}`; + +const generateActionId = (operationId) => `${ACTION_ID_PREFIX}${getOperationUuid(operationId)}:${uuid()}`; + +const encodeOperations = (operations) => ({ + content_type: OPERATIONS_CONTENT_TYPE, + data: Buffer.from(JSON.stringify(operations)).toString('base64'), +}); + +const decodeOperations = (attachment) => JSON.parse(Buffer.from(attachment.data, 'base64').toString()); + +const buildActionDoc = (operationId, action, operations) => ({ + _id: generateActionId(operationId), + bulk_operation_id: operationId, + action, + cursor: 0, + total: operations.length, + _attachments: { + [OPERATIONS_ATTACHMENT]: encodeOperations(operations), + }, +}); + +const buildLogAction = (action, totalChangesCount, date) => ({ + status: STATUSES.QUEUED, + action, + updated_date: date, + total_changes_count: totalChangesCount, +}); + +/** + * Builds the documents that make up a single bulk operation. + * @param {Array<{action: string, operations: Object[]}>} actionOperations - one entry per action type, each holding + * its list of per-item params + * @param {Date} date - the operation start date, also the initial updated_date of every action + * @returns {{log: Object, actions: Object[]}} the log document (for medic-logs) and the action documents + * (for medic-sentinel); the log's `actions` map is keyed by each action document's `_id` + */ +const buildBulkOperation = (actionOperations, date) => { + const operationId = generateOperationId(); + const actions = actionOperations.map(({ action, operations }) => buildActionDoc(operationId, action, operations)); + + const logActions = {}; + actions.forEach((actionDoc) => { + logActions[actionDoc._id] = buildLogAction(actionDoc.action, actionDoc.total, date); + }); + + const log = { + _id: operationId, + start_date: date, + actions: logActions, + }; + + return { log, actions }; +}; + +module.exports = { + LOG_ID_PREFIX, + ACTION_ID_PREFIX, + ACTIONS, + STATUSES, + OPERATIONS_ATTACHMENT, + buildBulkOperation, + decodeOperations, +}; diff --git a/shared-libs/bulk-operations/test/index.js b/shared-libs/bulk-operations/test/index.js new file mode 100644 index 00000000000..c4fc5074008 --- /dev/null +++ b/shared-libs/bulk-operations/test/index.js @@ -0,0 +1,100 @@ +const chai = require('chai'); +const { validate: isUuid } = require('uuid'); + +const service = require('../src/index'); + +const expect = chai.expect; + +describe('bulk-operations document model', () => { + describe('constants', () => { + it('exposes the action types', () => { + expect(service.ACTIONS).to.deep.equal({ + ARCHIVE: 'archive', + SET_CONTACT: 'set-contact', + DELETE_USER: 'delete-user', + }); + }); + + it('exposes the statuses', () => { + expect(service.STATUSES).to.deep.equal({ + QUEUED: 'queued', + COMPLETED: 'completed', + FAILED: 'failed', + }); + }); + + it('exposes the document id prefixes', () => { + expect(service.LOG_ID_PREFIX).to.equal('bulk-operation:'); + expect(service.ACTION_ID_PREFIX).to.equal('bulk-operation-action:'); + }); + }); + + describe('buildBulkOperation', () => { + const date = new Date('2026-06-29T12:00:00.000Z'); + + it('builds a log document with a uuid-v7 operation id', () => { + const { log } = service.buildBulkOperation([{ action: 'archive', operations: [{ id: 'a' }] }], date); + + expect(log._id.startsWith('bulk-operation:')).to.equal(true); + expect(isUuid(log._id.slice('bulk-operation:'.length))).to.equal(true); + expect(log.start_date).to.equal(date); + }); + + it('builds one action document per action type, linked back to the operation', () => { + const groups = [ + { action: 'archive', operations: [{ id: 'person' }, { id: 'report' }] }, + { action: 'set-contact', operations: [{ id: 'place', current_contact_id: 'person' }] }, + { action: 'delete-user', operations: [{ id: 'org.couchdb.user:chw' }] }, + ]; + + const { log, actions } = service.buildBulkOperation(groups, date); + + expect(actions).to.have.length(3); + actions.forEach((actionDoc, i) => { + expect(actionDoc._id.startsWith('bulk-operation-action:')).to.equal(true); + // the action id embeds the operation's uuid, so the two ids stay parseable together + expect(actionDoc._id).to.include(log._id.slice('bulk-operation:'.length)); + expect(actionDoc.bulk_operation_id).to.equal(log._id); + expect(actionDoc.action).to.equal(groups[i].action); + expect(actionDoc.cursor).to.equal(0); + expect(actionDoc.total).to.equal(groups[i].operations.length); + expect(service.decodeOperations(actionDoc._attachments.operations)).to.deep.equal(groups[i].operations); + }); + }); + + it('stores the operation params in a base64 json attachment', () => { + const operations = [{ id: 'place', current_contact_id: 'person' }]; + const { actions } = service.buildBulkOperation([{ action: 'set-contact', operations }], date); + + const attachment = actions[0]._attachments.operations; + expect(attachment.content_type).to.equal('application/json'); + expect(attachment.data).to.be.a('string'); + expect(service.decodeOperations(attachment)).to.deep.equal(operations); + }); + + it('cross-links each action document to its entry in the log', () => { + const groups = [ + { action: 'archive', operations: [{ id: 'person' }] }, + { action: 'set-contact', operations: [{ id: 'place' }, { id: 'place2' }] }, + ]; + + const { log, actions } = service.buildBulkOperation(groups, date); + + expect(Object.keys(log.actions)).to.deep.equal(actions.map(actionDoc => actionDoc._id)); + actions.forEach((actionDoc) => { + const logAction = log.actions[actionDoc._id]; + expect(logAction.status).to.equal('queued'); + expect(logAction.action).to.equal(actionDoc.action); + expect(logAction.updated_date).to.equal(date); + expect(logAction.total_changes_count).to.equal(actionDoc.total); + }); + }); + + it('generates a distinct operation id on each call', () => { + const first = service.buildBulkOperation([{ action: 'archive', operations: [] }], date); + const second = service.buildBulkOperation([{ action: 'archive', operations: [] }], date); + + expect(first.log._id).to.not.equal(second.log._id); + }); + }); +}); From 25f8e201f979873c037edae3319c24f21f00b1e9 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 29 Jun 2026 15:04:25 +0530 Subject: [PATCH 02/16] build(#10706): add @medic/bulk-operations to package-lock --- package-lock.json | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/package-lock.json b/package-lock.json index 53a1d09fb49..f16699ff667 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5750,6 +5750,10 @@ "resolved": "shared-libs/bulk-docs-utils", "link": true }, + "node_modules/@medic/bulk-operations": { + "resolved": "shared-libs/bulk-operations", + "link": true + }, "node_modules/@medic/calendar-interval": { "resolved": "shared-libs/calendar-interval", "link": true @@ -44275,6 +44279,11 @@ "version": "1.0.0", "license": "Apache-2.0" }, + "shared-libs/bulk-operations": { + "name": "@medic/bulk-operations", + "version": "1.0.0", + "license": "Apache-2.0" + }, "shared-libs/calendar-interval": { "name": "@medic/calendar-interval", "version": "1.0.0", From a088c6bbf04f7ce1fc086c87aa4e0e613afac0bf Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 29 Jun 2026 16:00:54 +0530 Subject: [PATCH 03/16] feat(#10706): add bulk operation status endpoint Add GET /api/v1/bulk-operations/{id}, which returns the bulk operation log document from the medic-logs database so a caller can poll the progress of a delete, move or merge it started. The service only resolves ids carrying the bulk-operation prefix, so the endpoint cannot be used to read the other log documents that share the database. Online users only; missing or non-bulk-operation ids return 404. --- api/src/controllers/bulk-operations.js | 66 ++++++++++++++++++ api/src/routing.js | 3 + api/src/services/bulk-operations.js | 25 +++++++ .../mocha/controllers/bulk-operations.spec.js | 68 +++++++++++++++++++ .../mocha/services/bulk-operations.spec.js | 64 +++++++++++++++++ 5 files changed, 226 insertions(+) create mode 100644 api/src/controllers/bulk-operations.js create mode 100644 api/src/services/bulk-operations.js create mode 100644 api/tests/mocha/controllers/bulk-operations.spec.js create mode 100644 api/tests/mocha/services/bulk-operations.spec.js diff --git a/api/src/controllers/bulk-operations.js b/api/src/controllers/bulk-operations.js new file mode 100644 index 00000000000..b4c88da9a8d --- /dev/null +++ b/api/src/controllers/bulk-operations.js @@ -0,0 +1,66 @@ +const service = require('../services/bulk-operations'); +const serverUtils = require('../server-utils'); +const auth = require('../auth'); + +/** + * @openapi + * tags: + * - name: Bulk operations + * description: Status of long-running bulk operations (delete, move, merge) + */ +module.exports = { + v1: { + /** + * @openapi + * /api/v1/bulk-operations/{id}: + * get: + * summary: Get the status of a bulk operation + * operationId: v1BulkOperationIdGet + * description: > + * Returns the log document for a bulk operation, including the per-action status and the + * count of changes applied so far. Used to poll the progress of an operation that was + * started through one of the bulk endpoints. + * tags: [Bulk operations] + * x-since: 5.1.0 + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * description: The id of the bulk operation, as returned when it was started. + * responses: + * '200': + * description: The bulk operation log + * content: + * application/json: + * schema: + * type: object + * properties: + * _id: + * type: string + * description: The bulk operation id. + * start_date: + * type: string + * format: date-time + * description: When the operation was started. + * actions: + * type: object + * description: Per-action status, keyed by action id. + * '401': + * $ref: '#/components/responses/Unauthorized' + * '403': + * $ref: '#/components/responses/Forbidden' + * '404': + * $ref: '#/components/responses/NotFound' + */ + get: serverUtils.doOrError(async (req, res) => { + await auth.assertPermissions(req, { isOnline: true }); + const log = await service.getLog(req.params.id); + if (!log) { + return serverUtils.error({ status: 404, message: 'Bulk operation not found' }, req, res); + } + res.json(log); + }) + } +}; diff --git a/api/src/routing.js b/api/src/routing.js index 370f0f2cf3a..a3f55b50ac6 100644 --- a/api/src/routing.js +++ b/api/src/routing.js @@ -46,6 +46,7 @@ const { people, places } = require('@medic/contacts')(config, db, dataContext); const upgrade = require('./controllers/upgrade'); const settings = require('./controllers/settings'); const bulkDocs = require('./controllers/bulk-docs'); +const bulkOperations = require('./controllers/bulk-operations'); const monitoring = require('./controllers/monitoring'); const africasTalking = require('./controllers/africas-talking'); const rapidPro = require('./controllers/rapidpro'); @@ -478,6 +479,8 @@ app.get('/api/v1/monitoring', deprecation.deprecate('/api/v2/monitoring'), monit app.get('/api/v2/monitoring', monitoring.getV2); app.get('/api/v1/impact', impact.v1.get); +app.get('/api/v1/bulk-operations/:id', bulkOperations.v1.get); + app.post('/api/v1/upgrade', jsonParser, upgrade.upgrade); app.post('/api/v1/upgrade/stage', jsonParser, upgrade.stage); app.post('/api/v1/upgrade/complete', jsonParser, upgrade.complete); diff --git a/api/src/services/bulk-operations.js b/api/src/services/bulk-operations.js new file mode 100644 index 00000000000..332384932dd --- /dev/null +++ b/api/src/services/bulk-operations.js @@ -0,0 +1,25 @@ +const db = require('../db'); +const { LOG_ID_PREFIX } = require('@medic/bulk-operations'); + +// Reads only bulk operation log documents from the medic-logs database. The prefix guard keeps the +// endpoint from returning the other kinds of log documents that share this database. +const getLog = async (id) => { + if (!id?.startsWith(LOG_ID_PREFIX)) { + return null; + } + + try { + const log = await db.medicLogs.get(id); + delete log._rev; + return log; + } catch (err) { + if (err.status === 404) { + return null; + } + throw err; + } +}; + +module.exports = { + getLog, +}; diff --git a/api/tests/mocha/controllers/bulk-operations.spec.js b/api/tests/mocha/controllers/bulk-operations.spec.js new file mode 100644 index 00000000000..fbfd2a2547c --- /dev/null +++ b/api/tests/mocha/controllers/bulk-operations.spec.js @@ -0,0 +1,68 @@ +const sinon = require('sinon'); +const chai = require('chai'); + +const serverUtils = require('../../../src/server-utils'); +const controller = require('../../../src/controllers/bulk-operations'); +const service = require('../../../src/services/bulk-operations'); +const auth = require('../../../src/auth'); +const { PermissionError } = require('../../../src/errors'); + +describe('Bulk operations controller', () => { + let req; + let res; + + beforeEach(() => { + sinon.stub(serverUtils, 'error'); + req = { params: { id: 'bulk-operation:abc' } }; + res = { json: sinon.stub() }; + }); + + afterEach(() => sinon.restore()); + + describe('v1 get', () => { + it('returns the bulk operation log for an authorised online user', () => { + const log = { _id: 'bulk-operation:abc', start_date: 'date', actions: {} }; + sinon.stub(auth, 'assertPermissions').resolves(); + sinon.stub(service, 'getLog').resolves(log); + + return controller.v1.get(req, res).then(() => { + chai.expect(auth.assertPermissions.calledOnceWithExactly(req, { isOnline: true })).to.equal(true); + chai.expect(service.getLog.calledOnceWithExactly('bulk-operation:abc')).to.equal(true); + chai.expect(res.json.calledOnceWithExactly(log)).to.equal(true); + chai.expect(serverUtils.error.called).to.equal(false); + }); + }); + + it('returns a 404 when the operation is not found', () => { + sinon.stub(auth, 'assertPermissions').resolves(); + sinon.stub(service, 'getLog').resolves(null); + + return controller.v1.get(req, res).then(() => { + chai.expect(res.json.called).to.equal(false); + chai.expect(serverUtils.error.calledOnce).to.equal(true); + chai.expect(serverUtils.error.args[0][0]).to.deep.equal({ status: 404, message: 'Bulk operation not found' }); + }); + }); + + it('does not reach the service when the user is not permitted', () => { + sinon.stub(auth, 'assertPermissions').rejects(new PermissionError('Insufficient privileges')); + sinon.stub(service, 'getLog').resolves({}); + + return controller.v1.get(req, res).then(() => { + chai.expect(service.getLog.called).to.equal(false); + chai.expect(res.json.called).to.equal(false); + chai.expect(serverUtils.error.calledOnce).to.equal(true); + }); + }); + + it('handles a service rejection gracefully', () => { + sinon.stub(auth, 'assertPermissions').resolves(); + sinon.stub(service, 'getLog').rejects(new Error('db down')); + + return controller.v1.get(req, res).then(() => { + chai.expect(res.json.called).to.equal(false); + chai.expect(serverUtils.error.calledOnce).to.equal(true); + }); + }); + }); +}); diff --git a/api/tests/mocha/services/bulk-operations.spec.js b/api/tests/mocha/services/bulk-operations.spec.js new file mode 100644 index 00000000000..e6043e22315 --- /dev/null +++ b/api/tests/mocha/services/bulk-operations.spec.js @@ -0,0 +1,64 @@ +const chai = require('chai'); +const sinon = require('sinon'); + +const db = require('../../../src/db'); +const service = require('../../../src/services/bulk-operations'); + +describe('Bulk operations service', () => { + afterEach(() => sinon.restore()); + + describe('getLog', () => { + it('returns the log document without the couch _rev', () => { + const doc = { + _id: 'bulk-operation:abc', + _rev: '1-xyz', + start_date: 'date', + actions: { 'bulk-operation-action:abc:1': { status: 'queued' } }, + }; + sinon.stub(db.medicLogs, 'get').resolves(doc); + + return service.getLog('bulk-operation:abc').then((log) => { + chai.expect(db.medicLogs.get.calledOnceWithExactly('bulk-operation:abc')).to.equal(true); + chai.expect(log).to.deep.equal({ + _id: 'bulk-operation:abc', + start_date: 'date', + actions: { 'bulk-operation-action:abc:1': { status: 'queued' } }, + }); + chai.expect(log._rev).to.equal(undefined); + }); + }); + + it('returns null when the operation does not exist', () => { + sinon.stub(db.medicLogs, 'get').rejects({ status: 404 }); + + return service.getLog('bulk-operation:missing').then((log) => { + chai.expect(log).to.equal(null); + }); + }); + + it('does not query the database for an id that is not a bulk operation', () => { + const get = sinon.stub(db.medicLogs, 'get'); + + return Promise + .all([ + service.getLog(undefined), + service.getLog(''), + service.getLog('upgrade_log:something'), + service.getLog('some-other-doc'), + ]) + .then((results) => { + chai.expect(results).to.deep.equal([null, null, null, null]); + chai.expect(get.called).to.equal(false); + }); + }); + + it('rethrows errors that are not a 404', () => { + sinon.stub(db.medicLogs, 'get').rejects({ status: 500 }); + + return service + .getLog('bulk-operation:boom') + .then(() => chai.expect.fail('should have thrown')) + .catch((err) => chai.expect(err.status).to.equal(500)); + }); + }); +}); From aa5a218423e81241d6231d9b60235448db74cbe4 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 09:56:29 +0530 Subject: [PATCH 04/16] feat(#10706): add bulk operation queue helper Add queue(actionOperations) to the bulk-operations service. It builds the log and action documents and writes the log to medic-logs first, then one action document per type to medic-sentinel, skipping empty action groups, and returns the bulk operation id. --- api/src/services/bulk-operations.js | 17 ++++++- .../mocha/services/bulk-operations.spec.js | 46 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/api/src/services/bulk-operations.js b/api/src/services/bulk-operations.js index 332384932dd..63d282cd67b 100644 --- a/api/src/services/bulk-operations.js +++ b/api/src/services/bulk-operations.js @@ -1,5 +1,5 @@ const db = require('../db'); -const { LOG_ID_PREFIX } = require('@medic/bulk-operations'); +const { LOG_ID_PREFIX, buildBulkOperation } = require('@medic/bulk-operations'); // Reads only bulk operation log documents from the medic-logs database. The prefix guard keeps the // endpoint from returning the other kinds of log documents that share this database. @@ -20,6 +20,21 @@ const getLog = async (id) => { } }; +// Queues a bulk operation: writes the log document to medic-logs (the status record the polling +// endpoint reads) and one action document per action type to medic-sentinel (what the Sentinel +// listener processes). The log is written first so it exists before the listener picks up an action. +// Action groups with no operations are skipped. Returns the bulk operation id. +const queue = async (actionOperations) => { + const nonEmpty = actionOperations.filter(({ operations }) => operations.length); + const { log, actions } = buildBulkOperation(nonEmpty, new Date()); + + await db.medicLogs.put(log); + await db.sentinel.bulkDocs(actions); + + return log._id; +}; + module.exports = { getLog, + queue, }; diff --git a/api/tests/mocha/services/bulk-operations.spec.js b/api/tests/mocha/services/bulk-operations.spec.js index e6043e22315..f135a45c5d8 100644 --- a/api/tests/mocha/services/bulk-operations.spec.js +++ b/api/tests/mocha/services/bulk-operations.spec.js @@ -61,4 +61,50 @@ describe('Bulk operations service', () => { .catch((err) => chai.expect(err.status).to.equal(500)); }); }); + + describe('queue', () => { + const actionOperations = [ + { action: 'archive', operations: [{ id: 'person' }, { id: 'report' }] }, + { action: 'set-contact', operations: [{ id: 'place', current_contact_id: 'person' }] }, + { action: 'delete-user', operations: [{ id: 'org.couchdb.user:chw' }] }, + ]; + + it('writes the log to medic-logs and the actions to medic-sentinel, and returns the operation id', () => { + const put = sinon.stub(db.medicLogs, 'put').resolves(); + const bulkDocs = sinon.stub(db.sentinel, 'bulkDocs').resolves(); + + return service.queue(actionOperations).then((operationId) => { + chai.expect(operationId.startsWith('bulk-operation:')).to.equal(true); + + chai.expect(put.calledOnce).to.equal(true); + const log = put.args[0][0]; + chai.expect(log._id).to.equal(operationId); + chai.expect(Object.keys(log.actions)).to.have.length(3); + + chai.expect(bulkDocs.calledOnce).to.equal(true); + const actions = bulkDocs.args[0][0]; + chai.expect(actions).to.have.length(3); + chai.expect(actions.map(action => action.action)).to.deep.equal(['archive', 'set-contact', 'delete-user']); + chai.expect(actions[0].bulk_operation_id).to.equal(operationId); + + // the log must exist before the listener can pick up an action + chai.expect(put.calledBefore(bulkDocs)).to.equal(true); + }); + }); + + it('skips action groups that have no operations', () => { + sinon.stub(db.medicLogs, 'put').resolves(); + const bulkDocs = sinon.stub(db.sentinel, 'bulkDocs').resolves(); + const groups = [ + { action: 'archive', operations: [{ id: 'person' }] }, + { action: 'set-contact', operations: [] }, + { action: 'delete-user', operations: [{ id: 'org.couchdb.user:chw' }] }, + ]; + + return service.queue(groups).then(() => { + const actions = bulkDocs.args[0][0]; + chai.expect(actions.map(action => action.action)).to.deep.equal(['archive', 'delete-user']); + }); + }); + }); }); From aaa05463a889ef9289c8da8e537b4ca7138f652d Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 09:56:29 +0530 Subject: [PATCH 05/16] feat(#10706): process bulk operations in sentinel Add a medic-sentinel changes-feed listener that queues bulk-operation-action docs (existing ones on startup, new ones from the feed) and a processor that works through each in batches, tracking the cursor, then records the result on the log doc and removes the action doc. Includes the set-contact and delete-user handlers; the archive handler follows with the archiving work. Adds the medic-logs handle and getAttachment to sentinel's db. --- sentinel/server.js | 3 + sentinel/src/db.js | 8 + sentinel/src/lib/bulk-operations.js | 177 ++++++++++++++++++ .../tests/unit/lib/bulk-operations.spec.js | 162 ++++++++++++++++ 4 files changed, 350 insertions(+) create mode 100644 sentinel/src/lib/bulk-operations.js create mode 100644 sentinel/tests/unit/lib/bulk-operations.spec.js diff --git a/sentinel/server.js b/sentinel/server.js index 74bf0ee206f..b310bc621c2 100755 --- a/sentinel/server.js +++ b/sentinel/server.js @@ -47,6 +47,9 @@ logger.info('Running server checks...'); const schedule = require('./src/schedule'); schedule.init(); + const bulkOperations = require('./src/lib/bulk-operations'); + bulkOperations.listen(); + logger.info('startup complete.'); const processHooks = require('./src/process-hooks'); diff --git a/sentinel/src/db.js b/sentinel/src/db.js index bf873e61cad..0353facdc33 100644 --- a/sentinel/src/db.js +++ b/sentinel/src/db.js @@ -35,9 +35,16 @@ if (UNIT_TEST_ENV) { post: stubMe('post'), query: stubMe('query'), get: stubMe('get'), + getAttachment: stubMe('getAttachment'), changes: stubMe('changes'), }; + module.exports.medicLogs = { + allDocs: stubMe('allDocs'), + get: stubMe('get'), + put: stubMe('put'), + }; + module.exports.users = { allDocs: stubMe('allDocs'), bulkDocs: stubMe('bulkDocs'), @@ -77,6 +84,7 @@ if (UNIT_TEST_ENV) { module.exports.medic = new PouchDB(couchUrl, { fetch: fetchFn }); module.exports.sentinel = new PouchDB(`${couchUrl}-sentinel`, { fetch: fetchFn}); + module.exports.medicLogs = new PouchDB(`${couchUrl}-logs`, { fetch: fetchFn }); module.exports.allDbs = () => request.get({ url: `${environment.serverUrl}/_all_dbs`, json: true }); module.exports.get = db => new PouchDB(`${environment.serverUrl}/${db}`); module.exports.close = db => { diff --git a/sentinel/src/lib/bulk-operations.js b/sentinel/src/lib/bulk-operations.js new file mode 100644 index 00000000000..86fffc77894 --- /dev/null +++ b/sentinel/src/lib/bulk-operations.js @@ -0,0 +1,177 @@ +const async = require('async'); +const logger = require('@medic/logger'); +const db = require('../db'); +const config = require('../config'); +const dataContext = require('../data-context'); +const { PREFIXES } = require('@medic/constants'); +const { ACTIONS, STATUSES, OPERATIONS_ATTACHMENT, ACTION_ID_PREFIX } = require('@medic/bulk-operations'); +const userManagement = require('@medic/user-management')(config, db, dataContext); + +const BATCH_SIZE = 100; +const RETRY_TIMEOUT = 60000; + +// set-contact: point a place's `contact` reference at a new value (or clear it when `contact` is +// absent). The change is applied only if the doc still has the contact we recorded when the +// operation was queued, so a concurrent edit is not clobbered. Operations whose doc is gone or +// whose contact has since changed are returned as failed. +const setContact = async (batch) => { + const result = await db.medic.allDocs({ keys: batch.map(op => op.id), include_docs: true }); + const docsById = {}; + result.rows.forEach(row => { + if (row.doc) { + docsById[row.doc._id] = row.doc; + } + }); + + const failed = []; + const toUpdate = []; + batch.forEach(op => { + const doc = docsById[op.id]; + const currentContactId = doc && (doc.contact?._id || doc.contact); + if (!doc || currentContactId !== op.current_contact_id) { + failed.push(op); + return; + } + doc.contact = op.contact; + toUpdate.push(doc); + }); + + if (toUpdate.length) { + await db.medic.bulkDocs(toUpdate); + } + return failed; +}; + +// delete-user: remove each linked user through the existing user-delete path. Failures are +// collected per user so one bad delete does not abort the batch. +const deleteUser = async (batch) => { + const failed = []; + for (const op of batch) { + try { + await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, '')); + } catch (err) { + logger.error(`bulk-operations: failed to delete user ${op.id}: %o`, err); + failed.push(op); + } + } + return failed; +}; + +const HANDLERS = { + [ACTIONS.SET_CONTACT]: setContact, + [ACTIONS.DELETE_USER]: deleteUser, + // ACTIONS.ARCHIVE handler is added when #6615 lands. +}; + +const readOperations = async (actionId) => { + const buffer = await db.sentinel.getAttachment(actionId, OPERATIONS_ATTACHMENT); + return JSON.parse(buffer.toString()); +}; + +// Persist progress on the action doc after each batch so a restart resumes from the cursor. The +// stored `operations` attachment is left as a stub, so advancing the cursor does not rewrite it. +const saveProgress = async (action, processedCount, failed) => { + const latest = await db.sentinel.get(action._id); + action._rev = latest._rev; + action.cursor += processedCount; + if (failed.length) { + action.failed_operations = [ ...(action.failed_operations || []), ...failed ]; + } + await db.sentinel.put(action); +}; + +const recordResultOnLog = async (action) => { + const log = await db.medicLogs.get(action.bulk_operation_id); + const failed = action.failed_operations || []; + log.actions[action._id] = { + status: failed.length ? STATUSES.FAILED : STATUSES.COMPLETED, + action: action.action, + updated_date: new Date(), + total_changes_count: action.total - failed.length, + failed_operations: failed.length ? failed : undefined, + }; + await db.medicLogs.put(log); +}; + +const deleteAction = async (action) => { + const latest = await db.sentinel.get(action._id); + await db.sentinel.put({ ...action, _rev: latest._rev, _deleted: true }); +}; + +// Processes a single bulk-operation action document: works through its operations in batches, +// advancing the cursor so a restart resumes where it left off, then records the outcome on the +// bulk operation log and removes the action document. +const processAction = async (action) => { + const handler = HANDLERS[action.action]; + if (!handler) { + throw new Error(`bulk-operations: no handler for action "${action.action}"`); + } + + const operations = await readOperations(action._id); + while (action.cursor < action.total) { + const batch = operations.slice(action.cursor, action.cursor + BATCH_SIZE); + if (!batch.length) { + break; + } + const failed = await handler(batch); + await saveProgress(action, batch.length, failed); + } + + await recordResultOnLog(action); + await deleteAction(action); + logger.info(`bulk-operations: completed action ${action._id}`); +}; + +// --- the medic-sentinel changes-feed listener --- +// On startup we queue any action docs already waiting, then watch the feed for new ones. Actions +// are processed one at a time. `inProgress` stops our own cursor writes (which also show up on the +// feed) from re-queueing an action while it is still being processed. +const inProgress = new Set(); + +const queue = async.queue((action, callback) => { + processAction(action) + .catch(err => logger.error(`bulk-operations: error processing action ${action._id}: %o`, err)) + .then(() => callback()); +}); + +const enqueue = (action) => { + if (inProgress.has(action._id)) { + return; + } + inProgress.add(action._id); + queue.push(action, () => inProgress.delete(action._id)); +}; + +const loadInitialQueue = async () => { + const result = await db.sentinel.allDocs({ + startkey: ACTION_ID_PREFIX, + endkey: `${ACTION_ID_PREFIX}￰`, + include_docs: true, + }); + result.rows.forEach(row => row.doc && enqueue(row.doc)); +}; + +const registerFeed = () => { + db.sentinel + .changes({ live: true, since: 'now', include_docs: true }) + .on('change', (change) => { + if (!change.deleted && change.id.startsWith(ACTION_ID_PREFIX) && change.doc) { + enqueue(change.doc); + } + }) + .on('error', (err) => { + logger.error('bulk-operations: changes feed error: %o', err); + setTimeout(registerFeed, RETRY_TIMEOUT); + }); +}; + +const listen = async () => { + logger.info('bulk-operations: listening for queued actions on medic-sentinel'); + await loadInitialQueue(); + registerFeed(); +}; + +module.exports = { + processAction, + listen, +}; diff --git a/sentinel/tests/unit/lib/bulk-operations.spec.js b/sentinel/tests/unit/lib/bulk-operations.spec.js new file mode 100644 index 00000000000..95e56f57395 --- /dev/null +++ b/sentinel/tests/unit/lib/bulk-operations.spec.js @@ -0,0 +1,162 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const rewire = require('rewire'); + +const db = require('../../../src/db'); + +const expect = chai.expect; + +describe('bulk-operations sentinel processor', () => { + let service; + + beforeEach(() => { + service = rewire('../../../src/lib/bulk-operations'); + }); + + afterEach(() => sinon.restore()); + + const buildAction = (overrides = {}) => ({ + _id: 'bulk-operation-action:op:1', + _rev: '1-a', + bulk_operation_id: 'bulk-operation:op', + action: 'set-contact', + cursor: 0, + total: 2, + _attachments: { operations: { stub: true } }, + ...overrides, + }); + + const stubActionWrites = () => { + sinon.stub(db.sentinel, 'get').resolves({ _rev: '2-b' }); + sinon.stub(db.sentinel, 'put').resolves({ rev: '3-c' }); + const log = { _id: 'bulk-operation:op', actions: {} }; + sinon.stub(db.medicLogs, 'get').resolves(log); + sinon.stub(db.medicLogs, 'put').resolves(); + return log; + }; + + describe('processAction', () => { + it('applies matching set-contact operations, advances the cursor, records the log and removes the action', () => { + const action = buildAction(); + const operations = [ + { id: 'place-1', contact: { _id: 'new-1' }, current_contact_id: 'old-1' }, + { id: 'place-2', current_contact_id: 'old-2' }, // no `contact` means clear it + ]; + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); + sinon.stub(db.medic, 'allDocs').resolves({ rows: [ + { doc: { _id: 'place-1', contact: { _id: 'old-1' } } }, + { doc: { _id: 'place-2', contact: 'old-2' } }, + ] }); + const medicBulk = sinon.stub(db.medic, 'bulkDocs').resolves(); + const log = stubActionWrites(); + + return service.processAction(action).then(() => { + expect(medicBulk.calledOnce).to.equal(true); + const updated = medicBulk.args[0][0]; + expect(updated.find(d => d._id === 'place-1').contact).to.deep.equal({ _id: 'new-1' }); + expect(updated.find(d => d._id === 'place-2').contact).to.equal(undefined); + + expect(action.cursor).to.equal(2); + expect(db.sentinel.put.called).to.equal(true); + + const entry = log.actions['bulk-operation-action:op:1']; + expect(entry.status).to.equal('completed'); + expect(entry.action).to.equal('set-contact'); + expect(entry.total_changes_count).to.equal(2); + expect(entry.failed_operations).to.equal(undefined); + + expect(db.sentinel.put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); + }); + }); + + it('records operations that no longer match as failed', () => { + const action = buildAction(); + const operations = [ + { id: 'place-1', contact: { _id: 'new-1' }, current_contact_id: 'old-1' }, + { id: 'gone', current_contact_id: 'old-2' }, + ]; + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); + sinon.stub(db.medic, 'allDocs').resolves({ rows: [ + { doc: { _id: 'place-1', contact: { _id: 'changed-since' } } }, // contact changed + { key: 'gone', error: 'not_found' }, // doc gone + ] }); + sinon.stub(db.medic, 'bulkDocs').resolves(); + const log = stubActionWrites(); + + return service.processAction(action).then(() => { + const entry = log.actions['bulk-operation-action:op:1']; + expect(entry.status).to.equal('failed'); + expect(entry.total_changes_count).to.equal(0); + expect(entry.failed_operations.map(op => op.id)).to.deep.equal(['place-1', 'gone']); + }); + }); + + it('deletes each linked user via the user-delete path for a delete-user action', () => { + const deleteUserStub = sinon.stub().resolves(); + service.__set__('userManagement', { deleteUser: deleteUserStub }); + + const action = buildAction({ action: 'delete-user' }); + const operations = [{ id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' }]; + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); + const log = stubActionWrites(); + + return service.processAction(action).then(() => { + expect(deleteUserStub.args.map(a => a[0])).to.deep.equal(['alice', 'bob']); + expect(log.actions['bulk-operation-action:op:1'].status).to.equal('completed'); + }); + }); + + it('records a user that fails to delete as a failed operation', () => { + const deleteUserStub = sinon.stub(); + deleteUserStub.withArgs('alice').resolves(); + deleteUserStub.withArgs('bob').rejects(new Error('boom')); + service.__set__('userManagement', { deleteUser: deleteUserStub }); + + const action = buildAction({ action: 'delete-user' }); + const operations = [{ id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' }]; + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); + const log = stubActionWrites(); + + return service.processAction(action).then(() => { + const entry = log.actions['bulk-operation-action:op:1']; + expect(entry.status).to.equal('failed'); + expect(entry.failed_operations.map(op => op.id)).to.deep.equal(['org.couchdb.user:bob']); + }); + }); + + it('throws when there is no handler for the action', () => { + const action = buildAction({ action: 'archive' }); + + return service + .processAction(action) + .then(() => chai.expect.fail('should have thrown')) + .catch(err => expect(err.message).to.contain('no handler')); + }); + }); + + describe('listen', () => { + it('queues and processes action docs already waiting in medic-sentinel', () => { + const processStub = sinon.stub().resolves(); + service.__set__('processAction', processStub); + sinon.stub(db.sentinel, 'allDocs').resolves({ rows: [ + { doc: { _id: 'bulk-operation-action:op:1' } }, + { doc: { _id: 'bulk-operation-action:op:2' } }, + ] }); + const changes = sinon.stub(db.sentinel, 'changes').returns({ on() { + return this; + } }); + + return service + .listen() + .then(() => new Promise(resolve => setTimeout(resolve, 20))) + .then(() => { + expect(changes.calledOnce).to.equal(true); + expect(processStub.callCount).to.equal(2); + expect(processStub.args.map(a => a[0]._id)).to.deep.equal([ + 'bulk-operation-action:op:1', + 'bulk-operation-action:op:2', + ]); + }); + }); + }); +}); From 50ed392371d7684549169d5f3b41c50c603ca010 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 09:56:30 +0530 Subject: [PATCH 06/16] feat(#10706): add contact hierarchy delete endpoints Add DELETE /api/v1/person/{id} and /api/v1/place/{id} over a shared delete-contact service that gathers the subtree, its reports (matched by uuid and shortcode), the primary-contact places to clear, and the linked users, then queues the bulk operation and returns the breakdown. Gated on can_delete_contact_hierarchy, with delete_users (requires can_delete_users) and dry_run query params. --- api/src/controllers/person.js | 72 +++++++++++ api/src/controllers/place.js | 72 +++++++++++ api/src/routing.js | 2 + api/src/services/delete-contact.js | 114 ++++++++++++++++++ api/tests/mocha/controllers/person.spec.js | 47 ++++++++ api/tests/mocha/controllers/place.spec.js | 47 ++++++++ .../mocha/services/delete-contact.spec.js | 113 +++++++++++++++++ 7 files changed, 467 insertions(+) create mode 100644 api/src/services/delete-contact.js create mode 100644 api/tests/mocha/services/delete-contact.spec.js diff --git a/api/src/controllers/person.js b/api/src/controllers/person.js index f9468ab39e5..6bcc62b6a76 100644 --- a/api/src/controllers/person.js +++ b/api/src/controllers/person.js @@ -2,6 +2,7 @@ const { Person, Qualifier } = require('@medic/cht-datasource'); const ctx = require('../services/data-context'); const serverUtils = require('../server-utils'); const auth = require('../auth'); +const deleteContactService = require('../services/delete-contact'); const getPerson = ctx.bind(Person.v1.get); const getPersonWithLineage = ctx.bind(Person.v1.getWithLineage); @@ -208,5 +209,76 @@ module.exports = { const updatedPersonDoc = await updatePerson(updatePersonInput); return res.json(updatedPersonDoc); }), + + /** + * @openapi + * /api/v1/person/{id}: + * delete: + * summary: Delete a person and their hierarchy + * operationId: v1PersonIdDelete + * description: > + * Queues an asynchronous bulk operation that removes the person, every descendant contact, + * and the reports they are the subject of, clears any dangling primary-contact references, + * and (with delete_users=true) removes linked user accounts. Returns a breakdown of the + * changes and the bulk operation id to poll. + * tags: [Person] + * x-since: 5.2.0 + * x-permissions: + * hasAll: [can_delete_contact_hierarchy] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * description: The id of the person to delete + * - in: query + * name: delete_users + * schema: + * type: boolean + * description: > + * Also delete user accounts linked to the removed contacts. Requires the + * can_delete_users permission. When not set, the request is rejected with 400 if any + * linked users exist. + * - in: query + * name: dry_run + * schema: + * type: boolean + * description: Return the breakdown of changes without queuing anything. + * responses: + * '202': + * description: The bulk operation was queued + * content: + * application/json: + * schema: + * type: object + * properties: + * breakdown: + * type: object + * id: + * type: string + * description: The bulk operation id to poll. + * '200': + * description: The dry-run breakdown (nothing queued) + * '400': + * $ref: '#/components/responses/BadRequest' + * '401': + * $ref: '#/components/responses/Unauthorized' + * '403': + * $ref: '#/components/responses/Forbidden' + * '404': + * $ref: '#/components/responses/NotFound' + */ + delete: serverUtils.doOrError(async (req, res) => { + const deleteUsers = req.query.delete_users === 'true'; + const dryRun = req.query.dry_run === 'true'; + const permissions = deleteUsers + ? ['can_delete_contact_hierarchy', 'can_delete_users'] + : ['can_delete_contact_hierarchy']; + await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); + + const result = await deleteContactService.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); + return res.status(dryRun ? 200 : 202).json(result); + }), }, }; diff --git a/api/src/controllers/place.js b/api/src/controllers/place.js index 02dfc657f9d..39b3cfa891e 100644 --- a/api/src/controllers/place.js +++ b/api/src/controllers/place.js @@ -2,6 +2,7 @@ const { Place, Qualifier } = require('@medic/cht-datasource'); const ctx = require('../services/data-context'); const serverUtils = require('../server-utils'); const auth = require('../auth'); +const deleteContactService = require('../services/delete-contact'); const getPlace = ctx.bind(Place.v1.get); const getPlaceWithLineage = ctx.bind(Place.v1.getWithLineage); @@ -214,6 +215,77 @@ module.exports = { }; const updatedPlaceDoc = await update(updatePlaceInput); return res.json(updatedPlaceDoc); + }), + + /** + * @openapi + * /api/v1/place/{id}: + * delete: + * summary: Delete a place and its hierarchy + * operationId: v1PlaceIdDelete + * description: > + * Queues an asynchronous bulk operation that removes the place, every descendant contact, + * and the reports they are the subject of, clears any dangling primary-contact references, + * and (with delete_users=true) removes linked user accounts. Returns a breakdown of the + * changes and the bulk operation id to poll. + * tags: [Place] + * x-since: 5.2.0 + * x-permissions: + * hasAll: [can_delete_contact_hierarchy] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * description: The id of the place to delete + * - in: query + * name: delete_users + * schema: + * type: boolean + * description: > + * Also delete user accounts linked to the removed contacts. Requires the + * can_delete_users permission. When not set, the request is rejected with 400 if any + * linked users exist. + * - in: query + * name: dry_run + * schema: + * type: boolean + * description: Return the breakdown of changes without queuing anything. + * responses: + * '202': + * description: The bulk operation was queued + * content: + * application/json: + * schema: + * type: object + * properties: + * breakdown: + * type: object + * id: + * type: string + * description: The bulk operation id to poll. + * '200': + * description: The dry-run breakdown (nothing queued) + * '400': + * $ref: '#/components/responses/BadRequest' + * '401': + * $ref: '#/components/responses/Unauthorized' + * '403': + * $ref: '#/components/responses/Forbidden' + * '404': + * $ref: '#/components/responses/NotFound' + */ + delete: serverUtils.doOrError(async (req, res) => { + const deleteUsers = req.query.delete_users === 'true'; + const dryRun = req.query.dry_run === 'true'; + const permissions = deleteUsers + ? ['can_delete_contact_hierarchy', 'can_delete_users'] + : ['can_delete_contact_hierarchy']; + await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); + + const result = await deleteContactService.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); + return res.status(dryRun ? 200 : 202).json(result); }) } }; diff --git a/api/src/routing.js b/api/src/routing.js index a3f55b50ac6..1c20e015fc9 100644 --- a/api/src/routing.js +++ b/api/src/routing.js @@ -664,6 +664,7 @@ app.get('/api/v1/place', place.v1.getAll); app.get('/api/v1/place/:uuid', place.v1.get); app.postJson('/api/v1/place', place.v1.create); app.putJson('/api/v1/place/:uuid', place.v1.update); +app.delete('/api/v1/place/:uuid', place.v1.delete); /** * @openapi @@ -739,6 +740,7 @@ app.get('/api/v1/person', person.v1.getAll); app.get('/api/v1/person/:uuid', person.v1.get); app.postJson('/api/v1/person', person.v1.create); app.putJson('/api/v1/person/:uuid', person.v1.update); +app.delete('/api/v1/person/:uuid', person.v1.delete); app.get('/api/v1/contact/uuid', contact.v1.getUuids); app.get('/api/v1/contact/:uuid', contact.v1.get); diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js new file mode 100644 index 00000000000..1ac9a485cc1 --- /dev/null +++ b/api/src/services/delete-contact.js @@ -0,0 +1,114 @@ +const db = require('../db'); +const bulkOperations = require('./bulk-operations'); +const { ACTIONS } = require('@medic/bulk-operations'); + +const httpError = (status, message) => { + const err = new Error(message); + err.status = status; + return err; +}; + +// `contacts_by_depth` keyed by a single id returns one row per contact in that subtree (the target +// and every descendant). Each row carries the contact's uuid (row.id) and its shortcode +// (row.value.shortcode), the same pairing the replication authorization service uses. +const getSubtree = (id) => db.medic.query('medic/contacts_by_depth', { key: [id] }); + +const getSubjectKeys = (rows) => { + const keys = []; + rows.forEach(row => { + keys.push(row.id); + if (row.value?.shortcode) { + keys.push(row.value.shortcode); + } + }); + return keys; +}; + +// Reports are matched by both the contact uuid and its shortcode, so a report that only records the +// shortcode is not missed. +const getReportIds = async (subjectKeys) => { + if (!subjectKeys.length) { + return []; + } + const result = await db.medic.query('medic-client/reports_by_subject', { keys: subjectKeys }); + return [ ...new Set(result.rows.map(row => row.id)) ]; +}; + +// Places whose primary contact is one of the deleted contacts, excluding places that are being +// deleted themselves. Each keeps the current primary id so the set-contact handler can verify it +// has not changed before clearing the reference. +const getPrimaryContactClears = async (contactIds) => { + const result = await db.medic.query('medic/contacts_by_primary_contact', { keys: contactIds }); + const deleted = new Set(contactIds); + const seen = new Set(); + const operations = []; + result.rows.forEach(row => { + if (deleted.has(row.id) || seen.has(row.id)) { + return; + } + seen.add(row.id); + operations.push({ id: row.id, current_contact_id: row.key }); + }); + return operations; +}; + +// Users whose facility is one of the deleted contacts (only places have linked users). +const getLinkedUserIds = async (contactIds) => { + const result = await db.users.query('users/users_by_field', { + keys: contactIds.map(id => [ 'facility_id', id ]), + }); + return [ ...new Set(result.rows.map(row => row.id)) ]; +}; + +/** + * Gathers everything a contact-hierarchy delete touches and queues it as a bulk operation. + * @param {string} id - the target contact id + * @param {{deleteUsers: boolean, dryRun: boolean}} options + * @returns {Promise<{breakdown: Object, id?: string}>} the breakdown of changes, and the bulk + * operation id when the operation was queued + * @throws a 404 when the contact does not exist, or a 400 when linked users would be left behind + * and `deleteUsers` was not requested + */ +const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { + const subtree = await getSubtree(id); + const contactIds = subtree.rows.map(row => row.id); + if (!contactIds.length) { + throw httpError(404, `Contact "${id}" not found`); + } + + const [ reportIds, setContactOperations, userIds ] = await Promise.all([ + getReportIds(getSubjectKeys(subtree.rows)), + getPrimaryContactClears(contactIds), + getLinkedUserIds(contactIds), + ]); + + if (userIds.length && !deleteUsers) { + throw httpError( + 400, + `${userIds.length} user(s) are linked to contacts in this hierarchy. ` + + `Set delete_users=true (requires can_delete_users) to remove them.` + ); + } + + const breakdown = { + archive: { contacts: contactIds.length, reports: reportIds.length }, + 'set-contact': setContactOperations.length, + 'delete-user': deleteUsers ? userIds.length : 0, + }; + + if (dryRun) { + return { breakdown }; + } + + const bulkOperationId = await bulkOperations.queue([ + { action: ACTIONS.ARCHIVE, operations: [ ...contactIds, ...reportIds ].map(docId => ({ id: docId })) }, + { action: ACTIONS.SET_CONTACT, operations: setContactOperations }, + { action: ACTIONS.DELETE_USER, operations: deleteUsers ? userIds.map(userId => ({ id: userId })) : [] }, + ]); + + return { breakdown, id: bulkOperationId }; +}; + +module.exports = { + deleteContactHierarchy, +}; diff --git a/api/tests/mocha/controllers/person.spec.js b/api/tests/mocha/controllers/person.spec.js index b94ad7817dd..d407858e31f 100644 --- a/api/tests/mocha/controllers/person.spec.js +++ b/api/tests/mocha/controllers/person.spec.js @@ -204,5 +204,52 @@ describe('Person Controller', () => { expect(res.json.calledOnceWithExactly(updatePersonDoc)).to.be.true; }); }); + + describe('delete', () => { + let deleteContact; + beforeEach(() => { + deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); + res.status = sinon.stub().returns(res); + }); + + it('queues the delete and responds 202 with the result', async () => { + req = { params: { uuid: '123' }, query: {} }; + const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; + deleteContact.resolves(result); + + await controller.v1.delete(req, res); + + expect(assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } + )).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; + expect(res.status.calledOnceWithExactly(202)).to.be.true; + expect(res.json.calledOnceWithExactly(result)).to.be.true; + }); + + it('also requires can_delete_users when delete_users=true', async () => { + req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; + deleteContact.resolves({ breakdown: {}, id: 'x' }); + + await controller.v1.delete(req, res); + + expect(assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } + )).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: true, dryRun: false })).to.be.true; + }); + + it('responds 200 for a dry run', async () => { + req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; + deleteContact.resolves({ breakdown: {} }); + + await controller.v1.delete(req, res); + + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: true })).to.be.true; + expect(res.status.calledOnceWithExactly(200)).to.be.true; + }); + }); }); }); diff --git a/api/tests/mocha/controllers/place.spec.js b/api/tests/mocha/controllers/place.spec.js index e29776761d8..fd531195f2c 100644 --- a/api/tests/mocha/controllers/place.spec.js +++ b/api/tests/mocha/controllers/place.spec.js @@ -212,5 +212,52 @@ describe('Place Controller', () => { expect(res.json.calledOnceWithExactly(updatePlaceDoc)).to.be.true; }); }); + + describe('delete', () => { + let deleteContact; + beforeEach(() => { + deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); + res.status = sinon.stub().returns(res); + }); + + it('queues the delete and responds 202 with the result', async () => { + req = { params: { uuid: '123' }, query: {} }; + const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; + deleteContact.resolves(result); + + await controller.v1.delete(req, res); + + expect(assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } + )).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; + expect(res.status.calledOnceWithExactly(202)).to.be.true; + expect(res.json.calledOnceWithExactly(result)).to.be.true; + }); + + it('also requires can_delete_users when delete_users=true', async () => { + req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; + deleteContact.resolves({ breakdown: {}, id: 'x' }); + + await controller.v1.delete(req, res); + + expect(assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } + )).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: true, dryRun: false })).to.be.true; + }); + + it('responds 200 for a dry run', async () => { + req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; + deleteContact.resolves({ breakdown: {} }); + + await controller.v1.delete(req, res); + + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: true })).to.be.true; + expect(res.status.calledOnceWithExactly(200)).to.be.true; + }); + }); }); }); diff --git a/api/tests/mocha/services/delete-contact.spec.js b/api/tests/mocha/services/delete-contact.spec.js new file mode 100644 index 00000000000..5c906cc05e6 --- /dev/null +++ b/api/tests/mocha/services/delete-contact.spec.js @@ -0,0 +1,113 @@ +const chai = require('chai'); +const sinon = require('sinon'); + +const db = require('../../../src/db'); +const bulkOperations = require('../../../src/services/bulk-operations'); +const service = require('../../../src/services/delete-contact'); + +const expect = chai.expect; + +describe('Delete contact service', () => { + afterEach(() => sinon.restore()); + + describe('deleteContactHierarchy', () => { + it('gathers the hierarchy and queues the bulk operation', () => { + sinon.stub(db.medic, 'query').callsFake((view) => { + if (view === 'medic/contacts_by_depth') { + return Promise.resolve({ rows: [ + { id: 'target', value: { shortcode: 'PID-1' } }, + { id: 'child', value: { shortcode: null } }, + ] }); + } + if (view === 'medic-client/reports_by_subject') { + return Promise.resolve({ rows: [ { id: 'report-1' }, { id: 'report-1' }, { id: 'report-2' } ] }); + } + if (view === 'medic/contacts_by_primary_contact') { + return Promise.resolve({ rows: [ + { id: 'parent-place', key: 'target' }, // parent whose primary is the target -> clear + { id: 'child', key: 'target' }, // child is itself being deleted -> skip + ] }); + } + return Promise.resolve({ rows: [] }); + }); + sinon.stub(db.users, 'query').resolves({ rows: [ { id: 'org.couchdb.user:chw' } ] }); + const queue = sinon.stub(bulkOperations, 'queue').resolves('bulk-operation:xyz'); + + return service.deleteContactHierarchy('target', { deleteUsers: true, dryRun: false }).then(result => { + expect(result).to.deep.equal({ + breakdown: { + archive: { contacts: 2, reports: 2 }, + 'set-contact': 1, + 'delete-user': 1, + }, + id: 'bulk-operation:xyz', + }); + + // reports matched by uuid + shortcode + const reportsCall = db.medic.query.getCalls().find(c => c.args[0] === 'medic-client/reports_by_subject'); + expect(reportsCall.args[1].keys).to.deep.equal([ 'target', 'PID-1', 'child' ]); + + // linked users looked up by facility + expect(db.users.query.args[0][1].keys).to.deep.equal([ + [ 'facility_id', 'target' ], + [ 'facility_id', 'child' ], + ]); + + // the queued action lists + const byAction = Object.fromEntries(queue.args[0][0].map(a => [ a.action, a.operations ])); + expect(byAction.archive.map(o => o.id)).to.deep.equal([ 'target', 'child', 'report-1', 'report-2' ]); + expect(byAction['set-contact']).to.deep.equal([ { id: 'parent-place', current_contact_id: 'target' } ]); + expect(byAction['delete-user']).to.deep.equal([ { id: 'org.couchdb.user:chw' } ]); + }); + }); + + it('throws 404 when the contact does not exist', () => { + sinon.stub(db.medic, 'query').resolves({ rows: [] }); + + return service + .deleteContactHierarchy('missing', {}) + .then(() => chai.expect.fail('should have thrown')) + .catch(err => expect(err.status).to.equal(404)); + }); + + it('throws 400 when linked users exist and delete_users is not set', () => { + sinon.stub(db.medic, 'query').callsFake((view) => { + if (view === 'medic/contacts_by_depth') { + return Promise.resolve({ rows: [ { id: 'place', value: {} } ] }); + } + return Promise.resolve({ rows: [] }); + }); + sinon.stub(db.users, 'query').resolves({ rows: [ { id: 'org.couchdb.user:chw' } ] }); + const queue = sinon.stub(bulkOperations, 'queue').resolves('x'); + + return service + .deleteContactHierarchy('place', { deleteUsers: false }) + .then(() => chai.expect.fail('should have thrown')) + .catch(err => { + expect(err.status).to.equal(400); + expect(queue.called).to.equal(false); + }); + }); + + it('returns the breakdown without queuing for a dry run', () => { + sinon.stub(db.medic, 'query').callsFake((view) => { + if (view === 'medic/contacts_by_depth') { + return Promise.resolve({ rows: [ { id: 'place', value: {} } ] }); + } + if (view === 'medic-client/reports_by_subject') { + return Promise.resolve({ rows: [ { id: 'r1' } ] }); + } + return Promise.resolve({ rows: [] }); + }); + sinon.stub(db.users, 'query').resolves({ rows: [] }); + const queue = sinon.stub(bulkOperations, 'queue').resolves('x'); + + return service.deleteContactHierarchy('place', { dryRun: true }).then(result => { + expect(result).to.deep.equal({ + breakdown: { archive: { contacts: 1, reports: 1 }, 'set-contact': 0, 'delete-user': 0 }, + }); + expect(queue.called).to.equal(false); + }); + }); + }); +}); From 77448a5497098b79a4ad3da69ae583cc48d8aee2 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 11:26:41 +0530 Subject: [PATCH 07/16] refactor(#10706): reduce delete-contact service complexity Compute the delete-user operations once (lowering the function's cognitive complexity) and drop an unreachable empty-subject guard. --- api/src/services/delete-contact.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js index 1ac9a485cc1..39613a6b0b8 100644 --- a/api/src/services/delete-contact.js +++ b/api/src/services/delete-contact.js @@ -27,9 +27,6 @@ const getSubjectKeys = (rows) => { // Reports are matched by both the contact uuid and its shortcode, so a report that only records the // shortcode is not missed. const getReportIds = async (subjectKeys) => { - if (!subjectKeys.length) { - return []; - } const result = await db.medic.query('medic-client/reports_by_subject', { keys: subjectKeys }); return [ ...new Set(result.rows.map(row => row.id)) ]; }; @@ -90,10 +87,11 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { ); } + const userOperations = deleteUsers ? userIds.map(userId => ({ id: userId })) : []; const breakdown = { archive: { contacts: contactIds.length, reports: reportIds.length }, 'set-contact': setContactOperations.length, - 'delete-user': deleteUsers ? userIds.length : 0, + 'delete-user': userOperations.length, }; if (dryRun) { @@ -103,7 +101,7 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { const bulkOperationId = await bulkOperations.queue([ { action: ACTIONS.ARCHIVE, operations: [ ...contactIds, ...reportIds ].map(docId => ({ id: docId })) }, { action: ACTIONS.SET_CONTACT, operations: setContactOperations }, - { action: ACTIONS.DELETE_USER, operations: deleteUsers ? userIds.map(userId => ({ id: userId })) : [] }, + { action: ACTIONS.DELETE_USER, operations: userOperations }, ]); return { breakdown, id: bulkOperationId }; From 93d333cfade49b249c2514dd94f6f40819c3b147 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 11:26:41 +0530 Subject: [PATCH 08/16] test(#10706): cover the sentinel bulk-operation listener Exercise the feed change handler, dedup, the empty-batch guard and a failing action, use specific assertions, and mark the changes-feed error retry (live feed infrastructure) as ignored for coverage. --- sentinel/src/lib/bulk-operations.js | 2 +- .../tests/unit/lib/bulk-operations.spec.js | 47 ++++++++++++++----- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/sentinel/src/lib/bulk-operations.js b/sentinel/src/lib/bulk-operations.js index 86fffc77894..51fafa7936f 100644 --- a/sentinel/src/lib/bulk-operations.js +++ b/sentinel/src/lib/bulk-operations.js @@ -159,7 +159,7 @@ const registerFeed = () => { enqueue(change.doc); } }) - .on('error', (err) => { + .on('error', /* istanbul ignore next: feed-error retry, exercised via integration tests */ (err) => { logger.error('bulk-operations: changes feed error: %o', err); setTimeout(registerFeed, RETRY_TIMEOUT); }); diff --git a/sentinel/tests/unit/lib/bulk-operations.spec.js b/sentinel/tests/unit/lib/bulk-operations.spec.js index 95e56f57395..afc54eb750e 100644 --- a/sentinel/tests/unit/lib/bulk-operations.spec.js +++ b/sentinel/tests/unit/lib/bulk-operations.spec.js @@ -54,7 +54,7 @@ describe('bulk-operations sentinel processor', () => { expect(medicBulk.calledOnce).to.equal(true); const updated = medicBulk.args[0][0]; expect(updated.find(d => d._id === 'place-1').contact).to.deep.equal({ _id: 'new-1' }); - expect(updated.find(d => d._id === 'place-2').contact).to.equal(undefined); + expect(updated.find(d => d._id === 'place-2').contact).to.be.undefined; expect(action.cursor).to.equal(2); expect(db.sentinel.put.called).to.equal(true); @@ -63,7 +63,7 @@ describe('bulk-operations sentinel processor', () => { expect(entry.status).to.equal('completed'); expect(entry.action).to.equal('set-contact'); expect(entry.total_changes_count).to.equal(2); - expect(entry.failed_operations).to.equal(undefined); + expect(entry.failed_operations).to.be.undefined; expect(db.sentinel.put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); }); @@ -132,31 +132,52 @@ describe('bulk-operations sentinel processor', () => { .then(() => chai.expect.fail('should have thrown')) .catch(err => expect(err.message).to.contain('no handler')); }); + + it('stops safely when an action has fewer operations than its total', () => { + const action = buildAction({ total: 1 }); + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify([]))); + stubActionWrites(); + + return service.processAction(action).then(() => { + expect(db.sentinel.put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); + }); + }); }); describe('listen', () => { - it('queues and processes action docs already waiting in medic-sentinel', () => { - const processStub = sinon.stub().resolves(); + it('processes waiting and feed-delivered actions, dedupes, and ignores irrelevant changes', () => { + const processStub = sinon.stub(); + processStub.onFirstCall().rejects(new Error('processing error')); // exercises the queue error handler + processStub.resolves(); service.__set__('processAction', processStub); - sinon.stub(db.sentinel, 'allDocs').resolves({ rows: [ - { doc: { _id: 'bulk-operation-action:op:1' } }, - { doc: { _id: 'bulk-operation-action:op:2' } }, - ] }); - const changes = sinon.stub(db.sentinel, 'changes').returns({ on() { - return this; - } }); + sinon.stub(db.sentinel, 'allDocs').resolves({ rows: [{ doc: { _id: 'bulk-operation-action:op:1' } }] }); + let onChange; + const changes = sinon.stub(db.sentinel, 'changes').returns({ + on(event, cb) { + if (event === 'change') { + onChange = cb; + } + return this; + }, + }); return service .listen() - .then(() => new Promise(resolve => setTimeout(resolve, 20))) .then(() => { expect(changes.calledOnce).to.equal(true); - expect(processStub.callCount).to.equal(2); + onChange({ id: 'bulk-operation-action:op:2', doc: { _id: 'bulk-operation-action:op:2' } }); + onChange({ id: 'bulk-operation-action:op:2', doc: { _id: 'bulk-operation-action:op:2' } }); // already queued + onChange({ id: 'bulk-operation-action:op:3', deleted: true }); // ignored: deleted + onChange({ id: 'not-an-action', doc: { _id: 'not-an-action' } }); // ignored: wrong prefix + return new Promise(resolve => setTimeout(resolve, 20)); + }) + .then(() => { expect(processStub.args.map(a => a[0]._id)).to.deep.equal([ 'bulk-operation-action:op:1', 'bulk-operation-action:op:2', ]); }); }); + }); }); From 1526ef0fc6b0423814a410f723af9e654f5faf04 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 11:58:03 +0530 Subject: [PATCH 09/16] docs(#10706): use standard JSDoc types so the generated docs build passes The docs build's JSDoc parser rejected the TypeScript-style optional-property type on deleteContactHierarchy's @returns; replace it and the record-in-generic param types with plain JSDoc types. --- api/src/services/delete-contact.js | 12 +++++++----- shared-libs/bulk-operations/src/index.js | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js index 39613a6b0b8..133f91183d9 100644 --- a/api/src/services/delete-contact.js +++ b/api/src/services/delete-contact.js @@ -60,11 +60,13 @@ const getLinkedUserIds = async (contactIds) => { /** * Gathers everything a contact-hierarchy delete touches and queues it as a bulk operation. * @param {string} id - the target contact id - * @param {{deleteUsers: boolean, dryRun: boolean}} options - * @returns {Promise<{breakdown: Object, id?: string}>} the breakdown of changes, and the bulk - * operation id when the operation was queued - * @throws a 404 when the contact does not exist, or a 400 when linked users would be left behind - * and `deleteUsers` was not requested + * @param {Object} options + * @param {boolean} options.deleteUsers - also remove users linked to the deleted contacts + * @param {boolean} options.dryRun - return the breakdown without queuing anything + * @returns {Promise} the breakdown of changes, plus the bulk operation id when the + * operation was queued (omitted for a dry run) + * @throws {Error} a 404 when the contact does not exist, or a 400 when linked users would be left + * behind and `deleteUsers` was not requested */ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { const subtree = await getSubtree(id); diff --git a/shared-libs/bulk-operations/src/index.js b/shared-libs/bulk-operations/src/index.js index d84078cce83..2e26eaaab83 100644 --- a/shared-libs/bulk-operations/src/index.js +++ b/shared-libs/bulk-operations/src/index.js @@ -63,11 +63,11 @@ const buildLogAction = (action, totalChangesCount, date) => ({ /** * Builds the documents that make up a single bulk operation. - * @param {Array<{action: string, operations: Object[]}>} actionOperations - one entry per action type, each holding + * @param {Object[]} actionOperations - one entry per action type (`{ action, operations }`), each holding * its list of per-item params * @param {Date} date - the operation start date, also the initial updated_date of every action - * @returns {{log: Object, actions: Object[]}} the log document (for medic-logs) and the action documents - * (for medic-sentinel); the log's `actions` map is keyed by each action document's `_id` + * @returns {Object} the log document (for medic-logs) and the action documents (for medic-sentinel) + * as `{ log, actions }`; the log's `actions` map is keyed by each action document's `_id` */ const buildBulkOperation = (actionOperations, date) => { const operationId = generateOperationId(); From 2b7b945add7605f86592a1130fb0ef1dd4f4dd08 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Sat, 4 Jul 2026 12:30:09 +0530 Subject: [PATCH 10/16] feat(#10706): declare the can_delete_contact_hierarchy permission Add can_delete_contact_hierarchy to the default permissions with no role, so it is assignable and shows in the admin UI. Admins still pass the gate by default. --- config/default/app_settings.json | 1 + 1 file changed, 1 insertion(+) diff --git a/config/default/app_settings.json b/config/default/app_settings.json index 1d91b088dbb..db2b1987ec9 100644 --- a/config/default/app_settings.json +++ b/config/default/app_settings.json @@ -122,6 +122,7 @@ "can_create_users": [ "program_officer" ], + "can_delete_contact_hierarchy": [], "can_delete_contacts": [ "program_officer", "chw_supervisor", From a21c0632e6bda8d8e456b475278239138360129e Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 6 Jul 2026 16:21:51 +0530 Subject: [PATCH 11/16] refactor(#10706): share the contact-delete handler and OpenAPI docs The person and place DELETE endpoints delete a hierarchy the same way, so their controller handlers and @openapi blocks were near-identical. Move the shared handler into the delete-contact service (the controllers now just point their delete at it) and pull the delete_users/dry_run params and the response shapes into shared OpenAPI components referenced from both. --- api/src/controllers/person.js | 41 ++----------- api/src/controllers/place.js | 41 ++----------- api/src/services/delete-contact.js | 21 +++++++ api/tests/mocha/controllers/person.spec.js | 45 +------------- api/tests/mocha/controllers/place.spec.js | 45 +------------- .../mocha/services/delete-contact.spec.js | 59 +++++++++++++++++++ scripts/build/generate-openapi.js | 31 +++++++++- 7 files changed, 124 insertions(+), 159 deletions(-) diff --git a/api/src/controllers/person.js b/api/src/controllers/person.js index 6bcc62b6a76..04c7b58b801 100644 --- a/api/src/controllers/person.js +++ b/api/src/controllers/person.js @@ -232,34 +232,13 @@ module.exports = { * schema: * type: string * description: The id of the person to delete - * - in: query - * name: delete_users - * schema: - * type: boolean - * description: > - * Also delete user accounts linked to the removed contacts. Requires the - * can_delete_users permission. When not set, the request is rejected with 400 if any - * linked users exist. - * - in: query - * name: dry_run - * schema: - * type: boolean - * description: Return the breakdown of changes without queuing anything. + * - $ref: '#/components/parameters/deleteUsers' + * - $ref: '#/components/parameters/dryRun' * responses: * '202': - * description: The bulk operation was queued - * content: - * application/json: - * schema: - * type: object - * properties: - * breakdown: - * type: object - * id: - * type: string - * description: The bulk operation id to poll. + * $ref: '#/components/responses/BulkOperationQueued' * '200': - * description: The dry-run breakdown (nothing queued) + * $ref: '#/components/responses/BulkOperationDryRun' * '400': * $ref: '#/components/responses/BadRequest' * '401': @@ -269,16 +248,6 @@ module.exports = { * '404': * $ref: '#/components/responses/NotFound' */ - delete: serverUtils.doOrError(async (req, res) => { - const deleteUsers = req.query.delete_users === 'true'; - const dryRun = req.query.dry_run === 'true'; - const permissions = deleteUsers - ? ['can_delete_contact_hierarchy', 'can_delete_users'] - : ['can_delete_contact_hierarchy']; - await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); - - const result = await deleteContactService.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); - return res.status(dryRun ? 200 : 202).json(result); - }), + delete: deleteContactService.handleDelete, }, }; diff --git a/api/src/controllers/place.js b/api/src/controllers/place.js index 39b3cfa891e..5589c47be10 100644 --- a/api/src/controllers/place.js +++ b/api/src/controllers/place.js @@ -239,34 +239,13 @@ module.exports = { * schema: * type: string * description: The id of the place to delete - * - in: query - * name: delete_users - * schema: - * type: boolean - * description: > - * Also delete user accounts linked to the removed contacts. Requires the - * can_delete_users permission. When not set, the request is rejected with 400 if any - * linked users exist. - * - in: query - * name: dry_run - * schema: - * type: boolean - * description: Return the breakdown of changes without queuing anything. + * - $ref: '#/components/parameters/deleteUsers' + * - $ref: '#/components/parameters/dryRun' * responses: * '202': - * description: The bulk operation was queued - * content: - * application/json: - * schema: - * type: object - * properties: - * breakdown: - * type: object - * id: - * type: string - * description: The bulk operation id to poll. + * $ref: '#/components/responses/BulkOperationQueued' * '200': - * description: The dry-run breakdown (nothing queued) + * $ref: '#/components/responses/BulkOperationDryRun' * '400': * $ref: '#/components/responses/BadRequest' * '401': @@ -276,16 +255,6 @@ module.exports = { * '404': * $ref: '#/components/responses/NotFound' */ - delete: serverUtils.doOrError(async (req, res) => { - const deleteUsers = req.query.delete_users === 'true'; - const dryRun = req.query.dry_run === 'true'; - const permissions = deleteUsers - ? ['can_delete_contact_hierarchy', 'can_delete_users'] - : ['can_delete_contact_hierarchy']; - await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); - - const result = await deleteContactService.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); - return res.status(dryRun ? 200 : 202).json(result); - }) + delete: deleteContactService.handleDelete } }; diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js index 133f91183d9..e4b417848e3 100644 --- a/api/src/services/delete-contact.js +++ b/api/src/services/delete-contact.js @@ -1,4 +1,6 @@ const db = require('../db'); +const auth = require('../auth'); +const serverUtils = require('../server-utils'); const bulkOperations = require('./bulk-operations'); const { ACTIONS } = require('@medic/bulk-operations'); @@ -109,6 +111,25 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { return { breakdown, id: bulkOperationId }; }; +/** + * Express handler shared by the person and place DELETE endpoints. Deleting a contact hierarchy is + * the same regardless of contact type, so both controllers point their `delete` at this: it reads + * the `delete_users`/`dry_run` query params, asserts the required permissions, hands off to + * `deleteContactHierarchy`, and responds with the breakdown (202 when queued, 200 for a dry run). + */ +const handleDelete = serverUtils.doOrError(async (req, res) => { + const deleteUsers = req.query.delete_users === 'true'; + const dryRun = req.query.dry_run === 'true'; + const permissions = deleteUsers + ? ['can_delete_contact_hierarchy', 'can_delete_users'] + : ['can_delete_contact_hierarchy']; + await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); + + const result = await module.exports.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); + return res.status(dryRun ? 200 : 202).json(result); +}); + module.exports = { deleteContactHierarchy, + handleDelete, }; diff --git a/api/tests/mocha/controllers/person.spec.js b/api/tests/mocha/controllers/person.spec.js index d407858e31f..a8bb2aa0a86 100644 --- a/api/tests/mocha/controllers/person.spec.js +++ b/api/tests/mocha/controllers/person.spec.js @@ -206,49 +206,8 @@ describe('Person Controller', () => { }); describe('delete', () => { - let deleteContact; - beforeEach(() => { - deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); - res.status = sinon.stub().returns(res); - }); - - it('queues the delete and responds 202 with the result', async () => { - req = { params: { uuid: '123' }, query: {} }; - const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; - deleteContact.resolves(result); - - await controller.v1.delete(req, res); - - expect(assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } - )).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; - expect(res.status.calledOnceWithExactly(202)).to.be.true; - expect(res.json.calledOnceWithExactly(result)).to.be.true; - }); - - it('also requires can_delete_users when delete_users=true', async () => { - req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; - deleteContact.resolves({ breakdown: {}, id: 'x' }); - - await controller.v1.delete(req, res); - - expect(assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } - )).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: true, dryRun: false })).to.be.true; - }); - - it('responds 200 for a dry run', async () => { - req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; - deleteContact.resolves({ breakdown: {} }); - - await controller.v1.delete(req, res); - - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: true })).to.be.true; - expect(res.status.calledOnceWithExactly(200)).to.be.true; + it('delegates to the shared delete-contact handler', () => { + expect(controller.v1.delete).to.equal(require('../../../src/services/delete-contact').handleDelete); }); }); }); diff --git a/api/tests/mocha/controllers/place.spec.js b/api/tests/mocha/controllers/place.spec.js index fd531195f2c..2f57ce4345d 100644 --- a/api/tests/mocha/controllers/place.spec.js +++ b/api/tests/mocha/controllers/place.spec.js @@ -214,49 +214,8 @@ describe('Place Controller', () => { }); describe('delete', () => { - let deleteContact; - beforeEach(() => { - deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); - res.status = sinon.stub().returns(res); - }); - - it('queues the delete and responds 202 with the result', async () => { - req = { params: { uuid: '123' }, query: {} }; - const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; - deleteContact.resolves(result); - - await controller.v1.delete(req, res); - - expect(assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } - )).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; - expect(res.status.calledOnceWithExactly(202)).to.be.true; - expect(res.json.calledOnceWithExactly(result)).to.be.true; - }); - - it('also requires can_delete_users when delete_users=true', async () => { - req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; - deleteContact.resolves({ breakdown: {}, id: 'x' }); - - await controller.v1.delete(req, res); - - expect(assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } - )).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: true, dryRun: false })).to.be.true; - }); - - it('responds 200 for a dry run', async () => { - req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; - deleteContact.resolves({ breakdown: {} }); - - await controller.v1.delete(req, res); - - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: true })).to.be.true; - expect(res.status.calledOnceWithExactly(200)).to.be.true; + it('delegates to the shared delete-contact handler', () => { + expect(controller.v1.delete).to.equal(require('../../../src/services/delete-contact').handleDelete); }); }); }); diff --git a/api/tests/mocha/services/delete-contact.spec.js b/api/tests/mocha/services/delete-contact.spec.js index 5c906cc05e6..589e805bece 100644 --- a/api/tests/mocha/services/delete-contact.spec.js +++ b/api/tests/mocha/services/delete-contact.spec.js @@ -2,6 +2,7 @@ const chai = require('chai'); const sinon = require('sinon'); const db = require('../../../src/db'); +const auth = require('../../../src/auth'); const bulkOperations = require('../../../src/services/bulk-operations'); const service = require('../../../src/services/delete-contact'); @@ -110,4 +111,62 @@ describe('Delete contact service', () => { }); }); }); + + describe('handleDelete', () => { + let req; + let res; + + beforeEach(() => { + sinon.stub(auth, 'assertPermissions').resolves(); + res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + }); + + it('queues the delete and responds 202 with the result', async () => { + req = { params: { uuid: '123' }, query: {} }; + const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; + sinon.stub(service, 'deleteContactHierarchy').resolves(result); + + await service.handleDelete(req, res); + + expect(auth.assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } + )).to.be.true; + expect(service.deleteContactHierarchy.calledOnceWithExactly( + '123', + { deleteUsers: false, dryRun: false } + )).to.be.true; + expect(res.status.calledOnceWithExactly(202)).to.be.true; + expect(res.json.calledOnceWithExactly(result)).to.be.true; + }); + + it('also requires can_delete_users when delete_users=true', async () => { + req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; + sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {}, id: 'x' }); + + await service.handleDelete(req, res); + + expect(auth.assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } + )).to.be.true; + expect(service.deleteContactHierarchy.calledOnceWithExactly( + '123', + { deleteUsers: true, dryRun: false } + )).to.be.true; + }); + + it('responds 200 for a dry run', async () => { + req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; + sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {} }); + + await service.handleDelete(req, res); + + expect(service.deleteContactHierarchy.calledOnceWithExactly( + '123', + { deleteUsers: false, dryRun: true } + )).to.be.true; + expect(res.status.calledOnceWithExactly(200)).to.be.true; + }); + }); }); diff --git a/scripts/build/generate-openapi.js b/scripts/build/generate-openapi.js index 87bada620ba..db68bea1e1e 100644 --- a/scripts/build/generate-openapi.js +++ b/scripts/build/generate-openapi.js @@ -89,13 +89,42 @@ const SWAGGER_OPTIONS = { name: 'with_lineage', schema: { 'enum': ['true', 'false'], default: 'false' }, description: 'Include the full parent lineage.' + }, + deleteUsers: { + in: 'query', + name: 'delete_users', + schema: { type: 'boolean' }, + description: + 'Also delete user accounts linked to the removed contacts. Requires the can_delete_users ' + + 'permission. When not set, the request is rejected with 400 if any linked users exist.', + }, + dryRun: { + in: 'query', + name: 'dry_run', + schema: { type: 'boolean' }, + description: 'Return the breakdown of changes without queuing anything.', } }, responses: { NotFound: { description: 'Entity not found' }, BadRequest: { description: 'Invalid input (missing required fields, invalid types, etc.)' }, Unauthorized: { description: 'Not authenticated' }, - Forbidden: { description: 'Insufficient permissions' } + Forbidden: { description: 'Insufficient permissions' }, + BulkOperationQueued: { + description: 'The bulk operation was queued', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + breakdown: { type: 'object' }, + id: { type: 'string', description: 'The bulk operation id to poll.' } + } + } + } + } + }, + BulkOperationDryRun: { description: 'The dry-run breakdown (nothing queued)' } } }, }, From 8428c7f9aa29471d24d9709f67ad341d30525d28 Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 6 Jul 2026 18:28:27 +0530 Subject: [PATCH 12/16] feat(#10706): enforce contact type on the delete endpoints The person and place DELETE endpoints shared one handler and neither checked the target's type, so a place id sent to the person endpoint would have deleted the whole place hierarchy. Each endpoint now passes its own type getter to the shared handler, which rejects an id of the wrong type with a 404, the same way the GET endpoints already do. The gather-and-queue logic stays type-agnostic in deleteContactHierarchy. --- api/src/controllers/person.js | 5 ++- api/src/controllers/place.js | 5 ++- api/src/services/delete-contact.js | 25 +++++++++++---- api/tests/mocha/controllers/person.spec.js | 32 +++++++++++++++++-- api/tests/mocha/controllers/place.spec.js | 32 +++++++++++++++++-- .../mocha/services/delete-contact.spec.js | 30 ++++++++++++++--- 6 files changed, 113 insertions(+), 16 deletions(-) diff --git a/api/src/controllers/person.js b/api/src/controllers/person.js index 04c7b58b801..8cffadf174a 100644 --- a/api/src/controllers/person.js +++ b/api/src/controllers/person.js @@ -248,6 +248,9 @@ module.exports = { * '404': * $ref: '#/components/responses/NotFound' */ - delete: deleteContactService.handleDelete, + delete: deleteContactService.handleDelete({ + get: (uuid) => getPerson(Qualifier.byUuid(uuid)), + type: 'Person', + }), }, }; diff --git a/api/src/controllers/place.js b/api/src/controllers/place.js index 5589c47be10..6357239a1cd 100644 --- a/api/src/controllers/place.js +++ b/api/src/controllers/place.js @@ -255,6 +255,9 @@ module.exports = { * '404': * $ref: '#/components/responses/NotFound' */ - delete: deleteContactService.handleDelete + delete: deleteContactService.handleDelete({ + get: (uuid) => getPlace(Qualifier.byUuid(uuid)), + type: 'Place', + }) } }; diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js index e4b417848e3..7f721710d67 100644 --- a/api/src/services/delete-contact.js +++ b/api/src/services/delete-contact.js @@ -112,12 +112,19 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { }; /** - * Express handler shared by the person and place DELETE endpoints. Deleting a contact hierarchy is - * the same regardless of contact type, so both controllers point their `delete` at this: it reads - * the `delete_users`/`dry_run` query params, asserts the required permissions, hands off to - * `deleteContactHierarchy`, and responds with the breakdown (202 when queued, 200 for a dry run). + * Builds the DELETE express handler for a contact type. The person and place endpoints delete a + * hierarchy the same way, so they share this handler; each passes the pieces that make its endpoint + * type-specific. `get` fetches the target as its own type and returns null for the wrong type, so a + * place id cannot be deleted through the person endpoint or vice versa, and `type` names it for the + * not-found message. The handler reads the `delete_users`/`dry_run` query params, asserts the + * required permissions, hands the type-agnostic work off to `deleteContactHierarchy`, and responds + * with the breakdown (202 when queued, 200 for a dry run). + * @param {Object} options + * @param {Function} options.get - fetches the target contact by uuid, or null when it is not this type + * @param {string} options.type - the contact type name, used in the not-found message + * @returns {Function} the express request handler */ -const handleDelete = serverUtils.doOrError(async (req, res) => { +const handleDelete = ({ get, type }) => serverUtils.doOrError(async (req, res) => { const deleteUsers = req.query.delete_users === 'true'; const dryRun = req.query.dry_run === 'true'; const permissions = deleteUsers @@ -125,7 +132,13 @@ const handleDelete = serverUtils.doOrError(async (req, res) => { : ['can_delete_contact_hierarchy']; await auth.assertPermissions(req, { isOnline: true, hasAll: permissions }); - const result = await module.exports.deleteContactHierarchy(req.params.uuid, { deleteUsers, dryRun }); + const { uuid } = req.params; + const contact = await get(uuid); + if (!contact) { + return serverUtils.error({ status: 404, message: `${type} not found` }, req, res); + } + + const result = await module.exports.deleteContactHierarchy(uuid, { deleteUsers, dryRun }); return res.status(dryRun ? 200 : 202).json(result); }); diff --git a/api/tests/mocha/controllers/person.spec.js b/api/tests/mocha/controllers/person.spec.js index a8bb2aa0a86..0d2b9e69f42 100644 --- a/api/tests/mocha/controllers/person.spec.js +++ b/api/tests/mocha/controllers/person.spec.js @@ -206,8 +206,36 @@ describe('Person Controller', () => { }); describe('delete', () => { - it('delegates to the shared delete-contact handler', () => { - expect(controller.v1.delete).to.equal(require('../../../src/services/delete-contact').handleDelete); + let deleteContact; + beforeEach(() => { + deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); + res.status = sinon.stub().returns(res); + }); + + it('deletes the hierarchy when the id is a person', async () => { + req = { params: { uuid: '123' }, query: {} }; + personGet.resolves({ _id: '123' }); + deleteContact.resolves({ breakdown: {}, id: 'bulk-operation:1' }); + + await controller.v1.delete(req, res); + + expect(personGet.calledOnceWithExactly(Qualifier.byUuid('123'))).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; + expect(res.status.calledOnceWithExactly(202)).to.be.true; + }); + + it('responds 404 and does not delete when the id is not a person', async () => { + req = { params: { uuid: 'place-1' }, query: {} }; + personGet.resolves(null); + + await controller.v1.delete(req, res); + + expect(serverUtilsError.calledOnceWithExactly( + { status: 404, message: 'Person not found' }, + req, + res + )).to.be.true; + expect(deleteContact.notCalled).to.be.true; }); }); }); diff --git a/api/tests/mocha/controllers/place.spec.js b/api/tests/mocha/controllers/place.spec.js index 2f57ce4345d..a36aad2fadb 100644 --- a/api/tests/mocha/controllers/place.spec.js +++ b/api/tests/mocha/controllers/place.spec.js @@ -214,8 +214,36 @@ describe('Place Controller', () => { }); describe('delete', () => { - it('delegates to the shared delete-contact handler', () => { - expect(controller.v1.delete).to.equal(require('../../../src/services/delete-contact').handleDelete); + let deleteContact; + beforeEach(() => { + deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); + res.status = sinon.stub().returns(res); + }); + + it('deletes the hierarchy when the id is a place', async () => { + req = { params: { uuid: '123' }, query: {} }; + placeGet.resolves({ _id: '123' }); + deleteContact.resolves({ breakdown: {}, id: 'bulk-operation:1' }); + + await controller.v1.delete(req, res); + + expect(placeGet.calledOnceWithExactly(Qualifier.byUuid('123'))).to.be.true; + expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; + expect(res.status.calledOnceWithExactly(202)).to.be.true; + }); + + it('responds 404 and does not delete when the id is not a place', async () => { + req = { params: { uuid: 'person-1' }, query: {} }; + placeGet.resolves(null); + + await controller.v1.delete(req, res); + + expect(serverUtilsError.calledOnceWithExactly( + { status: 404, message: 'Place not found' }, + req, + res + )).to.be.true; + expect(deleteContact.notCalled).to.be.true; }); }); }); diff --git a/api/tests/mocha/services/delete-contact.spec.js b/api/tests/mocha/services/delete-contact.spec.js index 589e805bece..88c4d772711 100644 --- a/api/tests/mocha/services/delete-contact.spec.js +++ b/api/tests/mocha/services/delete-contact.spec.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const db = require('../../../src/db'); const auth = require('../../../src/auth'); +const serverUtils = require('../../../src/server-utils'); const bulkOperations = require('../../../src/services/bulk-operations'); const service = require('../../../src/services/delete-contact'); @@ -115,19 +116,25 @@ describe('Delete contact service', () => { describe('handleDelete', () => { let req; let res; + let get; + let handler; beforeEach(() => { sinon.stub(auth, 'assertPermissions').resolves(); + sinon.stub(serverUtils, 'error'); res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + get = sinon.stub().resolves({ _id: '123' }); + handler = service.handleDelete({ get, type: 'Person' }); }); - it('queues the delete and responds 202 with the result', async () => { + it('validates the type, queues the delete, and responds 202 with the result', async () => { req = { params: { uuid: '123' }, query: {} }; const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; sinon.stub(service, 'deleteContactHierarchy').resolves(result); - await service.handleDelete(req, res); + await handler(req, res); + expect(get.calledOnceWithExactly('123')).to.be.true; expect(auth.assertPermissions.calledOnceWithExactly( req, { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } @@ -144,7 +151,7 @@ describe('Delete contact service', () => { req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {}, id: 'x' }); - await service.handleDelete(req, res); + await handler(req, res); expect(auth.assertPermissions.calledOnceWithExactly( req, @@ -160,7 +167,7 @@ describe('Delete contact service', () => { req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {} }); - await service.handleDelete(req, res); + await handler(req, res); expect(service.deleteContactHierarchy.calledOnceWithExactly( '123', @@ -168,5 +175,20 @@ describe('Delete contact service', () => { )).to.be.true; expect(res.status.calledOnceWithExactly(200)).to.be.true; }); + + it('responds 404 and does not delete when the target is not the expected type', async () => { + req = { params: { uuid: 'place-1' }, query: {} }; + get.resolves(null); + const deleteStub = sinon.stub(service, 'deleteContactHierarchy'); + + await handler(req, res); + + expect(serverUtils.error.calledOnceWithExactly( + { status: 404, message: 'Person not found' }, + req, + res + )).to.be.true; + expect(deleteStub.notCalled).to.be.true; + }); }); }); From 3c454d19fa4e64a7b8932a39ba46d31e10d4d74b Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Fri, 10 Jul 2026 09:08:26 +0530 Subject: [PATCH 13/16] refactor(#10706): address review feedback on the bulk-operation delete - move the bulk-operation constants into @medic/constants and fold the doc model into the api service, dropping the @medic/bulk-operations shared-lib - split the sentinel processor into per-action files (set-contact, delete-user) - queue action ids on the feed instead of whole docs; register the feed before loading the initial queue - match linked users by contact_id as well as facility_id - run the archive action last, after the references to those contacts are cleared - keep the delete gather logic internal, tested through the handler - rename the response breakdown to summary and flesh out the OpenAPI schema --- api/src/controllers/bulk-operations.js | 29 ++- api/src/controllers/person.js | 14 +- api/src/controllers/place.js | 6 +- api/src/services/bulk-operations.js | 69 +++++- api/src/services/delete-contact.js | 47 ++-- api/tests/mocha/controllers/person.spec.js | 23 +- api/tests/mocha/controllers/place.spec.js | 23 +- .../mocha/services/bulk-operations.spec.js | 34 +++ .../mocha/services/delete-contact.spec.js | 206 ++++++++---------- package-lock.json | 9 - scripts/build/generate-openapi.js | 46 +++- sentinel/src/lib/bulk-operations.js | 177 --------------- .../src/lib/bulk-operations/delete-user.js | 26 +++ sentinel/src/lib/bulk-operations/index.js | 148 +++++++++++++ .../src/lib/bulk-operations/set-contact.js | 46 ++++ .../tests/unit/lib/bulk-operations.spec.js | 183 ---------------- .../lib/bulk-operations/delete-user.spec.js | 47 ++++ .../unit/lib/bulk-operations/index.spec.js | 155 +++++++++++++ .../lib/bulk-operations/set-contact.spec.js | 63 ++++++ shared-libs/bulk-operations/package.json | 11 - shared-libs/bulk-operations/src/index.js | 98 --------- shared-libs/bulk-operations/test/index.js | 100 --------- shared-libs/constants/src/index.js | 20 ++ 23 files changed, 789 insertions(+), 791 deletions(-) delete mode 100644 sentinel/src/lib/bulk-operations.js create mode 100644 sentinel/src/lib/bulk-operations/delete-user.js create mode 100644 sentinel/src/lib/bulk-operations/index.js create mode 100644 sentinel/src/lib/bulk-operations/set-contact.js delete mode 100644 sentinel/tests/unit/lib/bulk-operations.spec.js create mode 100644 sentinel/tests/unit/lib/bulk-operations/delete-user.spec.js create mode 100644 sentinel/tests/unit/lib/bulk-operations/index.spec.js create mode 100644 sentinel/tests/unit/lib/bulk-operations/set-contact.spec.js delete mode 100644 shared-libs/bulk-operations/package.json delete mode 100644 shared-libs/bulk-operations/src/index.js delete mode 100644 shared-libs/bulk-operations/test/index.js diff --git a/api/src/controllers/bulk-operations.js b/api/src/controllers/bulk-operations.js index b4c88da9a8d..d4211323bf1 100644 --- a/api/src/controllers/bulk-operations.js +++ b/api/src/controllers/bulk-operations.js @@ -2,12 +2,6 @@ const service = require('../services/bulk-operations'); const serverUtils = require('../server-utils'); const auth = require('../auth'); -/** - * @openapi - * tags: - * - name: Bulk operations - * description: Status of long-running bulk operations (delete, move, merge) - */ module.exports = { v1: { /** @@ -20,8 +14,8 @@ module.exports = { * Returns the log document for a bulk operation, including the per-action status and the * count of changes applied so far. Used to poll the progress of an operation that was * started through one of the bulk endpoints. - * tags: [Bulk operations] - * x-since: 5.1.0 + * tags: [Bulk] + * x-since: 5.3.0 * parameters: * - in: path * name: id @@ -47,6 +41,25 @@ module.exports = { * actions: * type: object * description: Per-action status, keyed by action id. + * additionalProperties: + * type: object + * properties: + * status: + * type: string + * enum: [queued, completed, failed] + * action: + * type: string + * enum: [archive, set-contact, delete-user] + * updated_date: + * type: string + * format: date-time + * total_changes_count: + * type: integer + * failed_operations: + * type: array + * description: The operations that failed, present only when status is failed. + * items: + * type: object * '401': * $ref: '#/components/responses/Unauthorized' * '403': diff --git a/api/src/controllers/person.js b/api/src/controllers/person.js index 8cffadf174a..6765a9044cb 100644 --- a/api/src/controllers/person.js +++ b/api/src/controllers/person.js @@ -214,17 +214,17 @@ module.exports = { * @openapi * /api/v1/person/{id}: * delete: - * summary: Delete a person and their hierarchy + * summary: Delete a person * operationId: v1PersonIdDelete * description: > - * Queues an asynchronous bulk operation that removes the person, every descendant contact, - * and the reports they are the subject of, clears any dangling primary-contact references, - * and (with delete_users=true) removes linked user accounts. Returns a breakdown of the - * changes and the bulk operation id to poll. + * Queues an asynchronous bulk operation that removes the person and the reports they are the + * subject of, clears any dangling primary-contact references, and (with delete_users=true) + * removes linked user accounts. Returns a summary of the changes and the bulk operation id + * to poll. * tags: [Person] - * x-since: 5.2.0 + * x-since: 5.3.0 * x-permissions: - * hasAll: [can_delete_contact_hierarchy] + * hasAll: [can_delete_contact_hierarchy, can_delete_users] * parameters: * - in: path * name: id diff --git a/api/src/controllers/place.js b/api/src/controllers/place.js index 6357239a1cd..6b3cb9e49ee 100644 --- a/api/src/controllers/place.js +++ b/api/src/controllers/place.js @@ -226,12 +226,12 @@ module.exports = { * description: > * Queues an asynchronous bulk operation that removes the place, every descendant contact, * and the reports they are the subject of, clears any dangling primary-contact references, - * and (with delete_users=true) removes linked user accounts. Returns a breakdown of the + * and (with delete_users=true) removes linked user accounts. Returns a summary of the * changes and the bulk operation id to poll. * tags: [Place] - * x-since: 5.2.0 + * x-since: 5.3.0 * x-permissions: - * hasAll: [can_delete_contact_hierarchy] + * hasAll: [can_delete_contact_hierarchy, can_delete_users] * parameters: * - in: path * name: id diff --git a/api/src/services/bulk-operations.js b/api/src/services/bulk-operations.js index 63d282cd67b..27dc15c78db 100644 --- a/api/src/services/bulk-operations.js +++ b/api/src/services/bulk-operations.js @@ -1,8 +1,57 @@ +const { v7: uuid } = require('uuid'); const db = require('../db'); -const { LOG_ID_PREFIX, buildBulkOperation } = require('@medic/bulk-operations'); +const { BULK_OPERATIONS } = require('@medic/constants'); -// Reads only bulk operation log documents from the medic-logs database. The prefix guard keeps the -// endpoint from returning the other kinds of log documents that share this database. +const { LOG_ID_PREFIX, ACTION_ID_PREFIX, OPERATIONS_ATTACHMENT, STATUSES } = BULK_OPERATIONS; +const OPERATIONS_CONTENT_TYPE = 'application/json'; + +const getOperationUuid = (operationId) => operationId.slice(LOG_ID_PREFIX.length); +const generateOperationId = () => `${LOG_ID_PREFIX}${uuid()}`; +const generateActionId = (operationId) => `${ACTION_ID_PREFIX}${getOperationUuid(operationId)}:${uuid()}`; + +// Params live in an attachment so advancing the cursor as Sentinel batches does not rewrite them. +const encodeOperations = (operations) => ({ + content_type: OPERATIONS_CONTENT_TYPE, + data: Buffer.from(JSON.stringify(operations)).toString('base64'), +}); + +const buildActionDoc = (operationId, action, operations) => ({ + _id: generateActionId(operationId), + bulk_operation_id: operationId, + action, + cursor: 0, + total: operations.length, + _attachments: { + [OPERATIONS_ATTACHMENT]: encodeOperations(operations), + }, +}); + +const buildLogAction = (action, totalChangesCount, date) => ({ + status: STATUSES.QUEUED, + action, + updated_date: date, + total_changes_count: totalChangesCount, +}); + +const buildBulkOperation = (actionOperations, date) => { + const operationId = generateOperationId(); + const actions = actionOperations.map(({ action, operations }) => buildActionDoc(operationId, action, operations)); + + const logActions = {}; + actions.forEach((actionDoc) => { + logActions[actionDoc._id] = buildLogAction(actionDoc.action, actionDoc.total, date); + }); + + const log = { + _id: operationId, + start_date: date, + actions: logActions, + }; + + return { log, actions }; +}; + +// Guards against returning the other kinds of log doc that share the medic-logs database. const getLog = async (id) => { if (!id?.startsWith(LOG_ID_PREFIX)) { return null; @@ -20,10 +69,16 @@ const getLog = async (id) => { } }; -// Queues a bulk operation: writes the log document to medic-logs (the status record the polling -// endpoint reads) and one action document per action type to medic-sentinel (what the Sentinel -// listener processes). The log is written first so it exists before the listener picks up an action. -// Action groups with no operations are skipped. Returns the bulk operation id. +/** + * Queues a bulk operation: writes the log document to medic-logs (the status record the polling + * endpoint reads) and one action document per action type to medic-sentinel (what the Sentinel + * listener processes). The log is written first so it exists before the listener picks up an action. + * Action groups with no operations are skipped. + * @param {Object[]} actionOperations - one group per action type + * @param {string} actionOperations[].action - the action type (`archive`, `set-contact`, `delete-user`) + * @param {Object[]} actionOperations[].operations - the per-item params for that action + * @returns {Promise} the bulk operation id + */ const queue = async (actionOperations) => { const nonEmpty = actionOperations.filter(({ operations }) => operations.length); const { log, actions } = buildBulkOperation(nonEmpty, new Date()); diff --git a/api/src/services/delete-contact.js b/api/src/services/delete-contact.js index 7f721710d67..abfb488fdef 100644 --- a/api/src/services/delete-contact.js +++ b/api/src/services/delete-contact.js @@ -2,7 +2,9 @@ const db = require('../db'); const auth = require('../auth'); const serverUtils = require('../server-utils'); const bulkOperations = require('./bulk-operations'); -const { ACTIONS } = require('@medic/bulk-operations'); +const { BULK_OPERATIONS } = require('@medic/constants'); + +const { ACTIONS } = BULK_OPERATIONS; const httpError = (status, message) => { const err = new Error(message); @@ -10,9 +12,7 @@ const httpError = (status, message) => { return err; }; -// `contacts_by_depth` keyed by a single id returns one row per contact in that subtree (the target -// and every descendant). Each row carries the contact's uuid (row.id) and its shortcode -// (row.value.shortcode), the same pairing the replication authorization service uses. +// contacts_by_depth returns one row per contact in the subtree, each carrying its uuid and shortcode. const getSubtree = (id) => db.medic.query('medic/contacts_by_depth', { key: [id] }); const getSubjectKeys = (rows) => { @@ -26,16 +26,13 @@ const getSubjectKeys = (rows) => { return keys; }; -// Reports are matched by both the contact uuid and its shortcode, so a report that only records the -// shortcode is not missed. +// Match reports by uuid and shortcode, so a report recording only the shortcode is not missed. const getReportIds = async (subjectKeys) => { const result = await db.medic.query('medic-client/reports_by_subject', { keys: subjectKeys }); return [ ...new Set(result.rows.map(row => row.id)) ]; }; -// Places whose primary contact is one of the deleted contacts, excluding places that are being -// deleted themselves. Each keeps the current primary id so the set-contact handler can verify it -// has not changed before clearing the reference. +// Surviving places whose primary contact is being deleted; the current id guards a since-changed ref. const getPrimaryContactClears = async (contactIds) => { const result = await db.medic.query('medic/contacts_by_primary_contact', { keys: contactIds }); const deleted = new Set(contactIds); @@ -51,10 +48,10 @@ const getPrimaryContactClears = async (contactIds) => { return operations; }; -// Users whose facility is one of the deleted contacts (only places have linked users). +// A place is a user's `facility_id` and a person can be their `contact_id`; either breaks the user. const getLinkedUserIds = async (contactIds) => { const result = await db.users.query('users/users_by_field', { - keys: contactIds.map(id => [ 'facility_id', id ]), + keys: contactIds.flatMap(id => [ [ 'facility_id', id ], [ 'contact_id', id ] ]), }); return [ ...new Set(result.rows.map(row => row.id)) ]; }; @@ -64,18 +61,14 @@ const getLinkedUserIds = async (contactIds) => { * @param {string} id - the target contact id * @param {Object} options * @param {boolean} options.deleteUsers - also remove users linked to the deleted contacts - * @param {boolean} options.dryRun - return the breakdown without queuing anything - * @returns {Promise} the breakdown of changes, plus the bulk operation id when the - * operation was queued (omitted for a dry run) - * @throws {Error} a 404 when the contact does not exist, or a 400 when linked users would be left - * behind and `deleteUsers` was not requested + * @param {boolean} options.dryRun - return the summary without queuing anything + * @returns {Promise} the summary of changes, plus the bulk operation id when the operation + * was queued (omitted for a dry run) + * @throws {Error} a 400 when linked users would be left behind and `deleteUsers` was not requested */ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { const subtree = await getSubtree(id); const contactIds = subtree.rows.map(row => row.id); - if (!contactIds.length) { - throw httpError(404, `Contact "${id}" not found`); - } const [ reportIds, setContactOperations, userIds ] = await Promise.all([ getReportIds(getSubjectKeys(subtree.rows)), @@ -91,24 +84,25 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { ); } - const userOperations = deleteUsers ? userIds.map(userId => ({ id: userId })) : []; - const breakdown = { + const userOperations = userIds.map(userId => ({ id: userId })); + const summary = { archive: { contacts: contactIds.length, reports: reportIds.length }, 'set-contact': setContactOperations.length, 'delete-user': userOperations.length, }; if (dryRun) { - return { breakdown }; + return { summary }; } + // Archive last, so contacts are removed only after the references to them are cleared. const bulkOperationId = await bulkOperations.queue([ - { action: ACTIONS.ARCHIVE, operations: [ ...contactIds, ...reportIds ].map(docId => ({ id: docId })) }, { action: ACTIONS.SET_CONTACT, operations: setContactOperations }, { action: ACTIONS.DELETE_USER, operations: userOperations }, + { action: ACTIONS.ARCHIVE, operations: [ ...reportIds, ...contactIds ].map(docId => ({ id: docId })) }, ]); - return { breakdown, id: bulkOperationId }; + return { summary, id: bulkOperationId }; }; /** @@ -118,7 +112,7 @@ const deleteContactHierarchy = async (id, { deleteUsers, dryRun } = {}) => { * place id cannot be deleted through the person endpoint or vice versa, and `type` names it for the * not-found message. The handler reads the `delete_users`/`dry_run` query params, asserts the * required permissions, hands the type-agnostic work off to `deleteContactHierarchy`, and responds - * with the breakdown (202 when queued, 200 for a dry run). + * with the summary (202 when queued, 200 for a dry run). * @param {Object} options * @param {Function} options.get - fetches the target contact by uuid, or null when it is not this type * @param {string} options.type - the contact type name, used in the not-found message @@ -138,11 +132,10 @@ const handleDelete = ({ get, type }) => serverUtils.doOrError(async (req, res) = return serverUtils.error({ status: 404, message: `${type} not found` }, req, res); } - const result = await module.exports.deleteContactHierarchy(uuid, { deleteUsers, dryRun }); + const result = await deleteContactHierarchy(uuid, { deleteUsers, dryRun }); return res.status(dryRun ? 200 : 202).json(result); }); module.exports = { - deleteContactHierarchy, handleDelete, }; diff --git a/api/tests/mocha/controllers/person.spec.js b/api/tests/mocha/controllers/person.spec.js index 0d2b9e69f42..bd62c43cbe6 100644 --- a/api/tests/mocha/controllers/person.spec.js +++ b/api/tests/mocha/controllers/person.spec.js @@ -206,36 +206,19 @@ describe('Person Controller', () => { }); describe('delete', () => { - let deleteContact; - beforeEach(() => { - deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); - res.status = sinon.stub().returns(res); - }); - - it('deletes the hierarchy when the id is a person', async () => { - req = { params: { uuid: '123' }, query: {} }; - personGet.resolves({ _id: '123' }); - deleteContact.resolves({ breakdown: {}, id: 'bulk-operation:1' }); - - await controller.v1.delete(req, res); - - expect(personGet.calledOnceWithExactly(Qualifier.byUuid('123'))).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; - expect(res.status.calledOnceWithExactly(202)).to.be.true; - }); - - it('responds 404 and does not delete when the id is not a person', async () => { + // the delete logic itself is covered in the delete-contact service spec + it('responds 404 for an id that is not a person', async () => { req = { params: { uuid: 'place-1' }, query: {} }; personGet.resolves(null); await controller.v1.delete(req, res); + expect(personGet.calledOnceWithExactly(Qualifier.byUuid('place-1'))).to.be.true; expect(serverUtilsError.calledOnceWithExactly( { status: 404, message: 'Person not found' }, req, res )).to.be.true; - expect(deleteContact.notCalled).to.be.true; }); }); }); diff --git a/api/tests/mocha/controllers/place.spec.js b/api/tests/mocha/controllers/place.spec.js index a36aad2fadb..12dd599ba16 100644 --- a/api/tests/mocha/controllers/place.spec.js +++ b/api/tests/mocha/controllers/place.spec.js @@ -214,36 +214,19 @@ describe('Place Controller', () => { }); describe('delete', () => { - let deleteContact; - beforeEach(() => { - deleteContact = sinon.stub(require('../../../src/services/delete-contact'), 'deleteContactHierarchy'); - res.status = sinon.stub().returns(res); - }); - - it('deletes the hierarchy when the id is a place', async () => { - req = { params: { uuid: '123' }, query: {} }; - placeGet.resolves({ _id: '123' }); - deleteContact.resolves({ breakdown: {}, id: 'bulk-operation:1' }); - - await controller.v1.delete(req, res); - - expect(placeGet.calledOnceWithExactly(Qualifier.byUuid('123'))).to.be.true; - expect(deleteContact.calledOnceWithExactly('123', { deleteUsers: false, dryRun: false })).to.be.true; - expect(res.status.calledOnceWithExactly(202)).to.be.true; - }); - - it('responds 404 and does not delete when the id is not a place', async () => { + // the delete logic itself is covered in the delete-contact service spec + it('responds 404 for an id that is not a place', async () => { req = { params: { uuid: 'person-1' }, query: {} }; placeGet.resolves(null); await controller.v1.delete(req, res); + expect(placeGet.calledOnceWithExactly(Qualifier.byUuid('person-1'))).to.be.true; expect(serverUtilsError.calledOnceWithExactly( { status: 404, message: 'Place not found' }, req, res )).to.be.true; - expect(deleteContact.notCalled).to.be.true; }); }); }); diff --git a/api/tests/mocha/services/bulk-operations.spec.js b/api/tests/mocha/services/bulk-operations.spec.js index f135a45c5d8..4a3d6991c34 100644 --- a/api/tests/mocha/services/bulk-operations.spec.js +++ b/api/tests/mocha/services/bulk-operations.spec.js @@ -92,6 +92,40 @@ describe('Bulk operations service', () => { }); }); + it('stores each action\'s params in a base64 json attachment and records the log action detail', () => { + const put = sinon.stub(db.medicLogs, 'put').resolves(); + const bulkDocs = sinon.stub(db.sentinel, 'bulkDocs').resolves(); + + return service.queue(actionOperations).then(() => { + const log = put.args[0][0]; + const actions = bulkDocs.args[0][0]; + + // per-item params live in a base64 json attachment + const attachment = actions[0]._attachments.operations; + chai.expect(attachment.content_type).to.equal('application/json'); + chai.expect(JSON.parse(Buffer.from(attachment.data, 'base64').toString())) + .to.deep.equal(actionOperations[0].operations); + chai.expect(actions[0].cursor).to.equal(0); + chai.expect(actions[0].total).to.equal(2); + + // the log action entry cross-links to the action doc and starts queued + const logAction = log.actions[actions[0]._id]; + chai.expect(logAction.status).to.equal('queued'); + chai.expect(logAction.action).to.equal('archive'); + chai.expect(logAction.total_changes_count).to.equal(2); + chai.expect(logAction.updated_date).to.equal(log.start_date); + }); + }); + + it('generates a distinct operation id on each call', () => { + sinon.stub(db.medicLogs, 'put').resolves(); + sinon.stub(db.sentinel, 'bulkDocs').resolves(); + + return Promise + .all([ service.queue(actionOperations), service.queue(actionOperations) ]) + .then(([ first, second ]) => chai.expect(first).to.not.equal(second)); + }); + it('skips action groups that have no operations', () => { sinon.stub(db.medicLogs, 'put').resolves(); const bulkDocs = sinon.stub(db.sentinel, 'bulkDocs').resolves(); diff --git a/api/tests/mocha/services/delete-contact.spec.js b/api/tests/mocha/services/delete-contact.spec.js index 88c4d772711..86412a69b4b 100644 --- a/api/tests/mocha/services/delete-contact.spec.js +++ b/api/tests/mocha/services/delete-contact.spec.js @@ -10,10 +10,24 @@ const service = require('../../../src/services/delete-contact'); const expect = chai.expect; describe('Delete contact service', () => { + let res; + + beforeEach(() => { + sinon.stub(auth, 'assertPermissions').resolves(); + sinon.stub(serverUtils, 'error'); + res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + }); + afterEach(() => sinon.restore()); - describe('deleteContactHierarchy', () => { - it('gathers the hierarchy and queues the bulk operation', () => { + // The delete logic is only reachable through the shared handler, so it is tested through it: each + // test builds the handler with a `get` (the type-specific fetch the controllers pass in) and stubs + // the db layer the gather runs against. + const handlerFor = (get) => service.handleDelete({ get, type: 'Person' }); + + describe('handleDelete', () => { + it('gathers the hierarchy, queues the actions in order, and responds 202 with the summary', () => { + const get = sinon.stub().resolves({ _id: 'target' }); sinon.stub(db.medic, 'query').callsFake((view) => { if (view === 'medic/contacts_by_depth') { return Promise.resolve({ rows: [ @@ -35,63 +49,64 @@ describe('Delete contact service', () => { sinon.stub(db.users, 'query').resolves({ rows: [ { id: 'org.couchdb.user:chw' } ] }); const queue = sinon.stub(bulkOperations, 'queue').resolves('bulk-operation:xyz'); - return service.deleteContactHierarchy('target', { deleteUsers: true, dryRun: false }).then(result => { - expect(result).to.deep.equal({ - breakdown: { - archive: { contacts: 2, reports: 2 }, - 'set-contact': 1, - 'delete-user': 1, - }, - id: 'bulk-operation:xyz', - }); + const req = { params: { uuid: 'target' }, query: { delete_users: 'true' } }; + return handlerFor(get)(req, res).then(() => { + expect(auth.assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: [ 'can_delete_contact_hierarchy', 'can_delete_users' ] } + )).to.be.true; // reports matched by uuid + shortcode const reportsCall = db.medic.query.getCalls().find(c => c.args[0] === 'medic-client/reports_by_subject'); expect(reportsCall.args[1].keys).to.deep.equal([ 'target', 'PID-1', 'child' ]); - // linked users looked up by facility + // users looked up by both facility_id and contact_id expect(db.users.query.args[0][1].keys).to.deep.equal([ - [ 'facility_id', 'target' ], - [ 'facility_id', 'child' ], + [ 'facility_id', 'target' ], [ 'contact_id', 'target' ], + [ 'facility_id', 'child' ], [ 'contact_id', 'child' ], ]); - // the queued action lists - const byAction = Object.fromEntries(queue.args[0][0].map(a => [ a.action, a.operations ])); - expect(byAction.archive.map(o => o.id)).to.deep.equal([ 'target', 'child', 'report-1', 'report-2' ]); + // queued in order: set-contact, delete-user, then archive (reports before their subject contacts) + const actions = queue.args[0][0]; + expect(actions.map(a => a.action)).to.deep.equal([ 'set-contact', 'delete-user', 'archive' ]); + const byAction = Object.fromEntries(actions.map(a => [ a.action, a.operations ])); expect(byAction['set-contact']).to.deep.equal([ { id: 'parent-place', current_contact_id: 'target' } ]); expect(byAction['delete-user']).to.deep.equal([ { id: 'org.couchdb.user:chw' } ]); - }); - }); + expect(byAction.archive.map(o => o.id)).to.deep.equal([ 'report-1', 'report-2', 'target', 'child' ]); - it('throws 404 when the contact does not exist', () => { - sinon.stub(db.medic, 'query').resolves({ rows: [] }); - - return service - .deleteContactHierarchy('missing', {}) - .then(() => chai.expect.fail('should have thrown')) - .catch(err => expect(err.status).to.equal(404)); + expect(res.status.calledOnceWithExactly(202)).to.be.true; + expect(res.json.calledOnceWithExactly({ + summary: { archive: { contacts: 2, reports: 2 }, 'set-contact': 1, 'delete-user': 1 }, + id: 'bulk-operation:xyz', + })).to.be.true; + }); }); - it('throws 400 when linked users exist and delete_users is not set', () => { + it('asserts only can_delete_contact_hierarchy when delete_users is not set', () => { + const get = sinon.stub().resolves({ _id: 'place' }); sinon.stub(db.medic, 'query').callsFake((view) => { if (view === 'medic/contacts_by_depth') { return Promise.resolve({ rows: [ { id: 'place', value: {} } ] }); } return Promise.resolve({ rows: [] }); }); - sinon.stub(db.users, 'query').resolves({ rows: [ { id: 'org.couchdb.user:chw' } ] }); - const queue = sinon.stub(bulkOperations, 'queue').resolves('x'); - - return service - .deleteContactHierarchy('place', { deleteUsers: false }) - .then(() => chai.expect.fail('should have thrown')) - .catch(err => { - expect(err.status).to.equal(400); - expect(queue.called).to.equal(false); - }); + sinon.stub(db.users, 'query').resolves({ rows: [] }); + const queue = sinon.stub(bulkOperations, 'queue').resolves('bulk-operation:1'); + + const req = { params: { uuid: 'place' }, query: {} }; + return handlerFor(get)(req, res).then(() => { + expect(auth.assertPermissions.calledOnceWithExactly( + req, + { isOnline: true, hasAll: [ 'can_delete_contact_hierarchy' ] } + )).to.be.true; + const byAction = Object.fromEntries(queue.args[0][0].map(a => [ a.action, a.operations ])); + expect(byAction['delete-user']).to.deep.equal([]); + expect(res.status.calledOnceWithExactly(202)).to.be.true; + }); }); - it('returns the breakdown without queuing for a dry run', () => { + it('responds 200 with the summary and queues nothing for a dry run', () => { + const get = sinon.stub().resolves({ _id: 'place' }); sinon.stub(db.medic, 'query').callsFake((view) => { if (view === 'medic/contacts_by_depth') { return Promise.resolve({ rows: [ { id: 'place', value: {} } ] }); @@ -104,91 +119,50 @@ describe('Delete contact service', () => { sinon.stub(db.users, 'query').resolves({ rows: [] }); const queue = sinon.stub(bulkOperations, 'queue').resolves('x'); - return service.deleteContactHierarchy('place', { dryRun: true }).then(result => { - expect(result).to.deep.equal({ - breakdown: { archive: { contacts: 1, reports: 1 }, 'set-contact': 0, 'delete-user': 0 }, - }); + const req = { params: { uuid: 'place' }, query: { dry_run: 'true' } }; + return handlerFor(get)(req, res).then(() => { expect(queue.called).to.equal(false); + expect(res.status.calledOnceWithExactly(200)).to.be.true; + expect(res.json.calledOnceWithExactly({ + summary: { archive: { contacts: 1, reports: 1 }, 'set-contact': 0, 'delete-user': 0 }, + })).to.be.true; }); }); - }); - - describe('handleDelete', () => { - let req; - let res; - let get; - let handler; - - beforeEach(() => { - sinon.stub(auth, 'assertPermissions').resolves(); - sinon.stub(serverUtils, 'error'); - res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - get = sinon.stub().resolves({ _id: '123' }); - handler = service.handleDelete({ get, type: 'Person' }); - }); - - it('validates the type, queues the delete, and responds 202 with the result', async () => { - req = { params: { uuid: '123' }, query: {} }; - const result = { breakdown: { archive: { contacts: 1, reports: 0 } }, id: 'bulk-operation:1' }; - sinon.stub(service, 'deleteContactHierarchy').resolves(result); - - await handler(req, res); - - expect(get.calledOnceWithExactly('123')).to.be.true; - expect(auth.assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy'] } - )).to.be.true; - expect(service.deleteContactHierarchy.calledOnceWithExactly( - '123', - { deleteUsers: false, dryRun: false } - )).to.be.true; - expect(res.status.calledOnceWithExactly(202)).to.be.true; - expect(res.json.calledOnceWithExactly(result)).to.be.true; - }); - it('also requires can_delete_users when delete_users=true', async () => { - req = { params: { uuid: '123' }, query: { delete_users: 'true' } }; - sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {}, id: 'x' }); - - await handler(req, res); - - expect(auth.assertPermissions.calledOnceWithExactly( - req, - { isOnline: true, hasAll: ['can_delete_contact_hierarchy', 'can_delete_users'] } - )).to.be.true; - expect(service.deleteContactHierarchy.calledOnceWithExactly( - '123', - { deleteUsers: true, dryRun: false } - )).to.be.true; - }); - - it('responds 200 for a dry run', async () => { - req = { params: { uuid: '123' }, query: { dry_run: 'true' } }; - sinon.stub(service, 'deleteContactHierarchy').resolves({ breakdown: {} }); - - await handler(req, res); - - expect(service.deleteContactHierarchy.calledOnceWithExactly( - '123', - { deleteUsers: false, dryRun: true } - )).to.be.true; - expect(res.status.calledOnceWithExactly(200)).to.be.true; + it('responds 404 and does not gather when the target is not the expected type', () => { + const get = sinon.stub().resolves(null); + const query = sinon.stub(db.medic, 'query'); + const queue = sinon.stub(bulkOperations, 'queue'); + + const req = { params: { uuid: 'wrong' }, query: {} }; + return handlerFor(get)(req, res).then(() => { + expect(serverUtils.error.calledOnceWithExactly( + { status: 404, message: 'Person not found' }, + req, + res + )).to.be.true; + expect(query.called).to.equal(false); + expect(queue.called).to.equal(false); + }); }); - it('responds 404 and does not delete when the target is not the expected type', async () => { - req = { params: { uuid: 'place-1' }, query: {} }; - get.resolves(null); - const deleteStub = sinon.stub(service, 'deleteContactHierarchy'); - - await handler(req, res); + it('responds 400 and queues nothing when linked users exist and delete_users is not set', () => { + const get = sinon.stub().resolves({ _id: 'place' }); + sinon.stub(db.medic, 'query').callsFake((view) => { + if (view === 'medic/contacts_by_depth') { + return Promise.resolve({ rows: [ { id: 'place', value: {} } ] }); + } + return Promise.resolve({ rows: [] }); + }); + sinon.stub(db.users, 'query').resolves({ rows: [ { id: 'org.couchdb.user:chw' } ] }); + const queue = sinon.stub(bulkOperations, 'queue'); - expect(serverUtils.error.calledOnceWithExactly( - { status: 404, message: 'Person not found' }, - req, - res - )).to.be.true; - expect(deleteStub.notCalled).to.be.true; + const req = { params: { uuid: 'place' }, query: {} }; + return handlerFor(get)(req, res).then(() => { + expect(serverUtils.error.calledOnce).to.be.true; + expect(serverUtils.error.args[0][0].status).to.equal(400); + expect(queue.called).to.equal(false); + }); }); }); }); diff --git a/package-lock.json b/package-lock.json index 45052195e36..a1236060078 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5750,10 +5750,6 @@ "resolved": "shared-libs/bulk-docs-utils", "link": true }, - "node_modules/@medic/bulk-operations": { - "resolved": "shared-libs/bulk-operations", - "link": true - }, "node_modules/@medic/calendar-interval": { "resolved": "shared-libs/calendar-interval", "link": true @@ -44303,11 +44299,6 @@ "version": "1.0.0", "license": "Apache-2.0" }, - "shared-libs/bulk-operations": { - "name": "@medic/bulk-operations", - "version": "1.0.0", - "license": "Apache-2.0" - }, "shared-libs/calendar-interval": { "name": "@medic/calendar-interval", "version": "1.0.0", diff --git a/scripts/build/generate-openapi.js b/scripts/build/generate-openapi.js index db68bea1e1e..e5ab3702c1e 100644 --- a/scripts/build/generate-openapi.js +++ b/scripts/build/generate-openapi.js @@ -62,6 +62,28 @@ const SWAGGER_OPTIONS = { ok: { const: true }, }, }, + BulkOperationSummary: { + type: 'object', + description: 'A count of the changes an operation will make, grouped by action.', + properties: { + archive: { + type: 'object', + description: 'Documents to remove: the contacts in the hierarchy and their reports.', + properties: { + contacts: { type: 'integer' }, + reports: { type: 'integer' }, + }, + }, + 'set-contact': { + type: 'integer', + description: 'Primary-contact references on surviving places that will be cleared.', + }, + 'delete-user': { + type: 'integer', + description: 'Linked user accounts that will be removed.', + }, + }, + }, }, parameters: { cursor: { @@ -93,7 +115,7 @@ const SWAGGER_OPTIONS = { deleteUsers: { in: 'query', name: 'delete_users', - schema: { type: 'boolean' }, + schema: { type: 'boolean', default: false }, description: 'Also delete user accounts linked to the removed contacts. Requires the can_delete_users ' + 'permission. When not set, the request is rejected with 400 if any linked users exist.', @@ -101,8 +123,10 @@ const SWAGGER_OPTIONS = { dryRun: { in: 'query', name: 'dry_run', - schema: { type: 'boolean' }, - description: 'Return the breakdown of changes without queuing anything.', + schema: { type: 'boolean', default: false }, + description: + 'Return the summary of what would be changed by executing this operation. Nothing is ' + + 'applied when dry_run is set.', } }, responses: { @@ -117,14 +141,26 @@ const SWAGGER_OPTIONS = { schema: { type: 'object', properties: { - breakdown: { type: 'object' }, + summary: { $ref: '#/components/schemas/BulkOperationSummary' }, id: { type: 'string', description: 'The bulk operation id to poll.' } } } } } }, - BulkOperationDryRun: { description: 'The dry-run breakdown (nothing queued)' } + BulkOperationDryRun: { + description: 'The dry-run summary (nothing queued)', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + summary: { $ref: '#/components/schemas/BulkOperationSummary' } + } + } + } + } + } } }, }, diff --git a/sentinel/src/lib/bulk-operations.js b/sentinel/src/lib/bulk-operations.js deleted file mode 100644 index 51fafa7936f..00000000000 --- a/sentinel/src/lib/bulk-operations.js +++ /dev/null @@ -1,177 +0,0 @@ -const async = require('async'); -const logger = require('@medic/logger'); -const db = require('../db'); -const config = require('../config'); -const dataContext = require('../data-context'); -const { PREFIXES } = require('@medic/constants'); -const { ACTIONS, STATUSES, OPERATIONS_ATTACHMENT, ACTION_ID_PREFIX } = require('@medic/bulk-operations'); -const userManagement = require('@medic/user-management')(config, db, dataContext); - -const BATCH_SIZE = 100; -const RETRY_TIMEOUT = 60000; - -// set-contact: point a place's `contact` reference at a new value (or clear it when `contact` is -// absent). The change is applied only if the doc still has the contact we recorded when the -// operation was queued, so a concurrent edit is not clobbered. Operations whose doc is gone or -// whose contact has since changed are returned as failed. -const setContact = async (batch) => { - const result = await db.medic.allDocs({ keys: batch.map(op => op.id), include_docs: true }); - const docsById = {}; - result.rows.forEach(row => { - if (row.doc) { - docsById[row.doc._id] = row.doc; - } - }); - - const failed = []; - const toUpdate = []; - batch.forEach(op => { - const doc = docsById[op.id]; - const currentContactId = doc && (doc.contact?._id || doc.contact); - if (!doc || currentContactId !== op.current_contact_id) { - failed.push(op); - return; - } - doc.contact = op.contact; - toUpdate.push(doc); - }); - - if (toUpdate.length) { - await db.medic.bulkDocs(toUpdate); - } - return failed; -}; - -// delete-user: remove each linked user through the existing user-delete path. Failures are -// collected per user so one bad delete does not abort the batch. -const deleteUser = async (batch) => { - const failed = []; - for (const op of batch) { - try { - await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, '')); - } catch (err) { - logger.error(`bulk-operations: failed to delete user ${op.id}: %o`, err); - failed.push(op); - } - } - return failed; -}; - -const HANDLERS = { - [ACTIONS.SET_CONTACT]: setContact, - [ACTIONS.DELETE_USER]: deleteUser, - // ACTIONS.ARCHIVE handler is added when #6615 lands. -}; - -const readOperations = async (actionId) => { - const buffer = await db.sentinel.getAttachment(actionId, OPERATIONS_ATTACHMENT); - return JSON.parse(buffer.toString()); -}; - -// Persist progress on the action doc after each batch so a restart resumes from the cursor. The -// stored `operations` attachment is left as a stub, so advancing the cursor does not rewrite it. -const saveProgress = async (action, processedCount, failed) => { - const latest = await db.sentinel.get(action._id); - action._rev = latest._rev; - action.cursor += processedCount; - if (failed.length) { - action.failed_operations = [ ...(action.failed_operations || []), ...failed ]; - } - await db.sentinel.put(action); -}; - -const recordResultOnLog = async (action) => { - const log = await db.medicLogs.get(action.bulk_operation_id); - const failed = action.failed_operations || []; - log.actions[action._id] = { - status: failed.length ? STATUSES.FAILED : STATUSES.COMPLETED, - action: action.action, - updated_date: new Date(), - total_changes_count: action.total - failed.length, - failed_operations: failed.length ? failed : undefined, - }; - await db.medicLogs.put(log); -}; - -const deleteAction = async (action) => { - const latest = await db.sentinel.get(action._id); - await db.sentinel.put({ ...action, _rev: latest._rev, _deleted: true }); -}; - -// Processes a single bulk-operation action document: works through its operations in batches, -// advancing the cursor so a restart resumes where it left off, then records the outcome on the -// bulk operation log and removes the action document. -const processAction = async (action) => { - const handler = HANDLERS[action.action]; - if (!handler) { - throw new Error(`bulk-operations: no handler for action "${action.action}"`); - } - - const operations = await readOperations(action._id); - while (action.cursor < action.total) { - const batch = operations.slice(action.cursor, action.cursor + BATCH_SIZE); - if (!batch.length) { - break; - } - const failed = await handler(batch); - await saveProgress(action, batch.length, failed); - } - - await recordResultOnLog(action); - await deleteAction(action); - logger.info(`bulk-operations: completed action ${action._id}`); -}; - -// --- the medic-sentinel changes-feed listener --- -// On startup we queue any action docs already waiting, then watch the feed for new ones. Actions -// are processed one at a time. `inProgress` stops our own cursor writes (which also show up on the -// feed) from re-queueing an action while it is still being processed. -const inProgress = new Set(); - -const queue = async.queue((action, callback) => { - processAction(action) - .catch(err => logger.error(`bulk-operations: error processing action ${action._id}: %o`, err)) - .then(() => callback()); -}); - -const enqueue = (action) => { - if (inProgress.has(action._id)) { - return; - } - inProgress.add(action._id); - queue.push(action, () => inProgress.delete(action._id)); -}; - -const loadInitialQueue = async () => { - const result = await db.sentinel.allDocs({ - startkey: ACTION_ID_PREFIX, - endkey: `${ACTION_ID_PREFIX}￰`, - include_docs: true, - }); - result.rows.forEach(row => row.doc && enqueue(row.doc)); -}; - -const registerFeed = () => { - db.sentinel - .changes({ live: true, since: 'now', include_docs: true }) - .on('change', (change) => { - if (!change.deleted && change.id.startsWith(ACTION_ID_PREFIX) && change.doc) { - enqueue(change.doc); - } - }) - .on('error', /* istanbul ignore next: feed-error retry, exercised via integration tests */ (err) => { - logger.error('bulk-operations: changes feed error: %o', err); - setTimeout(registerFeed, RETRY_TIMEOUT); - }); -}; - -const listen = async () => { - logger.info('bulk-operations: listening for queued actions on medic-sentinel'); - await loadInitialQueue(); - registerFeed(); -}; - -module.exports = { - processAction, - listen, -}; diff --git a/sentinel/src/lib/bulk-operations/delete-user.js b/sentinel/src/lib/bulk-operations/delete-user.js new file mode 100644 index 00000000000..b284574d61c --- /dev/null +++ b/sentinel/src/lib/bulk-operations/delete-user.js @@ -0,0 +1,26 @@ +const logger = require('@medic/logger'); +const db = require('../../db'); +const config = require('../../config'); +const dataContext = require('../../data-context'); +const { PREFIXES } = require('@medic/constants'); + +const userManagement = require('@medic/user-management')(config, db, dataContext); + +// Remove each linked user via the existing user-delete path; a failure fails only that op. +module.exports = async (batch, actionId) => { + const failed = []; + for (const op of batch) { + if (!op.id) { + logger.error(`bulk-operations: delete-user skipped an operation with no id (action ${actionId})`); + failed.push(op); + continue; + } + try { + await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, '')); + } catch (err) { + logger.error(`bulk-operations: delete-user failed for ${op.id} (action ${actionId}): %o`, err); + failed.push(op); + } + } + return failed; +}; diff --git a/sentinel/src/lib/bulk-operations/index.js b/sentinel/src/lib/bulk-operations/index.js new file mode 100644 index 00000000000..154700a4b54 --- /dev/null +++ b/sentinel/src/lib/bulk-operations/index.js @@ -0,0 +1,148 @@ +const async = require('async'); +const logger = require('@medic/logger'); +const db = require('../../db'); +const { BULK_OPERATIONS } = require('@medic/constants'); +const setContact = require('./set-contact'); +const deleteUser = require('./delete-user'); + +const { ACTIONS, STATUSES, OPERATIONS_ATTACHMENT, ACTION_ID_PREFIX } = BULK_OPERATIONS; + +const BATCH_SIZE = 100; +const RETRY_TIMEOUT = 60000; + +const HANDLERS = { + [ACTIONS.SET_CONTACT]: setContact, + [ACTIONS.DELETE_USER]: deleteUser, + // ACTIONS.ARCHIVE handler is added when #6615 lands. +}; + +const readOperations = async (actionId) => { + const buffer = await db.sentinel.getAttachment(actionId, OPERATIONS_ATTACHMENT); + return JSON.parse(buffer.toString()); +}; + +// Base the cursor update on the latest doc (not our in-memory copy) and hand it back to the caller. +const saveProgress = async (action, processedCount, failed) => { + const updated = await db.sentinel.get(action._id); + updated.cursor = (updated.cursor || 0) + processedCount; + if (failed.length) { + updated.failed_operations = [ ...(updated.failed_operations || []), ...failed ]; + } + await db.sentinel.put(updated); + return updated; +}; + +// A missing log doc is skipped (and logged) rather than crashing Sentinel. +const recordResultOnLog = async (action) => { + let log; + try { + log = await db.medicLogs.get(action.bulk_operation_id); + } catch (err) { + if (err.status === 404) { + logger.error( + `bulk-operations: log doc ${action.bulk_operation_id} not found; skipping result for ${action._id}` + ); + return; + } + throw err; + } + + const failed = action.failed_operations || []; + log.actions = log.actions || {}; + log.actions[action._id] = { + status: failed.length ? STATUSES.FAILED : STATUSES.COMPLETED, + action: action.action, + updated_date: new Date(), + total_changes_count: action.total - failed.length, + failed_operations: failed.length ? failed : undefined, + }; + await db.medicLogs.put(log); +}; + +const deleteAction = async (action) => { + const latest = await db.sentinel.get(action._id); + await db.sentinel.put({ ...latest, _deleted: true }); +}; + +// A missing action doc (already processed and removed, e.g. queued twice at startup) is a no-op. +const processAction = async (actionId) => { + let action; + try { + action = await db.sentinel.get(actionId); + } catch (err) { + if (err.status === 404) { + return; + } + throw err; + } + + const handler = HANDLERS[action.action]; + if (!handler) { + throw new Error(`bulk-operations: no handler for action "${action.action}"`); + } + + if (action.cursor < action.total) { + const operations = await readOperations(actionId); + while (action.cursor < operations.length) { + const batch = operations.slice(action.cursor, action.cursor + BATCH_SIZE); + const failed = await handler(batch, actionId); + action = await saveProgress(action, batch.length, failed); + } + } + + await recordResultOnLog(action); + await deleteAction(action); + logger.info(`bulk-operations: completed action ${actionId}`); +}; + +// inProgress stops our own cursor writes (which show up on the feed) from re-queueing an in-flight +// action. +const inProgress = new Set(); + +const queue = async.queue((actionId, callback) => { + processAction(actionId) + .catch(err => logger.error(`bulk-operations: error processing action ${actionId}: %o`, err)) + .then(() => callback()); +}); + +const enqueue = (actionId) => { + if (inProgress.has(actionId)) { + return; + } + inProgress.add(actionId); + queue.push(actionId, () => inProgress.delete(actionId)); +}; + +const loadInitialQueue = async () => { + const result = await db.sentinel.allDocs({ + startkey: ACTION_ID_PREFIX, + endkey: `${ACTION_ID_PREFIX}\ufff0`, + }); + result.rows.forEach(row => enqueue(row.id)); +}; + +const registerFeed = () => { + db.sentinel + .changes({ live: true, since: 'now' }) + .on('change', (change) => { + if (!change.deleted && change.id.startsWith(ACTION_ID_PREFIX)) { + enqueue(change.id); + } + }) + .on('error', /* istanbul ignore next: feed-error retry, exercised via integration tests */ (err) => { + logger.error('bulk-operations: changes feed error: %o', err); + setTimeout(registerFeed, RETRY_TIMEOUT); + }); +}; + +const listen = async () => { + // Register the feed before loading the queue so an action written in between is not missed. + registerFeed(); + await loadInitialQueue(); + logger.info('bulk-operations: listening for queued actions on medic-sentinel'); +}; + +module.exports = { + processAction, + listen, +}; diff --git a/sentinel/src/lib/bulk-operations/set-contact.js b/sentinel/src/lib/bulk-operations/set-contact.js new file mode 100644 index 00000000000..d0a9ddb341f --- /dev/null +++ b/sentinel/src/lib/bulk-operations/set-contact.js @@ -0,0 +1,46 @@ +const logger = require('@medic/logger'); +const db = require('../../db'); + +// Point a place's contact at a new value (or clear it), only when the doc still holds the contact we +// recorded, so a concurrent edit is not clobbered. A missing id/doc or a changed contact is failed. +module.exports = async (batch, actionId) => { + const withId = batch.filter(op => op.id); + const result = withId.length + ? await db.medic.allDocs({ keys: withId.map(op => op.id), include_docs: true }) + : { rows: [] }; + const docsById = {}; + result.rows.forEach(row => { + if (row.doc) { + docsById[row.doc._id] = row.doc; + } + }); + + const failed = []; + const toUpdate = []; + batch.forEach(op => { + if (!op.id) { + logger.error(`bulk-operations: set-contact skipped an operation with no id (action ${actionId})`); + failed.push(op); + return; + } + const doc = docsById[op.id]; + if (!doc) { + logger.error(`bulk-operations: set-contact failed for ${op.id}: doc missing (action ${actionId})`); + failed.push(op); + return; + } + const currentContactId = doc.contact?._id || doc.contact; + if (currentContactId !== op.current_contact_id) { + logger.error(`bulk-operations: set-contact failed for ${op.id}: contact changed (action ${actionId})`); + failed.push(op); + return; + } + doc.contact = op.contact; + toUpdate.push(doc); + }); + + if (toUpdate.length) { + await db.medic.bulkDocs(toUpdate); + } + return failed; +}; diff --git a/sentinel/tests/unit/lib/bulk-operations.spec.js b/sentinel/tests/unit/lib/bulk-operations.spec.js deleted file mode 100644 index afc54eb750e..00000000000 --- a/sentinel/tests/unit/lib/bulk-operations.spec.js +++ /dev/null @@ -1,183 +0,0 @@ -const chai = require('chai'); -const sinon = require('sinon'); -const rewire = require('rewire'); - -const db = require('../../../src/db'); - -const expect = chai.expect; - -describe('bulk-operations sentinel processor', () => { - let service; - - beforeEach(() => { - service = rewire('../../../src/lib/bulk-operations'); - }); - - afterEach(() => sinon.restore()); - - const buildAction = (overrides = {}) => ({ - _id: 'bulk-operation-action:op:1', - _rev: '1-a', - bulk_operation_id: 'bulk-operation:op', - action: 'set-contact', - cursor: 0, - total: 2, - _attachments: { operations: { stub: true } }, - ...overrides, - }); - - const stubActionWrites = () => { - sinon.stub(db.sentinel, 'get').resolves({ _rev: '2-b' }); - sinon.stub(db.sentinel, 'put').resolves({ rev: '3-c' }); - const log = { _id: 'bulk-operation:op', actions: {} }; - sinon.stub(db.medicLogs, 'get').resolves(log); - sinon.stub(db.medicLogs, 'put').resolves(); - return log; - }; - - describe('processAction', () => { - it('applies matching set-contact operations, advances the cursor, records the log and removes the action', () => { - const action = buildAction(); - const operations = [ - { id: 'place-1', contact: { _id: 'new-1' }, current_contact_id: 'old-1' }, - { id: 'place-2', current_contact_id: 'old-2' }, // no `contact` means clear it - ]; - sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); - sinon.stub(db.medic, 'allDocs').resolves({ rows: [ - { doc: { _id: 'place-1', contact: { _id: 'old-1' } } }, - { doc: { _id: 'place-2', contact: 'old-2' } }, - ] }); - const medicBulk = sinon.stub(db.medic, 'bulkDocs').resolves(); - const log = stubActionWrites(); - - return service.processAction(action).then(() => { - expect(medicBulk.calledOnce).to.equal(true); - const updated = medicBulk.args[0][0]; - expect(updated.find(d => d._id === 'place-1').contact).to.deep.equal({ _id: 'new-1' }); - expect(updated.find(d => d._id === 'place-2').contact).to.be.undefined; - - expect(action.cursor).to.equal(2); - expect(db.sentinel.put.called).to.equal(true); - - const entry = log.actions['bulk-operation-action:op:1']; - expect(entry.status).to.equal('completed'); - expect(entry.action).to.equal('set-contact'); - expect(entry.total_changes_count).to.equal(2); - expect(entry.failed_operations).to.be.undefined; - - expect(db.sentinel.put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); - }); - }); - - it('records operations that no longer match as failed', () => { - const action = buildAction(); - const operations = [ - { id: 'place-1', contact: { _id: 'new-1' }, current_contact_id: 'old-1' }, - { id: 'gone', current_contact_id: 'old-2' }, - ]; - sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); - sinon.stub(db.medic, 'allDocs').resolves({ rows: [ - { doc: { _id: 'place-1', contact: { _id: 'changed-since' } } }, // contact changed - { key: 'gone', error: 'not_found' }, // doc gone - ] }); - sinon.stub(db.medic, 'bulkDocs').resolves(); - const log = stubActionWrites(); - - return service.processAction(action).then(() => { - const entry = log.actions['bulk-operation-action:op:1']; - expect(entry.status).to.equal('failed'); - expect(entry.total_changes_count).to.equal(0); - expect(entry.failed_operations.map(op => op.id)).to.deep.equal(['place-1', 'gone']); - }); - }); - - it('deletes each linked user via the user-delete path for a delete-user action', () => { - const deleteUserStub = sinon.stub().resolves(); - service.__set__('userManagement', { deleteUser: deleteUserStub }); - - const action = buildAction({ action: 'delete-user' }); - const operations = [{ id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' }]; - sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); - const log = stubActionWrites(); - - return service.processAction(action).then(() => { - expect(deleteUserStub.args.map(a => a[0])).to.deep.equal(['alice', 'bob']); - expect(log.actions['bulk-operation-action:op:1'].status).to.equal('completed'); - }); - }); - - it('records a user that fails to delete as a failed operation', () => { - const deleteUserStub = sinon.stub(); - deleteUserStub.withArgs('alice').resolves(); - deleteUserStub.withArgs('bob').rejects(new Error('boom')); - service.__set__('userManagement', { deleteUser: deleteUserStub }); - - const action = buildAction({ action: 'delete-user' }); - const operations = [{ id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' }]; - sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); - const log = stubActionWrites(); - - return service.processAction(action).then(() => { - const entry = log.actions['bulk-operation-action:op:1']; - expect(entry.status).to.equal('failed'); - expect(entry.failed_operations.map(op => op.id)).to.deep.equal(['org.couchdb.user:bob']); - }); - }); - - it('throws when there is no handler for the action', () => { - const action = buildAction({ action: 'archive' }); - - return service - .processAction(action) - .then(() => chai.expect.fail('should have thrown')) - .catch(err => expect(err.message).to.contain('no handler')); - }); - - it('stops safely when an action has fewer operations than its total', () => { - const action = buildAction({ total: 1 }); - sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify([]))); - stubActionWrites(); - - return service.processAction(action).then(() => { - expect(db.sentinel.put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); - }); - }); - }); - - describe('listen', () => { - it('processes waiting and feed-delivered actions, dedupes, and ignores irrelevant changes', () => { - const processStub = sinon.stub(); - processStub.onFirstCall().rejects(new Error('processing error')); // exercises the queue error handler - processStub.resolves(); - service.__set__('processAction', processStub); - sinon.stub(db.sentinel, 'allDocs').resolves({ rows: [{ doc: { _id: 'bulk-operation-action:op:1' } }] }); - let onChange; - const changes = sinon.stub(db.sentinel, 'changes').returns({ - on(event, cb) { - if (event === 'change') { - onChange = cb; - } - return this; - }, - }); - - return service - .listen() - .then(() => { - expect(changes.calledOnce).to.equal(true); - onChange({ id: 'bulk-operation-action:op:2', doc: { _id: 'bulk-operation-action:op:2' } }); - onChange({ id: 'bulk-operation-action:op:2', doc: { _id: 'bulk-operation-action:op:2' } }); // already queued - onChange({ id: 'bulk-operation-action:op:3', deleted: true }); // ignored: deleted - onChange({ id: 'not-an-action', doc: { _id: 'not-an-action' } }); // ignored: wrong prefix - return new Promise(resolve => setTimeout(resolve, 20)); - }) - .then(() => { - expect(processStub.args.map(a => a[0]._id)).to.deep.equal([ - 'bulk-operation-action:op:1', - 'bulk-operation-action:op:2', - ]); - }); - }); - - }); -}); diff --git a/sentinel/tests/unit/lib/bulk-operations/delete-user.spec.js b/sentinel/tests/unit/lib/bulk-operations/delete-user.spec.js new file mode 100644 index 00000000000..2859c621be7 --- /dev/null +++ b/sentinel/tests/unit/lib/bulk-operations/delete-user.spec.js @@ -0,0 +1,47 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const rewire = require('rewire'); + +const expect = chai.expect; + +describe('bulk-operations delete-user handler', () => { + let deleteUser; + let deleteUserStub; + + beforeEach(() => { + deleteUser = rewire('../../../../src/lib/bulk-operations/delete-user'); + deleteUserStub = sinon.stub(); + deleteUser.__set__('userManagement', { deleteUser: deleteUserStub }); + }); + + afterEach(() => sinon.restore()); + + it('deletes each user via the user-delete path, stripping the couch prefix', () => { + deleteUserStub.resolves(); + + return deleteUser([ { id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' } ], 'action-1') + .then(failed => { + expect(failed).to.deep.equal([]); + expect(deleteUserStub.args.map(a => a[0])).to.deep.equal([ 'alice', 'bob' ]); + }); + }); + + it('records a user that fails to delete as failed and keeps going', () => { + deleteUserStub.withArgs('alice').resolves(); + deleteUserStub.withArgs('bob').rejects(new Error('boom')); + + return deleteUser([ { id: 'org.couchdb.user:alice' }, { id: 'org.couchdb.user:bob' } ], 'action-1') + .then(failed => { + expect(failed.map(op => op.id)).to.deep.equal([ 'org.couchdb.user:bob' ]); + }); + }); + + it('fails an operation with no id without calling the delete path', () => { + deleteUserStub.resolves(); + + return deleteUser([ {} ], 'action-1').then(failed => { + expect(failed).to.have.length(1); + expect(deleteUserStub.called).to.equal(false); + }); + }); +}); diff --git a/sentinel/tests/unit/lib/bulk-operations/index.spec.js b/sentinel/tests/unit/lib/bulk-operations/index.spec.js new file mode 100644 index 00000000000..28c3ce29233 --- /dev/null +++ b/sentinel/tests/unit/lib/bulk-operations/index.spec.js @@ -0,0 +1,155 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const rewire = require('rewire'); + +const db = require('../../../../src/db'); + +const expect = chai.expect; + +describe('bulk-operations sentinel processor', () => { + let service; + + beforeEach(() => { + service = rewire('../../../../src/lib/bulk-operations'); + }); + + afterEach(() => sinon.restore()); + + describe('processAction', () => { + const actionId = 'bulk-operation-action:op:1'; + const buildAction = (overrides = {}) => ({ + _id: actionId, + bulk_operation_id: 'bulk-operation:op', + action: 'set-contact', + cursor: 0, + total: 2, + ...overrides, + }); + + const stubDb = (action, operations) => { + sinon.stub(db.sentinel, 'get').resolves(action); + const put = sinon.stub(db.sentinel, 'put').resolves(); + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify(operations))); + const log = { _id: action.bulk_operation_id, actions: {} }; + sinon.stub(db.medicLogs, 'get').resolves(log); + sinon.stub(db.medicLogs, 'put').resolves(); + return { put, log }; + }; + + it('fetches the action, runs the handler in batches, records the log, and deletes the action', () => { + const action = buildAction(); + const { put, log } = stubDb(action, [ { id: 'a' }, { id: 'b' } ]); + const handler = sinon.stub().resolves([]); + service.__set__('HANDLERS', { 'set-contact': handler }); + + return service.processAction(actionId).then(() => { + expect(db.sentinel.get.firstCall.args[0]).to.equal(actionId); + expect(handler.calledOnce).to.equal(true); + expect(handler.args[0][0].map(op => op.id)).to.deep.equal([ 'a', 'b' ]); + expect(handler.args[0][1]).to.equal(actionId); // the action id is passed for logging + + const entry = log.actions[actionId]; + expect(entry.status).to.equal('completed'); + expect(entry.total_changes_count).to.equal(2); + expect(entry.failed_operations).to.be.undefined; + + expect(put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); + }); + }); + + it('records failed operations on the log', () => { + const action = buildAction(); + const { log } = stubDb(action, [ { id: 'a' }, { id: 'b' } ]); + service.__set__('HANDLERS', { 'set-contact': sinon.stub().resolves([ { id: 'b' } ]) }); + + return service.processAction(actionId).then(() => { + const entry = log.actions[actionId]; + expect(entry.status).to.equal('failed'); + expect(entry.total_changes_count).to.equal(1); + expect(entry.failed_operations.map(op => op.id)).to.deep.equal([ 'b' ]); + }); + }); + + it('throws when there is no handler for the action', () => { + stubDb(buildAction({ action: 'archive' }), []); + service.__set__('HANDLERS', {}); + + return service + .processAction(actionId) + .then(() => chai.expect.fail('should have thrown')) + .catch(err => expect(err.message).to.contain('no handler')); + }); + + it('is a no-op when the action doc is already gone', () => { + sinon.stub(db.sentinel, 'get').rejects({ status: 404 }); + const medicGet = sinon.stub(db.medicLogs, 'get'); + + return service.processAction(actionId).then(() => { + expect(medicGet.called).to.equal(false); + }); + }); + + it('skips recording when the log doc is missing, without crashing', () => { + const action = buildAction(); + sinon.stub(db.sentinel, 'get').resolves(action); + const put = sinon.stub(db.sentinel, 'put').resolves(); + sinon.stub(db.sentinel, 'getAttachment').resolves(Buffer.from(JSON.stringify([ { id: 'a' }, { id: 'b' } ]))); + sinon.stub(db.medicLogs, 'get').rejects({ status: 404 }); + const logPut = sinon.stub(db.medicLogs, 'put').resolves(); + service.__set__('HANDLERS', { 'set-contact': sinon.stub().resolves([]) }); + + return service.processAction(actionId).then(() => { + expect(logPut.called).to.equal(false); + // the action is still removed + expect(put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); + }); + }); + + it('resolves safely when there are no operations to process', () => { + const action = buildAction({ total: 0 }); + const { put } = stubDb(action, []); + service.__set__('HANDLERS', { 'set-contact': sinon.stub().resolves([]) }); + + return service.processAction(actionId).then(() => { + expect(put.args.some(callArgs => callArgs[0]._deleted === true)).to.equal(true); + }); + }); + }); + + describe('listen', () => { + it('registers the feed before loading the queue, enqueues ids, dedupes, and ignores irrelevant changes', () => { + const processStub = sinon.stub(); + processStub.onFirstCall().rejects(new Error('processing error')); // exercises the queue error handler + processStub.resolves(); + service.__set__('processAction', processStub); + + let onChange; + const changes = sinon.stub(db.sentinel, 'changes').returns({ + on(event, cb) { + if (event === 'change') { + onChange = cb; + } + return this; + }, + }); + const allDocs = sinon.stub(db.sentinel, 'allDocs').resolves({ rows: [ { id: 'bulk-operation-action:op:1' } ] }); + + return service + .listen() + .then(() => { + expect(changes.calledBefore(allDocs)).to.equal(true); // feed registered before the initial queue + onChange({ id: 'bulk-operation-action:op:2' }); + onChange({ id: 'bulk-operation-action:op:2' }); // already queued -> deduped + onChange({ id: 'bulk-operation-action:op:3', deleted: true }); // ignored: deleted + onChange({ id: 'not-an-action' }); // ignored: wrong prefix + return new Promise(resolve => setTimeout(resolve, 20)); + }) + .then(() => { + expect(processStub.args.map(a => a[0])).to.deep.equal([ + 'bulk-operation-action:op:1', + 'bulk-operation-action:op:2', + ]); + }); + }); + }); +}); diff --git a/sentinel/tests/unit/lib/bulk-operations/set-contact.spec.js b/sentinel/tests/unit/lib/bulk-operations/set-contact.spec.js new file mode 100644 index 00000000000..8015cbfc4b0 --- /dev/null +++ b/sentinel/tests/unit/lib/bulk-operations/set-contact.spec.js @@ -0,0 +1,63 @@ +const chai = require('chai'); +const sinon = require('sinon'); + +const db = require('../../../../src/db'); +const setContact = require('../../../../src/lib/bulk-operations/set-contact'); + +const expect = chai.expect; + +describe('bulk-operations set-contact handler', () => { + afterEach(() => sinon.restore()); + + it('applies matching operations and returns no failures', () => { + const batch = [ + { id: 'place-1', contact: { _id: 'new-1' }, current_contact_id: 'old-1' }, + { id: 'place-2', current_contact_id: 'old-2' }, // no `contact` means clear it + ]; + sinon.stub(db.medic, 'allDocs').resolves({ rows: [ + { doc: { _id: 'place-1', contact: { _id: 'old-1' } } }, + { doc: { _id: 'place-2', contact: 'old-2' } }, + ] }); + const bulkDocs = sinon.stub(db.medic, 'bulkDocs').resolves(); + + return setContact(batch, 'action-1').then(failed => { + expect(failed).to.deep.equal([]); + const updated = bulkDocs.args[0][0]; + expect(updated.find(d => d._id === 'place-1').contact).to.deep.equal({ _id: 'new-1' }); + expect(updated.find(d => d._id === 'place-2').contact).to.be.undefined; + }); + }); + + it('fails an operation whose doc is missing', () => { + sinon.stub(db.medic, 'allDocs').resolves({ rows: [ { key: 'gone', error: 'not_found' } ] }); + const bulkDocs = sinon.stub(db.medic, 'bulkDocs').resolves(); + + return setContact([ { id: 'gone', current_contact_id: 'old' } ], 'action-1').then(failed => { + expect(failed.map(op => op.id)).to.deep.equal([ 'gone' ]); + expect(bulkDocs.called).to.equal(false); + }); + }); + + it('fails an operation whose contact has changed since it was queued', () => { + sinon.stub(db.medic, 'allDocs').resolves({ rows: [ + { doc: { _id: 'place-1', contact: { _id: 'changed-since' } } }, + ] }); + const bulkDocs = sinon.stub(db.medic, 'bulkDocs').resolves(); + + return setContact([ { id: 'place-1', contact: { _id: 'new' }, current_contact_id: 'old-1' } ], 'action-1') + .then(failed => { + expect(failed.map(op => op.id)).to.deep.equal([ 'place-1' ]); + expect(bulkDocs.called).to.equal(false); + }); + }); + + it('fails an operation with no id without querying', () => { + const allDocs = sinon.stub(db.medic, 'allDocs').resolves({ rows: [] }); + sinon.stub(db.medic, 'bulkDocs').resolves(); + + return setContact([ { current_contact_id: 'x' } ], 'action-1').then(failed => { + expect(failed).to.have.length(1); + expect(allDocs.called).to.equal(false); + }); + }); +}); diff --git a/shared-libs/bulk-operations/package.json b/shared-libs/bulk-operations/package.json deleted file mode 100644 index 24d1de4831d..00000000000 --- a/shared-libs/bulk-operations/package.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "name": "@medic/bulk-operations", - "version": "1.0.0", - "description": "Shared document model for the CHT-Core bulk operation framework (delete, move, merge)", - "main": "src/index.js", - "scripts": { - "test": "nyc --nycrcPath='../nyc.config.js' mocha ./test" - }, - "author": "", - "license": "Apache-2.0" -} diff --git a/shared-libs/bulk-operations/src/index.js b/shared-libs/bulk-operations/src/index.js deleted file mode 100644 index 2e26eaaab83..00000000000 --- a/shared-libs/bulk-operations/src/index.js +++ /dev/null @@ -1,98 +0,0 @@ -const { v7: uuid } = require('uuid'); - -/** - * Document model for the bulk operation framework. - * - * A bulk operation is recorded with two kinds of document: - * - a single LOG document (stored in medic-logs) that tracks overall status and is read by the polling endpoint. - * - one ACTION document per action type (stored in medic-sentinel) that Sentinel processes in batches. - * - * Delete, move and merge all express their work as a set of these actions. - */ - -const LOG_ID_PREFIX = 'bulk-operation:'; -const ACTION_ID_PREFIX = 'bulk-operation-action:'; - -const ACTIONS = { - ARCHIVE: 'archive', - SET_CONTACT: 'set-contact', - DELETE_USER: 'delete-user', -}; - -const STATUSES = { - QUEUED: 'queued', - COMPLETED: 'completed', - FAILED: 'failed', -}; - -// The per-item params for each action live in an attachment rather than inline, so updating the -// action document's cursor as Sentinel works through the batches does not rewrite the whole list. -const OPERATIONS_ATTACHMENT = 'operations'; -const OPERATIONS_CONTENT_TYPE = 'application/json'; - -const getOperationUuid = (operationId) => operationId.slice(LOG_ID_PREFIX.length); - -const generateOperationId = () => `${LOG_ID_PREFIX}${uuid()}`; - -const generateActionId = (operationId) => `${ACTION_ID_PREFIX}${getOperationUuid(operationId)}:${uuid()}`; - -const encodeOperations = (operations) => ({ - content_type: OPERATIONS_CONTENT_TYPE, - data: Buffer.from(JSON.stringify(operations)).toString('base64'), -}); - -const decodeOperations = (attachment) => JSON.parse(Buffer.from(attachment.data, 'base64').toString()); - -const buildActionDoc = (operationId, action, operations) => ({ - _id: generateActionId(operationId), - bulk_operation_id: operationId, - action, - cursor: 0, - total: operations.length, - _attachments: { - [OPERATIONS_ATTACHMENT]: encodeOperations(operations), - }, -}); - -const buildLogAction = (action, totalChangesCount, date) => ({ - status: STATUSES.QUEUED, - action, - updated_date: date, - total_changes_count: totalChangesCount, -}); - -/** - * Builds the documents that make up a single bulk operation. - * @param {Object[]} actionOperations - one entry per action type (`{ action, operations }`), each holding - * its list of per-item params - * @param {Date} date - the operation start date, also the initial updated_date of every action - * @returns {Object} the log document (for medic-logs) and the action documents (for medic-sentinel) - * as `{ log, actions }`; the log's `actions` map is keyed by each action document's `_id` - */ -const buildBulkOperation = (actionOperations, date) => { - const operationId = generateOperationId(); - const actions = actionOperations.map(({ action, operations }) => buildActionDoc(operationId, action, operations)); - - const logActions = {}; - actions.forEach((actionDoc) => { - logActions[actionDoc._id] = buildLogAction(actionDoc.action, actionDoc.total, date); - }); - - const log = { - _id: operationId, - start_date: date, - actions: logActions, - }; - - return { log, actions }; -}; - -module.exports = { - LOG_ID_PREFIX, - ACTION_ID_PREFIX, - ACTIONS, - STATUSES, - OPERATIONS_ATTACHMENT, - buildBulkOperation, - decodeOperations, -}; diff --git a/shared-libs/bulk-operations/test/index.js b/shared-libs/bulk-operations/test/index.js deleted file mode 100644 index c4fc5074008..00000000000 --- a/shared-libs/bulk-operations/test/index.js +++ /dev/null @@ -1,100 +0,0 @@ -const chai = require('chai'); -const { validate: isUuid } = require('uuid'); - -const service = require('../src/index'); - -const expect = chai.expect; - -describe('bulk-operations document model', () => { - describe('constants', () => { - it('exposes the action types', () => { - expect(service.ACTIONS).to.deep.equal({ - ARCHIVE: 'archive', - SET_CONTACT: 'set-contact', - DELETE_USER: 'delete-user', - }); - }); - - it('exposes the statuses', () => { - expect(service.STATUSES).to.deep.equal({ - QUEUED: 'queued', - COMPLETED: 'completed', - FAILED: 'failed', - }); - }); - - it('exposes the document id prefixes', () => { - expect(service.LOG_ID_PREFIX).to.equal('bulk-operation:'); - expect(service.ACTION_ID_PREFIX).to.equal('bulk-operation-action:'); - }); - }); - - describe('buildBulkOperation', () => { - const date = new Date('2026-06-29T12:00:00.000Z'); - - it('builds a log document with a uuid-v7 operation id', () => { - const { log } = service.buildBulkOperation([{ action: 'archive', operations: [{ id: 'a' }] }], date); - - expect(log._id.startsWith('bulk-operation:')).to.equal(true); - expect(isUuid(log._id.slice('bulk-operation:'.length))).to.equal(true); - expect(log.start_date).to.equal(date); - }); - - it('builds one action document per action type, linked back to the operation', () => { - const groups = [ - { action: 'archive', operations: [{ id: 'person' }, { id: 'report' }] }, - { action: 'set-contact', operations: [{ id: 'place', current_contact_id: 'person' }] }, - { action: 'delete-user', operations: [{ id: 'org.couchdb.user:chw' }] }, - ]; - - const { log, actions } = service.buildBulkOperation(groups, date); - - expect(actions).to.have.length(3); - actions.forEach((actionDoc, i) => { - expect(actionDoc._id.startsWith('bulk-operation-action:')).to.equal(true); - // the action id embeds the operation's uuid, so the two ids stay parseable together - expect(actionDoc._id).to.include(log._id.slice('bulk-operation:'.length)); - expect(actionDoc.bulk_operation_id).to.equal(log._id); - expect(actionDoc.action).to.equal(groups[i].action); - expect(actionDoc.cursor).to.equal(0); - expect(actionDoc.total).to.equal(groups[i].operations.length); - expect(service.decodeOperations(actionDoc._attachments.operations)).to.deep.equal(groups[i].operations); - }); - }); - - it('stores the operation params in a base64 json attachment', () => { - const operations = [{ id: 'place', current_contact_id: 'person' }]; - const { actions } = service.buildBulkOperation([{ action: 'set-contact', operations }], date); - - const attachment = actions[0]._attachments.operations; - expect(attachment.content_type).to.equal('application/json'); - expect(attachment.data).to.be.a('string'); - expect(service.decodeOperations(attachment)).to.deep.equal(operations); - }); - - it('cross-links each action document to its entry in the log', () => { - const groups = [ - { action: 'archive', operations: [{ id: 'person' }] }, - { action: 'set-contact', operations: [{ id: 'place' }, { id: 'place2' }] }, - ]; - - const { log, actions } = service.buildBulkOperation(groups, date); - - expect(Object.keys(log.actions)).to.deep.equal(actions.map(actionDoc => actionDoc._id)); - actions.forEach((actionDoc) => { - const logAction = log.actions[actionDoc._id]; - expect(logAction.status).to.equal('queued'); - expect(logAction.action).to.equal(actionDoc.action); - expect(logAction.updated_date).to.equal(date); - expect(logAction.total_changes_count).to.equal(actionDoc.total); - }); - }); - - it('generates a distinct operation id on each call', () => { - const first = service.buildBulkOperation([{ action: 'archive', operations: [] }], date); - const second = service.buildBulkOperation([{ action: 'archive', operations: [] }], date); - - expect(first.log._id).to.not.equal(second.log._id); - }); - }); -}); diff --git a/shared-libs/constants/src/index.js b/shared-libs/constants/src/index.js index ae4309d9984..e2ce52240e0 100644 --- a/shared-libs/constants/src/index.js +++ b/shared-libs/constants/src/index.js @@ -69,6 +69,25 @@ const PREFIXES = { UI_EXTENSION: `${DOC_TYPES.UI_EXTENSION}:` }; +// Bulk operation framework (delete, move, merge): the document id prefixes, action types, statuses, +// and the attachment name shared between the api (which queues operations) and sentinel (which runs +// them). +const BULK_OPERATIONS = { + LOG_ID_PREFIX: 'bulk-operation:', + ACTION_ID_PREFIX: 'bulk-operation-action:', + OPERATIONS_ATTACHMENT: 'operations', + ACTIONS: { + ARCHIVE: 'archive', + SET_CONTACT: 'set-contact', + DELETE_USER: 'delete-user', + }, + STATUSES: { + QUEUED: 'queued', + COMPLETED: 'completed', + FAILED: 'failed', + }, +}; + module.exports = { DOC_IDS, DOC_TYPES, @@ -79,4 +98,5 @@ module.exports = { CONTACT_TYPES, STANDARD_HTTP_HEADERS, PREFIXES, + BULK_OPERATIONS, }; From 2a1b6ce5970b11ba58a51c425418c9ae8f95b8fd Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Fri, 10 Jul 2026 09:17:07 +0530 Subject: [PATCH 14/16] fix(#10706): resolve sonarcloud code smells in the bulk-operation actions Name the set-contact and delete-user handler functions (they were anonymous exported arrows), and reduce processAction's cognitive complexity by pulling the action fetch and the batch loop into helpers. --- .../src/lib/bulk-operations/delete-user.js | 4 ++- sentinel/src/lib/bulk-operations/index.js | 33 ++++++++++++------- .../src/lib/bulk-operations/set-contact.js | 4 ++- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/sentinel/src/lib/bulk-operations/delete-user.js b/sentinel/src/lib/bulk-operations/delete-user.js index b284574d61c..ac407f1c91b 100644 --- a/sentinel/src/lib/bulk-operations/delete-user.js +++ b/sentinel/src/lib/bulk-operations/delete-user.js @@ -7,7 +7,7 @@ const { PREFIXES } = require('@medic/constants'); const userManagement = require('@medic/user-management')(config, db, dataContext); // Remove each linked user via the existing user-delete path; a failure fails only that op. -module.exports = async (batch, actionId) => { +const deleteUser = async (batch, actionId) => { const failed = []; for (const op of batch) { if (!op.id) { @@ -24,3 +24,5 @@ module.exports = async (batch, actionId) => { } return failed; }; + +module.exports = deleteUser; diff --git a/sentinel/src/lib/bulk-operations/index.js b/sentinel/src/lib/bulk-operations/index.js index 154700a4b54..67c2a780828 100644 --- a/sentinel/src/lib/bulk-operations/index.js +++ b/sentinel/src/lib/bulk-operations/index.js @@ -64,17 +64,33 @@ const deleteAction = async (action) => { await db.sentinel.put({ ...latest, _deleted: true }); }; -// A missing action doc (already processed and removed, e.g. queued twice at startup) is a no-op. -const processAction = async (actionId) => { - let action; +// null when the action doc is gone (already processed and removed, e.g. queued twice at startup). +const getAction = async (actionId) => { try { - action = await db.sentinel.get(actionId); + return await db.sentinel.get(actionId); } catch (err) { if (err.status === 404) { - return; + return null; } throw err; } +}; + +const runOperations = async (action, handler, actionId) => { + const operations = await readOperations(actionId); + while (action.cursor < operations.length) { + const batch = operations.slice(action.cursor, action.cursor + BATCH_SIZE); + const failed = await handler(batch, actionId); + action = await saveProgress(action, batch.length, failed); + } + return action; +}; + +const processAction = async (actionId) => { + let action = await getAction(actionId); + if (!action) { + return; + } const handler = HANDLERS[action.action]; if (!handler) { @@ -82,12 +98,7 @@ const processAction = async (actionId) => { } if (action.cursor < action.total) { - const operations = await readOperations(actionId); - while (action.cursor < operations.length) { - const batch = operations.slice(action.cursor, action.cursor + BATCH_SIZE); - const failed = await handler(batch, actionId); - action = await saveProgress(action, batch.length, failed); - } + action = await runOperations(action, handler, actionId); } await recordResultOnLog(action); diff --git a/sentinel/src/lib/bulk-operations/set-contact.js b/sentinel/src/lib/bulk-operations/set-contact.js index d0a9ddb341f..eb1f22a3fe1 100644 --- a/sentinel/src/lib/bulk-operations/set-contact.js +++ b/sentinel/src/lib/bulk-operations/set-contact.js @@ -3,7 +3,7 @@ const db = require('../../db'); // Point a place's contact at a new value (or clear it), only when the doc still holds the contact we // recorded, so a concurrent edit is not clobbered. A missing id/doc or a changed contact is failed. -module.exports = async (batch, actionId) => { +const setContact = async (batch, actionId) => { const withId = batch.filter(op => op.id); const result = withId.length ? await db.medic.allDocs({ keys: withId.map(op => op.id), include_docs: true }) @@ -44,3 +44,5 @@ module.exports = async (batch, actionId) => { } return failed; }; + +module.exports = setContact; From 1060de6f6cd860fa7937d34a502fbdd4af35df6b Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 13 Jul 2026 21:11:45 +0530 Subject: [PATCH 15/16] test(#10706): add integration tests for the contact deletion endpoints Covers permissions, dry-run summaries and the type/linked-user error cases for DELETE /api/v1/person, DELETE /api/v1/place and GET /api/v1/bulk-operations. The removal assertions poll the bulk operation to completion, which needs the archive action handler from #6615, so those are skipped until it lands. --- .../api/controllers/bulk-operations.spec.js | 92 ++++++++++++++ .../api/controllers/person.spec.js | 71 +++++++++++ .../integration/api/controllers/place.spec.js | 117 ++++++++++++++++++ 3 files changed, 280 insertions(+) create mode 100644 tests/integration/api/controllers/bulk-operations.spec.js diff --git a/tests/integration/api/controllers/bulk-operations.spec.js b/tests/integration/api/controllers/bulk-operations.spec.js new file mode 100644 index 00000000000..bc84c39e9bf --- /dev/null +++ b/tests/integration/api/controllers/bulk-operations.spec.js @@ -0,0 +1,92 @@ +const utils = require('@utils'); +const placeFactory = require('@factories/cht/contacts/place'); +const personFactory = require('@factories/cht/contacts/person'); +const userFactory = require('@factories/cht/users/users'); +const reportFactory = require('@factories/cht/reports/generic-report'); +const { CONTACT_TYPES } = require('@medic/constants'); +const { expect } = require('chai'); + +describe('Bulk operations API', () => { + const contact0 = utils.deepFreeze(personFactory.build({ name: 'contact0', role: 'chw' })); + const placeMap = utils.deepFreeze(placeFactory.generateHierarchy()); + const place0 = utils.deepFreeze({ ...placeMap.get(CONTACT_TYPES.CLINIC), contact: { _id: contact0._id } }); + const place1 = utils.deepFreeze(placeMap.get(CONTACT_TYPES.HEALTH_CENTER)); + const place2 = utils.deepFreeze(placeMap.get('district_hospital')); + + const offlineUser = utils.deepFreeze(userFactory.build({ + username: 'offline-bulk', + place: place0._id, + contact: { + _id: 'fixture:user:offline-bulk', + name: 'Offline User', + }, + roles: ['chw'] + })); + + const allDocItems = [contact0, place0, place1, place2]; + + before(async () => { + await utils.saveDocs(allDocItems); + await utils.createUsers([offlineUser]); + }); + + after(async () => { + await utils.revertDb([], true); + await utils.deleteUsers([offlineUser]); + }); + + describe('GET /api/v1/bulk-operations/:id', async () => { + const endpoint = '/api/v1/bulk-operations'; + + it('throws 404 when no operation matches the id', async () => { + await expect(utils.request({ path: `${endpoint}/not-a-real-id` })) + .to.be.rejectedWith('404 - {"code":404,"error":"Bulk operation not found"}'); + }); + + it('throws 403 for an offline user', async () => { + const opts = { + path: `${endpoint}/whatever`, + auth: { username: offlineUser.username, password: offlineUser.password }, + }; + await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); + }); + + it('returns the queued log for an operation started by a delete', async () => { + const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); + const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); + await utils.saveDocs([person, report]); + + const { id } = await utils.request({ path: `/api/v1/person/${person._id}`, method: 'DELETE' }); + expect(id).to.be.a('string'); + + const log = await utils.request({ path: `${endpoint}/${id}` }); + expect(log._id).to.equal(id); + const actions = Object.values(log.actions || {}); + expect(actions).to.have.lengthOf(1); + expect(actions[0]).to.include({ action: 'archive', status: 'queued' }); + }); + + describe.skip('once the archive handler is wired (#6615)', () => { + it('reports the operation as completed and removes the contacts', async () => { + const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); + await utils.saveDocs([person]); + + const { id } = await utils.request({ path: `/api/v1/person/${person._id}`, method: 'DELETE' }); + + let log; + for (let i = 0; i < 30; i++) { + log = await utils.request({ path: `${endpoint}/${id}` }); + const actions = Object.values(log.actions || {}); + if (actions.length && actions.every(action => action.status !== 'queued')) { + break; + } + await new Promise(resolve => setTimeout(resolve, 1000)); + } + + const actions = Object.values(log.actions || {}); + expect(actions.every(action => action.status === 'completed')).to.equal(true); + await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); + }); + }); + }); +}); diff --git a/tests/integration/api/controllers/person.spec.js b/tests/integration/api/controllers/person.spec.js index 5eea2894d1a..e37f6116cc8 100644 --- a/tests/integration/api/controllers/person.spec.js +++ b/tests/integration/api/controllers/person.spec.js @@ -2,6 +2,7 @@ const utils = require('@utils'); const placeFactory = require('@factories/cht/contacts/place'); const personFactory = require('@factories/cht/contacts/person'); const userFactory = require('@factories/cht/users/users'); +const reportFactory = require('@factories/cht/reports/generic-report'); const { USER_ROLES, CONTACT_TYPES } = require('@medic/constants'); const { expect } = require('chai'); @@ -492,4 +493,74 @@ describe('Person API', () => { }); }); }); + + describe('DELETE /api/v1/person/:uuid', async () => { + const endpoint = '/api/v1/person'; + + it('returns a dry-run summary and deletes nothing', async () => { + const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); + const reports = [ + reportFactory.report().build({ form: 'test-report' }, { patient: person }), + reportFactory.report().build({ form: 'test-report' }, { patient: person }), + ]; + await utils.saveDocs([person, ...reports]); + + const response = await utils.request({ + path: `${endpoint}/${person._id}`, + method: 'DELETE', + qs: { dry_run: true }, + }); + + expect(response).to.deep.equal({ + summary: { archive: { contacts: 1, reports: 2 }, 'set-contact': 0, 'delete-user': 0 }, + }); + const stillThere = await utils.getDoc(person._id); + expect(stillThere._id).to.equal(person._id); + }); + + it('throws 404 when the id is a place, not a person', async () => { + await expect(utils.request({ path: `${endpoint}/${place0._id}`, method: 'DELETE' })) + .to.be.rejectedWith('404 - {"code":404,"error":"Person not found"}'); + }); + + [ + ['does not have can_delete_contact_hierarchy permission', userNoPerms], + ['is not an online user', offlineUser] + ].forEach(([description, user]) => { + it(`throws 403 when user ${description}`, async () => { + const opts = { + path: `${endpoint}/${patient._id}`, + method: 'DELETE', + auth: { username: user.username, password: user.password }, + }; + await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); + }); + }); + + describe.skip('once the archive handler is wired (#6615)', () => { + const pollBulkOperation = async (id, tries = 30) => { + for (let i = 0; i < tries; i++) { + const log = await utils.request({ path: `/api/v1/bulk-operations/${id}` }); + const actions = Object.values(log.actions || {}); + if (actions.length && actions.every(action => action.status !== 'queued')) { + return log; + } + await new Promise(resolve => setTimeout(resolve, 1000)); + } + throw new Error(`bulk operation ${id} did not complete`); + }; + + it('removes the person and their reports', async () => { + const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); + const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); + await utils.saveDocs([person, report]); + + const { id } = await utils.request({ path: `${endpoint}/${person._id}`, method: 'DELETE' }); + await pollBulkOperation(id); + + await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); + await expect(utils.getDoc(report._id)).to.be.rejectedWith('404'); + }); + }); + }); }); diff --git a/tests/integration/api/controllers/place.spec.js b/tests/integration/api/controllers/place.spec.js index f58fe1f9d1b..5f8dd013295 100644 --- a/tests/integration/api/controllers/place.spec.js +++ b/tests/integration/api/controllers/place.spec.js @@ -2,6 +2,7 @@ const utils = require('@utils'); const placeFactory = require('@factories/cht/contacts/place'); const personFactory = require('@factories/cht/contacts/person'); const userFactory = require('@factories/cht/users/users'); +const reportFactory = require('@factories/cht/reports/generic-report'); const { USER_ROLES, CONTACT_TYPES } = require('@medic/constants'); const { expect } = require('chai'); @@ -591,4 +592,120 @@ describe('Place API', () => { }); }); }); + + describe('DELETE /api/v1/place/:uuid', async () => { + const endpoint = '/api/v1/place'; + + it('returns a dry-run summary of the subtree and deletes nothing', async () => { + const hc = placeFactory.place().build({ name: 'del-hc', type: CONTACT_TYPES.HEALTH_CENTER, contact: {} }); + const clinic = placeFactory.place().build({ + name: 'del-clinic', + type: CONTACT_TYPES.CLINIC, + contact: {}, + parent: { _id: hc._id }, + }); + const person = personFactory.build({ parent: { _id: clinic._id, parent: { _id: hc._id } } }); + const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); + await utils.saveDocs([hc, clinic, person, report]); + + const response = await utils.request({ + path: `${endpoint}/${hc._id}`, + method: 'DELETE', + qs: { dry_run: true }, + }); + + expect(response).to.deep.equal({ + summary: { archive: { contacts: 3, reports: 1 }, 'set-contact': 0, 'delete-user': 0 }, + }); + const stillThere = await utils.getDoc(hc._id); + expect(stillThere._id).to.equal(hc._id); + }); + + it('throws 400 when a linked user would be left behind and delete_users is not set', async () => { + const clinic = placeFactory.place().build({ + name: 'del-clinic-user', + type: CONTACT_TYPES.CLINIC, + contact: {}, + parent: { _id: place1._id, parent: { _id: place2._id } }, + }); + await utils.saveDocs([clinic]); + const linkedUser = userFactory.build({ + username: 'del-linked-user', + place: clinic._id, + contact: { _id: 'fixture:user:del-linked-user', name: 'Linked User' }, + roles: ['chw'], + }); + await utils.createUsers([linkedUser]); + + try { + await expect(utils.request({ path: `${endpoint}/${clinic._id}`, method: 'DELETE' })) + .to.be.rejectedWith(/400 - .*user\(s\) are linked to contacts/); + } finally { + await utils.deleteUsers([linkedUser]); + } + }); + + it('throws 404 when the id is a person, not a place', async () => { + await expect(utils.request({ path: `${endpoint}/${contact0._id}`, method: 'DELETE' })) + .to.be.rejectedWith('404 - {"code":404,"error":"Place not found"}'); + }); + + [ + ['does not have can_delete_contact_hierarchy permission', userNoPerms], + ['is not an online user', offlineUser] + ].forEach(([description, user]) => { + it(`throws 403 when user ${description}`, async () => { + const opts = { + path: `${endpoint}/${place0._id}`, + method: 'DELETE', + auth: { username: user.username, password: user.password }, + }; + await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); + }); + }); + + describe.skip('once the archive handler is wired (#6615)', () => { + const pollBulkOperation = async (id, tries = 30) => { + for (let i = 0; i < tries; i++) { + const log = await utils.request({ path: `/api/v1/bulk-operations/${id}` }); + const actions = Object.values(log.actions || {}); + if (actions.length && actions.every(action => action.status !== 'queued')) { + return log; + } + await new Promise(resolve => setTimeout(resolve, 1000)); + } + throw new Error(`bulk operation ${id} did not complete`); + }; + + it('removes the place, its subtree and (with delete_users) the linked users', async () => { + const clinic = placeFactory.place().build({ + name: 'del-clinic-full', + type: CONTACT_TYPES.CLINIC, + contact: {}, + parent: { _id: place1._id, parent: { _id: place2._id } }, + }); + const person = personFactory.build({ parent: { _id: clinic._id, parent: clinic.parent } }); + const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); + await utils.saveDocs([clinic, person, report]); + const linkedUser = userFactory.build({ + username: 'del-full-user', + place: clinic._id, + contact: { _id: 'fixture:user:del-full-user', name: 'Full User' }, + roles: ['chw'], + }); + await utils.createUsers([linkedUser]); + + const { id } = await utils.request({ + path: `${endpoint}/${clinic._id}`, + method: 'DELETE', + qs: { delete_users: true }, + }); + await pollBulkOperation(id); + + await expect(utils.getDoc(clinic._id)).to.be.rejectedWith('404'); + await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); + await expect(utils.getDoc(report._id)).to.be.rejectedWith('404'); + }); + }); + }); }); From 336134067e1d6f692d09c2dfac6f4772e5f0320f Mon Sep 17 00:00:00 2001 From: Vikrant Kumar Date: Mon, 13 Jul 2026 21:29:57 +0530 Subject: [PATCH 16/16] test(#10706): drop the archive-gated skipped tests flagged by sonarcloud The poll-to-removal tests depend on the #6615 archive handler and were parked in describe.skip, which trips sonar's "no skipped tests" rule. They land with that handler instead. Also make the new describe callbacks synchronous. --- .../api/controllers/bulk-operations.spec.js | 25 +--------- .../api/controllers/person.spec.js | 28 +---------- .../integration/api/controllers/place.spec.js | 46 +------------------ 3 files changed, 3 insertions(+), 96 deletions(-) diff --git a/tests/integration/api/controllers/bulk-operations.spec.js b/tests/integration/api/controllers/bulk-operations.spec.js index bc84c39e9bf..dea3560e315 100644 --- a/tests/integration/api/controllers/bulk-operations.spec.js +++ b/tests/integration/api/controllers/bulk-operations.spec.js @@ -35,7 +35,7 @@ describe('Bulk operations API', () => { await utils.deleteUsers([offlineUser]); }); - describe('GET /api/v1/bulk-operations/:id', async () => { + describe('GET /api/v1/bulk-operations/:id', () => { const endpoint = '/api/v1/bulk-operations'; it('throws 404 when no operation matches the id', async () => { @@ -65,28 +65,5 @@ describe('Bulk operations API', () => { expect(actions).to.have.lengthOf(1); expect(actions[0]).to.include({ action: 'archive', status: 'queued' }); }); - - describe.skip('once the archive handler is wired (#6615)', () => { - it('reports the operation as completed and removes the contacts', async () => { - const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); - await utils.saveDocs([person]); - - const { id } = await utils.request({ path: `/api/v1/person/${person._id}`, method: 'DELETE' }); - - let log; - for (let i = 0; i < 30; i++) { - log = await utils.request({ path: `${endpoint}/${id}` }); - const actions = Object.values(log.actions || {}); - if (actions.length && actions.every(action => action.status !== 'queued')) { - break; - } - await new Promise(resolve => setTimeout(resolve, 1000)); - } - - const actions = Object.values(log.actions || {}); - expect(actions.every(action => action.status === 'completed')).to.equal(true); - await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); - }); - }); }); }); diff --git a/tests/integration/api/controllers/person.spec.js b/tests/integration/api/controllers/person.spec.js index e37f6116cc8..e2c2210855d 100644 --- a/tests/integration/api/controllers/person.spec.js +++ b/tests/integration/api/controllers/person.spec.js @@ -494,7 +494,7 @@ describe('Person API', () => { }); }); - describe('DELETE /api/v1/person/:uuid', async () => { + describe('DELETE /api/v1/person/:uuid', () => { const endpoint = '/api/v1/person'; it('returns a dry-run summary and deletes nothing', async () => { @@ -536,31 +536,5 @@ describe('Person API', () => { await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); }); }); - - describe.skip('once the archive handler is wired (#6615)', () => { - const pollBulkOperation = async (id, tries = 30) => { - for (let i = 0; i < tries; i++) { - const log = await utils.request({ path: `/api/v1/bulk-operations/${id}` }); - const actions = Object.values(log.actions || {}); - if (actions.length && actions.every(action => action.status !== 'queued')) { - return log; - } - await new Promise(resolve => setTimeout(resolve, 1000)); - } - throw new Error(`bulk operation ${id} did not complete`); - }; - - it('removes the person and their reports', async () => { - const person = personFactory.build({ parent: { _id: place0._id, parent: place0.parent } }); - const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); - await utils.saveDocs([person, report]); - - const { id } = await utils.request({ path: `${endpoint}/${person._id}`, method: 'DELETE' }); - await pollBulkOperation(id); - - await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); - await expect(utils.getDoc(report._id)).to.be.rejectedWith('404'); - }); - }); }); }); diff --git a/tests/integration/api/controllers/place.spec.js b/tests/integration/api/controllers/place.spec.js index 5f8dd013295..3494e49da8e 100644 --- a/tests/integration/api/controllers/place.spec.js +++ b/tests/integration/api/controllers/place.spec.js @@ -593,7 +593,7 @@ describe('Place API', () => { }); }); - describe('DELETE /api/v1/place/:uuid', async () => { + describe('DELETE /api/v1/place/:uuid', () => { const endpoint = '/api/v1/place'; it('returns a dry-run summary of the subtree and deletes nothing', async () => { @@ -663,49 +663,5 @@ describe('Place API', () => { await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); }); }); - - describe.skip('once the archive handler is wired (#6615)', () => { - const pollBulkOperation = async (id, tries = 30) => { - for (let i = 0; i < tries; i++) { - const log = await utils.request({ path: `/api/v1/bulk-operations/${id}` }); - const actions = Object.values(log.actions || {}); - if (actions.length && actions.every(action => action.status !== 'queued')) { - return log; - } - await new Promise(resolve => setTimeout(resolve, 1000)); - } - throw new Error(`bulk operation ${id} did not complete`); - }; - - it('removes the place, its subtree and (with delete_users) the linked users', async () => { - const clinic = placeFactory.place().build({ - name: 'del-clinic-full', - type: CONTACT_TYPES.CLINIC, - contact: {}, - parent: { _id: place1._id, parent: { _id: place2._id } }, - }); - const person = personFactory.build({ parent: { _id: clinic._id, parent: clinic.parent } }); - const report = reportFactory.report().build({ form: 'test-report' }, { patient: person }); - await utils.saveDocs([clinic, person, report]); - const linkedUser = userFactory.build({ - username: 'del-full-user', - place: clinic._id, - contact: { _id: 'fixture:user:del-full-user', name: 'Full User' }, - roles: ['chw'], - }); - await utils.createUsers([linkedUser]); - - const { id } = await utils.request({ - path: `${endpoint}/${clinic._id}`, - method: 'DELETE', - qs: { delete_users: true }, - }); - await pollBulkOperation(id); - - await expect(utils.getDoc(clinic._id)).to.be.rejectedWith('404'); - await expect(utils.getDoc(person._id)).to.be.rejectedWith('404'); - await expect(utils.getDoc(report._id)).to.be.rejectedWith('404'); - }); - }); }); });