Email Executor generic template data handling#2485
Merged
ThaminduDilshan merged 1 commit intoasgardeo:mainfrom Apr 29, 2026
Merged
Email Executor generic template data handling#2485ThaminduDilshan merged 1 commit intoasgardeo:mainfrom
ThaminduDilshan merged 1 commit intoasgardeo:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new forwarded-template-data key ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant InviteExecutor as Invite\r\nExecutor
participant EmailExecutor as Email\r\nExecutor
participant TemplateService as Template\r\nService
participant SMTP as Email\r\nService
Client->>InviteExecutor: Execute GenerateInvite
InviteExecutor->>InviteExecutor: create inviteLink / token
InviteExecutor->>InviteExecutor: set ExecutorResponse.ForwardedData.templateData(inviteLink...)
InviteExecutor-->>Client: return ExecutorResponse (with ForwardedData)
Client->>EmailExecutor: ExecuteSend (receives/uses ForwardedData)
EmailExecutor->>EmailExecutor: select templateName (forwarded or node property)
EmailExecutor->>TemplateService: Render(templateName, templateData)
TemplateService-->>EmailExecutor: rendered content
EmailExecutor->>SMTP: Send(email data)
SMTP-->>EmailExecutor: send result
EmailExecutor-->>Client: ExecComplete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Contributor
Author
|
Please mark this as an improvement, and a breaking change. Note, that MagicLink implementation #1879 depends on this PR. |
Contributor
|
@RandithaK Why this is a breaking change? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/common/constants.go`:
- Around line 256-259: The change introduces a breaking public contract by
adding ForwardedDataKeyTemplateName and ForwardedDataKeyTemplateData (used as
ForwardedData["templateName"] and ForwardedData["templateData"]) for email
templating but the docs were not updated; add documentation under docs/
describing the forwarded-data contract (keys ForwardedDataKeyTemplateName:
string and ForwardedDataKeyTemplateData: map[string]string), show example
payloads and formats, include migration guidance for existing callers/flows
(notably User Invitation) and update any API/reference pages (e.g.,
docs/content/guides/... and docs/content/apis.mdx) to reflect the new keys
before merging.
In `@backend/internal/flow/executor/email_executor.go`:
- Around line 125-127: The forwarded-template path must validate the contract
and fail fast: in the branch using e.resolveForwardedTemplateScenario and when
reading ctx.NodeProperties[propertyKeyEmailTemplate], ensure that if a forwarded
structure is present you check both that the scenario string is non-empty and
that templateName and templateData exist and have the expected types (e.g.,
templateName is string, templateData is map/struct or expected type); if any of
these checks fail return an ExecFailure (or error) immediately rather than
silently continuing. Apply the same strict validation logic to the other
forwarded-template handling area referenced around the later branch (the block
covering the other forwarded-template processing) so both paths validate types
and presence of templateName/templateData and produce explicit ExecFailure with
a clear message when malformed.
In `@backend/internal/system/email/smtp_client.go`:
- Around line 45-47: The change tightens sender/recipient validation in
smtp_client.go (IsValidEmail used with config.from and related recipient checks
that now return ErrorInvalidSender) and requires updating user-facing
documentation; add or update docs under docs/content/guides (e.g., an email
configuration/usage guide) clarifying that SMTP sender/recipient must be plain
addr-spec (no display-name forms like "Name <a@b.com>") and that CR/LF or
display-name formatted addresses will be rejected, and mention the
ErrorInvalidSender behavior so callers know to sanitize payloads; ensure any
API/flow docs that describe email payload shapes are updated accordingly.
- Around line 45-47: The email validation uses IsValidEmail which trims
internally but leaves config.from unmodified, causing MAIL FROM failures for
values with surrounding whitespace; after the IsValidEmail check in the SMTP
client setup (the block referencing IsValidEmail and ErrorInvalidSender for
config.from) assign the trimmed value back to config.from (and apply the same
change to the analogous check referencing lines 59-61, e.g. for reply-to or
other sender fields) so the stored envelope sender is normalized before
returning/using it; use strings.TrimSpace to normalize the value immediately
after the validation succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9703c31a-0bbc-4d72-b0c4-ade09cff5c6b
📒 Files selected for processing (8)
backend/internal/flow/common/constants.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/system/email/smtp_client.gobackend/internal/system/email/smtp_client_test.gobackend/internal/system/email/validator.gobackend/internal/system/email/validator_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2485 +/- ##
==========================================
- Coverage 87.44% 87.43% -0.02%
==========================================
Files 889 889
Lines 62489 62499 +10
==========================================
Hits 54645 54645
- Misses 5899 5910 +11
+ Partials 1945 1944 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR generalizes email sending to support choosing a template scenario and supplying template parameters via forwarded data, and centralizes email address validation (including CR/LF injection protection).
Changes:
- Add
IsValidEmailhelper + unit tests and switch SMTP sender/recipient validation to use it. - Introduce forwarded data contract keys (
templateName,templateData) and populate them in the invite executor. - Update email executor to resolve template scenario/data from
ForwardedData, and adjust executor unit tests accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/internal/system/email/validator.go | Adds IsValidEmail with CR/LF rejection and strict parsing. |
| backend/internal/system/email/validator_test.go | Adds unit tests for IsValidEmail valid/invalid and injection cases. |
| backend/internal/system/email/smtp_client.go | Replaces net/mail parsing with IsValidEmail; refactors error var naming; adds docs. |
| backend/internal/system/email/smtp_client_test.go | Refactors variable naming (error → err, config → conf) for clarity/consistency. |
| backend/internal/flow/common/constants.go | Adds forwarded data keys for template scenario and template data. |
| backend/internal/flow/executor/invite_executor.go | Populates forwarded templateName and templateData for invite flows. |
| backend/internal/flow/executor/email_executor.go | Resolves template scenario/data via forwarded data and removes inviteLink runtime enforcement. |
| backend/internal/flow/executor/email_executor_test.go | Updates expectations to use forwarded template contract and empty template data defaults. |
Contributor
Author
The email templating will not support the previous pattern of use, and need to have a templateName, templateData, in forwarded data to apply templates |
RandithaK
force-pushed
the
improvement/adaptive-email-template
branch
from
726414e to
8176c60
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/internal/flow/executor/invite_executor.go (1)
113-117: Prefer explicit flow-type mapping for template scenario selection.Current
elsemaps every non-registration flow toUSER_INVITE. Aswitchwith explicit supported flow types plus a guarded default avoids silent misclassification when new flow types are introduced.Refactor sketch
- if ctx.FlowType == common.FlowTypeRegistration { - execResp.ForwardedData[common.ForwardedDataKeyTemplateName] = string(template.ScenarioSelfRegistration) - } else { - execResp.ForwardedData[common.ForwardedDataKeyTemplateName] = string(template.ScenarioUserInvite) - } + switch ctx.FlowType { + case common.FlowTypeRegistration: + execResp.ForwardedData[common.ForwardedDataKeyTemplateName] = string(template.ScenarioSelfRegistration) + case common.FlowTypeUserOnboarding: + execResp.ForwardedData[common.ForwardedDataKeyTemplateName] = string(template.ScenarioUserInvite) + default: + execResp.ForwardedData[common.ForwardedDataKeyTemplateName] = string(template.ScenarioUserInvite) + logger.Debug("Unhandled flow type for invite template scenario", log.String("flowType", string(ctx.FlowType))) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/executor/invite_executor.go` around lines 113 - 117, Replace the loose if/else that sets execResp.ForwardedData[common.ForwardedDataKeyTemplateName] based on ctx.FlowType with an explicit switch over ctx.FlowType that handles supported flow types (e.g., common.FlowTypeRegistration -> template.ScenarioSelfRegistration, common.FlowTypeUserInvite -> template.ScenarioUserInvite) and includes a default branch that returns an error or logs and fails the execution to avoid silent misclassification; update any callers to handle the new error path if necessary and keep references to execResp.ForwardedData[common.ForwardedDataKeyTemplateName], ctx.FlowType, template.ScenarioSelfRegistration and template.ScenarioUserInvite to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/system/email/validator.go`:
- Around line 26-44: The change to IsValidEmail tightens accepted formats (it
now rejects CR/LF control characters and requires the parsed address to exactly
match the input, thus rejecting display-name forms like "Name
<user@example.com>"); add documentation describing these email validation
constraints by creating/updating a guide under docs/content/guides/ (e.g.,
"email-sending-constraints.md") that lists the new rules (no CR/LF, no
display-name formats, examples of accepted/rejected values) and update
docs/content/apis.mdx to reference this guide where email request payloads or
sender/recipient behavior is documented so users are aware of the behavior
change.
---
Nitpick comments:
In `@backend/internal/flow/executor/invite_executor.go`:
- Around line 113-117: Replace the loose if/else that sets
execResp.ForwardedData[common.ForwardedDataKeyTemplateName] based on
ctx.FlowType with an explicit switch over ctx.FlowType that handles supported
flow types (e.g., common.FlowTypeRegistration ->
template.ScenarioSelfRegistration, common.FlowTypeUserInvite ->
template.ScenarioUserInvite) and includes a default branch that returns an error
or logs and fails the execution to avoid silent misclassification; update any
callers to handle the new error path if necessary and keep references to
execResp.ForwardedData[common.ForwardedDataKeyTemplateName], ctx.FlowType,
template.ScenarioSelfRegistration and template.ScenarioUserInvite to locate the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 059481b9-5c8e-4ffa-8500-dd06999c5e82
📒 Files selected for processing (8)
backend/internal/flow/common/constants.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/system/email/smtp_client.gobackend/internal/system/email/smtp_client_test.gobackend/internal/system/email/validator.gobackend/internal/system/email/validator_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/internal/flow/executor/email_executor.go
- backend/internal/flow/executor/email_executor_test.go
- backend/internal/system/email/smtp_client.go
Contributor
Author
|
Manually tested and ready |
RandithaK
force-pushed
the
improvement/adaptive-email-template
branch
2 times, most recently
from
f49ce25 to
0fe3cf6
Compare
RandithaK
force-pushed
the
improvement/adaptive-email-template
branch
2 times, most recently
from
5a8c61f to
884589f
Compare
ThaminduDilshan
previously approved these changes
Apr 29, 2026
Signed-off-by: RandithaK <me@randitha.net>
RandithaK
force-pushed
the
improvement/adaptive-email-template
branch
from
884589f to
0a95155
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
backend/internal/system/email/smtp_client.go (1)
42-47:⚠️ Potential issue | 🟠 Major🔴 Documentation Required for stricter SMTP address format behavior.
At Line 42 and Line 135, validation now enforces stricter addr-spec inputs (e.g., rejecting display-name formats like
Name <a@b.com>). This changes operator/runtime behavior for SMTP sender and recipient values, but no matching docs update is included in this PR context.Please update docs under
docs/content/guides/(email/SMTP usage) and any related flow docs to explicitly document accepted sender/recipient formats and rejection behavior.As per coding guidelines: if a user-facing change is introduced without corresponding updates under
docs/, it must be flagged as a major issue with a documentation-required note.Also applies to: 135-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/system/email/smtp_client.go` around lines 42 - 47, The PR tightens SMTP addr-spec validation in smtp_client.go by rejecting display-name formats (e.g., "Name <a@b.com>") when building the sender (variable sender and IsValidEmail usage) and similarly for recipients around the recipient validation block; update documentation under docs/content/guides/ (email/SMTP usage) and any related flow docs to clearly state the accepted formats (pure addr-spec only, no display-name or angle-bracketed forms), list examples of accepted and rejected inputs, and note the runtime rejection behavior so operators know to adjust configuration/inputs accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/internal/system/email/smtp_client.go`:
- Around line 42-47: The PR tightens SMTP addr-spec validation in smtp_client.go
by rejecting display-name formats (e.g., "Name <a@b.com>") when building the
sender (variable sender and IsValidEmail usage) and similarly for recipients
around the recipient validation block; update documentation under
docs/content/guides/ (email/SMTP usage) and any related flow docs to clearly
state the accepted formats (pure addr-spec only, no display-name or
angle-bracketed forms), list examples of accepted and rejected inputs, and note
the runtime rejection behavior so operators know to adjust configuration/inputs
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5bab9ddd-d7f9-4f07-883e-e7578d17ca08
📒 Files selected for processing (9)
backend/internal/flow/common/constants.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/flow/executor/invite_executor_test.gobackend/internal/system/email/smtp_client.gobackend/internal/system/email/smtp_client_test.gobackend/internal/system/email/validator.gobackend/internal/system/email/validator_test.go
✅ Files skipped from review due to trivial changes (1)
- backend/internal/system/email/smtp_client_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/internal/system/email/validator.go
- backend/internal/flow/executor/email_executor.go
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.
Purpose
The email sending capability has been generalised to support templating without code changes.
Now the email system accepts template name and data and applies as needed
Approach
Make the email system templating easier to scale with diverging use-cases, using forwarded data.
The caller should use the following pattern.
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
Tests