Locales: More use of locale objects, Addressed failing tests

This commit is contained in:
Dan Brown 2023-09-17 16:20:21 +01:00
parent ac9a65945f
commit 8994c1b9d9
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
17 changed files with 92 additions and 66 deletions

View File

@ -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));
}
}

View File

@ -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'),
);
}
}

View File

@ -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));
}
}

View File

@ -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));
}
}

View File

@ -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));
}
}

View File

@ -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',

View File

@ -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());
}
}

View File

@ -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);
}
}

View File

@ -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);
}
/**

View File

@ -1,6 +1,6 @@
<!DOCTYPE html>
<html lang="{{ $locale->htmlLang() }}"
dir="{{ $locale->htmlDirection() }}"
<html lang="{{ $locale?->htmlLang() ?? config('app.default_locale') }}"
dir="{{ $locale?->htmlDirection() ?? 'auto' }}"
class="{{ setting()->getForCurrentUser('dark-mode-enabled') ? 'dark-mode ' : '' }}">
<head>
<title>{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}</title>

View File

@ -1,5 +1,5 @@
<!doctype html>
<html lang="{{ $locale->htmlLang() }}">
<html lang="{{ $locale?->htmlLang() ?? config('app.default_locale') }}">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
<title>@yield('title')</title>

View File

@ -1,6 +1,6 @@
<!DOCTYPE html>
<html lang="{{ $locale->htmlLang() }}"
dir="{{ $locale->htmlDirection() }}"
<html lang="{{ $locale?->htmlLang() ?? config('app.default_locale') }}"
dir="{{ $locale?->htmlDirection() ?? 'auto' }}"
class="@yield('document-class')">
<head>
<title>{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}</title>

View File

@ -14,7 +14,7 @@
<div class="setting-list">
@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')])
</div>
<div class="form-group text-right">

View File

@ -16,7 +16,8 @@
<div class="grid half gap-xl">
<div>
<label for="user-avatar" class="setting-list-label">{{ trans('settings.users_avatar') }}</label>
<label for="user-avatar"
class="setting-list-label">{{ trans('settings.users_avatar') }}</label>
<p class="small">{{ trans('settings.users_avatar_desc') }}</p>
</div>
<div>
@ -33,13 +34,15 @@
</div>
</div>
@include('users.parts.language-option-row', ['value' => $user->getLanguage())])
@include('users.parts.language-option-row', ['value' => old('language') ?? $user->getLocale()->appLocale()])
</div>
<div class="text-right">
<a href="{{ url(userCan('users-manage') ? "/settings/users" : "/") }}" class="button outline">{{ trans('common.cancel') }}</a>
<a href="{{ url(userCan('users-manage') ? "/settings/users" : "/") }}"
class="button outline">{{ trans('common.cancel') }}</a>
@if($authMethod !== 'system')
<a href="{{ url("/settings/users/{$user->id}/delete") }}" class="button outline">{{ trans('settings.users_delete') }}</a>
<a href="{{ url("/settings/users/{$user->id}/delete") }}"
class="button outline">{{ trans('settings.users_delete') }}</a>
@endif
<button class="button" type="submit">{{ trans('common.save') }}</button>
</div>
@ -60,7 +63,8 @@
</div>
<div class="text-m-right">
@if($user->id === user()->id)
<a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('settings.users_mfa_configure') }}</a>
<a href="{{ url('/mfa/setup') }}"
class="button outline">{{ trans('settings.users_mfa_configure') }}</a>
@endif
</div>
</div>
@ -84,7 +88,8 @@
class="button small outline">{{ trans('settings.users_social_disconnect') }}</button>
</form>
@else
<a href="{{ url("/login/service/{$driver}") }}" aria-label="{{ trans('settings.users_social_connect') }} - {{ $driver }}"
<a href="{{ url("/login/service/{$driver}") }}"
aria-label="{{ trans('settings.users_social_connect') }} - {{ $driver }}"
class="button small outline">{{ trans('settings.users_social_connect') }}</a>
@endif
</div>

View File

@ -159,7 +159,7 @@ $style = [
<tr>
<td style="{{ $fontFamily }}">
<p style="{{ $style['paragraph-sub'] }}">
{{ trans('common.email_action_help', ['actionText' => $actionText], $language) }}
{{ $locale->trans('common.email_action_help', ['actionText' => $actionText]) }}
</p>
<p style="{{ $style['paragraph-sub'] }}">
@ -187,7 +187,7 @@ $style = [
<p style="{{ $style['paragraph-sub'] }}">
&copy; {{ date('Y') }}
<a style="{{ $style['anchor'] }}" href="{{ url('/') }}" target="_blank">{{ setting('app-name') }}</a>.
{{ trans('common.email_rights', [], $language) }}
{{ $locale->trans('common.email_rights') }}
</p>
</td>
</tr>

View File

@ -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()

View File

@ -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]');
}