diff --git a/AGENTS.md b/AGENTS.md index 54eeab672a..6f41460d08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -563,13 +563,22 @@ enforces that). from the picked language — for a guest that is the *only* source of direction, since the middleware renders `htmlDir` from the default on every request. - **Register submits `locale` as a required hidden field** and creates the user - with it. **Login submits it only once the visitor picks a language** — the - field goes out empty otherwise, and an empty value leaves `users.locale` + with it: the language the visitor picked, else the browser's language + (`detectBrowserLanguage` over the supported codes, so `pt-PT` becomes `pt-BR`), + else `Locale::DEFAULT` (`signupLocale` in `useGuestLocale`). The page still + renders in the default until the visitor picks; only the account gets the + browser language. Lifecycle emails pick their language from it, so a signup + must not start in English and switch minutes later. + **Login submits it only once the visitor picks a language** — the field goes + out empty otherwise, and an empty value leaves `users.locale` untouched. This asymmetry is load-bearing: the login screen always renders in `Locale::DEFAULT`, so an always-sent field would reset every non-English user to English on each login. Forgot and reset password do not send it at all. -- Google and GitHub signups store `Locale::DEFAULT`: they have no picker, and the - OAuth callback tells you nothing reliable about the person. +- Google and GitHub signups send the same `signupLocale` as a query param on the + start route, kept by `PreservesSignupPreferences` with the zone and clock (the + OAuth callback itself tells you nothing reliable about the person). `CreateUser` + falls back to `Locale::DEFAULT` for a missing or unsupported value. A returning + user logging in through OAuth keeps their stored locale. ## User preferences (`/settings/preferences`) @@ -632,8 +641,9 @@ rolls the switch back and toasts. `WeekStart::firstDay()/lastDay()` to Carbon. The weekly posting goal (`CountPostsSentThisWeek`) and the Insights weekly buckets (`PeriodBuckets`) follow the **viewing** user's `week_starts_on`, never Monday or the UI language. -- **Signup detection:** email, Google and GitHub signups store the browser zone, - the week start (`Intl.Locale` week info, else the Sunday-first zone list in +- **Signup detection:** email, Google and GitHub signups store the browser + language (see "UI locale"), the browser zone, the week start (`Intl.Locale` + week info, else the Sunday-first zone list in `resources/js/lib/detectPreferences.ts`) and the hour cycle. OAuth sends them as query params on the start route and `PreservesSignupPreferences` keeps them in the session; `CreateUser` validates each one. Fallbacks: UTC, Monday, the diff --git a/app/Actions/User/CreateUser.php b/app/Actions/User/CreateUser.php index fd00cbb122..61e85fc0ed 100644 --- a/app/Actions/User/CreateUser.php +++ b/app/Actions/User/CreateUser.php @@ -22,7 +22,7 @@ class CreateUser { /** - * @param array{name: string, email: string, password?: string, google_id?: string, github_id?: string, email_verified_at?: \DateTimeInterface|null, is_invite?: bool, registration_ip?: string|null, locale?: string, timezone?: string|null, week_starts_on?: string|null, time_format?: string|null} $data + * @param array{name: string, email: string, password?: string, google_id?: string, github_id?: string, email_verified_at?: \DateTimeInterface|null, is_invite?: bool, registration_ip?: string|null, locale?: string|null, timezone?: string|null, week_starts_on?: string|null, time_format?: string|null} $data * @param array $attributionParameters UTM parameters and ad click IDs (gclid, fbclid, etc.) captured before signup */ public static function execute(array $data, array $attributionParameters = []): User @@ -47,7 +47,7 @@ public static function execute(array $data, array $attributionParameters = []): $account = Account::query()->forceCreate($accountAttributes); - $locale = Locale::from(data_get($data, 'locale', Locale::DEFAULT->value)); + $locale = Locale::tryFrom((string) data_get($data, 'locale')) ?? Locale::DEFAULT; $user = User::create(array_merge([ 'name' => data_get($data, 'name'), diff --git a/app/Http/Controllers/Auth/Concerns/PreservesSignupPreferences.php b/app/Http/Controllers/Auth/Concerns/PreservesSignupPreferences.php index f1dfbaff79..5be065a92a 100644 --- a/app/Http/Controllers/Auth/Concerns/PreservesSignupPreferences.php +++ b/app/Http/Controllers/Auth/Concerns/PreservesSignupPreferences.php @@ -11,14 +11,14 @@ trait PreservesSignupPreferences { private function storeSignupPreferences(Request $request): void { - $request->session()->put('signup_preferences', collect($request->only(['timezone', 'week_starts_on', 'time_format'])) + $request->session()->put('signup_preferences', collect($request->only(['locale', 'timezone', 'week_starts_on', 'time_format'])) ->filter(fn (mixed $value): bool => is_string($value) && $value !== '') ->map(fn (string $value): string => Str::limit($value, 64, '')) ->all()); } /** - * @return array{timezone?: string, week_starts_on?: string, time_format?: string} + * @return array{locale?: string, timezone?: string, week_starts_on?: string, time_format?: string} */ private function retrieveSignupPreferences(): array { diff --git a/app/Http/Controllers/Auth/GitHubController.php b/app/Http/Controllers/Auth/GitHubController.php index d6a2a4b916..3810ae9722 100644 --- a/app/Http/Controllers/Auth/GitHubController.php +++ b/app/Http/Controllers/Auth/GitHubController.php @@ -124,6 +124,7 @@ private function registerNewUser(\Laravel\Socialite\Contracts\User $githubUser): 'email_verified_at' => now(), 'is_invite' => $invite !== null, 'registration_ip' => request()->ip(), + 'locale' => data_get($preferences, 'locale'), 'timezone' => data_get($preferences, 'timezone'), 'week_starts_on' => data_get($preferences, 'week_starts_on'), 'time_format' => data_get($preferences, 'time_format'), diff --git a/app/Http/Controllers/Auth/GoogleController.php b/app/Http/Controllers/Auth/GoogleController.php index f231390a19..9c3b157533 100644 --- a/app/Http/Controllers/Auth/GoogleController.php +++ b/app/Http/Controllers/Auth/GoogleController.php @@ -116,6 +116,7 @@ private function registerNewUser(\Laravel\Socialite\Contracts\User $googleUser): 'email_verified_at' => now(), 'is_invite' => $invite !== null, 'registration_ip' => request()->ip(), + 'locale' => data_get($preferences, 'locale'), 'timezone' => data_get($preferences, 'timezone'), 'week_starts_on' => data_get($preferences, 'week_starts_on'), 'time_format' => data_get($preferences, 'time_format'), diff --git a/resources/js/components/auth/SocialLogin.vue b/resources/js/components/auth/SocialLogin.vue index 80dfc266ae..76914dfa71 100644 --- a/resources/js/components/auth/SocialLogin.vue +++ b/resources/js/components/auth/SocialLogin.vue @@ -3,6 +3,7 @@ import { usePage } from '@inertiajs/vue3'; import { computed } from 'vue'; import { Button } from '@/components/ui/button'; +import { useGuestLocale } from '@/composables/useGuestLocale'; import { detectPreferences } from '@/lib/detectPreferences'; import { redirect as githubRedirect } from '@/routes/auth/github'; import { redirect as googleRedirect } from '@/routes/auth/google'; @@ -22,9 +23,11 @@ const githubEnabled = computed(() => Boolean(page.props.githubAuthEnabled)); const hasSocial = computed(() => googleEnabled.value || githubEnabled.value); const detected = detectPreferences(); +const { signupLocale } = useGuestLocale(); const query = computed(() => { const params: Record = { + locale: signupLocale.value, timezone: detected.timezone, week_starts_on: detected.week_starts_on, }; diff --git a/resources/js/composables/useGuestLocale.ts b/resources/js/composables/useGuestLocale.ts index 7fa4f4f6c6..27db3dbe66 100644 --- a/resources/js/composables/useGuestLocale.ts +++ b/resources/js/composables/useGuestLocale.ts @@ -2,6 +2,7 @@ import { usePage } from '@inertiajs/vue3'; import { computed } from 'vue'; import { chooseGuestLocale, guestLocale } from '@/language'; +import { detectBrowserLanguage } from '@/lib/detectPreferences'; import type { Language } from '@/types'; export const useGuestLocale = () => { @@ -11,8 +12,14 @@ export const useGuestLocale = () => { () => page.props.languages as Language[], ); + const pageLocale = (): string => page.props.locale as string; + + const browserLocale = computed(() => + detectBrowserLanguage(languages.value.map((language) => language.code)), + ); + const locale = computed({ - get: () => guestLocale() ?? (page.props.locale as string), + get: () => guestLocale() ?? pageLocale(), set: (value) => { const language = languages.value.find( (candidate) => candidate.code === value, @@ -24,5 +31,9 @@ export const useGuestLocale = () => { }, }); - return { locale, chosen: computed(guestLocale), languages }; + const signupLocale = computed( + () => guestLocale() ?? browserLocale.value ?? pageLocale(), + ); + + return { locale, signupLocale, chosen: computed(guestLocale), languages }; }; diff --git a/resources/js/pages/auth/Register.vue b/resources/js/pages/auth/Register.vue index 8ba59cc5c8..aa1751c3ec 100644 --- a/resources/js/pages/auth/Register.vue +++ b/resources/js/pages/auth/Register.vue @@ -31,7 +31,7 @@ defineProps<{ passwordRequirements: PasswordRequirement[]; }>(); -const { locale } = useGuestLocale(); +const { signupLocale } = useGuestLocale(); const detected = detectPreferences(); const showPassword = ref(false); @@ -68,7 +68,7 @@ const hasSocial = computed( class="flex flex-col gap-6" > - + @@ -163,7 +163,7 @@ const hasSocial = computed( class="w-full" :tabindex="4" :disabled="processing" - data-test="register-user-button" + data-testid="register-user-button" > {{ $t('auth.register.submit') }} diff --git a/tests/Browser/SignupPreferencesTest.php b/tests/Browser/SignupPreferencesTest.php index ea9a39e1c2..42c931bcfe 100644 --- a/tests/Browser/SignupPreferencesTest.php +++ b/tests/Browser/SignupPreferencesTest.php @@ -2,33 +2,25 @@ declare(strict_types=1); -function waitForSignupPreferencesTestId(mixed $page, string $testId): void -{ - $page->script(<< { - for (let attempt = 0; attempt < 100; attempt++) { - if (document.querySelector('[data-testid="{$testId}"]')) return; - await new Promise((resolve) => setTimeout(resolve, 50)); - } - })(); - JS); -} - -function signupPreferencesValue(mixed $page, string $testId): string -{ - return (string) $page->script("document.querySelector('[data-testid=\"{$testId}\"]').value"); -} +use App\Enums\User\Locale; +use App\Models\Account; +use App\Models\Invite; +use App\Models\User; +use App\Models\Workspace; +use Illuminate\Support\Uri; +use Laravel\Socialite\Facades\Socialite; +use Laravel\Socialite\Two\User as SocialiteUser; test('the register form sends the zone, week start and clock the browser reports', function (string $locale, string $timezone, string $weekStart, string $timeFormat) { config()->set('trypost.self_hosted', false); $page = visit(route('register'))->withLocale($locale)->withTimezone($timezone); - waitForSignupPreferencesTestId($page, 'register-time-format'); + waitForState($page, '@register-time-format', "element.value !== ''"); - expect(signupPreferencesValue($page, 'register-timezone'))->toBe($timezone) - ->and(signupPreferencesValue($page, 'register-week-start'))->toBe($weekStart) - ->and(signupPreferencesValue($page, 'register-time-format'))->toBe($timeFormat); - $page->assertNoJavaScriptErrors(); + $page->assertValue('@register-timezone', $timezone) + ->assertValue('@register-week-start', $weekStart) + ->assertValue('@register-time-format', $timeFormat) + ->assertNoJavaScriptErrors(); })->with([ 'United States' => ['en-US', 'America/New_York', 'sunday', '12h'], 'Germany' => ['de-DE', 'Europe/Berlin', 'monday', '24h'], @@ -39,13 +31,157 @@ function signupPreferencesValue(mixed $page, string $testId): string config()->set('trypost.github_auth_enabled', true); $page = visit(route('login'))->withLocale('en-US')->withTimezone('Asia/Tokyo'); - waitForSignupPreferencesTestId($page, 'social-login-github'); + waitFor($page, '@social-login-github'); + + foreach (['@social-login-google', '@social-login-github'] as $button) { + expect(Uri::of($page->attribute($button, 'href'))->query()->all()) + ->toMatchArray(['locale' => 'en', 'timezone' => 'Asia/Tokyo', 'week_starts_on' => 'sunday', 'time_format' => '12h']); + } + + $page->assertNoJavaScriptErrors(); +}); + +test('the register form signs up in the browser language when the visitor picks none', function (string $browserLocale, string $expected) { + config()->set('trypost.self_hosted', false); + + $page = visit(route('register'))->withLocale($browserLocale); + waitForState($page, '@register-locale', "element.value !== ''"); + + $page->assertValue('@register-locale', $expected) + ->assertNoJavaScriptErrors(); +})->with([ + 'Brazil' => ['pt-BR', 'pt-BR'], + 'Portugal, same base language' => ['pt-PT', 'pt-BR'], + 'Spain' => ['es-ES', 'es'], + 'unsupported falls back to the default' => ['sv-SE', 'en'], +]); + +test('the google and github buttons carry the browser language to the OAuth start', function () { + config()->set('trypost.self_hosted', false); + config()->set('trypost.google_auth_enabled', true); + config()->set('trypost.github_auth_enabled', true); + + $page = visit(route('register'))->withLocale('es-ES'); + waitFor($page, '@social-login-github'); + + foreach (['@social-login-google', '@social-login-github'] as $button) { + expect(Uri::of($page->attribute($button, 'href'))->query()->get('locale'))->toBe('es'); + } + + $page->assertNoJavaScriptErrors(); +}); + +test('a language picked on the register page wins over the browser language', function () { + config()->set('trypost.self_hosted', false); + config()->set('trypost.google_auth_enabled', true); + config()->set('trypost.github_auth_enabled', true); + + $page = visit(route('register'))->withLocale('es-ES'); + waitFor($page, '@language-picker-trigger'); + + $page->click('@language-picker-trigger'); + waitFor($page, '@language-picker-option-pt-BR'); + $page->click('@language-picker-option-pt-BR'); + waitForState($page, '@register-locale', "element.value === 'pt-BR'"); + + $page->assertValue('@register-locale', 'pt-BR'); + + foreach (['@social-login-google', '@social-login-github'] as $button) { + expect(Uri::of($page->attribute($button, 'href'))->query()->get('locale'))->toBe('pt-BR'); + } + + $page->assertNoJavaScriptErrors(); +}); + +test('signing up with the email form creates the account in the browser language', function () { + config()->set('trypost.self_hosted', false); + fakePwnedPasswords(); + + $page = visit(route('register'))->withLocale('pt-BR'); + waitForState($page, '@register-locale', "element.value !== ''"); + + $page->fill('@register-name', 'Bia') + ->fill('@register-email', 'bia-browser@example.com') + ->fill('@register-password', strongPassword()) + ->click('@register-user-button'); + waitForPath($page, parse_url(route('app.welcome.persona'), PHP_URL_PATH)); + + expect(User::query()->where('email', 'bia-browser@example.com')->sole()->locale)->toBe(Locale::PortugueseBrazil); + $page->assertNoJavaScriptErrors(); +}); + +test('a new person who starts on the login page signs up with google or github in the browser language', function (string $network, string $driver) { + config()->set('trypost.self_hosted', false); + config()->set('trypost.google_auth_enabled', true); + config()->set('trypost.github_auth_enabled', true); + + $page = visit(route('login'))->withLocale('es-ES'); + waitFor($page, "@social-login-{$network}"); + $href = $page->attribute("@social-login-{$network}", 'href'); + $page->assertNoJavaScriptErrors(); + + $socialiteUser = new SocialiteUser; + $socialiteUser->id = "{$network}-from-login"; + $socialiteUser->name = 'New Person'; + $socialiteUser->email = "{$network}-from-login@example.com"; + Socialite::shouldReceive('driver')->with($driver)->andReturn($socialite = Mockery::mock()); + $socialite->shouldReceive('redirect')->andReturn(redirect('/')); + $socialite->shouldReceive('scopes')->andReturnSelf(); + $socialite->shouldReceive('user')->andReturn($socialiteUser); - foreach (['social-login-google', 'social-login-github'] as $button) { - $params = $page->script("Object.fromEntries(new URL(document.querySelector('[data-testid=\"{$button}\"]').href).searchParams)"); + $this->get($href); + $this->get(route("auth.{$network}.callback"))->assertRedirect(route('app.welcome', absolute: false)); - expect($params)->toMatchArray(['timezone' => 'Asia/Tokyo', 'week_starts_on' => 'sunday', 'time_format' => '12h']); + expect(User::query()->where('email', "{$network}-from-login@example.com")->sole()->locale)->toBe(Locale::Spanish); +})->with([ + 'google' => ['google', 'google-auth'], + 'github' => ['github', 'github'], +]); + +test('the register page opened from an invite sends the browser language with the invite', function () { + config()->set('trypost.google_auth_enabled', true); + config()->set('trypost.github_auth_enabled', true); + + $account = Account::factory()->create(); + $owner = User::factory()->create(['account_id' => $account->id]); + $account->update(['owner_id' => $owner->id]); + $workspace = Workspace::factory()->create(['account_id' => $account->id, 'user_id' => $owner->id]); + $invite = Invite::factory()->create([ + 'account_id' => $account->id, + 'invited_by' => $owner->id, + 'email' => 'invitee-browser@example.com', + 'workspaces' => [$workspace->id], + ]); + + $page = visit(route('register', ['invite' => $invite->id, 'email' => 'invitee-browser@example.com']))->withLocale('pt-BR'); + waitForState($page, '@register-locale', "element.value !== ''"); + + $page->assertValue('@register-locale', 'pt-BR'); + + foreach (['@social-login-google', '@social-login-github'] as $button) { + expect(Uri::of($page->attribute($button, 'href'))->query()->all()) + ->toMatchArray(['invite' => $invite->id, 'locale' => 'pt-BR']); } $page->assertNoJavaScriptErrors(); }); + +test('with several browser languages the first one TryPost supports wins', function (array $browserLanguages, string $expected) { + config()->set('trypost.self_hosted', false); + + $page = visit(route('login'))->withLocale('en-US'); + waitFor($page, '@login-sign-up-link'); + + $languages = json_encode($browserLanguages); + $page->script("Object.defineProperty(navigator, 'languages', { get: () => {$languages} })"); + $page->click('@login-sign-up-link'); + waitForState($page, '@register-locale', "element.value !== ''"); + + $page->assertValue('@register-locale', $expected) + ->assertNoJavaScriptErrors(); +})->with([ + 'English first, then Portuguese' => [['en-US', 'pt-BR'], 'en'], + 'unsupported first, then Portuguese' => [['sv-SE', 'pt-BR'], 'pt-BR'], + 'Portugal first, then Spanish' => [['pt-PT', 'es-ES'], 'pt-BR'], + 'none supported' => [['sv-SE', 'fi-FI'], 'en'], +]); diff --git a/tests/Feature/Auth/SignupPreferencesTest.php b/tests/Feature/Auth/SignupPreferencesTest.php index 8b6d463a2d..6ffde405b2 100644 --- a/tests/Feature/Auth/SignupPreferencesTest.php +++ b/tests/Feature/Auth/SignupPreferencesTest.php @@ -2,9 +2,15 @@ declare(strict_types=1); +use App\Enums\User\Locale; use App\Enums\User\TimeFormat; use App\Enums\User\WeekStart; +use App\Jobs\PostHog\SyncUser; +use App\Models\Account; +use App\Models\Invite; use App\Models\User; +use App\Models\Workspace; +use Illuminate\Support\Facades\Queue; use Laravel\Socialite\Facades\Socialite; use Laravel\Socialite\Two\User as SocialiteUser; @@ -110,3 +116,110 @@ function signupPreferencesSocialite(string $driver, string $id, string $email): expect(User::query()->where('email', 'returning@example.com')->sole()->timezone)->toBe('Europe/Lisbon'); }); + +test('email registration stores the language the register form sent and pushes it to PostHog', function () { + enablePostHog(); + Queue::fake(); + + $this->post(route('register.store'), [ + 'name' => 'Bia', + 'email' => 'bia-locale@example.com', + 'password' => strongPassword(), + 'locale' => 'pt-BR', + ])->assertSessionHasNoErrors(); + + $user = User::query()->where('email', 'bia-locale@example.com')->sole(); + + expect($user->locale)->toBe(Locale::PortugueseBrazil); + Queue::assertPushed(SyncUser::class, fn (SyncUser $job): bool => $job->userId === (string) $user->id); +}); + +test('social signup stores the language sent to the OAuth start and pushes it to PostHog', function (string $network, string $driver) { + enablePostHog(); + Queue::fake(); + + $this->get(route("auth.{$network}.redirect", ['locale' => 'pt-BR'])); + signupPreferencesSocialite($driver, "{$network}-locale", "{$network}-locale@example.com"); + + $this->get(route("auth.{$network}.callback"))->assertRedirect(route('app.welcome', absolute: false)); + + $user = User::query()->where('email', "{$network}-locale@example.com")->sole(); + + expect($user->locale)->toBe(Locale::PortugueseBrazil); + Queue::assertPushed(SyncUser::class, fn (SyncUser $job): bool => $job->userId === (string) $user->id); +})->with([ + 'google' => ['google', 'google-auth'], + 'github' => ['github', 'github'], +]); + +test('social signup with an unsupported or missing language falls back to the default', function (string $network, string $driver, array $query) { + $this->get(route("auth.{$network}.redirect", $query)); + signupPreferencesSocialite($driver, "{$network}-fallback", "{$network}-fallback@example.com"); + + $this->get(route("auth.{$network}.callback"))->assertRedirect(route('app.welcome', absolute: false)); + + expect(User::query()->where('email', "{$network}-fallback@example.com")->sole()->locale)->toBe(Locale::DEFAULT); +})->with([ + 'google' => ['google', 'google-auth'], + 'github' => ['github', 'github'], +])->with([ + 'unsupported' => [['locale' => 'sv']], + 'missing' => [[]], +]); + +test('an existing user logging in with google or github keeps the language they already have', function (string $network, string $driver, string $idColumn) { + User::factory()->create(['email' => "{$network}-returning-locale@example.com", $idColumn => "{$network}-returning-locale", 'locale' => Locale::Japanese]); + + $this->get(route("auth.{$network}.redirect", ['locale' => 'pt-BR'])); + signupPreferencesSocialite($driver, "{$network}-returning-locale", "{$network}-returning-locale@example.com"); + + $this->get(route("auth.{$network}.callback")); + + expect(User::query()->where('email', "{$network}-returning-locale@example.com")->sole()->locale)->toBe(Locale::Japanese); +})->with([ + 'google' => ['google', 'google-auth', 'google_id'], + 'github' => ['github', 'github', 'github_id'], +]); + +function signupPreferencesInvite(string $email): Invite +{ + $account = Account::factory()->create(); + $owner = User::factory()->create(['account_id' => $account->id]); + $account->update(['owner_id' => $owner->id]); + $workspace = Workspace::factory()->create(['account_id' => $account->id, 'user_id' => $owner->id]); + + return Invite::factory()->create([ + 'account_id' => $account->id, + 'invited_by' => $owner->id, + 'email' => $email, + 'workspaces' => [$workspace->id], + ]); +} + +test('an invited email registration stores the language the register form sent', function () { + $invite = signupPreferencesInvite('invited-locale@example.com'); + + $this->post(route('register.store'), [ + 'name' => 'Invitee', + 'email' => 'invited-locale@example.com', + 'password' => strongPassword(), + 'invite' => $invite->id, + 'locale' => 'pt-BR', + ])->assertRedirect(route('app.invites.show', $invite, absolute: false)); + + expect(User::query()->where('email', 'invited-locale@example.com')->sole()->locale)->toBe(Locale::PortugueseBrazil); +}); + +test('an invited social signup stores the language sent to the OAuth start', function (string $network, string $driver) { + $invite = signupPreferencesInvite("{$network}-invited-locale@example.com"); + + $this->get(route("auth.{$network}.redirect", ['invite' => $invite->id, 'locale' => 'pt-BR'])); + signupPreferencesSocialite($driver, "{$network}-invited-locale", "{$network}-invited-locale@example.com"); + + $this->get(route("auth.{$network}.callback"))->assertRedirect(route('app.invites.show', $invite, absolute: false)); + + expect(User::query()->where('email', "{$network}-invited-locale@example.com")->sole()->locale)->toBe(Locale::PortugueseBrazil); +})->with([ + 'google' => ['google', 'google-auth'], + 'github' => ['github', 'github'], +]);