-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Customize email preview and heading between Console and projects #10723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1.8.x
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces conditional email verification template selection and updates translation keys. New project-specific translation keys ( Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/config/locale/translations/en.json (1)
9-9: Consider grammar consistency in translation string.The phrase "Verify your email for {{project}} account." appears to be missing the article "your" before "account" for consistency with line 7's "your {{project}} account."
Should this be either:
- "Verify your email for your {{project}} account." (matches line 7's structure), or
- "Verify your email for {{project}}" (matches line 10's simpler structure)?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/config/locale/translations/en.json(1 hunks)app/controllers/api/account.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (1)
app/config/locale/translations/en.json (1)
7-10: Good approach to separating Appwrite-branded vs generic messaging.The use of separate translation keys with the
-appwritesuffix allows for different messaging between Appwrite Cloud and other projects. The Appwrite-specific versions use more action-oriented language ("activate", "start using") which is appropriate for the branded experience.
| $preview = $projectName === 'Appwrite' ? $locale->getText("emails.verification.preview-appwrite") : $locale->getText("emails.verification.preview"); | ||
| $heading = $projectName === 'Appwrite' ? $locale->getText("emails.verification.heading-appwrite") : $locale->getText("emails.verification.heading"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify case-sensitivity and translation key availability across locales.
The case-sensitive comparison (===) means only the exact string 'Appwrite' will trigger the alternate template. If the project name could be 'appwrite', 'APPWRITE', or any other case variation, those won't match.
Additionally, this PR only adds the new translation keys to en.json. If the application supports multiple locales, you'll need to add emails.verification.preview-appwrite, emails.verification.heading-appwrite, emails.verification.preview, and emails.verification.heading to all other locale files, otherwise calls to $locale->getText() may fail or fall back to displaying the key names.
Run the following script to check for other locale files that may need updating:
🏁 Script executed:
#!/bin/bash
# Find all locale translation files
fd -e json . app/config/locale/translations
# Check if the new keys exist in other locale files
for file in $(fd -e json . app/config/locale/translations); do
echo "Checking $file"
if ! grep -q "emails.verification.preview-appwrite" "$file"; then
echo " Missing: emails.verification.preview-appwrite"
fi
if ! grep -q "emails.verification.heading-appwrite" "$file"; then
echo " Missing: emails.verification.heading-appwrite"
fi
doneLength of output: 13292
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find the getText method implementation
rg -n "function getText|public.*getText" --type=php -A 5 app/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if base keys exist in a sample non-English locale (French)
echo "=== Checking fr.json for base keys ==="
grep -E "emails\.verification\.(preview|heading)" app/config/locale/translations/fr.json | head -20Length of output: 38
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check what verification-related keys exist in en.json
echo "=== Keys in en.json with 'verification' ==="
grep -o '"emails\.verification[^"]*"' app/config/locale/translations/en.json | sort | uniq
echo ""
echo "=== Keys in fr.json with 'verification' ==="
grep -o '"emails\.verification[^"]*"' app/config/locale/translations/fr.json | sort | uniqLength of output: 642
🏁 Script executed:
#!/bin/bash
# Search for getText method across entire repo
rg -n "getText" --type=php -B 2 -A 5 | head -50Length of output: 4600
Add missing translation keys to all non-English locale files.
The issue is broader than initially assessed. Your code references four translation keys that are all missing from the 75 non-English locale files:
emails.verification.preview(base key)emails.verification.heading(base key)emails.verification.preview-appwrite(new key)emails.verification.heading-appwrite(new key)
While en.json contains all four keys, fr.json (and all other 74 locale files) are missing all of them. This means calls to $locale->getText() will fail or display untranslated key names for any non-English user, breaking both the Appwrite-branded and generic email templates for international users.
You must add all four keys to every locale file in app/config/locale/translations/ to ensure consistent behavior across all supported languages.
🤖 Prompt for AI Agents
In app/controllers/api/account.php around lines 3639-3640 the code references
four translation keys that exist only in en.json; add the keys
"emails.verification.preview", "emails.verification.heading",
"emails.verification.preview-appwrite" and
"emails.verification.heading-appwrite" to every non-English locale file under
app/config/locale/translations/ (all 75 files), populating them either with the
corresponding English strings from en.json as placeholders or with proper
translations, and ensure JSON syntax/encoding is valid in each file so
$locale->getText() returns the correct string for all languages.
✨ Benchmark results
⚡ Benchmark Comparison
|
Task: https://linear.app/appwrite/issue/SER-436/use-project-name-instead-of-appwrite-in-verification-email