From ac9a65945f6c9eb917b4449ad7d83b4402b4cea7 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 17 Sep 2023 13:29:06 +0100
Subject: [PATCH] Locales: Performed cleanup and alignment of locale handling

- Reduced app settings down to what's required.
- Used new view-shared $locale object instead of using globals via
  config.
- Aligned language used to default on "locale" instead of mixing
  locale/language.

For #4501
---
 app/Config/app.php                            |  9 +-
 app/Http/Middleware/Localization.php          | 31 ++-----
 app/Translation/LocaleDefinition.php          | 45 ++++++++++
 ...{LanguageManager.php => LocaleManager.php} | 84 ++++++++++---------
 app/Users/Models/User.php                     |  4 +-
 resources/views/layouts/base.blade.php        |  4 +-
 resources/views/layouts/export.blade.php      |  2 +-
 resources/views/layouts/plain.blade.php       |  4 +-
 .../pages/parts/markdown-editor.blade.php     |  2 +-
 .../pages/parts/wysiwyg-editor.blade.php      |  4 +-
 resources/views/users/create.blade.php        |  2 +-
 resources/views/users/edit.blade.php          |  2 +-
 .../vendor/notifications/email.blade.php      |  2 +-
 tests/LanguageTest.php                        |  1 +
 14 files changed, 116 insertions(+), 80 deletions(-)
 create mode 100644 app/Translation/LocaleDefinition.php
 rename app/Translation/{LanguageManager.php => LocaleManager.php} (70%)

