Bug 53532 - [5.0]: Add support for UMC translation file in ucs_registerLDAPExtension
[5.0]: Add support for UMC translation file in ucs_registerLDAPExtension
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: General
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0-0-errata
Assigned To: Christian Castens
Julia Bremer
:
Depends on: 53362
Blocks:
  Show dependency treegraph
 
Reported: 2021-06-30 14:12 CEST by Christian Castens
Modified: 2021-08-04 16:25 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Castens univentionstaff 2021-06-30 14:12:16 CEST
+++ This bug was initially created as a clone of Bug #53362 +++


The following changes of 4.4-8 have to be ported to UCS 5:

UCS 4.4-8:
f61968b46e8d3d9ec39dc924572e40cbfdf7cc46 - UMC translation file registration
c3748f701a1d1240056ff8629257e760a30308d1 - yamls

Packages changed:

Package: univention-lib
Version: 8.0.1-43A~4.4.0.202106291403
Branch: ucs_4.4-0
Scope: errata4.4-8

Package: univention-directory-manager-modules
Version: 14.0.20-12A~4.4.0.202106291412
Branch: ucs_4.4-0
Scope: errata4.4-8

Package: univention-ldap
Version: 15.0.3-4A~4.4.0.202106291430
Branch: ucs_4.4-0
Scope: errata4.4-8

Package: ucs-test
Version: 9.0.7-37A~4.4.0.202106291434
Branch: ucs_4.4-0
Scope: errata4.4-8


ucs_registerLDAPExtension from the Univention shell function library 
now allows the option "umcmessagecatalog". This option can be used to 
supply translation files in GNU message catalog format for the UMC.

Test 72_udm-extensions/33_umcmessagecatalog has been added.
Comment 1 Florian Best univentionstaff 2021-07-05 16:31:26 CEST
The changes look very good!

I am suggesting the following addition:

diff --git management/univention-directory-manager-modules/modules/univention/admin/syntax.py management/univention-directory-manager-modules/modules/univention/admin/syntax.py
index d7d2750c8f..deabbfc774 100644
--- management/univention-directory-manager-modules/modules/univention/admin/syntax.py
+++ management/univention-directory-manager-modules/modules/univention/admin/syntax.py
@@ -1082,11 +1082,12 @@ class UMCMessageCatalogFilename(string):
        """
 
        @classmethod
-       def parse(self, text):
+       def parse(cls, text):
+               text = string.parse(text)
                if '-' not in text:
-                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % str(text))
-               if not len(text.split('-')[0]) == 2:
-                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % str(text))
+                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % (text,))
+               if len(text.split('-')[0]) != 2:
+                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % (text,))
                return text

→ PEP8: "foo != bar" instead of "not foo == bar"
→ call super method, to ensure it's a string.
→ first argument of a classmethod should not be "self" (yes, other syntax classes are doing it wrong already)

TBD: what about handling None? (unset)
 

And after thinking a little bit, maybe:

 diff --git management/univention-directory-manager-modules/modules/univention/admin/syntax.py management/univention-directory-manager-modules/modules/univention/admin/syntax.py
index d7d2750c8f..481625b3fe 100644
--- management/univention-directory-manager-modules/modules/univention/admin/syntax.py
+++ management/univention-directory-manager-modules/modules/univention/admin/syntax.py
@@ -1082,11 +1082,11 @@ class UMCMessageCatalogFilename(string):
        """
 
        @classmethod
-       def parse(self, text):
-               if '-' not in text:
-                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % str(text))
-               if not len(text.split('-')[0]) == 2:
-                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. Must be the language code and UMCModuleID sperated by - %s') % str(text))
+       def parse(cls, text):
+               text = string.parse(text)
+               language_id, dash, module_id = text.partition('-')
+               if not dash:
+                       raise univention.admin.uexceptions.valueError(_('Not a valid filename for umcmessagecatalog. It must match "$language-$moduleid.mo" (e.g. "de-udm-foo.mo")'))
                return text
 
 
→ use str.partition() to enhance logic and remove duplicated error message
→ enhance the error message, give an example
→ don't print the acutal value in the error message, also for security reasons. if you want to return or log user input better do it with repr()
Comment 2 Julia Bremer univentionstaff 2021-07-08 08:55:44 CEST
Registration of umcmessagecatalog via joinscript: OK
multiple umcmessagecatalogs: OK
removing umcmessagecatalogs: OK
Setting univentionUMCMessageCatalog manually via LDAP: OK
.mo file is moved to the correct directory: OK
.mo file is removed correctly: OK
.mo file is encoded correctly in LDAP, UDM (binary, b64)
Translations of UMC module are working: OK
Automated test: OK
Changes for UCS5 port: OK
Code review: OK
YAML: OK

Verified