-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AUT-3675: make translation with new prefix #4035
Changes from all commits
617b0f9
5cacb4b
942d2a6
a04e9b1
a4e7f5f
e2c8cf2
f015a21
aa780ee
b388e44
eabb184
f429772
40a91ba
3c36d23
fdfb9b8
522a003
4218c85
e2dfd84
c19b6f9
f3d24ac
8954b07
762b7d7
12aea03
6e188af
b4c5faa
8ee971a
d28dca7
62d8ffd
97a942b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
/** | ||
* This program is free software; you can redistribute it and/or | ||
* modify it under the terms of the GNU General Public License | ||
* as published by the Free Software Foundation; under version 2 | ||
* of the License (non-upgradable). | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program; if not, write to the Free Software | ||
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
* | ||
* Copyright (c) 2024 (original work) Open Assessment Technologies SA; | ||
* | ||
* | ||
*/ | ||
|
||
namespace oat\tao\helpers; | ||
|
||
class LayoutHelper | ||
{ | ||
public function isSolarDesignEnabled(): bool | ||
bartlomiejmarszal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
return Layout::isSolarDesignEnabled(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is some kind o proxy? I could not find this reference There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was problematic to test a static method. So I wrapped it in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And it was less problematic when wrapped? Where have you test it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I didn't add new test, but it triggers here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as a Mock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How could you test a class by mocking it only? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
*/ | ||
|
||
use oat\generis\model\OntologyRdf; | ||
use oat\tao\helpers\Layout; | ||
|
||
/** | ||
* Internationalization helper. | ||
|
@@ -40,6 +41,12 @@ class tao_helpers_I18n | |
*/ | ||
public const AVAILABLE_LANGS_CACHEKEY = 'i18n_available_langs'; | ||
|
||
/** | ||
* locale prefix for New theme | ||
* | ||
*/ | ||
public const LANG_PREFIX = '-S'; | ||
|
||
/** | ||
* Short description of attribute availableLangs | ||
* | ||
|
@@ -60,6 +67,7 @@ public static function init(common_ext_Extension $extension, ?string $langCode): | |
if (empty($langCode)) { | ||
throw new Exception("Language is not defined"); | ||
} | ||
$langCode = self::checkPrefix($langCode); | ||
|
||
//init the ClearFw l10n tools | ||
$translations = tao_models_classes_LanguageService::singleton()->getServerBundle($langCode); | ||
|
@@ -78,7 +86,7 @@ public static function init(common_ext_Extension $extension, ?string $langCode): | |
*/ | ||
public static function getLangCode() | ||
{ | ||
return common_session_SessionManager::getSession()->getInterfaceLanguage(); | ||
return self::checkPrefix(common_session_SessionManager::getSession()->getInterfaceLanguage()); | ||
} | ||
|
||
/** | ||
|
@@ -193,4 +201,41 @@ public static function getAvailableLangsByUsage(core_kernel_classes_Resource $us | |
} | ||
return $returnValue; | ||
} | ||
|
||
/** | ||
* Check if the Solar design is enabled and the prefix has not yet been added | ||
* | ||
*/ | ||
private static function isContainPrefix(string $language): bool | ||
{ | ||
$pattern = '/' . self::LANG_PREFIX . '$/'; | ||
|
||
return !Layout::isSolarDesignEnabled() || preg_match($pattern, $language, $matches); | ||
} | ||
|
||
/** | ||
* Concatenate prefix for Solar design translations | ||
* | ||
*/ | ||
private static function addPrefix(string $language): string | ||
{ | ||
return $language . self::LANG_PREFIX; | ||
} | ||
|
||
/** | ||
* Check and Add prefix for Solar design translations | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this check about? If I check something and it fails I throw an error and you seem to modify return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the ticket there is a condition to check for a new translation, if there is no new translation then use a normal translation. That's why I check via |
||
* | ||
*/ | ||
private static function checkPrefix(string $language): string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is it static? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this class itself initiates statically. It's not using construct method |
||
{ | ||
if (!self::isContainPrefix($language)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be under feature flag? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feature flag is called inside |
||
$localesDir = 'views/locales'; | ||
$dir = dirname(__FILE__) . '/../' . $localesDir . '/' . self::addPrefix($language); | ||
if (is_dir($dir)) { | ||
$language = self::addPrefix($language); | ||
} | ||
} | ||
|
||
return $language; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||||
<?php | ||||||||
|
||||||||
/** | ||||||||
* This program is free software; you can redistribute it and/or | ||||||||
* modify it under the terms of the GNU General Public License | ||||||||
* as published by the Free Software Foundation; under version 2 | ||||||||
* of the License (non-upgradable). | ||||||||
* | ||||||||
* This program is distributed in the hope that it will be useful, | ||||||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||||||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||||||||
* GNU General Public License for more details. | ||||||||
* | ||||||||
* You should have received a copy of the GNU General Public License | ||||||||
* along with this program; if not, write to the Free Software | ||||||||
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||||||||
* | ||||||||
* Copyright (c) 2024 (original work) Open Assessment Technologies SA ; | ||||||||
*/ | ||||||||
|
||||||||
namespace oat\tao\helpers; | ||||||||
use oat\tao\helpers\Layout; | ||||||||
|
||||||||
class MapLabelNameService | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still not understand purpose of this class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Several places in the UI take the Label name from the PHP code. To set the correct Label for the translation, I need to map the PHP names with the translation IDs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you point me the UI that will be impacted by your change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was reviewing this with @pribi and I think I understand purpose of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||
{ | ||||||||
|
||||||||
private const ITEM = 'Item'; | ||||||||
private const MEDIA = 'Media'; | ||||||||
private const DELIVERY = 'Delivery'; | ||||||||
private const ASSETS = 'Assets'; | ||||||||
|
||||||||
// New terms for isSolarDesignEnabled FF | ||||||||
private $mapLabelNames = [ | ||||||||
self::ITEM => 'Item', | ||||||||
self::MEDIA => 'Asset', | ||||||||
self::DELIVERY => 'Delivery', | ||||||||
self::ASSETS => 'Asset' | ||||||||
]; | ||||||||
|
||||||||
/** | ||||||||
* @param $labelName | ||||||||
* @return string | ||||||||
*/ | ||||||||
public function mapLabelName(string $labelName): string | ||||||||
{ | ||||||||
$mapName = $labelName; | ||||||||
pribi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
if (Layout::isSolarDesignEnabled()) { | ||||||||
$mapName = __(self::$mapLabelNames[$labelName]); | ||||||||
} | ||||||||
return $mapName; | ||||||||
} | ||||||||
} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,62 @@ | ||||||||||||||||||
<?php | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* This program is free software; you can redistribute it and/or | ||||||||||||||||||
* modify it under the terms of the GNU General Public License | ||||||||||||||||||
* as published by the Free Software Foundation; under version 2 | ||||||||||||||||||
* of the License (non-upgradable). | ||||||||||||||||||
* | ||||||||||||||||||
* This program is distributed in the hope that it will be useful, | ||||||||||||||||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||||||||||||||||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||||||||||||||||||
* GNU General Public License for more details. | ||||||||||||||||||
* | ||||||||||||||||||
* You should have received a copy of the GNU General Public License | ||||||||||||||||||
* along with this program; if not, write to the Free Software | ||||||||||||||||||
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||||||||||||||||||
* | ||||||||||||||||||
* Copyright (c) 2024 (original work) Open Assessment Technologies SA ; | ||||||||||||||||||
*/ | ||||||||||||||||||
|
||||||||||||||||||
namespace oat\tao\helpers\translation; | ||||||||||||||||||
|
||||||||||||||||||
use oat\tao\helpers\LayoutHelper; | ||||||||||||||||||
|
||||||||||||||||||
abstract class AbstractSolarThemeHelper | ||||||||||||||||||
{ | ||||||||||||||||||
public const LANG_PREFIX = '-S'; | ||||||||||||||||||
|
||||||||||||||||||
private LayoutHelper $layoutHelper; | ||||||||||||||||||
|
||||||||||||||||||
public function __construct( | ||||||||||||||||||
LayoutHelper $layoutHelper | ||||||||||||||||||
) { | ||||||||||||||||||
$this->layoutHelper = $layoutHelper; | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Check if the Solar design is enabled and the prefix has not yet been added | ||||||||||||||||||
* | ||||||||||||||||||
*/ | ||||||||||||||||||
public function isContainPrefix(string $language): bool | ||||||||||||||||||
{ | ||||||||||||||||||
$pattern = '/' . self::LANG_PREFIX . '$/'; | ||||||||||||||||||
|
||||||||||||||||||
return !$this->layoutHelper->isSolarDesignEnabled() || preg_match($pattern, $language, $matches); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Concatenate prefix for Solar design translations | ||||||||||||||||||
* | ||||||||||||||||||
*/ | ||||||||||||||||||
protected function addPrefix(string $language): string | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You actually adding postfix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||||||||||||||||||
{ | ||||||||||||||||||
return $language . self::LANG_PREFIX; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Check and add prefix for Solar design translations | ||||||||||||||||||
* | ||||||||||||||||||
*/ | ||||||||||||||||||
abstract public function checkPrefix(string $language): string; | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
<?php | ||
|
||
/** | ||
* This program is free software; you can redistribute it and/or | ||
* modify it under the terms of the GNU General Public License | ||
* as published by the Free Software Foundation; under version 2 | ||
* of the License (non-upgradable). | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program; if not, write to the Free Software | ||
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
* | ||
* Copyright (c) 2024 (original work) Open Assessment Technologies SA ; | ||
*/ | ||
|
||
namespace oat\tao\helpers\translation; | ||
|
||
class SolarThemeFileHelper extends AbstractSolarThemeHelper | ||
{ | ||
/** | ||
* Check and add prefix for Solar design translations | ||
* | ||
*/ | ||
public function checkPrefix(string $language): string | ||
{ | ||
if (!$this->isContainPrefix($language)) { | ||
$localesDir = 'views/locales'; | ||
$dir = dirname(__FILE__) . '/../../' . $localesDir . '/' . $this->addPrefix($language); | ||
if (is_dir($dir)) { | ||
$language = $this->addPrefix($language); | ||
} | ||
} | ||
|
||
return $language; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you call it as you would call service?