feat(#10904): route file attachments to correct sub-docs in report forms by YASHSHARMAOFFICIALLY · Pull Request #10922 · medic/cht-core

YASHSHARMAOFFICIALLY · 2026-04-24T15:11:41Z

Summary

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.

Changes

resolveOwnerDoc() — walks up the XML tree from a binary node to find the nearest [db-doc="true"] ancestor, returning the corresponding prepared sub-doc (or the main report doc as fallback)
findBinaryNodeByFilename() — locates the [type=binary] XML node matching a FileManager filename so its position in the tree can determine the owner
FileManager file routing — uploaded files are now attached to the correct owner doc based on their position in the XML form
Inline binary blob routing — base64-encoded binary fields inside sub-docs are attached to those sub-docs

Test plan

Unit tests pass (2652 passing)
Lint passes
Binary field in main doc attaches to main report doc
Binary field inside [db-doc="true"] attaches to that sub-doc
FileManager file upload inside sub-doc routes correctly
Existing report forms without sub-docs are unaffected

benkags · 2026-04-24T21:01:59Z

@YASHSHARMAOFFICIALLY let's use the branch medic:10700-photo-capture-in-sub-contacts-and-reports as the base branch.

YASHSHARMAOFFICIALLY · 2026-04-25T15:23:38Z

Done, updated the base branch to 10700-photo-capture-in-sub-contacts-and-reports. Ready for review! @benkags @jkuester @dianabarsan

benkags

Thank you @YASHSHARMAOFFICIALLY. I've left a few review comments.

benkags · 2026-04-28T21:11:40Z

+  private findBinaryNodeByFilename($record, filename: string) {
+    let match = null;
+    $record
+      .find('[type=binary]')


You want to ensure the you check for type=file. When testing #10923, I realized Enketo sets type to file; so uploads would fail silently. Did

Good catch — updated the selector from [type=binary] to [type=file] in 900c9550a. Enketo's Nodeset.setVal rewrites file-widget nodes to type="file" after upload, so this is the correct selector for FileManager files. Inline-binary blobs (draw/signature widgets) still use type="binary" and are handled via the separate xpath-based path.

benkags · 2026-04-28T21:12:48Z

    const attachLegacyFile = (elem, file, type, alreadyEncoded) => {
+      const ownerDoc = resolveOwnerDoc(elem);
      const xpath = Xpath.getElementXPath(elem);
+      const formId = ownerDoc === doc ? doc.form : ownerDoc.type;


doc.form and doc.type are two different types. And type is not always unique. Let's just use doc.form. What's the rationale for using type here if may ask?

You're right — switched to doc.form for the xpath root segment in 900c9550a. Using doc.type was incorrect since it's not guaranteed to be unique and serves a different purpose. The xpath-based filename is now always rooted at the main form's internalId regardless of which sub-doc owns the attachment.

benkags · 2026-04-28T21:17:17Z

+      expect(actual.length).to.equal(1);
+      expect(AddAttachment.callCount).to.equal(1);
+      expect(AddAttachment.args[0][0]._id).to.equal(actual[0]._id);
+    });


We should add 2 more unit tests: 1) test repeats 2) test for clean up of attachments removed during edit

Added both in 5da5cc194:

Repeats — unit test verifies files inside db-doc blocks nested in repeat sections route to the corresponding sub-docs, with no files landing on the main doc.

Attachment cleanup on edit — unit test verifies that when a file field is cleared during edit, no attachment is created on the sub-doc.

benkags · 2026-05-05T07:30:55Z

@YASHSHARMAOFFICIALLY take a look at YASHSHARMAOFFICIALLY#1

benkags · 2026-05-07T04:50:56Z

@YASHSHARMAOFFICIALLY let me know if you are still working on this

YASHSHARMAOFFICIALLY · 2026-05-07T06:55:34Z

@benkags will raised a pr by today night working on these

benkags · 2026-05-07T18:22:15Z

@YASHSHARMAOFFICIALLY I added the fixes in the linked PR, please take a look, add an e2e test so that it is ready for review.

…rt 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

…oc attachments - findFileNodeByFilename now selects [type=file], matching Enketo's runtime XML state - use doc.form for the xpath-rooted filename - additional tests: orphan file fallback, db-doc nested in repeats, and filename shape for sub-doc binaries.

… sub-documents Verifies that when a form has a db-doc sub-document with a file upload field, the uploaded file is attached to the sub-doc and not the main report document.

YASHSHARMAOFFICIALLY · 2026-05-07T18:41:27Z

@benkags Do check and let me know if any improvment needed further

…removed during edit Verify that when a file is cleared from a sub-doc during editing, no attachment lands on the sub-doc while the main doc still receives its own file correctly.

YASHSHARMAOFFICIALLY · 2026-05-12T04:45:11Z

@benkags @dianabarsan @andrablaj Thank you for the fixes and the review guidance. Addressed the remaining items:

Unit tests added:

Repeats (db-doc-in-repeat-with-files): Verifies each repeat instance's file routes to the corresponding sub-doc, with nothing landing on the main doc.
Attachment cleanup on edit (db-doc-with-file-field-cleared): Verifies that when a file is cleared from a sub-doc during editing, no attachment lands on the sub-doc while the main doc still receives its own file correctly.

Both use dedicated XML fixtures to isolate the scenario.

The e2e test (submit-db-doc-file-upload.wdio-spec.js) from the previous commit covers the full end-to-end flow: uploading files to both the main doc and a sub-doc, then verifying each attachment ended up on the correct document in the database.

Ready for re-review.

dianabarsan

Code looks solid, and makes sense on a quick read. Appreciate that.
I need to request that we add an e2e test for this feature, that involves having a form with multiple sub-docs and test that attachments are saved correctly. We should also cover sub-docs in repeats.

Appreciate this contribution!

…-docs with file attachments Cover the two scenarios requested in review: a form with multiple sub-documents each owning separate file attachments, and sub-documents created inside repeat groups with per-instance file routing.

YASHSHARMAOFFICIALLY · 2026-05-12T14:11:36Z

@dianabarsan @andrablaj Thank you for the review. I've added the requested e2e coverage in e0229410f:

Multiple sub-docs — new form (db-doc-multi-file-upload) with two separate db-doc="true" sub-documents, each with its own file upload. The test verifies each sub-doc receives only its own attachment and checks for cross-contamination between all three documents (main, sub-doc A, sub-doc B).
Sub-docs in repeats — new form (db-doc-repeat-file-upload) with a repeat group containing a db-doc="true" block with a file upload. The test creates two repeat instances with different files and verifies each repeat's sub-doc gets the correct attachment, with no files leaking to the main doc or between repeat sub-docs.

Happy to contribute more

benkags · 2026-05-14T08:23:08Z

@dianabarsan please take another look and let us know if there is additional changes needed. Thanks.

dianabarsan

Thanks for the followup!

Edge case worth covering: if two sub-docs end up with files of the same name (e.g., default-named camera captures, or any form where the user picks files with overlapping names from disk), findFileNodeByFilename returns the first match and both files route to the same sub-doc — the other sub-doc gets no attachment, and the user-file-<name>
FileManager attachment key collides on top of that.

The e2e tests use distinct filenames (icon.png, layers.png, photo-for-upload-form.png), so this isn't exercised. Could you add a case with two sub-docs whose files have the same name and decide what the desired behavior is? At minimum I think we want to detect and warn rather than silently drop one file.

Could we also get an e2e test covering a draw/signature widget inside a db-doc="true" sub-doc? In production the draw widget puts its value in FileManager with a generated filename (drawing-HH_MM_SS.png — webapp/src/js/enketo/widgets/draw.js:674), so it goes through the same findFileNodeByFilename routing the PR
rewires. That path is only tested with upload widgets right now.
Can we also have an e2e test covering binary uploads in subdocs?

While you're there: those generated filenames are second-resolution, so a user signing two sub-doc sections within the same second produces colliding filenames — which is the same routing/collision concern I raised on the upload path, just more likely to hit because the user doesn't choose the filename.

dianabarsan · 2026-05-15T12:19:28Z

+      // 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(


I think this test asserts that the files were attached to repeat document, but not to which document.

benkags · 2026-05-16T16:54:31Z

Some of the work done here has leaked into #10923 when trying to handle the binary type input - should we track one PR instead?

I know you have done a lot of work here already @YASHSHARMAOFFICIALLY but at this point it is hard to separate out the reports logic in #10923's review. I suggest you you target my branch here benkags:10903-sub-contact-routing so that we track all the work you have already done here from there cc @dianabarsan @jkuester.

The <group> element wrapping the repeat caused Enketo to render it as a separate page. The test never navigated to that page, so the "Repeat photo" labels were not found. Removing the group keeps everything on a single page, matching the other test forms.

YASHSHARMAOFFICIALLY · 2026-05-18T21:58:45Z

@dianabarsan @benkags Pushed a fix for the failing e2e test should route file attachments to sub-docs inside repeat groups.

Root cause: The db-doc-repeat-file-upload.xml form had a <group> element wrapping the <repeat>, which caused Enketo to render the repeat section on a separate page. The test never navigated to that page, so $$(label*=Repeat photo) found 0 elements → Index out of bounds.

Fix: Removed the <group> wrapper so the repeat renders on the same page as the name field, matching the pattern of the other two test forms (db-doc-file-upload.xml, db-doc-multi-file-upload.xml).

jkuester · 2026-05-19T20:58:00Z

@YASHSHARMAOFFICIALLY thank you for the changes here!

