From 5760157937685c9184f587b0217367a83dde3d30 Mon Sep 17 00:00:00 2001 From: Yash Sharma Date: Wed, 20 May 2026 02:23:49 +0530 Subject: [PATCH 01/30] feat(#10904): route file attachments to correct sub-docs in report forms (#10922) Co-authored-by: Bernard K. --- .../enketo/forms/db-doc-file-upload.xml | 41 ++++ .../enketo/forms/db-doc-multi-file-upload.xml | 50 ++++ .../forms/db-doc-repeat-file-upload.xml | 40 ++++ .../submit-db-doc-file-upload.wdio-spec.js | 179 +++++++++++++++ webapp/src/ts/services/enketo.service.ts | 56 ++++- .../db-doc-in-repeat-with-files.xml | 15 ++ .../enketo-xml/db-doc-orphan-file.xml | 7 + .../enketo-xml/db-doc-with-binary.xml | 14 ++ .../db-doc-with-file-field-cleared.xml | 8 + .../enketo-xml/db-doc-with-file-field.xml | 8 + .../karma/ts/services/enketo.service.spec.ts | 214 ++++++++++++++++++ 11 files changed, 627 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/default/enketo/forms/db-doc-file-upload.xml create mode 100644 tests/e2e/default/enketo/forms/db-doc-multi-file-upload.xml create mode 100644 tests/e2e/default/enketo/forms/db-doc-repeat-file-upload.xml create mode 100644 tests/e2e/default/enketo/submit-db-doc-file-upload.wdio-spec.js create mode 100644 webapp/tests/karma/ts/services/enketo-xml/db-doc-in-repeat-with-files.xml create mode 100644 webapp/tests/karma/ts/services/enketo-xml/db-doc-orphan-file.xml create mode 100644 webapp/tests/karma/ts/services/enketo-xml/db-doc-with-binary.xml create mode 100644 webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field-cleared.xml create mode 100644 webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field.xml diff --git a/tests/e2e/default/enketo/forms/db-doc-file-upload.xml b/tests/e2e/default/enketo/forms/db-doc-file-upload.xml new file mode 100644 index 00000000000..99cdb0e021d --- /dev/null +++ b/tests/e2e/default/enketo/forms/db-doc-file-upload.xml @@ -0,0 +1,41 @@ + + + + db-doc-file-upload + + + + + + + + sub_record + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/e2e/default/enketo/forms/db-doc-multi-file-upload.xml b/tests/e2e/default/enketo/forms/db-doc-multi-file-upload.xml new file mode 100644 index 00000000000..231549f827d --- /dev/null +++ b/tests/e2e/default/enketo/forms/db-doc-multi-file-upload.xml @@ -0,0 +1,50 @@ + + + + db-doc-multi-file-upload + + + + + + + + sub_record_a + + + + + sub_record_b + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/e2e/default/enketo/forms/db-doc-repeat-file-upload.xml b/tests/e2e/default/enketo/forms/db-doc-repeat-file-upload.xml new file mode 100644 index 00000000000..90479783342 --- /dev/null +++ b/tests/e2e/default/enketo/forms/db-doc-repeat-file-upload.xml @@ -0,0 +1,40 @@ + + + + db-doc-repeat-file-upload + + + + + + + + repeat_record + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/e2e/default/enketo/submit-db-doc-file-upload.wdio-spec.js b/tests/e2e/default/enketo/submit-db-doc-file-upload.wdio-spec.js new file mode 100644 index 00000000000..6c0377478a4 --- /dev/null +++ b/tests/e2e/default/enketo/submit-db-doc-file-upload.wdio-spec.js @@ -0,0 +1,179 @@ +const commonPage = require('@page-objects/default/common/common.wdio.page'); +const commonEnketoPage = require('@page-objects/default/enketo/common-enketo.wdio.page'); +const reportsPage = require('@page-objects/default/reports/reports.wdio.page'); +const utils = require('@utils'); +const path = require('path'); +const loginPage = require('@page-objects/default/login/login.wdio.page'); +const genericForm = require('@page-objects/default/enketo/generic-form.wdio.page'); + +describe('Submit form with file uploads routed to db-doc sub-documents (#10904)', () => { + const mainImagePath = path.join(__dirname, '/images/photo-for-upload-form.png'); + const subImagePath = path.join(__dirname, '../../../../webapp/src/img/layers.png'); + const subImagePath2 = path.join(__dirname, '../../../../webapp/src/img/icon.png'); + + before(async () => { + await utils.saveDocIfNotExists(commonPage.createFormDoc(`${__dirname}/forms/db-doc-file-upload`)); + await utils.saveDocIfNotExists(commonPage.createFormDoc(`${__dirname}/forms/db-doc-multi-file-upload`)); + await utils.saveDocIfNotExists(commonPage.createFormDoc(`${__dirname}/forms/db-doc-repeat-file-upload`)); + await loginPage.cookieLogin(); + await commonPage.hideSnackbar(); + }); + + it('should route file attachments to the correct owner doc', async () => { + await commonPage.goToReports(); + await commonPage.openFastActionReport('db-doc-file-upload', false); + + await commonEnketoPage.setInputValue('Name', 'Test Sub Doc Upload'); + await commonEnketoPage.addFileInputValue('Main photo', mainImagePath); + await commonEnketoPage.addFileInputValue('Sub doc photo', subImagePath); + + await genericForm.submitForm(); + await commonPage.waitForPageLoaded(); + + const reportId = await reportsPage.getCurrentReportId(); + const report = await utils.getDoc(reportId); + + expect(report.fields.name).to.equal('Test Sub Doc Upload'); + + // The main doc should have the main_photo attachment (xpath-based name for binary widget) + const mainAttachments = Object.keys(report._attachments || {}); + const mainPhotoAttachment = mainAttachments.find(name => name.match(/^user-file-photo-for-upload-form.*\.png$/)); + expect(mainPhotoAttachment, 'Main photo should be attached to the report doc').to.exist; + + // The sub-doc ID is stored via db-doc-ref + const subDocId = report.fields.sub_doc_ref; + expect(subDocId, 'Sub-doc reference should be populated by db-doc-ref').to.exist; + + // Fetch the sub-doc and verify it has the sub_photo attachment + const subDoc = await utils.getDoc(subDocId); + expect(subDoc, 'Sub-doc should exist in the database').to.exist; + expect(subDoc.type).to.equal('sub_record'); + + const subAttachments = Object.keys(subDoc._attachments || {}); + const subPhotoAttachment = subAttachments.find(name => name.match(/^user-file-layers.*\.png$/)); + expect(subPhotoAttachment, 'Sub-doc photo should be attached to the sub-doc').to.exist; + expect(subDoc._attachments[subPhotoAttachment].content_type).to.equal('image/png'); + + // The sub-doc file should NOT be on the main doc + const mainHasSubFile = mainAttachments.find(name => name.match(/layers/)); + expect(mainHasSubFile, 'Sub-doc file should not appear on the main doc').to.not.exist; + }); + + it('should route file attachments correctly with multiple sub-docs', async () => { + await commonPage.goToReports(); + await commonPage.openFastActionReport('db-doc-multi-file-upload', false); + + await commonEnketoPage.setInputValue('Name', 'Test Multi Sub Doc'); + await commonEnketoPage.addFileInputValue('Main photo', mainImagePath); + await commonEnketoPage.addFileInputValue('Sub doc A photo', subImagePath); + await commonEnketoPage.addFileInputValue('Sub doc B photo', subImagePath2); + + await genericForm.submitForm(); + await commonPage.waitForPageLoaded(); + + const reportId = await reportsPage.getCurrentReportId(); + const report = await utils.getDoc(reportId); + + expect(report.fields.name).to.equal('Test Multi Sub Doc'); + + const mainAttachments = Object.keys(report._attachments || {}); + const mainPhotoAttachment = mainAttachments.find(name => name.match(/^user-file-photo-for-upload-form.*\.png$/)); + expect(mainPhotoAttachment, 'Main photo should be attached to the report doc').to.exist; + + // Verify sub-doc A + const subDocAId = report.fields.sub_doc_a_ref; + expect(subDocAId, 'Sub-doc A reference should be populated').to.exist; + + const subDocA = await utils.getDoc(subDocAId); + expect(subDocA.type).to.equal('sub_record_a'); + + const subAAttachments = Object.keys(subDocA._attachments || {}); + const subAPhoto = subAAttachments.find(name => name.match(/^user-file-layers.*\.png$/)); + expect(subAPhoto, 'Sub-doc A photo should be attached to sub-doc A').to.exist; + expect(subDocA._attachments[subAPhoto].content_type).to.equal('image/png'); + + // Verify sub-doc B + const subDocBId = report.fields.sub_doc_b_ref; + expect(subDocBId, 'Sub-doc B reference should be populated').to.exist; + + const subDocB = await utils.getDoc(subDocBId); + expect(subDocB.type).to.equal('sub_record_b'); + + const subBAttachments = Object.keys(subDocB._attachments || {}); + const subBPhoto = subBAttachments.find(name => name.match(/^user-file-icon.*\.png$/)); + expect(subBPhoto, 'Sub-doc B photo should be attached to sub-doc B').to.exist; + expect(subDocB._attachments[subBPhoto].content_type).to.equal('image/png'); + + // Cross-contamination checks: no sub-doc files on main doc + const mainHasSubAFile = mainAttachments.find(name => name.match(/layers/)); + expect(mainHasSubAFile, 'Sub-doc A file should not appear on the main doc').to.not.exist; + const mainHasSubBFile = mainAttachments.find(name => name.match(/icon/)); + expect(mainHasSubBFile, 'Sub-doc B file should not appear on the main doc').to.not.exist; + + // No cross-contamination between sub-docs + const subAHasSubBFile = subAAttachments.find(name => name.match(/icon/)); + expect(subAHasSubBFile, 'Sub-doc B file should not appear on sub-doc A').to.not.exist; + const subBHasSubAFile = subBAttachments.find(name => name.match(/layers/)); + expect(subBHasSubAFile, 'Sub-doc A file should not appear on sub-doc B').to.not.exist; + }); + + it('should route file attachments to sub-docs inside repeat groups', async () => { + await commonPage.goToReports(); + await commonPage.openFastActionReport('db-doc-repeat-file-upload', false); + + await commonEnketoPage.setInputValue('Name', 'Test Repeat Sub Doc'); + + // First repeat instance is already present; upload a file to it + await commonEnketoPage.addFileInputValue('Repeat photo', mainImagePath, { repeatIndex: 0 }); + + // Add a second repeat and upload a different file + await commonEnketoPage.addRepeatSection(); + await commonEnketoPage.addFileInputValue('Repeat photo', subImagePath, { repeatIndex: 1 }); + + await genericForm.submitForm(); + await commonPage.waitForPageLoaded(); + + const reportId = await reportsPage.getCurrentReportId(); + const report = await utils.getDoc(reportId); + + expect(report.fields.name).to.equal('Test Repeat Sub Doc'); + + // There should be two repeat sections, each with a db-doc-ref + const repeatSections = report.fields.repeat_section; + expect(repeatSections).to.be.an('array'); + expect(repeatSections.length).to.equal(2); + + // Verify first repeat sub-doc + const repeatDoc1Id = repeatSections[0].repeat_doc_ref; + expect(repeatDoc1Id, 'First repeat doc reference should be populated').to.exist; + + const repeatDoc1 = await utils.getDoc(repeatDoc1Id); + expect(repeatDoc1.type).to.equal('repeat_record'); + + const repeat1Attachments = Object.keys(repeatDoc1._attachments || {}); + const repeat1Photo = repeat1Attachments.find(name => name.match(/^user-file-photo-for-upload-form.*\.png$/)); + expect(repeat1Photo, 'First repeat doc should have its photo attached').to.exist; + + // Verify second repeat sub-doc + const repeatDoc2Id = repeatSections[1].repeat_doc_ref; + expect(repeatDoc2Id, 'Second repeat doc reference should be populated').to.exist; + + const repeatDoc2 = await utils.getDoc(repeatDoc2Id); + expect(repeatDoc2.type).to.equal('repeat_record'); + + const repeat2Attachments = Object.keys(repeatDoc2._attachments || {}); + const repeat2Photo = repeat2Attachments.find(name => name.match(/^user-file-layers.*\.png$/)); + expect(repeat2Photo, 'Second repeat doc should have its photo attached').to.exist; + + // No file attachments should be on the main report doc + const mainAttachments = Object.keys(report._attachments || {}); + const mainUserFiles = mainAttachments.filter(name => name.startsWith('user-file-')); + expect(mainUserFiles, 'No user files should be on the main doc').to.be.empty; + + // No cross-contamination between repeat sub-docs + const repeat1HasRepeat2File = repeat1Attachments.find(name => name.match(/layers/)); + expect(repeat1HasRepeat2File, 'Repeat 2 file should not appear on repeat 1 doc').to.not.exist; + const repeat2HasRepeat1File = repeat2Attachments.find(name => name.match(/photo-for-upload-form/)); + expect(repeat2HasRepeat1File, 'Repeat 1 file should not appear on repeat 2 doc').to.not.exist; + }); +}); diff --git a/webapp/src/ts/services/enketo.service.ts b/webapp/src/ts/services/enketo.service.ts index 2a219d00e0f..d3c66879957 100644 --- a/webapp/src/ts/services/enketo.service.ts +++ b/webapp/src/ts/services/enketo.service.ts @@ -362,6 +362,22 @@ export class EnketoService { return form; } + private findFileNodeByFilename($record, filename: string) { + // After upload, Enketo's Nodeset.setVal rewrites file-widget nodes from + // type="binary" to type="file" (#10903 §6.5). Inline-binary blobs from + // draw/signature widgets keep type="binary" and are handled separately. + let match = null; + $record + .find('[type=file]') + .each((_idx, element) => { + if ($(element).text() === filename) { + match = element; + return false; // break + } + }); + return match; + } + private xmlToDocs(doc, formXml, xmlVersion, record) { const recordDoc = $.parseXML(record); const $record = $($(recordDoc).children()[0]); @@ -481,16 +497,49 @@ export class EnketoService { } doc.hidden_fields = this.enketoTranslationService.getHiddenFieldList(record, dbDocTags); + // Build a lookup map: sub-doc _couchId -> prepared doc object + const subDocById = new Map(); + docsToStore.forEach(subDoc => subDocById.set(subDoc._id, subDoc)); + + // Resolve the owner document for a given XML element by walking + // up to the nearest [db-doc="true"] ancestor. Falls back to the + // main report doc when the element is not inside a sub-doc. + const resolveOwnerDoc = (element) => { + let node = element.parentNode; + while (node && node !== recordDoc) { + if (node._couchId && subDocById.has(node._couchId)) { + return subDocById.get(node._couchId); + } + node = node.parentNode; + } + return doc; + }; + + // Route FileManager files to the correct owner doc. + // For each file, find the [type=file] node whose text matches + // the filename, then resolve the owner from its position in the + // XML tree. FileManager .getCurrentFiles() - .forEach(file => this.attachmentService.add(doc, `user-file-${file.name}`, file, file.type, false)); + .forEach(file => { + const ownerDoc = resolveOwnerDoc( + this.findFileNodeByFilename($record, file.name) ?? $record[0] + ); + this.attachmentService.add(ownerDoc, `user-file-${file.name}`, file, file.type, false); + }); + // The legacy filename scheme is xpath-based (rooted at doc.form) because + // inline-binary widgets (draw/signature) do not store the filename as a + // question value, so xpath is the only stable mapping from answer to + // attachment. The xpath naming is unchanged when routing to a sub-doc; + // only the target doc differs. const attachLegacyFile = (elem, file, type, alreadyEncoded) => { + const ownerDoc = resolveOwnerDoc(elem); const xpath = Xpath.getElementXPath(elem); // replace instance root element node name with form internal ID const filename = 'user-file' + (xpath.startsWith('/' + doc.form) ? xpath : xpath.replace(/^\/[^/]+/, '/' + doc.form)); - this.attachmentService.add(doc, filename, file, type, alreadyEncoded); + this.attachmentService.add(ownerDoc, filename, file, type, alreadyEncoded); }; $record @@ -498,9 +547,6 @@ export class EnketoService { .each((idx, element) => { const file = $(element).text(); if (file) { - // Attach binary file with legacy-style filename because the actual filename is not stored as the question - // value in the form model (and so there is currently no way to map the answer in a saved report to the - // associated file attachment). attachLegacyFile(element, file, 'image/png', true); } }); diff --git a/webapp/tests/karma/ts/services/enketo-xml/db-doc-in-repeat-with-files.xml b/webapp/tests/karma/ts/services/enketo-xml/db-doc-in-repeat-with-files.xml new file mode 100644 index 00000000000..f06a7bf0aca --- /dev/null +++ b/webapp/tests/karma/ts/services/enketo-xml/db-doc-in-repeat-with-files.xml @@ -0,0 +1,15 @@ + + Sally + + + repeater + repeat_upload_1.png + + + + + repeater + repeat_upload_2.png + + + diff --git a/webapp/tests/karma/ts/services/enketo-xml/db-doc-orphan-file.xml b/webapp/tests/karma/ts/services/enketo-xml/db-doc-orphan-file.xml new file mode 100644 index 00000000000..db0d33d2be0 --- /dev/null +++ b/webapp/tests/karma/ts/services/enketo-xml/db-doc-orphan-file.xml @@ -0,0 +1,7 @@ + + Sally + + thing_1 + known_upload.png + + diff --git a/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-binary.xml b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-binary.xml new file mode 100644 index 00000000000..7c5e6180fe6 --- /dev/null +++ b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-binary.xml @@ -0,0 +1,14 @@ + + Sally + main_photo_data + + thing_1 + some_value_1 + sub_photo_data_1 + + + thing_2 + some_value_2 + sub_photo_data_2 + + diff --git a/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field-cleared.xml b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field-cleared.xml new file mode 100644 index 00000000000..3df2c585886 --- /dev/null +++ b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field-cleared.xml @@ -0,0 +1,8 @@ + + Sally + main_upload.png + + thing_1 + + + diff --git a/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field.xml b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field.xml new file mode 100644 index 00000000000..d1e71b0efda --- /dev/null +++ b/webapp/tests/karma/ts/services/enketo-xml/db-doc-with-file-field.xml @@ -0,0 +1,8 @@ + + Sally + main_upload.png + + thing_1 + sub_upload.png + + diff --git a/webapp/tests/karma/ts/services/enketo.service.spec.ts b/webapp/tests/karma/ts/services/enketo.service.spec.ts index e2b7dcf832a..1a0180b9878 100644 --- a/webapp/tests/karma/ts/services/enketo.service.spec.ts +++ b/webapp/tests/karma/ts/services/enketo.service.spec.ts @@ -1163,6 +1163,220 @@ describe('Enketo service', () => { expect(AddAttachment.args[0][2]).to.deep.equal('some image data'); expect(AddAttachment.args[0][3]).to.equal('image/png'); }); + + it('should route binary attachments to correct sub-docs', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-with-binary'); + form.getDataStr.returns(content); + dbGetAttachment.resolves('
'); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + // actual[0] = main doc, actual[1] = doc1, actual[2] = doc2 + expect(actual.length).to.equal(3); + + // 3 binary fields = 3 AddAttachment calls + expect(AddAttachment.callCount).to.equal(3); + + // main_photo should be attached to main doc (actual[0]) + const mainPhotoCall = AddAttachment.args.find(args => args[2] === 'main_photo_data'); + expect(mainPhotoCall).to.exist; + expect(mainPhotoCall[0]._id).to.equal(actual[0]._id); + + // sub_photo_data_1 should be attached to doc1 (actual[1]) + const subPhoto1Call = AddAttachment.args.find(args => args[2] === 'sub_photo_data_1'); + expect(subPhoto1Call).to.exist; + expect(subPhoto1Call[0]._id).to.equal(actual[1]._id); + + // sub_photo_data_2 should be attached to doc2 (actual[2]) + const subPhoto2Call = AddAttachment.args.find(args => args[2] === 'sub_photo_data_2'); + expect(subPhoto2Call).to.exist; + expect(subPhoto2Call[0]._id).to.equal(actual[2]._id); + }); + + it('should route FileManager files to correct sub-doc', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-with-file-field'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + const mainFile = { name: 'main_upload.png', type: 'image/png' }; + const subFile = { name: 'sub_upload.png', type: 'image/png' }; + getCurrentFiles.returns([mainFile, subFile]); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + expect(actual.length).to.equal(2); + + // FileManager file calls: main_upload.png and sub_upload.png + const mainFileCall = AddAttachment.args.find( + args => args[1] === 'user-file-main_upload.png' + ); + expect(mainFileCall).to.exist; + expect(mainFileCall[0]._id).to.equal(actual[0]._id); + + const subFileCall = AddAttachment.args.find( + args => args[1] === 'user-file-sub_upload.png' + ); + expect(subFileCall).to.exist; + expect(subFileCall[0]._id).to.equal(actual[1]._id); + }); + + it('should fall back to main doc when file is not inside a db-doc', async () => { + form.validate.resolves(true); + const content = loadXML('binary-field'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + expect(actual.length).to.equal(1); + expect(AddAttachment.callCount).to.equal(1); + expect(AddAttachment.args[0][0]._id).to.equal(actual[0]._id); + }); + + it('should fall back to main doc when FileManager file has no matching XML node', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-orphan-file'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + const orphanFile = { name: 'no_such_node.png', type: 'image/png' }; + getCurrentFiles.returns([orphanFile]); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + // actual[0] = main doc, actual[1] = doc1 + expect(actual.length).to.equal(2); + + const orphanCall = AddAttachment.args.find( + args => args[1] === 'user-file-no_such_node.png' + ); + expect(orphanCall).to.exist; + expect(orphanCall[0]._id).to.equal(actual[0]._id); + }); + + it('should route files inside db-doc nested in repeats to corresponding sub-docs', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-in-repeat-with-files'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + const file1 = { name: 'repeat_upload_1.png', type: 'image/png' }; + const file2 = { name: 'repeat_upload_2.png', type: 'image/png' }; + getCurrentFiles.returns([file1, file2]); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + // actual[0] = main doc, actual[1] = first repeat doc, actual[2] = second repeat doc + expect(actual.length).to.equal(3); + + const file1Call = AddAttachment.args.find( + args => args[1] === 'user-file-repeat_upload_1.png' + ); + expect(file1Call).to.exist; + expect(file1Call[0]._id).to.equal(actual[1]._id); + + const file2Call = AddAttachment.args.find( + args => args[1] === 'user-file-repeat_upload_2.png' + ); + expect(file2Call).to.exist; + expect(file2Call[0]._id).to.equal(actual[2]._id); + + // No file should land on the main doc. + const mainDocFileCalls = AddAttachment.args.filter( + args => args[0]._id === actual[0]._id && args[1].startsWith('user-file-') + ); + expect(mainDocFileCalls).to.be.empty; + }); + + it('should use the main form id (not sub-doc type) for xpath-named sub-doc attachments', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-with-binary'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + // The legacy xpath-named filename must be rooted at the main form id + // (`my-form`) regardless of which doc owns the attachment. Routing to + // a sub-doc must NOT swap the root segment to the sub-doc's `type`. + const subPhoto1Call = AddAttachment.args.find(args => args[2] === 'sub_photo_data_1'); + expect(subPhoto1Call).to.exist; + expect(subPhoto1Call[1]).to.equal('user-file/my-form/doc1/photo1'); + + const subPhoto2Call = AddAttachment.args.find(args => args[2] === 'sub_photo_data_2'); + expect(subPhoto2Call).to.exist; + expect(subPhoto2Call[1]).to.equal('user-file/my-form/doc2/photo2'); + + const mainPhotoCall = AddAttachment.args.find(args => args[2] === 'main_photo_data'); + expect(mainPhotoCall).to.exist; + expect(mainPhotoCall[1]).to.equal('user-file/my-form/main_photo'); + }); + + it('should not attach removed files to sub-docs during edit', async () => { + form.validate.resolves(true); + const content = loadXML('db-doc-with-file-field-cleared'); + form.getDataStr.returns(content); + dbGetAttachment.resolves(''); + + // Only the main file remains; sub_file was cleared during edit + const mainFile = { name: 'main_upload.png', type: 'image/png' }; + getCurrentFiles.returns([mainFile]); + + const actual = await service.completeNewReport( + 'my-form', + form, + { doc: {} }, + { _id: 'my-user', phone: '8989' } + ); + + // actual[0] = main doc, actual[1] = doc1 sub-doc + expect(actual.length).to.equal(2); + + // Only the main file should be attached + const mainFileCall = AddAttachment.args.find( + args => args[1] === 'user-file-main_upload.png' + ); + expect(mainFileCall).to.exist; + expect(mainFileCall[0]._id).to.equal(actual[0]._id); + + // No attachment should land on the sub-doc + const subDocAttachments = AddAttachment.args.filter( + args => args[0]._id === actual[1]._id + ); + expect(subDocAttachments).to.be.empty; + }); }); describe('multimedia', () => { From 455b339ff6ffb1ea1683669c7637d1333d4b2e79 Mon Sep 17 00:00:00 2001 From: Yash sharma Date: Fri, 24 Apr 2026 20:40:28 +0530 Subject: [PATCH 02/30] feat(#10904): route file attachments to correct sub-docs in report forms When a report form contains [db-doc="true"] sub-documents with binary/file fields, attachments are now routed to the owning sub-document instead of always attaching to the main report doc. - Add resolveOwnerDoc() to walk up XML tree to nearest db-doc ancestor - Route FileManager file uploads to correct owner doc - Route inline binary blobs to correct owner doc - Fall back to main report doc when element is not inside a sub-doc --- webapp/src/ts/services/enketo.service.ts | 20 ++++++++++++++----- .../karma/ts/services/enketo.service.spec.ts | 3 +++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/webapp/src/ts/services/enketo.service.ts b/webapp/src/ts/services/enketo.service.ts index d3c66879957..1c6293f881a 100644 --- a/webapp/src/ts/services/enketo.service.ts +++ b/webapp/src/ts/services/enketo.service.ts @@ -362,13 +362,10 @@ export class EnketoService { return form; } - private findFileNodeByFilename($record, filename: string) { - // After upload, Enketo's Nodeset.setVal rewrites file-widget nodes from - // type="binary" to type="file" (#10903 §6.5). Inline-binary blobs from - // draw/signature widgets keep type="binary" and are handled separately. + private findBinaryNodeByFilename($record, filename: string) { let match = null; $record - .find('[type=file]') + .find('[type=binary]') .each((_idx, element) => { if ($(element).text() === filename) { match = element; @@ -516,14 +513,22 @@ export class EnketoService { }; // Route FileManager files to the correct owner doc. +<<<<<<< HEAD // For each file, find the [type=file] node whose text matches +======= + // For each file, find the [type=binary] node whose text matches +>>>>>>> 1052a1878 (feat(#10904): route file attachments to correct sub-docs in report forms) // the filename, then resolve the owner from its position in the // XML tree. FileManager .getCurrentFiles() .forEach(file => { const ownerDoc = resolveOwnerDoc( +<<<<<<< HEAD this.findFileNodeByFilename($record, file.name) ?? $record[0] +======= + this.findBinaryNodeByFilename($record, file.name) ?? $record[0] +>>>>>>> 1052a1878 (feat(#10904): route file attachments to correct sub-docs in report forms) ); this.attachmentService.add(ownerDoc, `user-file-${file.name}`, file, file.type, false); }); @@ -536,9 +541,14 @@ export class EnketoService { const attachLegacyFile = (elem, file, type, alreadyEncoded) => { const ownerDoc = resolveOwnerDoc(elem); const xpath = Xpath.getElementXPath(elem); + const formId = ownerDoc === doc ? doc.form : ownerDoc.type; // replace instance root element node name with form internal ID const filename = 'user-file' + +<<<<<<< HEAD (xpath.startsWith('/' + doc.form) ? xpath : xpath.replace(/^\/[^/]+/, '/' + doc.form)); +======= + (xpath.startsWith('/' + formId) ? xpath : xpath.replace(/^\/[^/]+/, '/' + formId)); +>>>>>>> 1052a1878 (feat(#10904): route file attachments to correct sub-docs in report forms) this.attachmentService.add(ownerDoc, filename, file, type, alreadyEncoded); }; diff --git a/webapp/tests/karma/ts/services/enketo.service.spec.ts b/webapp/tests/karma/ts/services/enketo.service.spec.ts index 1a0180b9878..c5852088246 100644 --- a/webapp/tests/karma/ts/services/enketo.service.spec.ts +++ b/webapp/tests/karma/ts/services/enketo.service.spec.ts @@ -1249,6 +1249,7 @@ describe('Enketo service', () => { expect(AddAttachment.callCount).to.equal(1); expect(AddAttachment.args[0][0]._id).to.equal(actual[0]._id); }); +<<<<<<< HEAD it('should fall back to main doc when FileManager file has no matching XML node', async () => { form.validate.resolves(true); @@ -1377,6 +1378,8 @@ describe('Enketo service', () => { ); expect(subDocAttachments).to.be.empty; }); +======= +>>>>>>> 1052a1878 (feat(#10904): route file attachments to correct sub-docs in report forms) }); describe('multimedia', () => { From 6967caa8c141be19dfbff75bdae4eabe1c8e1486 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Thu, 23 Apr 2026 02:30:49 +0300 Subject: [PATCH 03/30] feat(#10903): route attachments per-doc in ContactSaveService processAllAttachments now walks the parsed XML to determine which prepared doc owns each [type=binary] element (main / sibling / repeat child) and attaches files accordingly, instead of dumping every upload on preparedDocs[0]. Field-value sanitization and orphan cleanup also run per-doc. Adds two private helpers: - resolveContactOwnerDoc: DOM-walk from any element to its section root, then to the owning prepared doc (with mainDoc fallback). - findContactOwnerForFilename: locates the [type=binary] node whose text matches a FileManager filename and resolves its owner. No public API changes; no new service dependencies. --- .../src/ts/services/contact-save.service.ts | 159 ++++++++++++++---- 1 file changed, 130 insertions(+), 29 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index 8d72dabd892..bf1de3d5afa 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -55,7 +55,7 @@ export class ContactSaveService { // Process attachments for all documents const preparedDocs = [ doc ].concat(repeated, siblings); // NB: order matters: #4200 - this.processAllAttachments(preparedDocs, xmlStr); + this.processAllAttachments(preparedDocs, submitted, xmlStr); return { docId: doc._id, @@ -173,51 +173,152 @@ export class ContactSaveService { }); } - private processAllAttachments(preparedDocs: Record[], xmlStr: string) { + private processAllAttachments( + preparedDocs: Record[], + submitted: { doc; siblings?; repeats? }, + xmlStr: string, + ) { const mainDoc = preparedDocs[0]; - const newAttachmentNames = new Set(); - const fileNameMap = new Map(); // Map of original file name -> sanitized file name + const $record = xmlStr ? $($.parseXML(xmlStr)) : null; + const root = ($record?.children(':first')[0]) as Element | undefined; + const formId = root ? $(root).attr('id') : undefined; + const submittedRepeatsLen = submitted?.repeats?.child_data?.length ?? 0; + + const newAttachmentNamesByDoc = new Map, Set>(); + const fileNameMapByDoc = new Map, Map>(); + const trackNew = (doc: Record, name: string) => { + const set = newAttachmentNamesByDoc.get(doc) ?? new Set(); + set.add(name); + newAttachmentNamesByDoc.set(doc, set); + }; + const trackFileName = (doc: Record, original: string, sanitized: string) => { + const map = fileNameMapByDoc.get(doc) ?? new Map(); + map.set(original, sanitized); + fileNameMapByDoc.set(doc, map); + }; - // Attach files from FileManager (uploaded via file widgets) + // Attach files from FileManager (uploaded via file widgets), routed per sub-doc FileManager .getCurrentFiles() .forEach(file => { + const ownerDoc = root + ? this.findContactOwnerForFilename(file.name, root, preparedDocs, mainDoc, submittedRepeatsLen) + : mainDoc; + const sanitizedFileName = this.sanitizeFileName(file.name); const attachmentName = `${this.USER_FILE_ATTACHMENT_PREFIX}${sanitizedFileName}`; - newAttachmentNames.add(attachmentName); - this.attachmentService.add(mainDoc, attachmentName, file, file.type, false); - // Track the mapping for field value sanitization - fileNameMap.set(file.name, sanitizedFileName); + this.attachmentService.add(ownerDoc, attachmentName, file, file.type, false); + trackNew(ownerDoc, attachmentName); + trackFileName(ownerDoc, file.name, sanitizedFileName); }); - // Process binary fields from XML - if (xmlStr) { - const $record = $($.parseXML(xmlStr)); - const formId = $record.find(':first').attr('id'); - - $record + // Process binary fields from XML, routed per sub-doc + if (root) { + $(root) .find('[type=binary]') - .each((_idx, element) => { + .each((_idx, element: Element) => { const content = $(element).text(); - if (content) { - const xpath = Xpath.getElementXPath(element); - // Replace instance root element node name with form internal ID - const filename = this.USER_FILE_ATTACHMENT_PREFIX.slice(0, -1) + - (xpath.startsWith('/' + formId) ? xpath : xpath.replace(/^\/[^/]+/, '/' + formId)); - newAttachmentNames.add(filename); - this.attachmentService.add(mainDoc, filename, content, 'image/png', true); + if (!content) { + return; } + const ownerDoc = this.resolveContactOwnerDoc( + element, root, preparedDocs, mainDoc, submittedRepeatsLen + ); + + const xpath = Xpath.getElementXPath(element); + // Replace instance root element node name with form internal ID + const filename = this.USER_FILE_ATTACHMENT_PREFIX.slice(0, -1) + + (xpath.startsWith('/' + formId) ? xpath : xpath.replace(/^\/[^/]+/, '/' + formId)); + + this.attachmentService.add(ownerDoc, filename, content, 'image/png', true); + trackNew(ownerDoc, filename); }); } - // Sanitize field values in the document to match sanitized attachment names - this.sanitizeFieldValues(mainDoc, fileNameMap); + // Per-doc field-value sanitization + orphan cleanup + for (const doc of preparedDocs) { + const nameMap = fileNameMapByDoc.get(doc) ?? new Map(); + this.sanitizeFieldValues(doc, nameMap); - // Remove orphaned user attachments that are no longer referenced - const referencedAttachmentNames = this.findReferencedAttachments(mainDoc); - const validAttachmentNames = new Set([...newAttachmentNames, ...referencedAttachmentNames]); - this.removeOrphanedAttachments(mainDoc, validAttachmentNames); + const newNames = newAttachmentNamesByDoc.get(doc) ?? new Set(); + const referenced = this.findReferencedAttachments(doc); + const valid = new Set([ ...newNames, ...referenced ]); + this.removeOrphanedAttachments(doc, valid); + } + } + + /** + * Locates the prepared sub-doc that owns a given XML element, by walking + * up the DOM from the element to its section root (a direct child of the + * form's root element). + * + * - main section (first non-meta/inputs/repeat element child of root) -> preparedDocs[0] + * - / peer of main section -> sibling doc with matching _id + * - 's i-th -> preparedDocs[1 + i] (repeats precede siblings in concat) + * - anything else (meta, inputs, unknown) -> mainDoc + */ + private resolveContactOwnerDoc( + el: Element, + root: Element, + preparedDocs: Record[], + mainDoc: Record, + submittedRepeatsLen: number, + ): Record { + let section: Element = el; + while (section.parentNode && section.parentNode !== root) { + section = section.parentNode as Element; + } + if (section.parentNode !== root) { + return mainDoc; + } + + const ignored = [ 'meta', 'inputs', 'repeat' ]; + const firstSection = (Array.from(root.children) as Element[]) + .find(c => !ignored.includes(c.tagName) && c.childElementCount > 0); + if (section === firstSection) { + return mainDoc; + } + + if (this.CONTACT_FIELD_NAMES.includes(section.tagName)) { + const siblingId = mainDoc[section.tagName]?._id; + return preparedDocs.find(d => d._id === siblingId) ?? mainDoc; + } + + if (section.tagName === 'repeat') { + const repeatChildren = Array.from(section.children) as Element[]; + const childIdx = repeatChildren.findIndex(c => c.contains(el)); + if (childIdx >= 0 && childIdx < submittedRepeatsLen) { + return preparedDocs[1 + childIdx]; + } + } + + return mainDoc; + } + + /** + * Locates the first [type=binary] element whose text content equals the + * given filename, then resolves the owning prepared doc. Falls back to + * mainDoc when no node matches (preserves behavior for forms with no + * matching upload field). + * + * Filename uniqueness within a form session is guaranteed by Enketo's + * timestamp suffix, so the first match is the only match. + */ + private findContactOwnerForFilename( + filename: string, + root: Element, + preparedDocs: Record[], + mainDoc: Record, + submittedRepeatsLen: number, + ): Record { + const match = $(root) + .find('[type=binary]') + .filter((_, el) => $(el).text() === filename)[0]; + if (!match) { + return mainDoc; + } + return this.resolveContactOwnerDoc(match, root, preparedDocs, mainDoc, submittedRepeatsLen); } private findReferencedAttachments(doc: Record): Set { From 12e59a5fdd562de309a67decadd9858fa41510c9 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Thu, 23 Apr 2026 02:31:10 +0300 Subject: [PATCH 04/30] feat(#10903): validate attachment size on contact save path saveContact now uses validateAttachments(preparedDocs.preparedDocs) --- webapp/src/ts/services/form.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/webapp/src/ts/services/form.service.ts b/webapp/src/ts/services/form.service.ts index f07f0040fd4..7229c7531cb 100644 --- a/webapp/src/ts/services/form.service.ts +++ b/webapp/src/ts/services/form.service.ts @@ -404,6 +404,7 @@ export class FormService { const docs = await this.contactSaveService.save(form, docId, typeFields, xmlVersion); const preparedDocs = await this.applyTransitions(docs); + await this.validateAttachments(preparedDocs.preparedDocs); const primaryDoc = preparedDocs.preparedDocs.find(doc => doc.type === type); From 5819064d163ddd00f2b803e92fe345a47e500491 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Thu, 23 Apr 2026 02:32:41 +0300 Subject: [PATCH 05/30] test(#10903): cover sub-contact attachment routing New 'attachment routing to sub-contacts' describe block exercising: - file uploaded inside a sibling section -> sibling doc - file uploaded inside a repeat child -> i-th repeat doc - mixed uploads across main / sibling / repeat -> each owner - FileManager file with no matching binary node -> main doc fallback - inline binary (draw widget) inside sibling -> sibling doc - per-doc field-value sanitization (sibling field rewritten, main untouched) - main-doc orphan cleanup on edit path with per-doc loop --- .../ts/services/contact-save.service.spec.ts | 253 ++++++++++++++++++ 1 file changed, 253 insertions(+) diff --git a/webapp/tests/karma/ts/services/contact-save.service.spec.ts b/webapp/tests/karma/ts/services/contact-save.service.spec.ts index ec045da2be4..e7a21e67429 100644 --- a/webapp/tests/karma/ts/services/contact-save.service.spec.ts +++ b/webapp/tests/karma/ts/services/contact-save.service.spec.ts @@ -728,4 +728,257 @@ describe('ContactSave service', () => { expect(signatureCall.args[4], 'Binary field should be pre-encoded').to.be.true; }); }); + + describe('attachment routing to sub-contacts', () => { + // Each test uses a parent-registration shaped XML with three potential + // owners: main section , sibling , and a + // repeated under . Tests vary which sections actually have + // [type=binary] nodes / FileManager files. + + const stubSiblingAndRepeat = () => { + // identity extract so prepared docs keep their _id and we can compare + extractLineageService.extract.callsFake(contact => contact); + }; + + it('routes a file uploaded inside a sibling section to that sibling doc', async () => { + const xml = + '' + + '' + + 'Kigali HF' + + '' + + 'Amina' + + 'amina.png' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF', contact: 'NEW' }, + siblings: { contact: { _id: 'sib1', type: 'person', name: 'Amina', parent: 'PARENT' } }, + repeats: {}, + }); + stubSiblingAndRepeat(); + + const aminaFile = new File(['x'], 'amina.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ aminaFile ]); + + await service.save(form, null, 'family'); + + const fileCall = attachmentService.add.getCalls() + .find(c => c.args[1] === 'user-file-amina.png'); + expect(fileCall, 'sibling-routed file attachment should exist').to.not.be.undefined; + expect(fileCall.args[0]._id, 'file should land on sibling doc').to.equal('sib1'); + expect(fileCall.args[2]).to.equal(aminaFile); + + const calls = attachmentService.add.getCalls(); + expect( + calls.some(c => c.args[0]._id === 'main1' && c.args[1] === 'user-file-amina.png'), + 'main doc should not receive the sibling file' + ).to.be.false; + }); + + it('routes a file uploaded inside a repeat child to the i-th repeat doc', async () => { + const xml = + '' + + '' + + 'Kigali HF' + + '' + + 'Child AchildA.png' + + 'Child BchildB.png' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF' }, + siblings: {}, + repeats: { child_data: [ + { _id: 'kid1', type: 'person', name: 'Child A', parent: 'PARENT' }, + { _id: 'kid2', type: 'person', name: 'Child B', parent: 'PARENT' }, + ] }, + }); + stubSiblingAndRepeat(); + + const fileA = new File(['a'], 'childA.png', { type: 'image/png' }); + const fileB = new File(['b'], 'childB.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ fileA, fileB ]); + + await service.save(form, null, 'family'); + + const calls = attachmentService.add.getCalls(); + const aTarget = calls.find(c => c.args[1] === 'user-file-childA.png'); + const bTarget = calls.find(c => c.args[1] === 'user-file-childB.png'); + expect(aTarget?.args[0]._id, 'childA file -> first repeat doc').to.equal('kid1'); + expect(bTarget?.args[0]._id, 'childB file -> second repeat doc').to.equal('kid2'); + }); + + it('routes mixed uploads to their respective sub-docs', async () => { + const xml = + '' + + '' + + 'Kigali HFfacility.jpg' + + '' + + 'Amina' + + 'amina.png' + + '' + + '' + + 'Child AchildA.png' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF', contact: 'NEW' }, + siblings: { contact: { _id: 'sib1', type: 'person', name: 'Amina', parent: 'PARENT' } }, + repeats: { child_data: [ + { _id: 'kid1', type: 'person', name: 'Child A', parent: 'PARENT' }, + ] }, + }); + stubSiblingAndRepeat(); + + const facilityFile = new File(['f'], 'facility.jpg', { type: 'image/jpeg' }); + const aminaFile = new File(['a'], 'amina.png', { type: 'image/png' }); + const childFile = new File(['c'], 'childA.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ facilityFile, aminaFile, childFile ]); + + await service.save(form, null, 'family'); + + const calls = attachmentService.add.getCalls(); + const ownerOf = (name: string) => calls.find(c => c.args[1] === name)?.args[0]._id; + + expect(ownerOf('user-file-facility.jpg'), 'facility upload -> main').to.equal('main1'); + expect(ownerOf('user-file-amina.png'), 'amina upload -> sibling').to.equal('sib1'); + expect(ownerOf('user-file-childA.png'), 'child upload -> repeat[0]').to.equal('kid1'); + + // each filename appears exactly once across all add calls + ['user-file-facility.jpg', 'user-file-amina.png', 'user-file-childA.png'] + .forEach(name => { + const matches = calls.filter(c => c.args[1] === name); + expect(matches.length, `${name} should be added exactly once`).to.equal(1); + }); + }); + + it('falls back to the main doc when no XML binary node matches the filename', async () => { + const xml = + '' + + '' + + 'Kigali HF' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF' }, + }); + + const orphan = new File(['o'], 'orphan.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ orphan ]); + + await service.save(form, null, 'family'); + + const call = attachmentService.add.getCalls() + .find(c => c.args[1] === 'user-file-orphan.png'); + expect(call?.args[0]._id, 'orphan file should fall back to main').to.equal('main1'); + }); + + it('routes inline binary content from a sibling section to the sibling doc', async () => { + const xml = + '' + + '' + + 'Kigali HF' + + '' + + 'Amina' + + 'BASE64_SIG_DATA' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF', contact: 'NEW' }, + siblings: { contact: { _id: 'sib1', type: 'person', name: 'Amina', parent: 'PARENT' } }, + repeats: {}, + }); + stubSiblingAndRepeat(); + sinon.stub(FileManager, 'getCurrentFiles').returns([]); + + await service.save(form, null, 'family'); + + const sigCall = attachmentService.add.getCalls() + .find(c => c.args[1] === 'user-file/contact:family:create/contact/signature'); + expect(sigCall, 'sibling-routed inline binary should exist').to.not.be.undefined; + expect(sigCall.args[0]._id, 'inline binary should land on sibling').to.equal('sib1'); + expect(sigCall.args[2]).to.equal('BASE64_SIG_DATA'); + expect(sigCall.args[3]).to.equal('image/png'); + expect(sigCall.args[4]).to.be.true; + }); + + it('sanitizes field values per-doc', async () => { + // Sibling has a special-char filename that must be sanitized in the + // sibling's own field; main doc fields stay untouched. + const xml = + '' + + '' + + 'Kigali HF' + + '' + + 'Amina' + + 'my photo.png' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF', contact: 'NEW' }, + siblings: { + contact: { _id: 'sib1', type: 'person', name: 'Amina', photo: 'my photo.png', parent: 'PARENT' } + }, + repeats: {}, + }); + stubSiblingAndRepeat(); + + const aminaFile = new File(['x'], 'my photo.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ aminaFile ]); + + const result = await service.save(form, null, 'family'); + + const sibling = result.preparedDocs.find(d => d._id === 'sib1'); + const main = result.preparedDocs.find(d => d._id === 'main1'); + expect(sibling.photo, 'sibling photo field should be sanitized').to.equal('myphoto.png'); + expect(main.name, 'main field should be untouched').to.equal('Kigali HF'); + }); + + it('runs orphan cleanup on the edit path for the main doc', async () => { + // Edit path: main doc has a stale user-file attachment that is no longer + // referenced. The per-doc orphan loop must still remove it from main. + const xml = + '' + + '' + + 'Kigali HF' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF' }, + }); + + // Original returned by the datasource has a stale user-file attachment. + getContact + .withArgs(Qualifier.byUuid('main1')) + .resolves({ + _id: 'main1', + type: 'family', + name: 'Kigali HF', + _attachments: { 'user-file-stale.png': { content_type: 'image/png' } } + }); + + sinon.stub(FileManager, 'getCurrentFiles').returns([]); + + await service.save(form, 'main1', 'family'); + + expect( + attachmentService.remove.calledWith( + sinon.match({ _id: 'main1' }), 'user-file-stale.png' + ), + 'stale main-doc attachment should be removed' + ).to.be.true; + }); + }); }); From 84bb2d0164ad3015a6e568a31af6b405f685c39e Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Thu, 23 Apr 2026 02:33:45 +0300 Subject: [PATCH 06/30] test(#10903): cover validateAttachments wiring on contact save path - main-doc & sub-doc oversize attachment fails saveContact - normal-sized attachment passes validation --- .../karma/ts/services/form.service.spec.ts | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/webapp/tests/karma/ts/services/form.service.spec.ts b/webapp/tests/karma/ts/services/form.service.spec.ts index 6ee36ff43dd..a97122650fc 100644 --- a/webapp/tests/karma/ts/services/form.service.spec.ts +++ b/webapp/tests/karma/ts/services/form.service.spec.ts @@ -1765,6 +1765,86 @@ describe('Form service', () => { expect(performanceService.track.notCalled).to.be.true; expect(performanceTracking.stop.notCalled).to.be.true; }); + + it('should reject and abort save when an attachment exceeds max size', async () => { + const form = { getDataStr: () => '' }; + const docId = null; + const type = 'some-contact-type'; + + translateService.get.returnsArg(0); + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'main', name: 'Main' } + }); + // applyTransitions returns the docs unchanged but with a bloated + // attachment so validateAttachments rejects. + transitionsService.applyTransitions.callsFake((docs) => { + docs[0]._attachments = { + 'user-file-huge.png': { data: { size: 100 * 1024 * 1024 } }, + }; + return Promise.resolve(docs); + }); + + await expect( + service.saveContact({ docId, type }, { form, xmlVersion: undefined, duplicateCheck: undefined }, true) + ).to.be.rejectedWith(/enketo\.error\.max_attachment_size/); + + assert.equal(dbBulkDocs.callCount, 0, 'bulkDocs should not be called when validation fails'); + assert.equal(setLastChangedDoc.callCount, 0, 'setLastChangedDoc should not be called'); + expect(globalActions.setSnackbarContent.calledWith('enketo.error.max_attachment_size')).to.be.true; + }); + + it('should reject when a sub-doc has oversize attachments', async () => { + // Per #10903, sub-contact attachments now route to the sibling/repeat doc. + // The validation must apply to every prepared doc, not just the main one. + const form = { getDataStr: () => '' }; + const docId = null; + const type = 'family'; + + translateService.get.returnsArg(0); + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'family', name: 'Kigali HF', contact: 'NEW' }, + siblings: { contact: { _id: 'sib1', type: 'person', name: 'Amina', parent: 'PARENT' } }, + repeats: {}, + }); + extractLineageService.extract.callsFake(c => c); + transitionsService.applyTransitions.callsFake((docs) => { + // Bloat the sibling's attachment, not the main's. + const sibling = docs.find(d => d._id === 'sib1'); + sibling._attachments = { + 'user-file-amina.png': { data: { size: 100 * 1024 * 1024 } }, + }; + return Promise.resolve(docs); + }); + + await expect( + service.saveContact({ docId, type }, { form, xmlVersion: undefined, duplicateCheck: undefined }, true) + ).to.be.rejectedWith(/enketo\.error\.max_attachment_size/); + + assert.equal(dbBulkDocs.callCount, 0, 'bulkDocs should not be called when sub-doc attachment is oversize'); + }); + + it('should pass validation and save when attachments are within size limit', async () => { + // Sanity: ensures the new validateAttachments call is a no-op for normal saves. + const form = { getDataStr: () => '' }; + const docId = null; + const type = 'some-contact-type'; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { _id: 'main1', type: 'main', name: 'Main' } + }); + transitionsService.applyTransitions.callsFake((docs) => { + docs[0]._attachments = { + 'user-file-tiny.png': { data: { size: 1024 } }, + }; + return Promise.resolve(docs); + }); + dbBulkDocs.resolves([]); + + await service.saveContact({ docId, type }, { form, xmlVersion: undefined, duplicateCheck: undefined }, true); + + assert.equal(dbBulkDocs.callCount, 1); + expect(globalActions.setSnackbarContent.notCalled).to.be.true; + }); }); describe('load contact summary', () => { From 7b8710d4bfce56d0359dfd4eeaf345c1afca903f Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Thu, 23 Apr 2026 07:39:41 +0300 Subject: [PATCH 07/30] fix(#10903): match type="file" upload nodes when routing Enketo's setVal rewrites uploaded binary nodes to type="file" the moment a value is set. --- .../src/ts/services/contact-save.service.ts | 11 ++-- .../ts/services/contact-save.service.spec.ts | 53 +++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index bf1de3d5afa..1b54513ed61 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -297,13 +297,16 @@ export class ContactSaveService { } /** - * Locates the first [type=binary] element whose text content equals the + * Locates the first upload-widget element whose text content equals the * given filename, then resolves the owning prepared doc. Falls back to * mainDoc when no node matches (preserves behavior for forms with no * matching upload field). * - * Filename uniqueness within a form session is guaranteed by Enketo's - * timestamp suffix, so the first match is the only match. + * The selector matches both `type="file"` (the runtime state Enketo + * writes via setVal after a value is set) and `type="binary"` (the + * historical / template attribute). Filename uniqueness within a form + * session is guaranteed by Enketo's timestamp suffix, so the first + * match is the only match. */ private findContactOwnerForFilename( filename: string, @@ -313,7 +316,7 @@ export class ContactSaveService { submittedRepeatsLen: number, ): Record { const match = $(root) - .find('[type=binary]') + .find('[type=file], [type=binary]') .filter((_, el) => $(el).text() === filename)[0]; if (!match) { return mainDoc; diff --git a/webapp/tests/karma/ts/services/contact-save.service.spec.ts b/webapp/tests/karma/ts/services/contact-save.service.spec.ts index e7a21e67429..c8bdb255e26 100644 --- a/webapp/tests/karma/ts/services/contact-save.service.spec.ts +++ b/webapp/tests/karma/ts/services/contact-save.service.spec.ts @@ -980,5 +980,58 @@ describe('ContactSave service', () => { 'stale main-doc attachment should be removed' ).to.be.true; }); + + it('routes uploads using the production type="file" attribute (regression for #10903)', async () => { + // Enketo's setVal rewrites uploaded binary nodes to type="file" at + // runtime (enketo-core form-model.js setVal). The earlier matcher + // only looked at [type=binary] and missed every production upload, + // routing every file to the main doc. + const xml = + '' + + '' + + '' + + 'Kigali HF' + + 'clinic' + + '' + + 'Amina' + + 'amina-14_39_7.png' + + '' + + '' + + '' + + 'Child A' + + 'childA-14_39_8.png' + + '' + + '' + + ''; + const form = { getDataStr: () => xml }; + + enketoTranslationService.contactRecordToJs.returns({ + doc: { + _id: 'main1', type: 'clinic', name: 'Kigali HF', + contact: 'NEW', + }, + siblings: { contact: { _id: 'sib1', type: 'person', name: 'Amina', parent: 'PARENT' } }, + repeats: { child_data: [ + { _id: 'kid1', type: 'person', name: 'Child A', parent: 'PARENT' }, + ] }, + }); + stubSiblingAndRepeat(); + + const aminaFile = new File(['a'], 'amina-14_39_7.png', { type: 'image/png' }); + const childFile = new File(['c'], 'childA-14_39_8.png', { type: 'image/png' }); + sinon.stub(FileManager, 'getCurrentFiles').returns([ aminaFile, childFile ]); + + await service.save(form, null, 'clinic'); + + const calls = attachmentService.add.getCalls(); + const ownerOf = (name: string) => calls.find(c => c.args[1] === name)?.args[0]._id; + + expect(ownerOf('user-file-amina-14_39_7.png'), 'sibling upload -> sibling doc').to.equal('sib1'); + expect(ownerOf('user-file-childA-14_39_8.png'), 'repeat upload -> repeat doc').to.equal('kid1'); + expect( + calls.some(c => c.args[0]._id === 'main1' && c.args[1].startsWith('user-file-')), + 'main doc should not receive any sub-contact uploads' + ).to.be.false; + }); }); }); From 3a90455caee8f790a7774d5ce2d0edfe82893151 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Fri, 24 Apr 2026 23:45:12 +0300 Subject: [PATCH 08/30] feat(#10903): clean up. --- .../tests/karma/ts/services/contact-save.service.spec.ts | 9 +++------ webapp/tests/karma/ts/services/form.service.spec.ts | 2 -- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/webapp/tests/karma/ts/services/contact-save.service.spec.ts b/webapp/tests/karma/ts/services/contact-save.service.spec.ts index c8bdb255e26..8414cb885f2 100644 --- a/webapp/tests/karma/ts/services/contact-save.service.spec.ts +++ b/webapp/tests/karma/ts/services/contact-save.service.spec.ts @@ -732,8 +732,7 @@ describe('ContactSave service', () => { describe('attachment routing to sub-contacts', () => { // Each test uses a parent-registration shaped XML with three potential // owners: main section , sibling , and a - // repeated under . Tests vary which sections actually have - // [type=binary] nodes / FileManager files. + // repeated under . const stubSiblingAndRepeat = () => { // identity extract so prepared docs keep their _id and we can compare @@ -981,11 +980,9 @@ describe('ContactSave service', () => { ).to.be.true; }); - it('routes uploads using the production type="file" attribute (regression for #10903)', async () => { + it('routes uploads using the type="file" attribute', async () => { // Enketo's setVal rewrites uploaded binary nodes to type="file" at - // runtime (enketo-core form-model.js setVal). The earlier matcher - // only looked at [type=binary] and missed every production upload, - // routing every file to the main doc. + // runtime (enketo-core form-model.js setVal). const xml = '' + '' + diff --git a/webapp/tests/karma/ts/services/form.service.spec.ts b/webapp/tests/karma/ts/services/form.service.spec.ts index a97122650fc..6231219c0e1 100644 --- a/webapp/tests/karma/ts/services/form.service.spec.ts +++ b/webapp/tests/karma/ts/services/form.service.spec.ts @@ -1794,7 +1794,6 @@ describe('Form service', () => { }); it('should reject when a sub-doc has oversize attachments', async () => { - // Per #10903, sub-contact attachments now route to the sibling/repeat doc. // The validation must apply to every prepared doc, not just the main one. const form = { getDataStr: () => '' }; const docId = null; @@ -1824,7 +1823,6 @@ describe('Form service', () => { }); it('should pass validation and save when attachments are within size limit', async () => { - // Sanity: ensures the new validateAttachments call is a no-op for normal saves. const form = { getDataStr: () => '' }; const docId = null; const type = 'some-contact-type'; From 7f46c972ae11f947a62530928099e2206c096c66 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Mon, 27 Apr 2026 22:17:23 +0300 Subject: [PATCH 09/30] refactor `resolveContactOwnerDoc` signature. --- .../src/ts/services/contact-save.service.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index 1b54513ed61..2e6dcb04947 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -11,6 +11,13 @@ import { Contact, Qualifier } from '@medic/cht-datasource'; import { Xpath } from '@mm-providers/xpath-element-path.provider'; import FileManager from '../../js/enketo/file-manager'; +type ContactRoutingContext = { + root: Element; + preparedDocs: Record[]; + mainDoc: Record; + submittedRepeatsLen: number; +}; + @Injectable({ providedIn: 'root' }) @@ -223,7 +230,7 @@ export class ContactSaveService { return; } const ownerDoc = this.resolveContactOwnerDoc( - element, root, preparedDocs, mainDoc, submittedRepeatsLen + element, { root, preparedDocs, mainDoc, submittedRepeatsLen } ); const xpath = Xpath.getElementXPath(element); @@ -258,13 +265,8 @@ export class ContactSaveService { * - 's i-th -> preparedDocs[1 + i] (repeats precede siblings in concat) * - anything else (meta, inputs, unknown) -> mainDoc */ - private resolveContactOwnerDoc( - el: Element, - root: Element, - preparedDocs: Record[], - mainDoc: Record, - submittedRepeatsLen: number, - ): Record { + private resolveContactOwnerDoc(el: Element, ctx: ContactRoutingContext): Record { + const { root, preparedDocs, mainDoc, submittedRepeatsLen } = ctx; let section: Element = el; while (section.parentNode && section.parentNode !== root) { section = section.parentNode as Element; @@ -321,7 +323,7 @@ export class ContactSaveService { if (!match) { return mainDoc; } - return this.resolveContactOwnerDoc(match, root, preparedDocs, mainDoc, submittedRepeatsLen); + return this.resolveContactOwnerDoc(match, { root, preparedDocs, mainDoc, submittedRepeatsLen }); } private findReferencedAttachments(doc: Record): Set { From f667aff771191759d0557a1050a1e035ff836a96 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Mon, 27 Apr 2026 22:34:31 +0300 Subject: [PATCH 10/30] refactor `resolveContactOwnerDoc` to reduce cognitive load --- .../src/ts/services/contact-save.service.ts | 84 +++++++++++-------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index 2e6dcb04947..b8efcc9c2d1 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -11,13 +11,6 @@ import { Contact, Qualifier } from '@medic/cht-datasource'; import { Xpath } from '@mm-providers/xpath-element-path.provider'; import FileManager from '../../js/enketo/file-manager'; -type ContactRoutingContext = { - root: Element; - preparedDocs: Record[]; - mainDoc: Record; - submittedRepeatsLen: number; -}; - @Injectable({ providedIn: 'root' }) @@ -230,7 +223,7 @@ export class ContactSaveService { return; } const ownerDoc = this.resolveContactOwnerDoc( - element, { root, preparedDocs, mainDoc, submittedRepeatsLen } + element, root, preparedDocs, mainDoc, submittedRepeatsLen ); const xpath = Xpath.getElementXPath(element); @@ -255,6 +248,44 @@ export class ContactSaveService { } } + private findSectionForElement(el: Element, root: Element): Element | null { + let section: Element = el; + while (section.parentNode && section.parentNode !== root) { + section = section.parentNode as Element; + } + return section.parentNode === root ? section : null; + } + + private isMainSection(section: Element, root: Element): boolean { + const ignored = [ 'meta', 'inputs', 'repeat' ]; + const firstSection = (Array.from(root.children) as Element[]) + .find(c => !ignored.includes(c.tagName) && c.childElementCount > 0); + return section === firstSection; + } + + private findSiblingDoc( + section: Element, + preparedDocs: Record[], + mainDoc: Record, + ): Record { + const siblingId = mainDoc[section.tagName]?._id; + return preparedDocs.find((d: Record) => d._id === siblingId) ?? mainDoc; + } + + private findRepeatDoc( + section: Element, + el: Element, + preparedDocs: Record[], + submittedRepeatsLen: number, + ): Record | null { + const repeatChildren = Array.from(section.children) as Element[]; + const childIdx = repeatChildren.findIndex(c => c.contains(el)); + if (childIdx >= 0 && childIdx < submittedRepeatsLen) { + return preparedDocs[1 + childIdx]; + } + return null; + } + /** * Locates the prepared sub-doc that owns a given XML element, by walking * up the DOM from the element to its section root (a direct child of the @@ -265,36 +296,23 @@ export class ContactSaveService { * - 's i-th -> preparedDocs[1 + i] (repeats precede siblings in concat) * - anything else (meta, inputs, unknown) -> mainDoc */ - private resolveContactOwnerDoc(el: Element, ctx: ContactRoutingContext): Record { - const { root, preparedDocs, mainDoc, submittedRepeatsLen } = ctx; - let section: Element = el; - while (section.parentNode && section.parentNode !== root) { - section = section.parentNode as Element; - } - if (section.parentNode !== root) { - return mainDoc; - } - - const ignored = [ 'meta', 'inputs', 'repeat' ]; - const firstSection = (Array.from(root.children) as Element[]) - .find(c => !ignored.includes(c.tagName) && c.childElementCount > 0); - if (section === firstSection) { + private resolveContactOwnerDoc( + el: Element, + root: Element, + preparedDocs: Record[], + mainDoc: Record, + submittedRepeatsLen: number, + ): Record { + const section = this.findSectionForElement(el, root); + if (!section || this.isMainSection(section, root)) { return mainDoc; } - if (this.CONTACT_FIELD_NAMES.includes(section.tagName)) { - const siblingId = mainDoc[section.tagName]?._id; - return preparedDocs.find(d => d._id === siblingId) ?? mainDoc; + return this.findSiblingDoc(section, preparedDocs, mainDoc); } - if (section.tagName === 'repeat') { - const repeatChildren = Array.from(section.children) as Element[]; - const childIdx = repeatChildren.findIndex(c => c.contains(el)); - if (childIdx >= 0 && childIdx < submittedRepeatsLen) { - return preparedDocs[1 + childIdx]; - } + return this.findRepeatDoc(section, el, preparedDocs, submittedRepeatsLen) ?? mainDoc; } - return mainDoc; } @@ -323,7 +341,7 @@ export class ContactSaveService { if (!match) { return mainDoc; } - return this.resolveContactOwnerDoc(match, { root, preparedDocs, mainDoc, submittedRepeatsLen }); + return this.resolveContactOwnerDoc(match, root, preparedDocs, mainDoc, submittedRepeatsLen); } private findReferencedAttachments(doc: Record): Set { From 3ac2cd5e50af41eb07558bcb278b7fb2bad3b83a Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Tue, 28 Apr 2026 06:39:48 +0300 Subject: [PATCH 11/30] refactor: use Set for ignored section tags --- webapp/src/ts/services/contact-save.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index b8efcc9c2d1..2113e5c8ea0 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -257,9 +257,9 @@ export class ContactSaveService { } private isMainSection(section: Element, root: Element): boolean { - const ignored = [ 'meta', 'inputs', 'repeat' ]; + const ignored = new Set([ 'meta', 'inputs', 'repeat' ]); const firstSection = (Array.from(root.children) as Element[]) - .find(c => !ignored.includes(c.tagName) && c.childElementCount > 0); + .find(c => !ignored.has(c.tagName) && c.childElementCount > 0); return section === firstSection; } From b66a67ede385b254384f6aa22e5782926815846f Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Tue, 28 Apr 2026 06:39:58 +0300 Subject: [PATCH 12/30] refactor: use Array.find over jQuery filter()[0] --- webapp/src/ts/services/contact-save.service.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index 2113e5c8ea0..e5ad1b338de 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -337,7 +337,8 @@ export class ContactSaveService { ): Record { const match = $(root) .find('[type=file], [type=binary]') - .filter((_, el) => $(el).text() === filename)[0]; + .toArray() + .find(el => $(el).text() === filename); if (!match) { return mainDoc; } From 61121a3fc17271485eadba9527b6a9208b30584c Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Tue, 28 Apr 2026 06:41:34 +0300 Subject: [PATCH 13/30] refactor: group contact-owner lookup params into context object --- .../src/ts/services/contact-save.service.ts | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/webapp/src/ts/services/contact-save.service.ts b/webapp/src/ts/services/contact-save.service.ts index e5ad1b338de..05b1ef3810a 100644 --- a/webapp/src/ts/services/contact-save.service.ts +++ b/webapp/src/ts/services/contact-save.service.ts @@ -11,6 +11,13 @@ import { Contact, Qualifier } from '@medic/cht-datasource'; import { Xpath } from '@mm-providers/xpath-element-path.provider'; import FileManager from '../../js/enketo/file-manager'; +interface ContactOwnerContext { + root: Element; + preparedDocs: Record[]; + mainDoc: Record; + submittedRepeatsLen: number; +} + @Injectable({ providedIn: 'root' }) @@ -182,7 +189,9 @@ export class ContactSaveService { const $record = xmlStr ? $($.parseXML(xmlStr)) : null; const root = ($record?.children(':first')[0]) as Element | undefined; const formId = root ? $(root).attr('id') : undefined; - const submittedRepeatsLen = submitted?.repeats?.child_data?.length ?? 0; + const ctx: ContactOwnerContext | null = root + ? { root, preparedDocs, mainDoc, submittedRepeatsLen: submitted?.repeats?.child_data?.length ?? 0 } + : null; const newAttachmentNamesByDoc = new Map, Set>(); const fileNameMapByDoc = new Map, Map>(); @@ -201,8 +210,8 @@ export class ContactSaveService { FileManager .getCurrentFiles() .forEach(file => { - const ownerDoc = root - ? this.findContactOwnerForFilename(file.name, root, preparedDocs, mainDoc, submittedRepeatsLen) + const ownerDoc = ctx + ? this.findContactOwnerForFilename(file.name, ctx) : mainDoc; const sanitizedFileName = this.sanitizeFileName(file.name); @@ -214,17 +223,15 @@ export class ContactSaveService { }); // Process binary fields from XML, routed per sub-doc - if (root) { - $(root) + if (ctx) { + $(ctx.root) .find('[type=binary]') .each((_idx, element: Element) => { const content = $(element).text(); if (!content) { return; } - const ownerDoc = this.resolveContactOwnerDoc( - element, root, preparedDocs, mainDoc, submittedRepeatsLen - ); + const ownerDoc = this.resolveContactOwnerDoc(element, ctx); const xpath = Xpath.getElementXPath(element); // Replace instance root element node name with form internal ID @@ -296,13 +303,8 @@ export class ContactSaveService { * - 's i-th -> preparedDocs[1 + i] (repeats precede siblings in concat) * - anything else (meta, inputs, unknown) -> mainDoc */ - private resolveContactOwnerDoc( - el: Element, - root: Element, - preparedDocs: Record[], - mainDoc: Record, - submittedRepeatsLen: number, - ): Record { + private resolveContactOwnerDoc(el: Element, ctx: ContactOwnerContext): Record { + const { root, preparedDocs, mainDoc, submittedRepeatsLen } = ctx; const section = this.findSectionForElement(el, root); if (!section || this.isMainSection(section, root)) { return mainDoc; @@ -328,21 +330,15 @@ export class ContactSaveService { * session is guaranteed by Enketo's timestamp suffix, so the first * match is the only match. */ - private findContactOwnerForFilename( - filename: string, - root: Element, - preparedDocs: Record[], - mainDoc: Record, - submittedRepeatsLen: number, - ): Record { - const match = $(root) + private findContactOwnerForFilename(filename: string, ctx: ContactOwnerContext): Record { + const match = $(ctx.root) .find('[type=file], [type=binary]') .toArray() .find(el => $(el).text() === filename); if (!match) { - return mainDoc; + return ctx.mainDoc; } - return this.resolveContactOwnerDoc(match, root, preparedDocs, mainDoc, submittedRepeatsLen); + return this.resolveContactOwnerDoc(match, ctx); } private findReferencedAttachments(doc: Record): Set { From dce22bdb2e7e20090d496729e438f7aa7b936c24 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Wed, 6 May 2026 11:02:37 +0300 Subject: [PATCH 14/30] test(e2e): assert sub-contact attachment routing on create and edit add sub-contact attachment routing form fixtures & testes --- .../forms/family-with-attachments-create.xml | 106 ++++++++ .../forms/family-with-attachments-edit.xml | 106 ++++++++ .../sub-contact-attachments.wdio-spec.js | 229 ++++++++++++++++++ 3 files changed, 441 insertions(+) create mode 100644 tests/e2e/default/contacts/forms/family-with-attachments-create.xml create mode 100644 tests/e2e/default/contacts/forms/family-with-attachments-edit.xml create mode 100644 tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js diff --git a/tests/e2e/default/contacts/forms/family-with-attachments-create.xml b/tests/e2e/default/contacts/forms/family-with-attachments-create.xml new file mode 100644 index 00000000000..44db0131817 --- /dev/null +++ b/tests/e2e/default/contacts/forms/family-with-attachments-create.xml @@ -0,0 +1,106 @@ + + + + New Family With Attachments + + + + + Family Name + + + Family Photo + + + Primary Contact Name + + + Primary Contact Photo + + + Children + + + Child Name + + + Child Photo + + + + + + + PARENT + contact + family_with_attachments + + + NEW + + + PARENT + person + + + + + + PARENT + person + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml b/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml new file mode 100644 index 00000000000..741791e5e9b --- /dev/null +++ b/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml @@ -0,0 +1,106 @@ + + + + Edit Family With Attachments + + + + + Family Name + + + Family Photo + + + Primary Contact Name + + + Primary Contact Photo + + + Children + + + Child Name + + + Child Photo + + + + + + + PARENT + contact + family_with_attachments + + + NEW + + + PARENT + person + + + + + + PARENT + person + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js b/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js new file mode 100644 index 00000000000..793961effed --- /dev/null +++ b/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js @@ -0,0 +1,229 @@ +const path = require('path'); +const fs = require('fs'); +const utils = require('@utils'); +const placeFactory = require('@factories/cht/contacts/place'); +const userFactory = require('@factories/cht/users/users'); +const loginPage = require('@page-objects/default/login/login.wdio.page'); +const commonPage = require('@page-objects/default/common/common.wdio.page'); +const commonEnketoPage = require('@page-objects/default/enketo/common-enketo.wdio.page'); +const genericForm = require('@page-objects/default/enketo/generic-form.wdio.page'); +const contactPage = require('@page-objects/default/contacts/contacts.wdio.page'); +const { CONTACT_TYPES } = require('@medic/constants'); + +describe('Sub-contact attachment routing', () => { + const familyPhotoPath = path.join(__dirname, '../enketo/images/photo-for-upload-form.png'); + const primaryContactPhotoPath = path.join(__dirname, '../../../../webapp/src/img/layers.png'); + const childPhotoPath = path.join(__dirname, '../../../../webapp/src/img/icon.png'); + + const places = placeFactory.generateHierarchy(); + const healthCenter = places.get(CONTACT_TYPES.HEALTH_CENTER); + + const onlineUser = userFactory.build({ + place: healthCenter._id, + roles: ['program_officer'] + }); + + const familyType = { + id: 'family_with_attachments', + name_key: 'contact.type.family_with_attachments', + group_key: 'contact.type.family_with_attachments.plural', + create_key: 'contact.type.family_with_attachments.new', + edit_key: 'contact.type.family_with_attachments.edit', + primary_contact_key: '', + parents: [CONTACT_TYPES.HEALTH_CENTER, CONTACT_TYPES.CLINIC, 'district_hospital'], + icon: 'medic-clinic', + create_form: 'form:contact:family_with_attachments:create', + edit_form: 'form:contact:family_with_attachments:edit', + person: false + }; + + const translations = { + 'contact.type.family_with_attachments': 'Family With Attachments', + 'contact.type.family_with_attachments.plural': 'Families With Attachments', + 'contact.type.family_with_attachments.new': 'New Family With Attachments', + 'contact.type.family_with_attachments.edit': 'Edit Family With Attachments' + }; + + const createFormXml = fs.readFileSync( + path.join(__dirname, 'forms/family-with-attachments-create.xml'), + 'utf8' + ); + + const editFormXml = fs.readFileSync( + path.join(__dirname, 'forms/family-with-attachments-edit.xml'), + 'utf8' + ); + + const createFormDoc = { + _id: 'form:contact:family_with_attachments:create', + internalId: 'contact:family_with_attachments:create', + title: 'New Family With Attachments', + type: 'form', + _attachments: { + xml: { + content_type: 'application/octet-stream', + data: Buffer.from(createFormXml).toString('base64'), + } + } + }; + + const editFormDoc = { + _id: 'form:contact:family_with_attachments:edit', + internalId: 'contact:family_with_attachments:edit', + title: 'Edit Family With Attachments', + type: 'form', + _attachments: { + xml: { + content_type: 'application/octet-stream', + data: Buffer.from(editFormXml).toString('base64'), + } + } + }; + + const fetchFamilyAndChildren = async (familyId) => { + const family = await utils.getDoc(familyId); + const [familyDoc, primaryContact] = await utils.getDocs([familyId, family.contact._id]); + const allRows = await utils.db.allDocs({ include_docs: true }); + const repeatChildren = allRows.rows + .map(row => row.doc) + .filter(doc => doc?.parent?._id === familyId && doc._id !== primaryContact._id); + return { family: familyDoc, primaryContact, repeatChildren }; + }; + + const submitFamilyForm = async ({ familyName, primaryContactName, repeatChildren }) => { + await commonEnketoPage.setInputValue('Family Name', familyName); + await commonEnketoPage.addFileInputValue('Family Photo', familyPhotoPath); + + await commonEnketoPage.setInputValue('Primary Contact Name', primaryContactName); + await commonEnketoPage.addFileInputValue('Primary Contact Photo', primaryContactPhotoPath); + + for (let i = 0; i < repeatChildren.length; i++) { + if (i > 0) { + await commonEnketoPage.addRepeatSection(); + } + await commonEnketoPage.setInputValue('Child Name', repeatChildren[i].name); + await commonEnketoPage.addFileInputValue('Child Photo', repeatChildren[i].photoPath, { repeatIndex: i }); + } + + await genericForm.submitForm(); + await commonPage.waitForPageLoaded(); + await contactPage.waitForContactLoaded(); + }; + + before(async () => { + await utils.saveDocs([...places.values()]); + await utils.createUsers([onlineUser]); + await utils.addTranslations('en', translations); + + const settings = await utils.getSettings(); + settings.contact_types.push(familyType); + await utils.updateSettings({ contact_types: settings.contact_types }, { ignoreReload: true }); + + await utils.saveDocs([createFormDoc, editFormDoc]); + + await loginPage.login(onlineUser); + }); + + after(async () => { + await utils.deleteDocs([createFormDoc._id, editFormDoc._id]); + await utils.revertDb([/^form:/], true); + }); + + afterEach(async () => { + await commonPage.goToPeople(); + await commonPage.waitForPageLoaded(); + }); + + it('should route uploads to main, sibling, and repeat docs', async () => { + await commonPage.goToPeople(healthCenter._id); + await commonPage.clickFastActionFAB({ actionId: familyType.id }); + + await submitFamilyForm({ + familyName: 'Routing Family', + primaryContactName: 'Amina', + repeatChildren: [{ name: 'Kid Alpha', photoPath: childPhotoPath }], + }); + + const familyId = await contactPage.getCurrentContactId(); + expect(familyId).to.exist; + + const { family, primaryContact, repeatChildren } = await fetchFamilyAndChildren(familyId); + + expect(family.name).to.equal('Routing Family'); + expect(family._attachments).to.exist; + const familyAttachmentNames = Object.keys(family._attachments); + expect(familyAttachmentNames).to.have.lengthOf(1); + expect(familyAttachmentNames[0]).to.match(/^user-file-photo-for-upload-form.*\.png$/); + + expect(primaryContact.name).to.equal('Amina'); + expect(primaryContact._attachments).to.exist; + const primaryAttachmentNames = Object.keys(primaryContact._attachments); + expect(primaryAttachmentNames).to.have.lengthOf(1); + expect(primaryAttachmentNames[0]).to.match(/^user-file-layers.*\.png$/); + + expect(repeatChildren).to.have.lengthOf(1); + const [child] = repeatChildren; + expect(child.name).to.equal('Kid Alpha'); + expect(child._attachments).to.exist; + const childAttachmentNames = Object.keys(child._attachments); + expect(childAttachmentNames).to.have.lengthOf(1); + expect(childAttachmentNames[0]).to.match(/^user-file-icon.*\.png$/); + + const allAttachmentNames = [ + ...familyAttachmentNames, + ...primaryAttachmentNames, + ...childAttachmentNames, + ]; + expect(new Set(allAttachmentNames).size).to.equal(3); + }); + + it('should keep saved attachments intact when adding a new repeat child on edit', async () => { + await commonPage.goToPeople(healthCenter._id); + await commonPage.clickFastActionFAB({ actionId: familyType.id }); + + await submitFamilyForm({ + familyName: 'Edit Family', + primaryContactName: 'Bilal', + repeatChildren: [{ name: 'Kid One', photoPath: childPhotoPath }], + }); + + const familyId = await contactPage.getCurrentContactId(); + const before = await fetchFamilyAndChildren(familyId); + + const snapshotAttachments = (doc) => Object.fromEntries( + Object.entries(doc._attachments || {}).map(([key, value]) => [key, value.length]) + ); + const beforeFamily = snapshotAttachments(before.family); + const beforePrimary = snapshotAttachments(before.primaryContact); + expect(before.repeatChildren).to.have.lengthOf(1); + const beforeKidOne = snapshotAttachments(before.repeatChildren[0]); + + await commonPage.accessEditOption(); + await commonPage.waitForPageLoaded(); + + await commonEnketoPage.addRepeatSection(); + await commonEnketoPage.setInputValue('Child Name', 'Kid Two'); + await commonEnketoPage.addFileInputValue('Child Photo', familyPhotoPath, { repeatIndex: 1 }); + + await genericForm.submitForm(); + await commonPage.waitForPageLoaded(); + await contactPage.waitForContactLoaded(); + + const after = await fetchFamilyAndChildren(familyId); + + expect(snapshotAttachments(after.family)).to.deep.equal(beforeFamily); + expect(snapshotAttachments(after.primaryContact)).to.deep.equal(beforePrimary); + + const kidOneAfter = after.repeatChildren.find(c => c.name === 'Kid One'); + const kidTwoAfter = after.repeatChildren.find(c => c.name === 'Kid Two'); + expect(kidOneAfter, 'Kid One should still exist').to.exist; + expect(kidTwoAfter, 'Kid Two should be created').to.exist; + + expect(snapshotAttachments(kidOneAfter)).to.deep.equal(beforeKidOne); + + expect(kidTwoAfter._attachments).to.exist; + const kidTwoAttachmentNames = Object.keys(kidTwoAfter._attachments); + expect(kidTwoAttachmentNames).to.have.lengthOf(1); + expect(kidTwoAttachmentNames[0]).to.match(/^user-file-photo-for-upload-form.*\.png$/); + }); +}); From c32e17dd27cf4bf93e7807e3311d34bd4ed7482d Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Wed, 6 May 2026 13:01:16 +0300 Subject: [PATCH 15/30] test(e2e): click add-repeat for the first child row, fix edit-flow repeatIndex --- .../default/contacts/sub-contact-attachments.wdio-spec.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js b/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js index 793961effed..c5792dd9037 100644 --- a/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js +++ b/tests/e2e/default/contacts/sub-contact-attachments.wdio-spec.js @@ -98,9 +98,7 @@ describe('Sub-contact attachment routing', () => { await commonEnketoPage.addFileInputValue('Primary Contact Photo', primaryContactPhotoPath); for (let i = 0; i < repeatChildren.length; i++) { - if (i > 0) { - await commonEnketoPage.addRepeatSection(); - } + await commonEnketoPage.addRepeatSection(); await commonEnketoPage.setInputValue('Child Name', repeatChildren[i].name); await commonEnketoPage.addFileInputValue('Child Photo', repeatChildren[i].photoPath, { repeatIndex: i }); } @@ -203,7 +201,7 @@ describe('Sub-contact attachment routing', () => { await commonEnketoPage.addRepeatSection(); await commonEnketoPage.setInputValue('Child Name', 'Kid Two'); - await commonEnketoPage.addFileInputValue('Child Photo', familyPhotoPath, { repeatIndex: 1 }); + await commonEnketoPage.addFileInputValue('Child Photo', familyPhotoPath, { repeatIndex: 0 }); await genericForm.submitForm(); await commonPage.waitForPageLoaded(); From 26a893b28b3c3c544ed9ceea88931c16e31ba366 Mon Sep 17 00:00:00 2001 From: "Bernard K." Date: Wed, 6 May 2026 13:08:09 +0300 Subject: [PATCH 16/30] test(e2e): drop primary contact section from edit form --- .../forms/family-with-attachments-edit.xml | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml b/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml index 741791e5e9b..cae229df087 100644 --- a/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml +++ b/tests/e2e/default/contacts/forms/family-with-attachments-edit.xml @@ -11,12 +11,6 @@ Family Photo - - Primary Contact Name - - - Primary Contact Photo - Children @@ -36,14 +30,8 @@ family_with_attachments - NEW + - - PARENT - person - - - PARENT @@ -63,10 +51,6 @@ - - - - @@ -83,14 +67,6 @@