chore: use env var to config email address#742
Conversation
revu-bot
left a comment
revu-bot
left a comment
There was a problem hiding this comment.
Summary
This PR refactors hardcoded email configuration values into environment variables, improving configurability across different environments. The changes are straightforward and follow good practices for externalizing configuration.
Key Observations
| Aspect | Status | Notes |
|---|---|---|
| Configuration externalization | ✅ Good | Email addresses now configurable via env vars |
| Backward compatibility | ✅ Good | Default values provided as fallbacks |
| Security | Email addresses exposed in .env.example |
Issues Found
- IMPORTANT: Inconsistent email domain between default values
- MINOR: Missing newline at end of .env.example file
The overall implementation is solid, but there's a critical inconsistency in the default email domain that needs attention.
| export const MAIL_DEFAULT_EMAIL = process.env.MAIL_DEFAULT_EMAIL || "jardinmental@assurance-maladie.fr"; | ||
| export const MAIL_DEFAULT_NAME = process.env.MAIL_DEFAULT_NAME || "Jardin Mental - Application"; |
There was a problem hiding this comment.
[IMPORTANT] Inconsistent email domain in defaults
Why it matters:
- The default values use
@assurance-maladie.frdomain - The original hardcoded values throughout the codebase used
@fabrique.social.gouv.frdomain - This changes the default behavior when env vars are not set, potentially breaking existing deployments
- Emails may be sent to/from the wrong domain in environments that don't explicitly set these variables
Proposed change:
Either:
- Keep the original domain as the default for backward compatibility, OR
- Document this breaking change clearly and ensure all environments set these variables explicitly
If option 1 (recommended for safety):
| MAIL_DEFAULT_EMAIL=jardinmental@assurance-maladie.fr | ||
| MAIL_DEFAULT_NAME=Jardin Mental - Application No newline at end of file |
There was a problem hiding this comment.
[MINOR] Missing newline at end of file
Why it matters:
- POSIX standard requires text files to end with a newline
- Can cause issues with some tools and version control systems
- Git will show "No newline at end of file" warning
Proposed change:
Add a newline character after the last line.
| MAIL_DEFAULT_EMAIL=jardinmental@assurance-maladie.fr | ||
| MAIL_DEFAULT_NAME=Jardin Mental - Application No newline at end of file |
There was a problem hiding this comment.
[IMPORTANT] Verify intended email domain change
Why it matters:
- The example shows
@assurance-maladie.frdomain - Previous hardcoded values used
@fabrique.social.gouv.frdomain - This appears to be an intentional domain migration, but should be verified
Action required:
Confirm this domain change is intentional and update documentation if this represents a migration from one email provider/domain to another. If unintentional, align with the original domain.
|
2 participants
No description provided.