Fix : custom email templates add translation support magic session keys#11235
Conversation
…nslation-support-magic-session-keys
📝 WalkthroughWalkthroughThe changes introduce email template translation and retrieval enhancements across multiple endpoints. Three new translation entries are added for magicsession email templates. Helper functions for custom template retrieval ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes ReasoningThe review demands attention to multiple distinct contexts: global helper functions in controllers, protected methods in a class-based module, and translation logic with placeholder resolution patterns. While the helper functions repeat across files, each implementation context requires separate reasoning. The translation logic is non-trivial, involving placeholder detection, key normalization with field remapping for magicsession variants, and fallback behavior. The changes span five files with heterogeneous purposes, though some patterns are consistent. Total line additions (~311) are substantial, and the logic density in template translation handling increases complexity beyond simple refactoring. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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. Comment |
commit: |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/controllers/api/teams.php`:
- Around line 879-882: The custom SMS template returned by
getCustomTemplate(...) may lack a 'message' key which can make $message null and
later break setParam; update the block that reads $customTemplate to check
isset($customTemplate['message']) or array_key_exists('message',
$customTemplate) before assigning $message, and if missing leave $message as the
base template value (or explicitly fall back to the base template variable) so
setParam always receives a valid string.
In
`@src/Appwrite/Platform/Modules/Account/Http/Account/MFA/Challenges/Create.php`:
- Around line 253-256: The code currently replaces the Template object in
$message with a string when a custom SMS template exists, causing fatal errors
at later $message->setParam() calls; instead, keep $message as the Template
instance and inject the custom text into it (e.g. call
$message->setParam('message', $customTemplate['message']) or use the Template
object's appropriate setter) after verifying $customTemplate is non-empty, so
you never reassign $message to a string; update the block around
getCustomTemplate(...) to set the template text on the Template object rather
than overwrite $message.
| $customTemplate = getCustomTemplate($project, 'invitation', $locale, 'sms'); | ||
| if (!empty($customTemplate)) { | ||
| $message = $customTemplate['message']; | ||
| } |
There was a problem hiding this comment.
Guard against missing SMS template message key.
If a custom SMS template lacks message, $message becomes null and setParam will fatal; fall back to the base template.
🔧 Suggested fix
- $message = $customTemplate['message'];
+ $message = $customTemplate['message'] ?? $message;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $customTemplate = getCustomTemplate($project, 'invitation', $locale, 'sms'); | |
| if (!empty($customTemplate)) { | |
| $message = $customTemplate['message']; | |
| } | |
| $customTemplate = getCustomTemplate($project, 'invitation', $locale, 'sms'); | |
| if (!empty($customTemplate)) { | |
| $message = $customTemplate['message'] ?? $message; | |
| } |
🤖 Prompt for AI Agents
In `@app/controllers/api/teams.php` around lines 879 - 882, The custom SMS
template returned by getCustomTemplate(...) may lack a 'message' key which can
make $message null and later break setParam; update the block that reads
$customTemplate to check isset($customTemplate['message']) or
array_key_exists('message', $customTemplate) before assigning $message, and if
missing leave $message as the base template value (or explicitly fall back to
the base template variable) so setParam always receives a valid string.
| $customTemplate = $this->getCustomTemplate($project, 'mfaChallenge', $locale, 'sms'); | ||
| if (!empty($customTemplate)) { | ||
| $message = $customTemplate['message'] ?? $message; | ||
| } |
There was a problem hiding this comment.
Custom SMS template replaces a Template object with a string.
Later calls to $message->setParam() will fatally error when a custom template is present.
🐛 Proposed fix
- $customTemplate = $this->getCustomTemplate($project, 'mfaChallenge', $locale, 'sms');
- if (!empty($customTemplate)) {
- $message = $customTemplate['message'] ?? $message;
- }
+ $customTemplate = $this->getCustomTemplate($project, 'mfaChallenge', $locale, 'sms');
+ if (!empty($customTemplate['message'])) {
+ $message = Template::fromString($customTemplate['message']);
+ }🤖 Prompt for AI Agents
In `@src/Appwrite/Platform/Modules/Account/Http/Account/MFA/Challenges/Create.php`
around lines 253 - 256, The code currently replaces the Template object in
$message with a string when a custom SMS template exists, causing fatal errors
at later $message->setParam() calls; instead, keep $message as the Template
instance and inject the custom text into it (e.g. call
$message->setParam('message', $customTemplate['message']) or use the Template
object's appropriate setter) after verifying $customTemplate is non-empty, so
you never reassign $message to a string; update the block around
getCustomTemplate(...) to set the template text on the Template object rather
than overwrite $message.
✨ Benchmark results
⚡ Benchmark Comparison
|
…nslation-support-magic-session-keys
| $oauthDefaultFailure = '/console/auth/oauth2/failure'; | ||
|
|
||
| if (!function_exists('getCustomTemplate')) { | ||
| function getCustomTemplate(Document $project, string $type, Locale $locale, string $prefix = 'email'): array |
There was a problem hiding this comment.
Avoid need for global function by fixing root cause - finding what write with lowercase instead of properCase
| "emails.magicSession.securityPhrase": "Security phrase for this email is {{b}}{{phrase}}{{/b}}. You can trust this email if this phrase matches the phrase shown during sign in.", | ||
| "emails.magicSession.thanks": "Thanks,", | ||
| "emails.magicSession.signature": "{{project}} team", | ||
| "emails.magicsession.body": "Click the button below to securely sign in to your {{b}}{{project}}{{/b}} account. This link will expire in 1 hour.", |
There was a problem hiding this comment.
Let's update tests too - add custom smtp translation scenrio that was a bug, and ensure it is now working
2 participants
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist