Skip to content

Conversation

@hmacr
Copy link
Contributor

@hmacr hmacr commented Oct 28, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

📝 Walkthrough

Walkthrough

This pull request introduces conditional email verification template selection and updates translation keys. New project-specific translation keys (emails.verification.preview-appwrite and emails.verification.heading-appwrite) are added to the English locale file, while existing keys are updated with revised wording. The account controller is modified to conditionally select these templates based on whether the project name is 'Appwrite', ensuring branded verification emails use project-specific messaging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Translation key additions and updates are straightforward replacements following a consistent naming pattern
  • Conditional template selection logic is a simple if/else statement with clear intent
  • Minimal behavioral impact beyond template routing

Possibly related PRs

Suggested reviewers

  • Meldiron

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Customize email preview and heading between Console and projects" accurately reflects the main changes in the changeset. The modifications add conditional template selection logic based on project name, creating different email verification text for Appwrite Console versus regular projects. The title is specific, clear, and directly conveys the primary change without unnecessary verbosity or vague terms.
Description Check ✅ Passed The PR description provides a reference to a specific task that directly relates to the changeset. The task ID "SER-436" appears in the source branch name and the task title explicitly references using project name instead of Appwrite in verification email, which aligns with the PR's objective. While minimal, the description is not completely off-topic and provides relevant context through the task link, meeting the lenient criteria for this check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ser-436

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.44-r2 CVE-2025-5244 HIGH
binutils 2.44-r2 CVE-2025-5245 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
pcre2 10.43-r1 CVE-2025-58050 CRITICAL
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2a260c3 and c2779de.

📒 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 -appwrite suffix 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.

Comment on lines +3639 to +3640
$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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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
done

Length 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 -20

Length 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 | uniq

Length of output: 642


🏁 Script executed:

#!/bin/bash
# Search for getText method across entire repo
rg -n "getText" --type=php -B 2 -A 5 | head -50

Length 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.

@github-actions
Copy link

✨ Benchmark results

  • Requests per second: 1,221
  • Requests with 200 status code: 219,863
  • P99 latency: 0.160403938

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,221 1,253
200 219,863 225,657
P99 0.160403938 0.164212441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants