10903 sub contact routing#10
Conversation
There was a problem hiding this comment.
I just reset this file to master since the original behavior of clearing the binary fields should work fine.
c660ebb to
3bbca5f
Compare
| const xpath = Xpath | ||
| .getElementXPath($element[0]) | ||
| .slice(ownerXpath.length + 1); | ||
| const formPrefix = ownerDoc.form ? `${ownerDoc.form}/` : ''; | ||
| const filename = `${USER_BINARY_FILE_PREFIX}${formPrefix}${xpath}`; |
There was a problem hiding this comment.
I spent an obscenely long time working on a naming approach here that uses a hash of the actual base64 contents and then had the attachment name set in the doc property value (similar to the type=file attachments).
It worked, but ultimately had very few benefits over this current strategy. 😓 The worst was that it ended up requiring bespoke handling code anyway. 😞
I ended circling back to this path-based approach in the end as I think it works with the least amount of code needing to be added. (And we can just fully repurpose the "legacy" pattern for type=binary attachments).
There was a problem hiding this comment.
I also had initially started with a guid as attachment name approach but that meant doing a clean up with every edit - quite inefficient. Only note with the updated attachment name logic is we need to confirm attachment retrieval for report view also works across all edge cases: reports existing pre-this feature, sub reports view within main report and sub reports standalone view.
There was a problem hiding this comment.
Only note with the updated attachment name logic is we need to confirm attachment retrieval for report view also works across all edge cases: reports existing pre-this feature, sub reports view within main report and sub reports standalone view.
100%! I did test against these cases (including pulling in a report with attachment created on a 4.8.0 instance) and viewing/editing the reports worked as expected. But, it would be really good to have someone else repro this as well. 👍
| // (`user-file/<xpath>`) for inline-binary fields. Required for sub-doc | ||
| // rendering: the parent-form prefix embedded in the name cannot be | ||
| // reconstructed from a sub-doc's own field path alone. | ||
| if (typeof value === 'string' && value.startsWith('user-file/') && isImagePath(value)) { |
There was a problem hiding this comment.
Just to note here that this logic here also fixed currently broken sub-reports standalone view whose attachment name was derived from the main/parent doc xpath, whose name reconstruction within the sub report fails because of missing parent doc xpath reference.
There was a problem hiding this comment.
Hmm, I am not following you here. 🤔 Based on my testing (and it is totally possible I just missed something 😬) the only time I saw the property value getting set to an attachment name with a corresponding user-file/ attachment was for type=file attachments pre-4.9.0. As far as I can tell, we have always cleared the property value for type=binary attachments. And, since 4.9.0 the type=file property values are mapped to user-file- attachments.
40daa0b to
39ccc3c
Compare
…rt forms (medic#10922) Co-authored-by: Bernard K. <kagondubernard81@gmail.com>
…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
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.
saveContact now uses validateAttachments(preparedDocs.preparedDocs)
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
- main-doc & sub-doc oversize attachment fails saveContact - normal-sized attachment passes validation
Enketo's setVal rewrites uploaded binary nodes to type="file" the moment a value is set.
add sub-contact attachment routing form fixtures & testes
Co-authored-by: Joshua Kuestersteffen <jkuester@kuester7.com>
…t attachments Inline-binary form fields (`<foo type="binary">…base64…</foo>` populated by form `<instance>` defaults, `calculate` expressions, or external `instanceData` injection) previously had attachments stored under `user-file/<xpath>` while the saved field was blanked, requiring downstream binary fields attachment name reconstruction from `label` at render time. That reconstruction was broken for sub-docs, where the parent-form prefix in the attachment name is not derivable from the sub-doc alone. With this change, a media field's value is the name of its attachment, regardless of how the attachment was produced. Inline-binary fields keep their stable, xml based name (`user-file/<xpath>`), which is also written back into the field value so the renderer can resolve the image by value alone, consistent with file-widget uploads. - `nodesToJs` no longer sets `[type=binary]` to ''. - `bindJsonToXml` skips binding into `[type=binary]` for empty & attachmentment name reference. inline base64 still binds. - `xmlToDocs` attaches `[type=binary]` blob under the xpath-derived name and rewrites the element text to that name. - `getImagePath` gets the value directly when it starts with `user-file/` and preserves the legacy fetch path Existing data behavior: - Pre-existing docs with `<binary>=""` and a `user-file/<xpath>` attachment, gets overwrite-in-place on edit-re-save as the base64 is supplied for re-attachment under the same xpath-derived key. - Un-edited legacy main-doc records continue to render via the legacy `user-file/<label-path>` fallback in `getImagePath`. - Sub-doc image rendering, previously broken because the parent-form prefix was unreachable from sub-doc field paths, now works once the field has been rewrittento the literal attachment name. - `calculate`-derived per-doc base64 is preserved across edits via re-computation at Enketo's form-init
The report path was searching [type=binary] for file-widget filenames, so modern [type=file] widgets in sub-docs fell back to the main report doc. Narrow both report and contact paths to [type=file] (inline binaries are handled by their own loops).
A media field's value is now uniformly its attachment name minus the
USER_FILE_PREFIX, for both file-widget uploads and inline-binary fields.
- export USER_FILE_PREFIX from enketo.service; use it for both file and
inline-binary attachment names (binary -> user-file-<formId>/<xpath>/<field>)
- store the bare reference (<formId>/<xpath>/<field>) as the binary field value
- add EnketoTranslationService.isAttachmentRef (path-pattern regex) and use it
in the three former startsWith('user-file/') guards
- remove the dedicated user-file/ branch from getImagePath; the bare value
resolves through the unified user-file- + value branch
- reduce processAllAttachments cognitive complexity (extract helpers)
- reduce bindJsonToXml cognitive complexity (extract shouldSkipBinaryBind,
bindArrayToXml, bindObjectToXml)
- namespace report sub-doc binaries by ownerDoc.type
- update unit specs; fix stale type="binary" file-widget fixtures to type="file"
Make the sub-contact badge assertions robust to the exact xpath: verify the inline-binary attachment exists and the field value is that name minus the user-file- prefix, and that the value is preserved unchanged on edit.
Sub-docs carry their own form, so revert attachInlineBinary to `ownerDoc.form || doc.form`. Update the db-doc-with-binary fixture so its sub-docs include a <form> element, matching real sub-report shape.
39ccc3c to
69fe840
Compare
- fill unit-coverage gaps: binary-in-repeat, empty binary, file dedup - gitignore tmep folder - tests hygiene: variable renaming and comments clean up
3bbca5f to
a8dfb10
Compare
19e33c9 to
d3f9c15
Compare
0c4ecea to
86e527e
Compare
All these changes have been made just with the report functionality in mind. I have not tested any support yet for contacts. 😓
Also, if you are not super happy with this approach, also check out just the changes in the first commit. That was a more ambitious refactoring that tried to bring the
type=binaryfunctionality to be closer totype=file.License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.