@dianabarsan, sorry for hijacking the PR and merging prematurely! 😬 🥷 I was on a call with @benkags and we were trying to find the most simple way to sync all of this with #10923 and get it shipped. The current plan is that @benkags is going to update his branch with the latest from https://github.com/medic/cht-core/tree/10700-photo-capture-in-sub-contacts-and-reports and then target master with github.com//pull/10923.

So, the complete unified changes will be available to review in that PR. 👍

…rt forms (medic#10922) Co-authored-by: Bernard K. <kagondubernard81@gmail.com>

YASHSHARMAOFFICIALLY force-pushed the 10904-sub-contact-photo-reports-routing branch from 8928897 to bfb5ec2 Compare April 25, 2026 15:22

YASHSHARMAOFFICIALLY changed the base branch from master to 10700-photo-capture-in-sub-contacts-and-reports April 25, 2026 15:23

benkags suggested changes Apr 28, 2026

View reviewed changes

dianabarsan self-requested a review May 6, 2026 06:10

YASHSHARMAOFFICIALLY and others added 3 commits May 8, 2026 00:04

test(medic#10904): add e2e test for file attachment routing to db-doc…

cb6c753

… sub-documents Verifies that when a form has a db-doc sub-document with a file upload field, the uploaded file is attached to the sub-doc and not the main report document.

YASHSHARMAOFFICIALLY force-pushed the 10904-sub-contact-photo-reports-routing branch from bfb5ec2 to cb6c753 Compare May 7, 2026 18:35

test(medic#10904): add unit test for attachment cleanup when file is …

5da5cc1

…removed during edit Verify that when a file is cleared from a sub-doc during editing, no attachment lands on the sub-doc while the main doc still receives its own file correctly.

dianabarsan requested changes May 12, 2026

View reviewed changes

dianabarsan self-requested a review May 14, 2026 18:01

dianabarsan requested changes May 15, 2026

View reviewed changes

benkags mentioned this pull request May 16, 2026

10903 sub contact routing benkags/cht-core#10

Open

jkuester merged commit cc34e08 into medic:10700-photo-capture-in-sub-contacts-and-reports May 19, 2026
26 of 28 checks passed

benkags added a commit to benkags/cht-core that referenced this pull request May 21, 2026

feat(medic#10904): route file attachments to correct sub-docs in repo…

05b38ac

…rt forms (medic#10922) Co-authored-by: Bernard K. <kagondubernard81@gmail.com>

jkuester mentioned this pull request May 26, 2026

feat(#10904): route file attachments to correct sub-docs in report fo… #11115

Draft

7 tasks

benkags added a commit to benkags/cht-core that referenced this pull request May 27, 2026

feat(medic#10904): route file attachments to correct sub-docs in repo…

1b32f2d

…rt forms (medic#10922) Co-authored-by: Bernard K. <kagondubernard81@gmail.com>

benkags added a commit to benkags/cht-core that referenced this pull request May 27, 2026

feat(medic#10904): route file attachments to correct sub-docs in repo…

5760157

…rt forms (medic#10922) Co-authored-by: Bernard K. <kagondubernard81@gmail.com>

Uh oh!

Conversation

YASHSHARMAOFFICIALLY commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

benkags commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YASHSHARMAOFFICIALLY commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benkags left a comment

Choose a reason for hiding this comment

Uh oh!

benkags Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

YASHSHARMAOFFICIALLY May 12, 2026

Choose a reason for hiding this comment

Uh oh!

benkags Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

YASHSHARMAOFFICIALLY May 12, 2026

Choose a reason for hiding this comment

Uh oh!

benkags Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

YASHSHARMAOFFICIALLY May 12, 2026

Choose a reason for hiding this comment

Uh oh!

benkags commented May 5, 2026

Uh oh!

benkags commented May 7, 2026

Uh oh!

YASHSHARMAOFFICIALLY commented May 7, 2026

Uh oh!

benkags commented May 7, 2026

Uh oh!

YASHSHARMAOFFICIALLY commented May 7, 2026

Uh oh!

YASHSHARMAOFFICIALLY commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dianabarsan left a comment

Choose a reason for hiding this comment

Uh oh!

YASHSHARMAOFFICIALLY commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benkags commented May 14, 2026

Uh oh!

dianabarsan left a comment

Choose a reason for hiding this comment

Uh oh!

dianabarsan May 15, 2026

Choose a reason for hiding this comment

Uh oh!

benkags commented May 16, 2026

Uh oh!

YASHSHARMAOFFICIALLY commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jkuester commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

YASHSHARMAOFFICIALLY commented Apr 24, 2026 •

edited

Loading

benkags commented Apr 24, 2026 •

edited

Loading

YASHSHARMAOFFICIALLY commented Apr 25, 2026 •

edited

Loading

YASHSHARMAOFFICIALLY commented May 12, 2026 •

edited

Loading

YASHSHARMAOFFICIALLY commented May 12, 2026 •

edited

Loading

YASHSHARMAOFFICIALLY commented May 18, 2026 •

edited

Loading