diff --git a/app/Config/app.php b/app/Config/app.php
index 3a843c512..dcd3ffc31 100644
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -83,10 +83,10 @@ return [
     'timezone' => env('APP_TIMEZONE', 'UTC'),
 
     // Default locale to use
+    // A default variant is also stored since Laravel can overwrite
+    // app.locale when dynamically setting the locale in-app.
     'locale' => env('APP_LANG', 'en'),
-
-    // Locales available
-    'locales' => ['en', 'ar', 'bg', 'bs', 'ca', 'cs', 'cy', 'da', 'de', 'de_informal', 'el', 'es', 'es_AR', 'et', 'eu', 'fa', 'fr', 'he', 'hr', 'hu', 'id', 'it', 'ja', 'ka', 'ko', 'lt', 'lv', 'nl', 'nb', 'pt', 'pt_BR', 'sk', 'sl', 'sv', 'pl',  'ro', 'ru', 'tr', 'uk', 'uz', 'vi', 'zh_CN', 'zh_TW'],
+    'default_locale' => env('APP_LANG', 'en'),
 
     //  Application Fallback Locale
     'fallback_locale' => 'en',
@@ -94,9 +94,6 @@ return [
     // Faker Locale
     'faker_locale' => 'en_GB',
 
-    // Enable right-to-left text control.
-    'rtl' => false,
-
     // Auto-detect the locale for public users
     // For public users their locale can be guessed by headers sent by their
     // browser. This is usually set by users in their browser settings.
diff --git a/app/Http/Middleware/Localization.php b/app/Http/Middleware/Localization.php
index 47723e242..0be0b77eb 100644
--- a/app/Http/Middleware/Localization.php
+++ b/app/Http/Middleware/Localization.php
@@ -2,17 +2,14 @@
 
 namespace BookStack\Http\Middleware;
 
-use BookStack\Translation\LanguageManager;
-use Carbon\Carbon;
+use BookStack\Translation\LocaleManager;
 use Closure;
 
 class Localization
 {
-    protected LanguageManager $languageManager;
-
-    public function __construct(LanguageManager $languageManager)
-    {
-        $this->languageManager = $languageManager;
+    public function __construct(
+        protected LocaleManager $localeManager
+    ) {
     }
 
     /**
@@ -25,22 +22,12 @@ class Localization
      */
     public function handle($request, Closure $next)
     {
-        // Get and record the default language in the config
-        $defaultLang = config('app.locale');
-        config()->set('app.default_locale', $defaultLang);
+        // Share details of the user's locale for use in views
+        $userLocale = $this->localeManager->getForUser(user());
+        view()->share('locale', $userLocale);
 
-        // Get the user's language and record that in the config for use in views
-        $userLang = $this->languageManager->getUserLanguage($request, $defaultLang);
-        config()->set('app.lang', str_replace('_', '-', $this->languageManager->getIsoName($userLang)));
-
-        // Set text direction
-        if ($this->languageManager->isRTL($userLang)) {
-            config()->set('app.rtl', true);
-        }
-
-        app()->setLocale($userLang);
-        Carbon::setLocale($userLang);
-        $this->languageManager->setPhpDateTimeLocale($userLang);
+        // Set locale for system components
+        $this->localeManager->setAppLocale($userLocale);
 
         return $next($request);
     }
diff --git a/app/Translation/LocaleDefinition.php b/app/Translation/LocaleDefinition.php
new file mode 100644
index 000000000..fe8640109
--- /dev/null
+++ b/app/Translation/LocaleDefinition.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace BookStack\Translation;
+
+class LocaleDefinition
+{
+    public function __construct(
+        protected string $appName,
+        protected string $isoName,
+        protected bool $isRtl
+    ) {
+    }
+
+    /**
+     * Provide the BookStack-specific locale name.
+     */
+    public function appLocale(): string
+    {
+        return $this->appName;
+    }
+
+    /**
+     * Provide the ISO-aligned locale name.
+     */
+    public function isoLocale(): string
+    {
+        return $this->isoName;
+    }
+
+    /**
+     * Returns a string suitable for the HTML "lang" attribute.
+     */
+    public function htmlLang(): string
+    {
+        return str_replace('_', '-', $this->isoName);
+    }
+
+    /**
+     * Returns a string suitable for the HTML "dir" attribute.
+     */
+    public function htmlDirection(): string
+    {
+        return $this->isRtl ? 'rtl' : 'ltr';
+    }
+}
diff --git a/app/Translation/LanguageManager.php b/app/Translation/LocaleManager.php
similarity index 70%
rename from app/Translation/LanguageManager.php
rename to app/Translation/LocaleManager.php
index f3432d038..171555293 100644
--- a/app/Translation/LanguageManager.php
+++ b/app/Translation/LocaleManager.php
@@ -3,17 +3,18 @@
 namespace BookStack\Translation;
 
 use BookStack\Users\Models\User;
+use Carbon\Carbon;
 use Illuminate\Http\Request;
 
-class LanguageManager
+class LocaleManager
 {
     /**
-     * Array of right-to-left language options.
+     * Array of right-to-left locale options.
      */
-    protected array $rtlLanguages = ['ar', 'fa', 'he'];
+    protected array $rtlLocales = ['ar', 'fa', 'he'];
 
     /**
-     * Map of BookStack language names to best-estimate ISO and windows locale names.
+     * Map of BookStack locale names to best-estimate ISO and windows locale names.
      * Locales can often be found by running `locale -a` on a linux system.
      * Windows locales can be found at:
      * https://docs.microsoft.com/en-us/cpp/c-runtime-library/language-strings?view=msvc-170.
@@ -29,8 +30,8 @@ class LanguageManager
         'da'          => ['iso' => 'da_DK', 'windows' => 'Danish'],
         'de'          => ['iso' => 'de_DE', 'windows' => 'German'],
         'de_informal' => ['iso' => 'de_DE', 'windows' => 'German'],
-        'en'          => ['iso' => 'en_GB', 'windows' => 'English'],
         'el'          => ['iso' => 'el_GR', 'windows' => 'Greek'],
+        'en'          => ['iso' => 'en_GB', 'windows' => 'English'],
         'es'          => ['iso' => 'es_ES', 'windows' => 'Spanish'],
         'es_AR'       => ['iso' => 'es_AR', 'windows' => 'Spanish'],
         'et'          => ['iso' => 'et_EE', 'windows' => 'Estonian'],
@@ -46,8 +47,8 @@ class LanguageManager
         'ko'          => ['iso' => 'ko_KR', 'windows' => 'Korean'],
         'lt'          => ['iso' => 'lt_LT', 'windows' => 'Lithuanian'],
         'lv'          => ['iso' => 'lv_LV', 'windows' => 'Latvian'],
-        'nl'          => ['iso' => 'nl_NL', 'windows' => 'Dutch'],
         'nb'          => ['iso' => 'nb_NO', 'windows' => 'Norwegian (Bokmal)'],
+        'nl'          => ['iso' => 'nl_NL', 'windows' => 'Dutch'],
         'pl'          => ['iso' => 'pl_PL', 'windows' => 'Polish'],
         'pt'          => ['iso' => 'pt_PT', 'windows' => 'Portuguese'],
         'pt_BR'       => ['iso' => 'pt_BR', 'windows' => 'Portuguese'],
@@ -56,47 +57,40 @@ class LanguageManager
         'sk'          => ['iso' => 'sk_SK', 'windows' => 'Slovak'],
         'sl'          => ['iso' => 'sl_SI', 'windows' => 'Slovenian'],
         'sv'          => ['iso' => 'sv_SE', 'windows' => 'Swedish'],
+        'tr'          => ['iso' => 'tr_TR', 'windows' => 'Turkish'],
         'uk'          => ['iso' => 'uk_UA', 'windows' => 'Ukrainian'],
         'uz'          => ['iso' => 'uz_UZ', 'windows' => 'Uzbek'],
         'vi'          => ['iso' => 'vi_VN', 'windows' => 'Vietnamese'],
         'zh_CN'       => ['iso' => 'zh_CN', 'windows' => 'Chinese (Simplified)'],
         'zh_TW'       => ['iso' => 'zh_TW', 'windows' => 'Chinese (Traditional)'],
-        'tr'          => ['iso' => 'tr_TR', 'windows' => 'Turkish'],
     ];
 
     /**
-     * Get the language specifically for the currently logged-in user if available.
+     * Get the BookStack locale string for the given user.
      */
-    public function getUserLanguage(Request $request, string $default): string
+    protected function getLocaleForUser(User $user): string
     {
-        try {
-            $user = user();
-        } catch (\Exception $exception) {
-            return $default;
-        }
+        $default = config('app.default_locale');
 
         if ($user->isGuest() && config('app.auto_detect_locale')) {
-            return $this->autoDetectLocale($request, $default);
+            return $this->autoDetectLocale(request(), $default);
         }
 
         return setting()->getUser($user, 'language', $default);
     }
 
     /**
-     * Get the language for the given user.
+     * Get a locale definition for the current user.
      */
-    public function getLanguageForUser(User $user): string
+    public function getForUser(User $user): LocaleDefinition
     {
-        $default = config('app.locale');
-        return setting()->getUser($user, 'language', $default);
-    }
+        $localeString = $this->getLocaleForUser($user);
 
-    /**
-     * Check if the given BookStack language value is a right-to-left language.
-     */
-    public function isRTL(string $language): bool
-    {
-        return in_array($language, $this->rtlLanguages);
+        return new LocaleDefinition(
+            $localeString,
+            $this->getIsoName($localeString),
+            in_array($localeString, $this->rtlLocales),
+        );
     }
 
     /**
@@ -105,7 +99,8 @@ class LanguageManager
      */
     protected function autoDetectLocale(Request $request, string $default): string
     {
-        $availableLocales = config('app.locales');
+        $availableLocales = array_keys($this->localeMap);
+
         foreach ($request->getLanguages() as $lang) {
             if (in_array($lang, $availableLocales)) {
                 return $lang;
@@ -116,29 +111,40 @@ class LanguageManager
     }
 
     /**
-     * Get the ISO version of a BookStack language name.
+     * Get the ISO version of a BookStack locale.
      */
-    public function getIsoName(string $language): string
+    protected function getIsoName(string $locale): string
     {
-        return $this->localeMap[$language]['iso'] ?? $language;
+        return $this->localeMap[$locale]['iso'] ?? $locale;
+    }
+
+    /**
+     * Sets the active locale for system level components.
+     */
+    public function setAppLocale(LocaleDefinition $locale): void
+    {
+        app()->setLocale($locale->appLocale());
+        Carbon::setLocale($locale->isoLocale());
+        $this->setPhpDateTimeLocale($locale);
     }
 
     /**
      * Set the system date locale for localized date formatting.
      * Will try both the standard locale name and the UTF8 variant.
      */
-    public function setPhpDateTimeLocale(string $language): void
+    public function setPhpDateTimeLocale(LocaleDefinition $locale): void
     {
-        $isoLang = $this->localeMap[$language]['iso'] ?? '';
-        $isoLangPrefix = explode('_', $isoLang)[0];
+        $appLocale = $locale->appLocale();
+        $isoLocale = $this->localeMap[$appLocale]['iso'] ?? '';
+        $isoLocalePrefix = explode('_', $isoLocale)[0];
 
         $locales = array_values(array_filter([
-            $isoLang ? $isoLang . '.utf8' : false,
-            $isoLang ?: false,
-            $isoLang ? str_replace('_', '-', $isoLang) : false,
-            $isoLang ? $isoLangPrefix . '.UTF-8' : false,
-            $this->localeMap[$language]['windows'] ?? false,
-            $language,
+            $isoLocale ? $isoLocale . '.utf8' : false,
+            $isoLocale ?: false,
+            $isoLocale ? str_replace('_', '-', $isoLocale) : false,
+            $isoLocale ? $isoLocalePrefix . '.UTF-8' : false,
+            $this->localeMap[$appLocale]['windows'] ?? false,
+            $appLocale,
         ]));
 
         if (!empty($locales)) {
diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php
index 232ea8832..78411e0d4 100644
--- a/app/Users/Models/User.php
+++ b/app/Users/Models/User.php
@@ -12,7 +12,7 @@ use BookStack\Api\ApiToken;
 use BookStack\App\Model;
 use BookStack\App\Sluggable;
 use BookStack\Entities\Tools\SlugGenerator;
-use BookStack\Translation\LanguageManager;
+use BookStack\Translation\LocaleManager;
 use BookStack\Uploads\Image;
 use Carbon\Carbon;
 use Exception;
@@ -346,7 +346,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      */
     public function getLanguage(): string
     {
-        return app()->make(LanguageManager::class)->getLanguageForUser($this);
+        return app()->make(LocaleManager::class)->getForUser($this)->appLocale();
     }
 
     /**
diff --git a/resources/views/layouts/base.blade.php b/resources/views/layouts/base.blade.php
index 8875788a6..0fd12b70f 100644
--- a/resources/views/layouts/base.blade.php
+++ b/resources/views/layouts/base.blade.php
@@ -1,6 +1,6 @@
 <!DOCTYPE html>
-<html lang="{{ config('app.lang') }}"
-      dir="{{ config('app.rtl') ? 'rtl' : 'ltr' }}"
+<html lang="{{ $locale->htmlLang() }}"
+      dir="{{ $locale->htmlDirection() }}"
       class="{{ setting()->getForCurrentUser('dark-mode-enabled') ? 'dark-mode ' : '' }}">
 <head>
     <title>{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}</title>
diff --git a/resources/views/layouts/export.blade.php b/resources/views/layouts/export.blade.php
index e041d8dea..eb2397a75 100644
--- a/resources/views/layouts/export.blade.php
+++ b/resources/views/layouts/export.blade.php
@@ -1,5 +1,5 @@
 <!doctype html>
-<html lang="{{ config('app.lang') }}">
+<html lang="{{ $locale->htmlLang() }}">
 <head>
     <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
     <title>@yield('title')</title>
diff --git a/resources/views/layouts/plain.blade.php b/resources/views/layouts/plain.blade.php
index 043d8aa48..360c404c1 100644
--- a/resources/views/layouts/plain.blade.php
+++ b/resources/views/layouts/plain.blade.php
@@ -1,6 +1,6 @@
 <!DOCTYPE html>
-<html lang="{{ config('app.lang') }}"
-      dir="{{ config('app.rtl') ? 'rtl' : 'ltr' }}"
+<html lang="{{ $locale->htmlLang() }}"
+      dir="{{ $locale->htmlDirection() }}"
       class="@yield('document-class')">
 <head>
     <title>{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}</title>
diff --git a/resources/views/pages/parts/markdown-editor.blade.php b/resources/views/pages/parts/markdown-editor.blade.php
index c488f0e11..18a418f10 100644
--- a/resources/views/pages/parts/markdown-editor.blade.php
+++ b/resources/views/pages/parts/markdown-editor.blade.php
@@ -1,6 +1,6 @@
 <div id="markdown-editor" component="markdown-editor"
      option:markdown-editor:page-id="{{ $model->id ?? 0 }}"
-     option:markdown-editor:text-direction="{{ config('app.rtl') ? 'rtl' : 'ltr' }}"
+     option:markdown-editor:text-direction="{{ $locale->htmlDirection() }}"
      option:markdown-editor:image-upload-error-text="{{ trans('errors.image_upload_error') }}"
      option:markdown-editor:server-upload-limit-text="{{ trans('errors.server_upload_limit') }}"
      class="flex-fill flex code-fill">
diff --git a/resources/views/pages/parts/wysiwyg-editor.blade.php b/resources/views/pages/parts/wysiwyg-editor.blade.php
index b7cd1bdaa..ca6b6da8a 100644
--- a/resources/views/pages/parts/wysiwyg-editor.blade.php
+++ b/resources/views/pages/parts/wysiwyg-editor.blade.php
@@ -3,9 +3,9 @@
 @endpush
 
 <div component="wysiwyg-editor"
-     option:wysiwyg-editor:language="{{ config('app.lang') }}"
+     option:wysiwyg-editor:language="{{ $locale->htmlLang() }}"
      option:wysiwyg-editor:page-id="{{ $model->id ?? 0 }}"
-     option:wysiwyg-editor:text-direction="{{ config('app.rtl') ? 'rtl' : 'ltr' }}"
+     option:wysiwyg-editor:text-direction="{{ $locale->htmlDirection() }}"
      option:wysiwyg-editor:image-upload-error-text="{{ trans('errors.image_upload_error') }}"
      option:wysiwyg-editor:server-upload-limit-text="{{ trans('errors.server_upload_limit') }}"
      class="flex-fill flex">
diff --git a/resources/views/users/create.blade.php b/resources/views/users/create.blade.php
index 540d7bd6a..0edae1d82 100644
--- a/resources/views/users/create.blade.php
+++ b/resources/views/users/create.blade.php
@@ -14,7 +14,7 @@
 
                 <div class="setting-list">
                     @include('users.parts.form')
-                    @include('users.parts.language-option-row', ['value' => old('setting.language') ?? config('app.default_locale')])
+                    @include('users.parts.language-option-row', ['value' => old('setting.language') ?? config('app.locale')])
                 </div>
 
                 <div class="form-group text-right">
diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php
index 4e31e785d..23cf87684 100644
--- a/resources/views/users/edit.blade.php
+++ b/resources/views/users/edit.blade.php
@@ -33,7 +33,7 @@
                         </div>
                     </div>
 
-                    @include('users.parts.language-option-row', ['value' => setting()->getUser($user, 'language', config('app.default_locale'))])
+                    @include('users.parts.language-option-row', ['value' => $user->getLanguage())])
                 </div>
 
                 <div class="text-right">
diff --git a/resources/views/vendor/notifications/email.blade.php b/resources/views/vendor/notifications/email.blade.php
index f5d9c328d..1b81c6fc6 100644
--- a/resources/views/vendor/notifications/email.blade.php
+++ b/resources/views/vendor/notifications/email.blade.php
@@ -1,5 +1,5 @@
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
-<html lang="{{ config('app.lang') }}">
+<html lang="{{ $locale->htmlLang() }}">
 <head>
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
diff --git a/tests/LanguageTest.php b/tests/LanguageTest.php
index a66227ff2..b6a7d1e87 100644
--- a/tests/LanguageTest.php
+++ b/tests/LanguageTest.php
@@ -78,6 +78,7 @@ class LanguageTest extends TestCase
     public function test_rtl_config_set_if_lang_is_rtl()
     {
         $this->asEditor();
+        // TODO - Alter
         $this->assertFalse(config('app.rtl'), 'App RTL config should be false by default');
         setting()->putUser($this->users->editor(), 'language', 'ar');
         $this->get('/');