OHRM5X-2480: Develop language package validation #1835

TharakaAthukorala · 2024-04-05T04:00:27Z

No description provided.

devishke-orange · 2024-04-05T04:18:37Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+              Use the sample template
+            </oxd-text>
+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Only edit the target field.
+            </oxd-text>
+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Do not change the template.
+            </oxd-text>
+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Sample XLIFF :


We need to add these as lang strings. Check these references:

create .yaml and add lang strings: https://github.com/orangehrm/orangehrm/blob/main/installer/Migration/V5_6_0/lang-string/admin.yaml

add insert code to Migration.php: https://github.com/orangehrm/orangehrm/blob/main/installer/Migration/V5_6_0/Migration.php#L44-L49

devishke-orange · 2024-04-05T04:19:28Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+      rules: {
+        attachment: [
+          required,
+          maxFileSize(1048576),


@RajithaKumara we might have to implement some way to increase max file size in this release since we have had people asking before.

check similar place and get as a config file

devishke-orange · 2024-04-05T04:25:02Z

src/client/src/orangehrmAdminPlugin/pages/languagePackage/LanguagePackage.vue

+              props: {
+                label: 'Import',
+                style: 'Text',
+                displayType: 'text',


we will change this to an icon later

devishke-orange · 2024-04-05T04:27:56Z

src/plugins/orangehrmAdminPlugin/Api/I18NTranslationImportAPI.php

+    public const PARAMETER_LANGUAGE_ID = 'languageId';
+    public const PARAMETER_ATTACHMENT = 'attachment';
+
+    public const PARAM_RULE_IMPORT_FILE_FORMAT = ['text/xml', 'application/xml', 'application/xliff+xml'];


@TharakaAthukorala is it possible for .xlf extension to take MIME types other than application/xliff+xml

If so we might have to apply sanitization here

The MIME type application/xliff+xml is the standard MIME type for .xlf files.

For .xlf files, it's theoretically possible for other MIME types to be associated with this extension.

.xliff seems this also valid extension

devishke-orange · 2024-04-05T04:29:54Z

src/plugins/orangehrmAdminPlugin/Api/I18NTranslationImportAPI.php

+    public const PARAMETER_ATTACHMENT = 'attachment';
+
+    public const PARAM_RULE_IMPORT_FILE_FORMAT = ['text/xml', 'application/xml', 'application/xliff+xml'];
+    public const PARAM_RULE_IMPORT_FILE_EXTENSIONS = ['xml', 'xlf'];


@RajithaKumara I feel we should only allow .xlf. Tharaka has changed to export to give an .xlf file as well. Not sure if we will have additional security concerns if we allow .xml.

devishke-orange · 2024-04-05T04:30:46Z

src/plugins/orangehrmAdminPlugin/Api/I18NTranslationImportAPI.php

@@ -0,0 +1,158 @@
+<?php


@TharakaAthukorala you need to add a new data group for this API in the Migration

devishke-orange · 2024-04-05T05:06:51Z

src/plugins/orangehrmAdminPlugin/Api/I18NTranslationImportAPI.php

+     * @inheritDoc
+     * @throws InvalidParamException
+     * @throws \Exception


remove exceptions

Suggested change

* @inheritDoc

* @throws InvalidParamException

* @throws \Exception

* @inheritDoc

devishke-orange · 2024-04-05T05:17:50Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

@@ -230,23 +233,26 @@ private function getXliffXmlSources(array $i18nGroups, I18NLanguage $language):
        $root->setAttribute('srcLang', 'en_US');
        $root->setAttribute('trgLang', $language->getCode());
        $root->setAttribute('xmlns', 'urn:oasis:names:tc:xliff:document:2.0');
-        $root->setAttribute('date', @date('Y-m-d\TH:i:s\Z'));
+//        $root->setAttribute('date', @date('Y-m-d\TH:i:s\Z'));


date attribute is not allowed right? we can remove this line

https://docs.oasis-open.org/xliff/xliff-core/v2.0/xliff-core-v2.0.html

devishke-orange · 2024-04-05T05:22:47Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

+        $document = new \DOMDocument();
+        $document->loadXML($content);
+
+        if (null !== $targetLanguage = $this->getTargetLanguageFromFile($document)) {
+            $normalizedLocalePattern = sprintf(
+                '(%s|%s)',
+                preg_quote($targetLanguage, '/'),
+                preg_quote(str_replace('-', '_', $targetLanguage), '/')
+            );
+            // strict file names require translation files to be named '____.locale.xlf'
+            $expectedFilenamePattern = sprintf('/^.*\.(?i:%s)\.(?:xlf|xliff)/', $normalizedLocalePattern);
+
+            if (0 === preg_match($expectedFilenamePattern, basename($file))) {
+                $errors[] = [
+                    'line' => -1,
+                    'column' => -1,
+                    'message' => sprintf(
+                        'There is a mismatch between the language included in the file name ("%s") and the "%s" value used in the "target-language" attribute of the file.',
+                        basename($file),
+                        $targetLanguage
+                    ),
+                ];
+            }
+        }
+
+        foreach (XliffUtils::validateSchema($document) as $xmlError) {
+            $errors[] = [
+                'line' => $xmlError['line'],
+                'column' => $xmlError['column'],
+                'message' => $xmlError['message'],
+            ];
+        }
+
+        libxml_clear_errors();
+        libxml_use_internal_errors($internal);
+
+        return ['file' => $file, 'isValid' => 0 === \count($errors), 'messages' => $errors];


just noting down that code taken from XliffLintCommand in symfony/translations
https://github.com/symfony/translation/blob/5.4/Command/XliffLintCommand.php#L123-L156

We'll have to discuss this a bit. Not sure if we need the filename checking part.

Better to create a utility class for XliffUtils for now. Reference: https://github.com/orangehrm/orangehrm/blob/main/src/plugins/orangehrmCorePlugin/Utility/Sanitizer.php

devishke-orange · 2024-04-05T05:24:17Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

+        $units = $xliffDocument->getElementsByTagName('unit');
+
+        // Define validation constraints
+        $maxLength = 255;


how did you decide this value?

need to discuss the max char length for source and target XLIFFs. If there is no limit we can remove this length validation.

RajithaKumara

API tests are pending

RajithaKumara · 2024-04-09T05:46:46Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+  <div class="orangehrm-background-container">
+    <div class="orangehrm-card-container">
+      <oxd-text class="orangehrm-main-title">
+        Import Language Package: {{ languageName }}


RajithaKumara · 2024-04-09T05:46:56Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+        <ul>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Use the sample template.


RajithaKumara · 2024-04-09T05:47:03Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Only edit the target field.


RajithaKumara · 2024-04-09T05:47:21Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Do not change the template.


Handle these language strings

RajithaKumara · 2024-04-09T05:47:28Z

src/client/src/orangehrmAdminPlugin/pages/languageImport/LanguageImport.vue

+          </li>
+          <li>
+            <oxd-text class="orangehrm-information-card-text">
+              Sample XLIFF :


RajithaKumara · 2024-04-09T05:58:19Z

src/plugins/orangehrmAdminPlugin/Api/I18NTranslationImportAPI.php

+    public const PARAMETER_LANGUAGE_ID = 'languageId';
+    public const PARAMETER_ATTACHMENT = 'attachment';
+
+    public const PARAM_RULE_IMPORT_FILE_FORMAT = ['text/xml', 'application/xml', 'application/xliff+xml'];


.xliff seems this also valid extension

RajithaKumara · 2024-04-09T06:05:58Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php


        $file = $xml->createElement('file');
+        $file->setAttribute('id', 1);


@devishke-orange check whether we have better id for this

RajithaKumara · 2024-04-09T06:07:37Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

@@ -270,4 +275,165 @@ private function getXliffXmlSources(array $i18nGroups, I18NLanguage $language):
        }
        return $xml;
    }
+
+    public function symfonyXliffValidations(string $content, ?string $file = null): array


add doc comment

RajithaKumara · 2024-04-09T06:15:25Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

+            $unitErrors = "";
+
+            // Validate source text
+            if (strlen($source) > $maxLength) {


Suggested change

if (strlen($source) > $maxLength) {

if ($this->getTextHelper()->strLength($source, '8bit') > $maxLength) {

reference: https://github.com/orangehrm/orangehrm/blob/main/src/plugins/orangehrmCorePlugin/Api/V2/Validator/Rules/Base64Attachment.php#L98

RajithaKumara · 2024-04-09T06:15:37Z

src/plugins/orangehrmAdminPlugin/Service/LocalizationService.php

+            }
+
+            // Validate target text
+            if (strlen($target) > $maxLength) {


…t the code

…test failure

…e unwanted langstrings

…erateCacheKey() into LocalizationService

This was referenced Apr 5, 2024

OHRM5X-2484: Develop admin - add getOne/delete to langauge API #1834

Merged

OHRM5X-2480: Develop language package validation #1825

Closed

TharakaAthukorala force-pushed the 5.7-old branch from 73f3852 to 8c57ace Compare April 5, 2024 04:56

TharakaAthukorala changed the base branch from 5.7-old to 5.7 April 5, 2024 04:58

TharakaAthukorala changed the title ~~5.7 old~~ OHRM5X-2480: Develop language package validation Apr 5, 2024

devishke-orange requested changes Apr 5, 2024

View reviewed changes

RajithaKumara requested changes Apr 9, 2024

View reviewed changes

TharakaAthukorala added 16 commits May 10, 2024 16:11

OHRM5X-2480: Develop language package validation

0ae696c

OHRM5X-2480: Develop language package validation

a704905

OHRM5X-2483: Add delete icon to language package list

95137fa

OHRM5X-2483: Add delete new icon to language package list

e0c9200

OHRM5X-2483: Add delete new icon to language package list and reforma…

ecfb4c1

…t the code

OHRM5X-2483: Add delete new icon to language package list and reforma…

19d8cb5

…t the code

OHRM5X-2480: Develop language package validation

c76a904

OHRM5X-2480: Develop language package validation fixed installation

8c0d237

OHRM5X-2481: Develop language package import

7dbd062

OHRM5X-2476: Add import button to language package list

78998ec

OHRM5X-2476: Add import button to language package list

516d4e4

OHRM5X-2476: Add icon buttons to language package list

443f46d

OHRM5X-2479: Develop language package import screen

7ffc1dd

Language import screen alignment fixing for 'Actions'

a310587

OHRM5X-2477: Change the note in the "Add Language Pack" modal

4db3c39

OHRM5X-2477: Change the note in the "Add Language Pack" modal & Unit …

38b3570

…test failure

TharakaAthukorala force-pushed the 5.7-old branch from 80b16b5 to 38b3570 Compare May 10, 2024 10:42

TharakaAthukorala added 5 commits May 10, 2024 16:46

OHRM5X-2477: Change the note in the "Add Language Pack" modal & delet…

7b27a78

…e unwanted langstrings

OHRM5X-2480: Refactor Xliff validation API

1aea84a

OHRM5X-2480: Refactor Xliff validation API & code comments & move gen…

f75bd9d

…erateCacheKey() into LocalizationService

OHRM5X-2479: Develop language string import modal

11bba6d

Unit tests for I18NTranslationImportAPI and LocalizationService

0856826

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OHRM5X-2480: Develop language package validation #1835

OHRM5X-2480: Develop language package validation #1835

TharakaAthukorala commented Apr 5, 2024

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

devishke-orange Apr 5, 2024

RajithaKumara Apr 9, 2024

devishke-orange Apr 5, 2024

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

RajithaKumara Apr 9, 2024

devishke-orange Apr 5, 2024

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

devishke-orange Apr 5, 2024 •

edited

devishke-orange Apr 5, 2024

TharakaAthukorala Apr 7, 2024

RajithaKumara left a comment

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024

RajithaKumara Apr 9, 2024


		$file = $xml->createElement('file');
		$file->setAttribute('id', 1);

	if (strlen($source) > $maxLength) {
	if ($this->getTextHelper()->strLength($source, '8bit') > $maxLength) {

OHRM5X-2480: Develop language package validation #1835

Are you sure you want to change the base?

OHRM5X-2480: Develop language package validation #1835

Conversation

TharakaAthukorala commented Apr 5, 2024

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

devishke-orange Apr 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RajithaKumara left a 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

devishke-orange Apr 5, 2024 •

edited