Skip to content

feat(#10706): add bulk operation framework and contact hierarchy delete#11236

Open
vikrantwiz02 wants to merge 17 commits into
medic:10706-dmp-2026-add-api-endpoints-for-advanced-contact-management-operationsfrom
vikrantwiz02:10706-delete-contact-hierarchy
Open

feat(#10706): add bulk operation framework and contact hierarchy delete#11236
vikrantwiz02 wants to merge 17 commits into
medic:10706-dmp-2026-add-api-endpoints-for-advanced-contact-management-operationsfrom
vikrantwiz02:10706-delete-contact-hierarchy

Conversation

@vikrantwiz02

@vikrantwiz02 vikrantwiz02 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Description

The server-side implementation of contact-hierarchy delete for #10706. A DELETE on the person and place endpoints gathers everything the delete touches and queues a bulk operation that Sentinel executes asynchronously, so the slow work runs off the request. The bulk operation framework is generic: delete is its first consumer, and move and merge will reuse it.

What's here, one commit per layer:

  • Document model + constants: the log doc and per-action doc shapes, with the id prefixes and enums shared through @medic/constants.
  • Queue helper: writes the log doc to medic-logs and one action doc per type to medic-sentinel.
  • GET /api/v1/bulk-operations/{id}: poll an operation's status.
  • Sentinel: a medic-sentinel changes-feed listener plus a batch/cursor processor, with the set-contact and delete-user handlers.
  • DELETE /api/v1/person/{id} and /api/v1/place/{id}: gated on can_delete_contact_hierarchy, with delete_users (requires can_delete_users, else a 400 when linked users exist) and dry_run query params; each endpoint rejects an id of the wrong type with a 404; returns a summary of the changes plus the operation id.

The archive action handler (copy to the archive database and purge) builds on the archiving work in #6615 / #11139, so it lands once that merges; the framework and the other handlers stand on their own.

Endpoint behaviour is covered by integration tests: the permissions, the dry-run summaries and the type/linked-user error cases. The removal assertions poll the operation to completion, so they land with the archive handler.

Supersedes the earlier delete PRs, now closed and folded in here: #11163, #11167, #11172, #11176 (the first synchronous attempt) and #11222, #11223 (the split framework pieces).

Part of #10706

Notes

The gather-and-queue logic lives once in deleteContactHierarchy and is contact-type-agnostic, a person delete is just the leaf case of a place-subtree delete. The person and place endpoints each call a shared handler with their own type, and the handler checks the target is that type before doing anything, so a place id sent to /person/{id} is rejected with a 404 instead of deleting the place hierarchy, and vice versa. The delete_users/dry_run params and the responses are shared OpenAPI components referenced from both.

Code review checklist

  • Readable: Concise, well named, follows the style guide
  • Documented: OpenAPI annotations for the new endpoints
  • Tested: Unit tests across each layer, plus integration tests for the endpoint permissions, dry-run summaries and error cases
  • Backwards compatible: New endpoints and code; works with existing data and configuration
  • AI disclosure: Please disclose use of AI per the guidelines.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

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.
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.
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.
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.
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.
Compute the delete-user operations once (lowering the function's cognitive
complexity) and drop an unreachable empty-subject guard.
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.
…ld 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.
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.
…anagement-operations' into 10706-delete-contact-hierarchy
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.
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.
@jkuester
jkuester self-requested a review July 8, 2026 19:08

@jkuester jkuester left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vikrantwiz02 this is great! Your code structure is very clear and tight and everything follows the design! ❤️

I am going to post my first-pass comments now, so you can start on them (have not had a change to look at the tests yet). Also, just disregard the comments starting with TODO 😅 . I am still thinking through some of the failure cases in Sentinel and do not have my thoughts completely formed yet on if we need to do anything different for some of that...

STATUSES,
OPERATIONS_ATTACHMENT,
buildBulkOperation,
decodeOperations,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used anywhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, wasn't used anywhere, so I dropped it. Constants moved into @medic/constants and the rest of the doc model into the api service.

@@ -0,0 +1,11 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this as a new shared-lib. Typically we have used shared-libs to avoid duplicated logic and not as an organizational tool for grouping related code.

In this case, it looks the only part of this lib that is actually shared between api and sentinel are some of the hardcoded constants. Any of these contstants that need to be shared can go into shared-libs/constants. For the rest of the logic here, I think we can just move it into the relevant api/sentinel code that is currently consuming it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, moved the constants into @medic/constants and folded the doc model into the api service. decodeOperations wasn't used anywhere so I dropped it.

Comment thread api/src/controllers/person.js Outdated
* summary: Delete a person and their hierarchy
* operationId: v1PersonIdDelete
* description: >
* Queues an asynchronous bulk operation that removes the person, every descendant contact,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not say "every descendant contact," here. In general, person contacts should not have descendants. (The contact hierarchy is location-based, not a literal ancestory of the contacts. So, a parent and their children might all be "descendants" of the same household place.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded, dropped the "every descendant contact" bit and shortened the summary to just "Delete a person".

Comment thread api/src/controllers/person.js Outdated
* 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* x-since: 5.2.0
* x-since: 5.3.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread api/src/controllers/person.js Outdated
* tags: [Person]
* x-since: 5.2.0
* x-permissions:
* hasAll: [can_delete_contact_hierarchy]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* hasAll: [can_delete_contact_hierarchy]
* hasAll: [can_delete_contact_hierarchy, can_delete_users]

We don't really have a format here for specifying permissions that are conditionally required. I think it is probably better to just mention both here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, listed both.

Comment thread sentinel/src/lib/bulk-operations.js Outdated
const failed = [];
for (const op of batch) {
try {
await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, ''));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to have an explicit check that op.id is valued and just skip straight to the failure if it does not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (added to both handlers).

Comment on lines +105 to +109
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 },
]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these operations are getting executed in order, I think we should be intentional so that we only actually delete the contacts once the rest of the related data has already been cleaned up:

Suggested change
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 },
]);
const bulkOperationId = await bulkOperations.queue([
{ action: ACTIONS.SET_CONTACT, operations: setContactOperations },
{ action: ACTIONS.DELETE_USER, operations: userOperations },
{ action: ACTIONS.ARCHIVE, operations: [ ...reportIds, ...contactIds ].map(docId => ({ id: docId })) },
]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, set-contact and delete-user run before archive now.

Comment thread sentinel/src/lib/bulk-operations.js Outdated
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically a report also has a contact property that could be updated with this action. (Not being used in our current workflow, but could possibly be leveraged in the future.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left it for now, can add updating the report's contact ref as a follow-up if we want it.

Comment thread sentinel/src/lib/bulk-operations.js Outdated
const doc = docsById[op.id];
const currentContactId = doc && (doc.contact?._id || doc.contact);
if (!doc || currentContactId !== op.current_contact_id) {
failed.push(op);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we log a message when failing the operation? Would be ideal to include the action id and indicate if it failed because the doc was missing or because the contact id changed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, logs the action id and whether it was a missing doc or a changed contact.

Comment thread sentinel/src/lib/bulk-operations.js Outdated
try {
await userManagement.deleteUser(op.id.replace(PREFIXES.COUCH_USER, ''));
} catch (err) {
logger.error(`bulk-operations: failed to delete user ${op.id}: %o`, err);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also include the action id in the log message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

…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
…n 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.
@vikrantwiz02
vikrantwiz02 requested a review from jkuester July 10, 2026 04:12
…points

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 medic#6615, so those are skipped until it lands.
…narcloud

The poll-to-removal tests depend on the medic#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants