Design: Config-driven PGR notifications (MDMS routing + templates, Novu delivery) #59
ChakshuGautam
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Design: Config-driven PGR notifications (MDMS routing + templates, Novu delivery)
1. TL;DR
This PR replaces PGR's hardcoded notification logic (routing baked into
NotificationService, message text in localization keys) with a config-driven pipeline: two MDMS masters (RAINMAKER-PGR.NotificationRoutingandNotificationTemplate) declare who gets notified on which workflow transition over which channel and with what message; pgr-services resolves audiences (including role-pool fan-out), renders and localizes the message, and emits one pre-rendered event per (recipient × channel) to Kafka; novu-bridge becomes a domain-dumb pass-through that identifies the subscriber in Novu and triggers a per-channel Novu workflow, logging every outcome tonb_dispatch_log. Both masters are authored in a single artifact (PgrWorkflowConfig.json, next to the workflow it belongs to) and split apart at seed time by default-data-handler; the configurator gains a per-transition "Notifications" management surface with a static config checker, plus read-only Logs and Providers screens. The whole runtime path is behind a feature flag (pgr.notification.config.driven) with a golden-output test gate proving SMS parity with the legacy path. PR: #58 (78 files, +11305/−543).2. Architecture
Authoring → seeding. Operators (and the repo's default seeds) author notifications inside
PgrWorkflowConfig.json, per workflow action:notifications[](audience, channel, optional fromState/assigneeOnly) plus top-levelnotificationTemplates[]. workflow-v2 has no extension point and silently drops unknown fields, so default-data-handler'screatePgrWorkflowConfigsplits the artifact: it strips the authoring-only fields from the raw tree before POSTing the BusinessService, then emits the routing and template rows to MDMS with deterministic uniqueIdentifiers (businessService.action.toState.audience.channelandaudience.action.toState.channel.locale). Emission is idempotent (duplicate creates tolerated), so tenant setup is re-runnable. Routing and template rows join 1:1 per (audience, action, toState, channel) — an active routing row without a matching template is a config error the checker flags (R2).Runtime. With the flag on,
NotificationService.processConfigDrivenasksNotificationRouterfor the rows matching (businessService, action, toState); each row yields an audience + channel.CITIZENresolves to the complaint filer;EMPLOYEEis kept as a legacy alias for the current assignee; any other value is treated as a workflow role code and expands to the tenant's holder pool via an egov-userroleCodessearch — deliberately mirroring the workflow engine's ownisRoleAvailablegate. Non-notifiable pseudo-audiences (AUTO_ESCALATE,SYSTEM) are dropped.TemplateRendererfills placeholders and applies default-locale fallback; PGR then publishes one fully-rendered event per (recipient × channel), deduped onchannel|subscriberId, with a stabletransactionId(complaintId:action:toState:subscriberId:channel) that doubles as the idempotency key downstream (Novu dedupes triggers on it;nb_dispatch_logupserts on it).Delivery. novu-bridge no longer resolves templates or providers for this path: it validates the envelope, optionally consults the consent gate (shipped disabled), identifies the Novu subscriber (contact + locale), triggers the per-channel Novu workflow with the rendered body as payload, and records the outcome in
nb_dispatch_log. Novu CE v2.3.0 only renders v2-native workflows (controlValues + Liquid{{ payload.body }}); v1-origin workflows accept triggers and render nothing — the "blank email" bug found during the pilot. SMS is delivered via Twilio, email via SMTP, both as Novu integrations.WhatsApp. The channel is defined in config — the MDMS schema enum keeps
WHATSAPP, seeds carry WHATSAPP rows, and PGR will emit WHATSAPP events per config — but delivery is not yet backed by a production provider. The Baileys client used during development was test-scaffolding only; it is not a legitimate provider and is being removed. WhatsApp delivery activates when a legitimate provider (Meta WhatsApp Cloud API or Twilio WhatsApp, as a Novu integration behind a v2-nativecomplaints-whatsappworkflow) is onboarded. Until then,channel=WHATSAPPmust terminate at the bridge as an explicit SKIPPED / no-provider dispatch-log row — never a thrown exception (that would DLQ-spam permanently undeliverable events) and never a fallback to another channel. Because the config layer is provider-agnostic and delivery is gated at the bridge edge, onboarding the real provider later is pure Novu configuration plus one flag — zero PGR/MDMS/configurator churn.Decommissioning the test WhatsApp path (Baileys removal scope)
Baileys is confined to the delivery edge; the MDMS model, PGR emitter (save one stale test Javadoc), and event contract are untouched by removal:
BaileysSendClient.javaandprovider/BaileysProviderStrategy.java; inDispatchPipelineServiceremove the client dependency and theWHATSAPP → Baileysbranch; remove the fournovu.bridge.whatsapp.baileys.*config fields/properties (one of which, the timeout, was already dead code); decide who reclaims the bare"whatsapp"alias in the provider-strategy factory (the diff removed it from the Meta strategy specifically to avoid shadowing Baileys); update the stale Javadoc "direct Baileys/Telegram sends bypass the log" notes — bridge-routed WHATSAPP sends already persist rows innb_dispatch_logtoday, so this is a comment correction, not a new observability gain. Also fix the stale "Baileys for WHATSAPP" Javadoc inNotificationConfigDrivenEmissionTest.java.novu.bridge.channel.whatsapp.enabled, default false, optionally backed by a cached probe of Novu's integrations list); when disabled, persistSKIPPED/NB_NO_PROVIDERvia the existing preference-denied pattern. FixgetNovuWorkflowId's default-to-SMS in the same change (see findings).utilities/baileys-send-service/entirely (~2.9k lines incl. lockfile).depends_on, and env vars fromlocal-setup/docker-compose.egov-digit.yaml; the baileys image/token variables and build tasks from the Ansible templates, playbook, and example host_vars; a stray comment indocker-compose.bomet.yml.BaileysProviderStrategyTest(4 tests); rewriteDispatchPipelinePassThroughTest.whatsappEvent_routesToBaileys_notNovuinto the WHATSAPP-no-provider safe-skip test; flip the e2e script's WHATSAPP "warn" branch into a hard SKIPPED assertion.active=false);SKIPPED/NB_NO_PROVIDERrows in the interim are the honest, debuggable representation.3. Configurator management surface
action.nextStateas a state UUID; the tab resolves UUID →applicationStatusso operators see and store real state names. Each transition row shows its (audience × channel) chips with inline add/edit/remove that writes both MDMS masters (routing + template) as a pair, relying on server-side x-unique uid derivation. A JSON toggle exposes the raw BusinessService.validateNotifications, pure/React-free): R1 audience must be a known role code (error), R2 every active routing row needs an active default-locale template (error, with a distinct message when only another locale exists), R3 channel must be in the allowed set (error), R4 the routing row's (action, toState) must be a real workflow transition (error — this is where the UUID resolution matters), R5 orphan templates with no routing (warn), R6 non-notifiable audiences like AUTO_ESCALATE (warn, and suppresses R1 for them). 11 vitest cases pass./novu-adapter/v1/logspagesnb_dispatch_log(tenant-scoped, filterable by reference number prefix/channel/status, clamped limits, honest totals) and/novu-adapter/v1/integrationslists Novu integrations with credentials redacted server-side (anycredentialsmap at any depth is masked wholesale).4. Key design decisions
isRoleAvailableactionability gate — the people who can act get told they should.CITIZENis the filer;EMPLOYEEsurvives only as a legacy alias for the assignee;assigneeOnlynarrows a role row to the assignee.complaintId:action:toState:subscriberId:channelis idempotent across redeliveries: Novu dedupes triggers on it andnb_dispatch_logupserts on (transaction_id, channel, recipient_value).NotificationUtiloutput — making the eventual default-flip an auditable no-op rather than a leap of faith.{{ payload.body }}); v1-origin workflows accept triggers and silently render nothing. The pilot's blank-email bug came from exactly this; workflow bootstrap must create v2-native definitions (see Open items).5. Review findings
Two review streams (backend + configurator) over the exact diff. Material findings, unsoftened:
useCreate/useUpdate/useDeletecalled without{ returnPromise: true }(ra-core 5.14.5) — everyawaitresolves void, all try/catch is dead code, and "Notification added" success toasts fire even when the MDMS write failed{ returnPromise: true }on every mutation call (mode is already pessimistic)getNovuWorkflowIddefault:branch). Reachable today via legacy envelopes; a future channel string would be silently delivered as SMS. (Note:getNovuWorkflowIdhas an explicit WHATSAPP case, so a naive Baileys removal routes WHATSAPP events to the unprovisionedcomplaints-whatsappNovu workflow —NB_NOVU_TRIGGER_FAILED→ DLQ spam, or phantom SENT rows if Novu acknowledges the unknown workflow — which is why the explicit channel-enable gate + SKIPPED/NB_NO_PROVIDERrow is required work, not what removal alone yields)NB_UNSUPPORTED_CHANNELor persist SKIPPED; never default to SMSRestTemplatehas no connect/read timeouts; one hung endpoint stalls the entire Kafka consumer (rebalance loops). The newbaileys.timeout.msproperty is dead config nothing readsClientHttpRequestFactorywith sane timeouts/novu-adapter/v1/*has no auth (no RequestInfo, Bearer token unchecked);recipient_value/transaction_idcan embed raw phone numbers; referenceNumber prefix search makes complaints enumerable. Simultaneously the repo ships no route for it (nginx template declines, no Kong route), so stock deploys 404 and the only way to make the screens work is hand-wiring the unauthenticated proxySENTrows (email step fails invisibly inside Novu); email-only users on phone channels produce guaranteed DLQ noisemdmsCreatethrow explicitly on an empty create responseisActive=truelocaledimension and recipient locale field are dead. Fine for the single-locale pilot, but undocumentedfromStateis silently ignored on the production path (router always called with null) — a row constrained to fromState=X applies on every transition into toStatePgrWorkflowConfig.jsonthe new role-pool policy (11 rows) — a tenant seeded by both gets a contradictory merged policy (double SMS on ASSIGN). The regen script also overwrites hand-added WHATSAPP/EMAIL rowsbusinessService: once a second workflow onboards, cross-BS template sharing yields R5 false positives and delete-time collateral; PGR's own REOPEN chip is silently shared across two transition rowsen_INhardcoded in checker + Configure tab while the backend default locale is a config property — a deployment flipping it gets false R2 errors and edits that miss the locale the runtime readsassigneeOnlyis schema-supported but unreachable from any UIRENDERED_BODY_MODE,IDENTIFY_ENABLED,channels.default, inert "rollback" properties whose client class was deleted); committed Playwright report artifacts; ~90 lines of duplicated ValidationPanel JSX; redaction is denylist-by-location (allowlist would be strictly safer in front of an unauthenticated route); unbounded identify-TTL mapWhat's done well (worth keeping as-is): the golden-output legacy-parity gate (including the all-transitions sweep and the explicit rejection of empty-set false proofs) is exactly the right harness for a rewrite-behind-a-flag;
DispatchLogRepositoryis cleanly parameterized with mandatory tenantId and clamped limits; the Flyway migration is correctly additive; the splitter's strip-before-POST + idempotent emit ordering is safely re-runnable;validateNotificationsis pure, well-typed, and fully rule-covered; the "custom" read-only dataProvider type is cleanly scoped; configurator type-check is clean.6. Automated test plan
Already covered (51 passing automated tests + 1 live E2E, all verified by running the suites)
NotificationRouterTestTemplateRendererTestNotificationConfigDrivenEmissionTestNotificationGoldenOutputTestDispatchPipelinePassThroughTestBaileysProviderStrategyTest"whatsapp"in the strategy factory — must be re-pinned against the future provider)ProviderAgnosticTestvalidateNotifications.test.tse2e-role-notifications.jsPgrWorkflowConfig.json. Hard-fails on any missing row (WhatsApp currently downgraded to a warning — flips to a hard assertion post-Baileys, see G18)Scope notes: Baileys-specific delivery tests are excluded — the provider is being removed. A WHATSAPP-no-provider safe-skip test is included (G2 unit, G18 e2e) to lock in the no-silent-fallback ruling.
To add
Unit
channel=WHATSAPPwith no provider → no Novu trigger, no send, explicit SKIPPED/NB_NO_PROVIDERdispatch row, and no SMS fallback (replaces the Baileys pass-through test)NotificationServiceassigneeOnly=truerestricts to assignee; EMPLOYEE alias → assignee (not pool search); CITIZEN who also holds a routed role → one message per channel; egov-user search failure → graceful skipPIGEON) rejected/skipped with logged status — never defaulted into the SMS workflow (the bridge must defend independently of PGR's router)SENTis persisted unconditionally) and the /logs proxy surfaces itMDMSUtilssw_KErecipient gets thesw_KEtemplate body) — service-level, not just renderer fallbackIntegration
PgrWorkflowConfig.jsonthrough with mocked workflow-v2 + MDMS clients; assert (a) POSTed workflow has zero notification-field residue at any depth, (b) emitted MDMS payloads exactly equal the checked-in seed fixtures, (c) uid schemes hold, (d) re-run is idempotent under phantom-200 semanticscomplaints.domain.eventsreaches the pipeline with the envelope intactE2E — all extend the existing Bomet script (same harness, new assertions/scenarios; no duplication of APPLY/ASSIGN/RESOLVE):
Configurator UI
validateNotificationsnextState, states carrying uuid + applicationStatus): R4 does not false-positive on valid transitions and does fire on a uuid pointing at a missing state7. Open items
/novu-adapter/v1/*route in the repo with Kong auth (EMPLOYEE/ADMIN), add server-side recipient masking, and correct the code comments that currently claim JWT gating. Until then the Logs/Providers screens either 404 on stock deploys or run over a hand-wired unauthenticated proxy — neither is acceptable beyond the pilot.complaints-whatsappworkflow, then flip the bridge's channel-enable gate. Config, seeds, and configurator need no changes. Interim behavior: explicit SKIPPED/no-provider rows (G2/G18 pin this).All reactions