AUT-3675: make translation with new prefix #4035

SultanSagi · 2024-06-04T09:37:03Z

Related to: https://oat-sa.atlassian.net/browse/AUT-3675

Summary

It makes translations with new prefix

To create and update new local files:

// from Docker container

php  tao/scripts/taoTranslate.php -v -a=create -e=taoQtiItem -l=en-us```

github-actions · 2024-06-04T09:38:11Z

Front-end summary Node 18

💯 Total	✅ Passed	⏭️ Skipped	❌ Failed
161	161	0	0

…logy Feature/aut 3624/add new terminology

bartlomiejmarszal · 2024-06-13T07:35:06Z

models/classes/class.LanguageService.php

+            $pattern = '/' . self::LANG_PREFIX . '$/';
+            if (!preg_match($pattern, $localeDir, $matches)) {
+                $path = $localesPath . '/' . $localeDir;
+                if ($localeDir[0] != '.' && @is_dir($path)) {
+                    // Look if the lang.rdf can be read.
+                    $languageModelFile = $path . '/lang.rdf';
+                    if (@file_exists($languageModelFile) && @is_readable($languageModelFile)) {
+                        $files[] = $languageModelFile;
+                    }


This may impact existing functionality. I recommend to extract this business logic into different service and unit service. I also recommend when replacing business logic here to use proxy approach maybe including Feature Flag to keep it safe

to clarify what do you mean with Business logic? Only my added if condition or whole logic inside foreach?

Not sure I understand this answer. You are adding new business logic into existing service. This will impact other parts of the code that is using models/classes/class.LanguageService.php. You should make sure it will not brake anything and for this you need to isolate impact of your change. I would recommend to create new Sprout class with method getLanguageFiles overloaded.

bartlomiejmarszal · 2024-06-13T07:36:04Z

models/classes/clientConfig/ClientConfigStorage.php

+    private 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;


Please extract this to exteranal service and unit test it.

scripts/class.TaoTranslate.php

marpesia · 2024-06-13T08:12:05Z

I'm still confused about mapClassesNames method

Please, re-check again @bartlomiejmarszal

bartlomiejmarszal

Missing empty lines on the end of php files

helpers/translation/SolarThemeHelper.php

bartlomiejmarszal · 2024-06-17T07:37:15Z

helpers/mapLabelNameService.php

+        }
+        return $mapName;
+    }
+}


Suggested change

}

}

pribi · 2024-06-21T10:22:30Z

Please resolve the merge conflicts.

Conflicts: views/js/loader/vendor.es5.min.js views/js/loader/vendor.es5.min.js.map views/js/loader/vendor.min.js.map

marpesia · 2024-06-21T10:40:02Z

Please resolve the merge conflicts.

Fixed @pribi

bartlomiejmarszal · 2024-06-21T11:02:38Z

helpers/translation/AbstractSolarThemeHelper.php

+     *
+     */
+    abstract public function checkPrefix(string $language): string;
+}


Empty line on the end of line

fixed @bartlomiejmarszal

github-actions · 2024-06-21T11:06:11Z

Version

Target Version	54.15.0
Last version	54.14.6

There are 0 BREAKING CHANGE, 12 features, 1 fix

bartlomiejmarszal · 2024-06-21T11:06:17Z

scripts/taoTranslate.php

This file change is not well readable. This is just reformatting? Could you skip reformatting for this?

There are new code added at the bottom of the array, though.
I can move the first square bracket to the line 29. Is that ok?

helpers/mapLabelNameService.php

bartlomiejmarszal · 2024-06-21T11:18:05Z

helpers/translation/AbstractSolarThemeHelper.php

+     * Concatenate prefix for Solar design translations
+     *
+     */
+    protected function addPrefix(string $language): string


You actually adding postfix

bartlomiejmarszal · 2024-06-21T11:21:17Z

actions/class.RdfController.php

@@ -467,7 +468,8 @@ public function editClassLabel()
            $this->setData('reload', true);
        }

-        $this->setData('formTitle', __('Edit class %s', \tao_helpers_Display::htmlize($class->getLabel())));
+        $getLabel = MapLabelNameService::mapLabelName($class->getLabel());


Why don't you call it as you would call service?

bartlomiejmarszal · 2024-06-21T11:22:18Z

helpers/translation/AbstractSolarThemeHelper.php

+    public function __construct(
+        LayoutHelper $layoutHelper
+    ) {
+        $this->layoutHelper = $layoutHelper;
+    }


Suggested change

public function __construct(

LayoutHelper $layoutHelper

) {

$this->layoutHelper = $layoutHelper;

}

public function __construct(LayoutHelper $layoutHelper) {

$this->layoutHelper = $layoutHelper;

}

bartlomiejmarszal · 2024-06-21T11:23:04Z

helpers/mapLabelNameService.php

+ namespace oat\tao\helpers;
+ use oat\tao\helpers\Layout;
+
+ class MapLabelNameService


I'm still not understand purpose of this class

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.

Could you point me the UI that will be impacted by your change?

I was reviewing this with @pribi and I think I understand purpose of this.
Looking into usage of actions/class.RdfController.php you will make a change translation for Items.

I understand that Items now should be renamed? WIll we rename root class Item? Do we want to apply this change everywhere? To each client and each instance?

The problem is that the new name is ONLY when the SolarUI Theme is applied.
Shall we continue these comments on the new PRs?
Locales only -> #4052
BE logic -> #4053
BE to change FE -> #4054
CSS FE fix -> #4055

bartlomiejmarszal · 2024-06-21T11:39:54Z

scripts/class.TaoTranslate.php

@@ -107,6 +110,7 @@ class tao_scripts_TaoTranslate extends tao_scripts_Runner
    protected $verbose = false;
    // --- OPERATIONS ---

+    private ?AbstractSolarThemeHelper $solarThemeHelper;


How this could be null? It is defined in ServiceProvider.

bartlomiejmarszal · 2024-06-21T13:00:12Z

helpers/class.I18n.php

+     */
+    private static function checkPrefix(string $language): string
+    {
+        if (!self::isContainPrefix($language)) {


Should this be under feature flag?

Feature flag is called inside isContainPrefix

bartlomiejmarszal · 2024-06-21T13:00:28Z

helpers/class.I18n.php

+     * Check and Add prefix for Solar design translations
+     *
+     */
+    private static function checkPrefix(string $language): string


Why is it static?

Because this class itself initiates statically. It's not using construct method

bartlomiejmarszal · 2024-06-21T13:01:24Z

helpers/class.I18n.php

+    }
+
+    /**
+     * Check and Add prefix for Solar design translations


What is this check about? If I check something and it fails I throw an error and you seem to modify return

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 is_dir. If the comment doesn't fit, is there any suggestion on how to change it?

feat: make translation with new prefix

617b0f9

marpesia requested review from marpesia and pribi June 4, 2024 10:06

SultanSagi changed the title ~~AUT-3765: make translation with new prefix~~ draft: AUT-3765: make translation with new prefix Jun 5, 2024

SultanSagi and others added 6 commits June 5, 2024 17:20

feat: make compile action

5cacb4b

feat: add mapClassesName helper to translate new terms

942d2a6

feat: use helper to map and translate new terms

a04e9b1

feat: map and translate new terms on tree panel

a4e7f5f

feat: add new terms locale

e2c8cf2

Merge pull request #4037 from oat-sa/feature/AUT-3624/add-new-termino…

f015a21

…logy Feature/aut 3624/add new terminology

SultanSagi added 4 commits June 7, 2024 11:05

chore: translate syntax

aa780ee

feat: set translation json regarding lang code

b388e44

feat: fix test mock

eabb184

fix: test mock

f429772

bartlomiejmarszal reviewed Jun 13, 2024

View reviewed changes

scripts/class.TaoTranslate.php Outdated Show resolved Hide resolved

bartlomiejmarszal reviewed Jun 13, 2024

View reviewed changes

scripts/class.TaoTranslate.php Show resolved Hide resolved

chore: refactor map label function

6e188af

refactor: extract to Solar theme helper

b4c5faa

bartlomiejmarszal requested changes Jun 17, 2024

View reviewed changes

SultanSagi added 2 commits June 17, 2024 19:10

chore: remove extra lines

8ee971a

feat: use common Theme helper

d28dca7

Merge branch 'develop' into feat/AUT-3675/new-prefix-translate

62d8ffd

Conflicts: views/js/loader/vendor.es5.min.js views/js/loader/vendor.es5.min.js.map views/js/loader/vendor.min.js.map

marpesia requested a review from bartlomiejmarszal June 21, 2024 10:38

bartlomiejmarszal reviewed Jun 21, 2024

View reviewed changes

chore: add empty line on AbstractSolarThemeHelper

97a942b

marpesia requested a review from bartlomiejmarszal June 21, 2024 11:05

bartlomiejmarszal reviewed Jun 21, 2024

View reviewed changes

helpers/mapLabelNameService.php Show resolved Hide resolved

bartlomiejmarszal reviewed Jun 21, 2024

View reviewed changes

SultanSagi changed the title ~~draft: AUT-3675: make translation with new prefix~~ AUT-3675: make translation with new prefix Jun 21, 2024

bartlomiejmarszal reviewed Jun 21, 2024

View reviewed changes

marpesia closed this Jun 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AUT-3675: make translation with new prefix #4035

AUT-3675: make translation with new prefix #4035

SultanSagi commented Jun 4, 2024 •

edited by marpesia

Loading

github-actions bot commented Jun 4, 2024 •

edited

Loading

bartlomiejmarszal Jun 13, 2024

SultanSagi Jun 13, 2024

bartlomiejmarszal Jun 17, 2024

bartlomiejmarszal Jun 13, 2024

SultanSagi Jun 21, 2024

marpesia commented Jun 13, 2024

bartlomiejmarszal left a comment

bartlomiejmarszal Jun 17, 2024

pribi commented Jun 21, 2024

marpesia commented Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

marpesia Jun 21, 2024

github-actions bot commented Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

marpesia Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 23, 2024

bartlomiejmarszal Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 23, 2024

bartlomiejmarszal Jun 21, 2024

marpesia Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

marpesia Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 21, 2024

bartlomiejmarszal Jun 21, 2024

SultanSagi Jun 23, 2024

AUT-3675: make translation with new prefix #4035

AUT-3675: make translation with new prefix #4035

Conversation

SultanSagi commented Jun 4, 2024 • edited by marpesia Loading

Related to: https://oat-sa.atlassian.net/browse/AUT-3675

Summary

github-actions bot commented Jun 4, 2024 • edited Loading

Front-end summary Node 18

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marpesia commented Jun 13, 2024

bartlomiejmarszal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pribi commented Jun 21, 2024

marpesia commented Jun 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 21, 2024

Version

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SultanSagi commented Jun 4, 2024 •

edited by marpesia

Loading

github-actions bot commented Jun 4, 2024 •

edited

Loading