From 8994c1b9d9dca616c74cdcdb17cc4722203e3299 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 17 Sep 2023 16:20:21 +0100 Subject: [PATCH] Locales: More use of locale objects, Addressed failing tests --- .../Notifications/UserInviteNotification.php | 12 +++++------ .../Messages/BaseActivityNotification.php | 7 ++++--- .../Messages/CommentCreationNotification.php | 18 ++++++++--------- .../Messages/PageCreationNotification.php | 16 +++++++-------- .../Messages/PageUpdateNotification.php | 18 ++++++++--------- app/App/MailNotification.php | 5 +++-- app/Translation/LocaleDefinition.php | 8 ++++++++ app/Translation/LocaleManager.php | 12 ++++++++++- app/Users/Models/User.php | 7 ++++--- resources/views/layouts/base.blade.php | 4 ++-- resources/views/layouts/export.blade.php | 2 +- resources/views/layouts/plain.blade.php | 4 ++-- resources/views/users/create.blade.php | 2 +- resources/views/users/edit.blade.php | 17 ++++++++++------ .../vendor/notifications/email.blade.php | 4 ++-- tests/LanguageTest.php | 20 +++++++++---------- tests/User/UserManagementTest.php | 2 +- 17 files changed, 92 insertions(+), 66 deletions(-) diff --git a/app/Access/Notifications/UserInviteNotification.php b/app/Access/Notifications/UserInviteNotification.php index b453fc95d..dacce574f 100644 --- a/app/Access/Notifications/UserInviteNotification.php +++ b/app/Access/Notifications/UserInviteNotification.php @@ -16,12 +16,12 @@ class UserInviteNotification extends MailNotification public function toMail(User $notifiable): MailMessage { $appName = ['appName' => setting('app-name')]; - $language = $notifiable->getLanguage(); + $locale = $notifiable->getLocale(); - return $this->newMailMessage($language) - ->subject(trans('auth.user_invite_email_subject', $appName, $language)) - ->greeting(trans('auth.user_invite_email_greeting', $appName, $language)) - ->line(trans('auth.user_invite_email_text', [], $language)) - ->action(trans('auth.user_invite_email_action', [], $language), url('/register/invite/' . $this->token)); + return $this->newMailMessage($locale) + ->subject($locale->trans('auth.user_invite_email_subject', $appName)) + ->greeting($locale->trans('auth.user_invite_email_greeting', $appName)) + ->line($locale->trans('auth.user_invite_email_text')) + ->action($locale->trans('auth.user_invite_email_action'), url('/register/invite/' . $this->token)); } } diff --git a/app/Activity/Notifications/Messages/BaseActivityNotification.php b/app/Activity/Notifications/Messages/BaseActivityNotification.php index 414859091..322df5d94 100644 --- a/app/Activity/Notifications/Messages/BaseActivityNotification.php +++ b/app/Activity/Notifications/Messages/BaseActivityNotification.php @@ -5,6 +5,7 @@ namespace BookStack\Activity\Notifications\Messages; use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\MessageParts\LinkedMailMessageLine; use BookStack\App\MailNotification; +use BookStack\Translation\LocaleDefinition; use BookStack\Users\Models\User; use Illuminate\Bus\Queueable; @@ -35,12 +36,12 @@ abstract class BaseActivityNotification extends MailNotification /** * Build the common reason footer line used in mail messages. */ - protected function buildReasonFooterLine(string $language): LinkedMailMessageLine + protected function buildReasonFooterLine(LocaleDefinition $locale): LinkedMailMessageLine { return new LinkedMailMessageLine( url('/preferences/notifications'), - trans('notifications.footer_reason', [], $language), - trans('notifications.footer_reason_link', [], $language), + $locale->trans('notifications.footer_reason'), + $locale->trans('notifications.footer_reason_link'), ); } } diff --git a/app/Activity/Notifications/Messages/CommentCreationNotification.php b/app/Activity/Notifications/Messages/CommentCreationNotification.php index 5db61b04b..094ab30b7 100644 --- a/app/Activity/Notifications/Messages/CommentCreationNotification.php +++ b/app/Activity/Notifications/Messages/CommentCreationNotification.php @@ -17,17 +17,17 @@ class CommentCreationNotification extends BaseActivityNotification /** @var Page $page */ $page = $comment->entity; - $language = $notifiable->getLanguage(); + $locale = $notifiable->getLocale(); - return $this->newMailMessage($language) - ->subject(trans('notifications.new_comment_subject', ['pageName' => $page->getShortName()], $language)) - ->line(trans('notifications.new_comment_intro', ['appName' => setting('app-name')], $language)) + return $this->newMailMessage($locale) + ->subject($locale->trans('notifications.new_comment_subject', ['pageName' => $page->getShortName()])) + ->line($locale->trans('notifications.new_comment_intro', ['appName' => setting('app-name')])) ->line(new ListMessageLine([ - trans('notifications.detail_page_name', [], $language) => $page->name, - trans('notifications.detail_commenter', [], $language) => $this->user->name, - trans('notifications.detail_comment', [], $language) => strip_tags($comment->html), + $locale->trans('notifications.detail_page_name') => $page->name, + $locale->trans('notifications.detail_commenter') => $this->user->name, + $locale->trans('notifications.detail_comment') => strip_tags($comment->html), ])) - ->action(trans('notifications.action_view_comment', [], $language), $page->getUrl('#comment' . $comment->local_id)) - ->line($this->buildReasonFooterLine($language)); + ->action($locale->trans('notifications.action_view_comment'), $page->getUrl('#comment' . $comment->local_id)) + ->line($this->buildReasonFooterLine($locale)); } } diff --git a/app/Activity/Notifications/Messages/PageCreationNotification.php b/app/Activity/Notifications/Messages/PageCreationNotification.php index 110829469..da028aa8c 100644 --- a/app/Activity/Notifications/Messages/PageCreationNotification.php +++ b/app/Activity/Notifications/Messages/PageCreationNotification.php @@ -14,16 +14,16 @@ class PageCreationNotification extends BaseActivityNotification /** @var Page $page */ $page = $this->detail; - $language = $notifiable->getLanguage(); + $locale = $notifiable->getLocale(); - return $this->newMailMessage($language) - ->subject(trans('notifications.new_page_subject', ['pageName' => $page->getShortName()], $language)) - ->line(trans('notifications.new_page_intro', ['appName' => setting('app-name')], $language)) + return $this->newMailMessage($locale) + ->subject($locale->trans('notifications.new_page_subject', ['pageName' => $page->getShortName()])) + ->line($locale->trans('notifications.new_page_intro', ['appName' => setting('app-name')], $locale)) ->line(new ListMessageLine([ - trans('notifications.detail_page_name', [], $language) => $page->name, - trans('notifications.detail_created_by', [], $language) => $this->user->name, + $locale->trans('notifications.detail_page_name') => $page->name, + $locale->trans('notifications.detail_created_by') => $this->user->name, ])) - ->action(trans('notifications.action_view_page', [], $language), $page->getUrl()) - ->line($this->buildReasonFooterLine($language)); + ->action($locale->trans('notifications.action_view_page'), $page->getUrl()) + ->line($this->buildReasonFooterLine($locale)); } } diff --git a/app/Activity/Notifications/Messages/PageUpdateNotification.php b/app/Activity/Notifications/Messages/PageUpdateNotification.php index 8e2d27fa4..1c8155d29 100644 --- a/app/Activity/Notifications/Messages/PageUpdateNotification.php +++ b/app/Activity/Notifications/Messages/PageUpdateNotification.php @@ -14,17 +14,17 @@ class PageUpdateNotification extends BaseActivityNotification /** @var Page $page */ $page = $this->detail; - $language = $notifiable->getLanguage(); + $locale = $notifiable->getLocale(); - return $this->newMailMessage($language) - ->subject(trans('notifications.updated_page_subject', ['pageName' => $page->getShortName()], $language)) - ->line(trans('notifications.updated_page_intro', ['appName' => setting('app-name')], $language)) + return $this->newMailMessage($locale) + ->subject($locale->trans('notifications.updated_page_subject', ['pageName' => $page->getShortName()])) + ->line($locale->trans('notifications.updated_page_intro', ['appName' => setting('app-name')])) ->line(new ListMessageLine([ - trans('notifications.detail_page_name', [], $language) => $page->name, - trans('notifications.detail_updated_by', [], $language) => $this->user->name, + $locale->trans('notifications.detail_page_name') => $page->name, + $locale->trans('notifications.detail_updated_by') => $this->user->name, ])) - ->line(trans('notifications.updated_page_debounce', [], $language)) - ->action(trans('notifications.action_view_page', [], $language), $page->getUrl()) - ->line($this->buildReasonFooterLine($language)); + ->line($locale->trans('notifications.updated_page_debounce')) + ->action($locale->trans('notifications.action_view_page'), $page->getUrl()) + ->line($this->buildReasonFooterLine($locale)); } } diff --git a/app/App/MailNotification.php b/app/App/MailNotification.php index 8c57b5621..50b7f69a7 100644 --- a/app/App/MailNotification.php +++ b/app/App/MailNotification.php @@ -2,6 +2,7 @@ namespace BookStack\App; +use BookStack\Translation\LocaleDefinition; use BookStack\Users\Models\User; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; @@ -32,9 +33,9 @@ abstract class MailNotification extends Notification implements ShouldQueue /** * Create a new mail message. */ - protected function newMailMessage(string $language = ''): MailMessage + protected function newMailMessage(?LocaleDefinition $locale = null): MailMessage { - $data = ['language' => $language ?: null]; + $data = ['locale' => $locale ?? user()->getLocale()]; return (new MailMessage())->view([ 'html' => 'vendor.notifications.email', diff --git a/app/Translation/LocaleDefinition.php b/app/Translation/LocaleDefinition.php index fe8640109..85d36afa0 100644 --- a/app/Translation/LocaleDefinition.php +++ b/app/Translation/LocaleDefinition.php @@ -42,4 +42,12 @@ class LocaleDefinition { return $this->isRtl ? 'rtl' : 'ltr'; } + + /** + * Translate using this locate. + */ + public function trans(string $key, array $replace = []): string + { + return trans($key, $replace, $this->appLocale()); + } } diff --git a/app/Translation/LocaleManager.php b/app/Translation/LocaleManager.php index 171555293..cf93aff06 100644 --- a/app/Translation/LocaleManager.php +++ b/app/Translation/LocaleManager.php @@ -27,6 +27,7 @@ class LocaleManager 'bs' => ['iso' => 'bs_BA', 'windows' => 'Bosnian (Latin)'], 'ca' => ['iso' => 'ca', 'windows' => 'Catalan'], 'cs' => ['iso' => 'cs_CZ', 'windows' => 'Czech'], + 'cy' => ['iso' => 'cy_GB', 'windows' => 'Welsh'], 'da' => ['iso' => 'da_DK', 'windows' => 'Danish'], 'de' => ['iso' => 'de_DE', 'windows' => 'German'], 'de_informal' => ['iso' => 'de_DE', 'windows' => 'German'], @@ -44,6 +45,7 @@ class LocaleManager 'id' => ['iso' => 'id_ID', 'windows' => 'Indonesian'], 'it' => ['iso' => 'it_IT', 'windows' => 'Italian'], 'ja' => ['iso' => 'ja', 'windows' => 'Japanese'], + 'ka' => ['iso' => 'ka_GE', 'windows' => 'Georgian'], 'ko' => ['iso' => 'ko_KR', 'windows' => 'Korean'], 'lt' => ['iso' => 'lt_LT', 'windows' => 'Lithuanian'], 'lv' => ['iso' => 'lv_LV', 'windows' => 'Latvian'], @@ -99,7 +101,7 @@ class LocaleManager */ protected function autoDetectLocale(Request $request, string $default): string { - $availableLocales = array_keys($this->localeMap); + $availableLocales = $this->getAllAppLocales(); foreach ($request->getLanguages() as $lang) { if (in_array($lang, $availableLocales)) { @@ -151,4 +153,12 @@ class LocaleManager setlocale(LC_TIME, $locales[0], ...array_slice($locales, 1)); } } + + /** + * Get all the available app-specific level locale strings. + */ + public function getAllAppLocales(): array + { + return array_keys($this->localeMap); + } } diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 78411e0d4..39236c7e4 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -12,6 +12,7 @@ use BookStack\Api\ApiToken; use BookStack\App\Model; use BookStack\App\Sluggable; use BookStack\Entities\Tools\SlugGenerator; +use BookStack\Translation\LocaleDefinition; use BookStack\Translation\LocaleManager; use BookStack\Uploads\Image; use Carbon\Carbon; @@ -342,11 +343,11 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon } /** - * Get the system language for this user. + * Get the locale for this user. */ - public function getLanguage(): string + public function getLocale(): LocaleDefinition { - return app()->make(LocaleManager::class)->getForUser($this)->appLocale(); + return app()->make(LocaleManager::class)->getForUser($this); } /** diff --git a/resources/views/layouts/base.blade.php b/resources/views/layouts/base.blade.php index 0fd12b70f..ca8570d36 100644 --- a/resources/views/layouts/base.blade.php +++ b/resources/views/layouts/base.blade.php @@ -1,6 +1,6 @@ - {{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }} diff --git a/resources/views/layouts/export.blade.php b/resources/views/layouts/export.blade.php index eb2397a75..c2c3880e4 100644 --- a/resources/views/layouts/export.blade.php +++ b/resources/views/layouts/export.blade.php @@ -1,5 +1,5 @@ - + @yield('title') diff --git a/resources/views/layouts/plain.blade.php b/resources/views/layouts/plain.blade.php index 360c404c1..7ce9078e2 100644 --- a/resources/views/layouts/plain.blade.php +++ b/resources/views/layouts/plain.blade.php @@ -1,6 +1,6 @@ - {{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }} diff --git a/resources/views/users/create.blade.php b/resources/views/users/create.blade.php index 0edae1d82..dafc623e1 100644 --- a/resources/views/users/create.blade.php +++ b/resources/views/users/create.blade.php @@ -14,7 +14,7 @@
@include('users.parts.form') - @include('users.parts.language-option-row', ['value' => old('setting.language') ?? config('app.locale')]) + @include('users.parts.language-option-row', ['value' => old('language') ?? config('app.default_locale')])
diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index 23cf87684..832186930 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -16,7 +16,8 @@
- +

{{ trans('settings.users_avatar_desc') }}

@@ -33,13 +34,15 @@
- @include('users.parts.language-option-row', ['value' => $user->getLanguage())]) + @include('users.parts.language-option-row', ['value' => old('language') ?? $user->getLocale()->appLocale()])
- {{ trans('common.cancel') }} + {{ trans('common.cancel') }} @if($authMethod !== 'system') - id}/delete") }}" class="button outline">{{ trans('settings.users_delete') }} + id}/delete") }}" + class="button outline">{{ trans('settings.users_delete') }} @endif
@@ -60,7 +63,8 @@
@if($user->id === user()->id) - {{ trans('settings.users_mfa_configure') }} + {{ trans('settings.users_mfa_configure') }} @endif
@@ -84,7 +88,8 @@ class="button small outline">{{ trans('settings.users_social_disconnect') }} @else - {{ trans('settings.users_social_connect') }} @endif diff --git a/resources/views/vendor/notifications/email.blade.php b/resources/views/vendor/notifications/email.blade.php index 1b81c6fc6..8e922fba5 100644 --- a/resources/views/vendor/notifications/email.blade.php +++ b/resources/views/vendor/notifications/email.blade.php @@ -159,7 +159,7 @@ $style = [

- {{ trans('common.email_action_help', ['actionText' => $actionText], $language) }} + {{ $locale->trans('common.email_action_help', ['actionText' => $actionText]) }}

@@ -187,7 +187,7 @@ $style = [

© {{ date('Y') }} {{ setting('app-name') }}. - {{ trans('common.email_rights', [], $language) }} + {{ $locale->trans('common.email_rights') }}

diff --git a/tests/LanguageTest.php b/tests/LanguageTest.php index b6a7d1e87..6b6856184 100644 --- a/tests/LanguageTest.php +++ b/tests/LanguageTest.php @@ -3,6 +3,7 @@ namespace Tests; use BookStack\Activity\ActivityType; +use BookStack\Translation\LocaleManager; class LanguageTest extends TestCase { @@ -17,12 +18,12 @@ class LanguageTest extends TestCase $this->langs = array_diff(scandir(lang_path('')), ['..', '.']); } - public function test_locales_config_key_set_properly() + public function test_locales_list_set_properly() { - $configLocales = config('app.locales'); - sort($configLocales); + $appLocales = $this->app->make(LocaleManager::class)->getAllAppLocales(); + sort($appLocales); sort($this->langs); - $this->assertEquals(implode(':', $configLocales), implode(':', $this->langs), 'app.locales configuration variable does not match those found in lang files'); + $this->assertEquals(implode(':', $this->langs), implode(':', $appLocales), 'app.locales configuration variable does not match those found in lang files'); } // Not part of standard phpunit test runs since we sometimes expect non-added langs. @@ -75,14 +76,13 @@ class LanguageTest extends TestCase } } - public function test_rtl_config_set_if_lang_is_rtl() + public function test_views_use_rtl_if_rtl_language_is_set() { - $this->asEditor(); - // TODO - Alter - $this->assertFalse(config('app.rtl'), 'App RTL config should be false by default'); + $this->asEditor()->withHtml($this->get('/'))->assertElementExists('html[dir="ltr"]'); + setting()->putUser($this->users->editor(), 'language', 'ar'); - $this->get('/'); - $this->assertTrue(config('app.rtl'), 'App RTL config should have been set to true by middleware'); + + $this->withHtml($this->get('/'))->assertElementExists('html[dir="rtl"]'); } public function test_unknown_lang_does_not_break_app() diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php index a6d869b2f..93d35f5d0 100644 --- a/tests/User/UserManagementTest.php +++ b/tests/User/UserManagementTest.php @@ -215,7 +215,7 @@ class UserManagementTest extends TestCase { $langs = ['en', 'fr', 'hr']; foreach ($langs as $lang) { - config()->set('app.locale', $lang); + config()->set('app.default_locale', $lang); $resp = $this->asAdmin()->get('/settings/users/create'); $this->withHtml($resp)->assertElementExists('select[name="language"] option[value="' . $lang . '"][selected]'); }