Adding a config-backed {appUrl} token for notify subject/body interpolation - #6642
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The notify block (shared by
notifications[],schedules[].notify,transitions[].notify, and aserviceTask'sargs.notify) interpolates{field}/{relation.field}placeholders insubject:/body:, but has no concept of the application's own external base URL. An author writing{appUrl}to link back to the record (e.g. "you have an approval waiting") got no error — theresolver silently treated the bare word as an entity field, PascalCased it, and emitted
entity.AppUrlinto the generated Java, which doesn't exist on any entity and failsjavacfor thewhole client-Java batch the moment the project is generated and published.
Fix
{appUrl}is now a reserved, config-backed token instead of falling through to the "must be a realentity field" assumption:
DirigibleConfig.APP_BASE_URL— new entry, env varDIRIGIBLE_APP_BASE_URL, default"".TenantConfigurationKeyPolicy— addedDIRIGIBLE_APP_BASE_URLto the explicit tenant-overrideallow-list.
NotificationSupport.Resolver.access()— special-cases the literal tokenappUrlbefore thedirect-field branch, resolving it to
org.eclipse.dirigible.sdk.core.Configurations.get("DIRIGIBLE_APP_BASE_URL", "")instead ofentity.AppUrl. Since this is the one shared resolver behind all four notify call sites, they allpick it up automatically — no template changes needed (they already import the
Configurationsfacade for
DIRIGIBLE_MAIL_SENDER).intent-assistant-guide.md— documented the token for the AI assistant: it supplies only theorigin (
https://app.example.com), never a full route — the intent layer stays path-agnostic, andthe author appends the rest of the URL as plain text/other placeholders
Example — before vs. after
Known limitation (documented, not fixed here)
Tenant overrides of DIRIGIBLE_APP_BASE_URL won't yet take effect when read from inside a generated
notification listener. The tenant-config thread map is only populated by
TenantConfigurationInitFilter, an HTTP-request-only filter; a notification always fires from an
async MessageHandler (ListenerClassConsumer), which re-establishes the tenant's identity but
not its config overrides — confirmed the same gap exists in the JS-side
AsynchronousMessageListener. This is a pre-existing, platform-wide limitation affecting every
listener that reads any tenant-configurable key, not something introduced by this change. It falls
back correctly to the global env var / default in the meantime. Flagged in
NotificationSupport.APP_URL_TOKEN's javadoc for discoverability; fixing it (wiring
Configuration.setThreadConfiguration(...) into listener dispatch) is a separate, cross-cutting
follow-up.