fix(auth): resolve session alert localization for non-English locales#11914
fix(auth): resolve session alert localization for non-English locales#11914jbaoho wants to merge 1 commit intoappwrite:1.9.xfrom
Conversation
1e92043 to
8dc4afd
Compare
Greptile SummaryFixes session alert emails rendering raw Confidence Score: 4/5Safe to merge for the core fix; two P2 issues in the helper's fallback logic and test timing are worth addressing but do not block the primary regression. The src/Appwrite/Bus/Listeners/Mails.php — Important Files Changed
Reviews (1): Last reviewed commit: "fix(auth): resolve session alert localiz..." | Re-trigger Greptile |
| $translate = static function (string $key) use ($locale): string { | ||
| $value = $locale->getText($key); | ||
|
|
||
| if (\preg_match('/^\{\{(.+)\}\}$/', $value, $matches) === 1) { | ||
| return $locale->getText($matches[1], $value); | ||
| } | ||
|
|
||
| return $value; | ||
| }; |
There was a problem hiding this comment.
$translate fallback is a no-op for self-references
When a locale file contains a self-referencing placeholder (e.g., "emails.sessionAlert.subject": "{{emails.sessionAlert.subject}}") — which can happen when Crowdin generates untranslated stubs — the function resolves $matches[1] to the same key it just failed to find. The second getText($matches[1], $value) call looks up the identical key, still finds nothing, and returns $value unchanged, so the raw placeholder leaks into the email.
At minimum, the greedy .+ in the regex should be changed to [^{}]+ to avoid mis-capturing values with multiple {{}} groups (e.g., {{a}}{{b}}):
| $translate = static function (string $key) use ($locale): string { | |
| $value = $locale->getText($key); | |
| if (\preg_match('/^\{\{(.+)\}\}$/', $value, $matches) === 1) { | |
| return $locale->getText($matches[1], $value); | |
| } | |
| return $value; | |
| }; | |
| if (\preg_match('/^\{\{([^{}]+)\}\}$/', $value, $matches) === 1) { |
|
|
||
| $this->assertEquals(201, $response['headers']['status-code']); | ||
|
|
||
| $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', $headers, [ | ||
| 'email' => $email, | ||
| 'password' => $password, | ||
| ]); | ||
|
|
||
| $this->assertEquals(201, $response['headers']['status-code']); | ||
|
|
There was a problem hiding this comment.
getLastEmailByAddress is called immediately after the second session creation with no wait or retry. If the Mails Bus listener is delayed (e.g., under load or queue backpressure), $lastEmail could be null or point to an earlier email, causing the assertStringContainsString assertions to fail spuriously. Consider using a poll/retry helper if one exists in the test framework.
Problem
Session alert emails render raw
{{emails.sessionAlert.*}}placeholders when a non-English locale is used.Solution
sendSessionAlert().X-Appwrite-Localeheader.Testing
docker compose exec appwrite test tests/e2e/Services/Account --filter=testSessionAlertWithLocaleHeader.Fixes #8659