fix(document): auto-reject modifications naming deleted linked entities

[ClemRz]

🤔 What

Validating a pending document modification could fail with a 500 that never cleared, leaving the document permanently unvalidatable. Fixes #1815.

• Check that every many-to-many member named in modifiedDocJson still exists before replaceCollection writes it
• Auto-reject the modification when one does not, instead of letting the foreign-key violation surface as a 500
• Return 400 from PUT /documents/{id}/new-entities when a client-supplied id does not resolve
• Write the canonical primary key back to the junction tables, and collapse duplicate members

🤷‍♂️ Why

modifiedDocJson is a snapshot of a proposed edit, taken at submission time and stored as json — the one place the foreign-key graph does not reach. Postgres will hold a reference to a row that no longer exists, indefinitely, and the failure only surfaces when a moderator next validates the document.

replaceM2MCollections handed those stored ids straight to replaceCollection, which destroys the junction rows and re-inserts them. Postgres rejected the insert with 23503, the request failed with a 500, and every retry failed the same way — the contributor's edit was stuck with no way for a moderator to clear it.

This fired twice in production on document 14958 (caver 23871, deleted). Document 229736 had been stuck since 2025-05-20 on subject code 2.11, pruned from the taxonomy — and that second case is why the guard belongs at validation time rather than in the delete controllers: no entity-delete controller was ever involved, and every constraint in the database was satisfied at every moment.

validateAndUpdateDocument already re-validates parent at validation time for exactly this reason — "the hierarchy may have changed since the modification was submitted". The seven m2m collections had no equivalent guard. This extends that principle and reuses the same auto-reject mechanism.

🔍 How

DocumentService.resolveM2MMembers(collectionData, db) returns { missing, resolved }: the members that no longer resolve, and the primary keys as read back from the target table. The target model is derived from TDocument.attributes[field].collection rather than a second hardcoded map, so the two cannot drift apart.

Two call sites, two policies:

Path	Behaviour
PUT /documents/validate	Auto-reject — clear modifiedDocJson, prefix validationComment with Auto-rejected:, notify the author with REJECT. A stale snapshot is not the moderator's fault, and the rest of the batch keeps processing.
PUT /documents/{id}/new-entities	400 listing the unknown ids. Here they come straight from the client, so it is a bad request.

Three subtleties worth a reviewer's attention:

• Blank padding. t_subject.code is bpchar(8), t_language.id bpchar(3), t_country.iso bpchar(2). node-postgres returns those padded, so a snapshot stores subject 2 as '2 '. A bpchar = text comparison casts the column to text — stripping its padding while leaving it on the parameter — so the two never match. Without trimming both sides the check reports every short code as missing and would have discarded valid edits. Confirmed against production: 229736's snapshot held "2.11 ".
• Returning resolved keys, not just a verdict. A padded member would otherwise be written verbatim into j_document_subject.code_subject and the populate join would find nothing — silent data loss. Resolving to the stored key also collapses duplicates such as ['1.0', '1.0 '], which were a 23505 primary-key violation on the junction composite key.
• validation_comment is varchar(300). The reason lists the missing members, so it is truncated to fit; the untruncated text always goes to the log.

null, empty and non-numeric members are now reported rather than letting Waterline raise E_INVALID_CRITERIA, which would have turned one 500 into another.

Out of scope. The batch is still non-atomic: a document that throws aborts the request after earlier documents have already committed, and fixing it properly means replacing the 204-with-no-body response with a per-document report, which the front end reads. Tracked in #1817, which needs a coordinated front-end change. This PR reduces how often it can fire without removing the fragility.

Production state. 229736's snapshot was repaired directly in the database (the pruned subject key removed from documentData) so its pending edit applies rather than being auto-rejected by this change. 14958 no longer shows the dangling author and needs no repair.

🧪 Testing

npm test — 19 new tests, all green.

Negative control: remove the trim() from normalizeMemberKey in DocumentService.js and four tests fail — both padded-code service tests, the duplicate-collapse test, and the padded-subject route test. The test schema is generated from the model definitions by Waterline migrate: 'drop', so those columns come out as character varying rather than bpchar and the trap is invisible from the column side; the tests reproduce it from the member side instead, which is exactly the production asymmetry.

To exercise the fix by hand: stage a modifiedDocJson naming a caver, delete the caver, then validate the document. Expect 204, modifiedDocJson cleared, validationComment starting Auto-rejected:, and a REJECT notification to the author — not a 500.

Unrelated pre-existing failure: test/integration/4_routes/Changes/get-recent-comment-relevance-swap.test.js fails 4 tests in shard 3, identically on develop with and without this branch. It passes in isolation, so it is a shard-ordering interaction.

📸 Previews

N/A — API only, no UI surface.

[Paul-AUB]

Issues (Must Fix)

1. [api/controllers/v1/document/multiple-validate.js:45] Auto-rejection leaves the document in the refused state instead of restoring its last validated state. A pending edit sets isValidated to false and dateValidation to null. This helper clears modifiedDocJson and sets dateValidation, but never restores isValidated. find-all.js then excludes the record from both the default validated list and the moderation queue (isValidated: false is only returned when dateValidation is null), so a stale linked entity makes the original document disappear rather than returning it to its last validated state. Please restore the validation flag for this rejection path and assert it in the stale-member route test; the existing parent-cycle asymmetry is also worth fixing now that this helper extends it to the new path.
2. [api/services/DocumentService.js:908] Finite numeric values are not necessarily valid IDs, so malformed members can still produce the 500 this guard is intended to prevent. For the serial/int4-backed collections, values such as 1.5 or 2147483648 pass Number.isFinite(Number(key)). Waterline accepts them as numbers, but PostgreSQL rejects them when evaluating the integer primary-key criterion, so Model.find() throws before the value can be reported in missing. The direct endpoint therefore returns 500 instead of 400, and the ordinary update path can persist the same value in modifiedDocJson, recreating a permanently failing moderation item. Please validate the integer PK domain (including range) or translate criterion/adapter failures into unresolved members, with route/service coverage for fractional and out-of-range IDs.

Suggestions (Should Consider)

3. [api/controllers/v1/document/multiple-validate.js:132] Consider closing the time-of-check/time-of-use gap between resolution and replacement. Both controllers resolve members before opening their write transaction. A target row can be deleted after Model.find() succeeds but before replaceCollection() inserts the junction row, which still yields a 23503/500 and, on validation, leaves the modification stuck. Resolving under an appropriate row-locking strategy, or catching the rolled-back FK failure and applying the same auto-reject/400 policy, would make the new guarantee hold under concurrent deletes as well.