feat(#11192): support extension libs in messages#11244
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aeba59b to
54db099
Compare
|
@binokaryg could you review this PR ? |
binokaryg
left a comment
binokaryg
left a comment
There was a problem hiding this comment.
Thanks @belwalshubham for working on this.
I tested manually on a local instance, and it was mostly working.
I added an extension-lib to_devanagari to my local config and tested it across:
- outgoing messages
- admin message queue
- registration reply SMS
- scheduled/reminder SMS
- validation error (muting)
- app form
Also confirmed:
- live reload of
extension-libworks - offline availability works
- UI-displayed translations work
Also thank you for adding the unit tests.
I did some review with Claude's assistance and find some issues, which I have flagged in the code inline.
One major fail was the silent content loss on a missing/incorrect helper:
A Mustache section over an unknown key renders nothing, dropping the entire block, not just the value, but any literal text inside it. Verified through the real message-utils render path:
Template (patient_id = 48549) |
Renders |
|---|---|
ID: {{#to_devanagari}}{{patient_id}}{{/to_devanagari}} (correct) |
ID: ४८५४९ |
ID: {{#to_devanagri}}{{patient_id}}{{/to_devanagri}} (typo) |
ID: |
ID: {{#to_nepali}}{{patient_id}}{{/to_nepali}} (never uploaded) |
ID: |
ID: {{to_devanagri}} (typo as plain var) |
ID: |
ID: {{#to_nepali}}12345{{/to_nepali}} (literal inside) |
ID: |
In the above example, a one-character typo in a helper name ships the id missing entirely (ID: ), with no server- or client-side warning. This is standard Mustache behavior (the existing built-ins share it, e.g. a typo'd {{#dat}}…{{/dat}}), but the feature widens the surface since configurers now author helper names. Mitigation is process (preview/test configured messages before deploy. The platform won't catch it) and strengthens the case for a debug/warn log on unknown-helper / name-collision.
We also need to add a documentation on the usage of this feature.
There was a problem hiding this comment.
Let's not include this image in the PR. It can stay on the PR body.
| translate: value => value, | ||
| doc: params || {}, | ||
| content: { message: expression }, | ||
| extensionLibs: extensionLibs.getAll(), |
There was a problem hiding this comment.
Translations can silently lose content before extension-libs load.
This is populated lazily, only when something first awaits CHTDatasourceService.isInitialized() (webapp/src/ts/services/cht-datasource.service.ts:31-42). Nothing guarantees this completes before translations render at startup.
Mustache drops a section whose key is missing (the PR's own test confirms: a non-function helper renders 'value: '). So a translation like Hello {{#uppercase}}{{name}}{{/uppercase}} rendered before the libs load produces Hello — silent content loss, and components won't re-render when the libs later arrive.
Suggestion: load extension-libs eagerly during bootstrap (before/alongside translation loading, e.g. in the bootstrapper or an APP_INITIALIZER), or make the parser fall back to default interpolation until the registry is populated.
| Object | ||
| .entries(doc?._attachments ?? {}) | ||
| .forEach(([k, v]) => this.extensionLibs[k] = this.decodeExtensionLib(k, v)); | ||
| return extensionLibs.load(this.dbService.get()); |
There was a problem hiding this comment.
API and Sentinel watch the changes feed and reload; Admin reloads per query; the webapp loads once per session and only watches DOC_IDS.SETTINGS (webapp/src/ts/services/cht-datasource.service.ts:65-71). This staleness predates the PR for getExtensionLib(), but translations now depend on it too, so the gap is more visible.
Suggestion: add a changes subscription for DOC_IDS.EXTENSION_LIBS, or explicitly document the reload-on-refresh behavior.
| const helperName = fileName.replace(/\.js$/, ''); | ||
| helpers[helperName] = function() { | ||
| return function(text, renderText) { | ||
| return extensionLib(renderText(text)); |
There was a problem hiding this comment.
A throwing extension lib halts the unprotected message paths
The helper wrapper calls the lib directly extensionLib(renderText(text)) in getExtensionLibHelpers. If a configured lib throws at render time, mustache.render throws. transitions/src/lib/messages.js wraps generate in try/catch, but due_tasks.js, the API message export, admin message queue, and webapp format-data-record do not.
I verified this on a local instance: with a helper that throws. A due scheduled reminder stayed stuck scheduled with no message for 7+ minutes (multiple due_tasks cycles) and only recovered once the lib was fixed (baseline with a working lib generated in ~220s). Because due_tasks.js has no try/catch around generate, the throw isn't isolated to the offending message. It rejects the whole batch run (caught only at runTasks' top-level logger), so one bad helper silently blocks all pending scheduled messages every cycle until the lib is corrected, with no user-visible error.
Suggestion: wrap the helper invocation in try/catch, log the error and fall back to the untransformed rendered text, so a single misbehaving lib degrades one message instead of halting the batch.
| DB({ remote: true }).query('medic-admin/message_queue', params.list), | ||
| DB({ remote: true }).query('medic-admin/message_queue', params.count) | ||
| DB({ remote: true }).query('medic-admin/message_queue', params.count), | ||
| MessageQueueUtils.loadExtensionLibs(), |
There was a problem hiding this comment.
Admin message queue hard-fails if extension-libs can't load.
API and Sentinel tolerate load failures but admin's query() rejects entirely. A transient failure fetching the doc shouldn't take down the whole message-queue tab.
Suggestion: catch and fall back to {}, matching the behavior everywhere else.
| exports.template = function(config, translate, doc, content, extraContext) { | ||
| extraContext = extraContext || {}; | ||
| exports.template = function(options, ...legacyArgs) { | ||
| const { config, translate, doc, content, extraContext, extensionLibs } = legacyArgs.length ? { |
There was a problem hiding this comment.
Duplicated legacy-args normalization: normalizeGenerateOptions exists for generate, but template re-implements the same logic inline. Extract one shared normalizer.
| const getExtensionLibHelpers = (extensionLibs = {}) => Object.entries(extensionLibs) | ||
| .filter(([, extensionLib]) => typeof extensionLib === 'function') | ||
| .reduce((helpers, [fileName, extensionLib]) => { | ||
| const helperName = fileName.replace(/\.js$/, ''); |
There was a problem hiding this comment.
Helper name collisions: uppercase.js and uppercase both map to helper uppercase — last one silently wins. A debug log would help configurers.
|
|
||
| const render = function(config, template, view, locale) { | ||
| return mustache.render(template, Object.assign(view, { | ||
| const getExtensionLibHelpers = (extensionLibs = {}) => Object.entries(extensionLibs) |
There was a problem hiding this comment.
nit: getExtensionLibHelpers(extensionLibs) rebuilds the helper map — iterating the registry, filtering non-functions, stripping .js suffixes, wrapping each lib in the Mustache lambda pattern — on every call to render, plus again for each nested render triggered by formatDate / local_phone. When Sentinel processes a batch of scheduled messages, that's the same map being reconstructed once per message from an unchanged registry. Since set() returns a fresh object on each reload, object identity could serve as the cache key. Worth caching?
| const findIdByKey = (contactsByReference, key) => { | ||
| const row = contactsByReference.rows.find((row) => row.key[1] === key); | ||
| return row && row.id; | ||
| return row && row.id; // NOSONAR: browserify-ngannotate does not support optional chaining. |
| const getLegacyGenerateArgs = call => { | ||
| const options = call[0]; | ||
| return [ | ||
| options.config, | ||
| options.translate, | ||
| options.doc, | ||
| options.content, | ||
| options.recipient, | ||
| options.extraContext, | ||
| ]; | ||
| }; |
There was a problem hiding this comment.
It looks misplaced, and the name also seems to be misleading. It destructures the new options object into a positional array to preserve old assertions. Please consider moving it below the imports and consider a clearer name like pickGenerateArgs.
3 participants
Summary
Adds project-defined
extension-libsas synchronous Mustache section helpers for translations and outgoing messages.This allows configurers to transform message values without adding a new built-in helper to CHT Core, for example:
Closes #11192.
Architecture
flowchart LR Couch[("CouchDB<br/>extension-libs")] Loader["Extension-libs loader<br/>decode + atomic registry replacement"] Couch --> Loader Loader --> Webapp["Webapp cache"] Loader --> API["API config cache"] Loader --> Sentinel["Sentinel config cache"] Loader --> Admin["Admin preview load"] Webapp --> Translation["Webapp translations"] Webapp --> Report["Report detail messages"] API --> Export["Message export rendering"] Sentinel --> Transitions["Transitions and scheduled SMS"] Admin --> Preview["Message queue preview"] Translation --> MessageUtils["message-utils<br/>template(opts) / generate(opts)"] Report --> MessageUtils Export --> MessageUtils Transitions --> MessageUtils Preview --> MessageUtils MessageUtils --> Output["Consistent rendered message"]Changes
@medic/extension-libsfor loading and decoding theextension-libsCouchDB document.message-utils.generateandmessage-utils.templateto accept an options object containingextensionLibs..jssuffix.Security
Extension-libs are trusted application configuration uploaded by authorized administrators. This change does not present extension-lib execution as sandboxed.
Verification
Manual end-to-end evidence
The following screenshot records the manually executed live CouchDB flow. It uses a real
extension-libsdocument and attachments, the production@medic/extension-libsloader, and the productionmessage-utilsrendering path.Manual steps verified:
cht_11192_e2edatabase on CouchDB 3.5.2.to_devanagari.jsas an attachment on theextension-libsdocument.message-utils.generateand confirmedID १२३४५.uppercase.js.message-utils.templateand confirmedHello ADA.This screenshot is evidence of the loader/reload/rendering end-to-end path. The Admin, API, Sentinel, and Webapp integrations are additionally covered by their regression suites listed below.
npm run build-devnpm run lintCOUCH_URL=http://medic:password@localhost:5984/medic npm run unit-api— 1,636 passingnpm run unit-admin— 435 passingTZ=UTC COUCH_URL=http://medic:password@localhost:5984/medic npm run unit-sentinel— 231 passingnpm test --workspace=@medic/extension-libs— 6 passing, 100% coveragenpm test --workspace=@medic/message-utils— 112 passing, 100% statements/linesto_devanagari.jsas an attachment;ID १२३४५throughmessage-utils.generate;uppercase.js;Hello ADAthroughmessage-utils.template;Notes for reviewers
message-utils.