fix: General fixing and clean-up on `translate()` #1364

phorward · 2025-01-09T00:07:40Z

This pull request summarizes several problems:

This pull request summarizes several problems: 1. Let all ViUR-core related translations have a `viur.core`-tr_key-prefix 2. Make `tr_key` required 3. Provide a way for `conf.i18n.add_missing_translations` to only add or ingore a specific pattern (fnmatch)

src/viur/core/i18n.py

src/viur/core/bones/select.py

src/viur/core/modules/translation.py

- SelectBone now allowes for a separate `add_missing_translations`-parameter - `conf.i18n.add_missing_translations` can now be also an iterable of fnmatch-patterns - `translate`-class allowes for individual `add_missing`-flag

Conflicts: src/viur/core/i18n.py

sveneberth · 2025-01-25T03:46:34Z

src/viur/core/i18n.py

-                )
+            if self.key not in systemTranslations:
+                # either the translate()-object has add_missing set
+                if not (add_missing := self.add_missing):


I find this behaviour confusing.

So I set add_missing set to False, but if conf.i18n.add_missing_translations is True, it is added anyway.
If add_missing is set to True, it doesn't matter what is in conf.i18n.add_missing_translations and it's added too.

I would see this as a common condition:
If self.add_missing and conf.i18n.add_missing_translations are both True, add, otherwise not.

Hi @sveneberth, sorry for the confusion. The initial consideration about this flag was, that either the translate-object has the add_missing-flag on first order, and when this is not True, the globally defined logic of conf.i18n.add_missing_translations takes place, which can either be just True, but also one or many fnmatch-style pattern to match the given translation key.

it seems we have different use cases for this and other expections (you want an or logic, I an and). Let's talk about this in a meet.

As discussed yesterday, I have implemented Always and Never in 58d2e30.

Conflicts: src/viur/core/modules/translation.py

… fix-translations

phorward · 2025-02-17T20:29:44Z

@sveneberth I'm done with my requests.

Can you please add changes for your requests:

Don't add translation for error messages --> 6ecdfa8
Don't do a str-cast on an existing translate to use it as key for the new object --> 5fc22d9

This issue is urgent, waiting for approval.

For non-static errors with variables like ```py raise errors.NotFound(f"No valid root node found for {node=}") ``` a new `TranslationSkel` for each `node`-key would be added.

phorward · 2025-02-24T22:56:32Z

@sveneberth thanks, this looks fine so far. Can you please approve the PR, I cannot self-approve.

sveneberth · 2025-02-25T13:31:23Z

src/viur/core/modules/translation.py

+
+        # Create the key from the name on initial write!
+        if not skel["key"]:
+            skel["key"] = db.Key(KINDNAME, skel["name"].lower())


This would overwrite data.

If there's already a complete skeleton with the key "foo", and someone clicks on add and use the same key, it would overwrite the other skel without any warning etc.

I didn't tested this, and this is kinda hakkish, but this might solve this issue:

@classmethod def write(cls, skel, **kwargs): def write_with_claimed_key(): skel["key"] = db.Key(KINDNAME, skel["name"].lower()) if db.Get(skel["key"]): raise ValueError(f'An TranslationSkel with {skel["name"]=} exists already!') return super().write(skel, **kwargs) # Create the key from the name on initial write! if not skel["key"]: if db.IsInTransaction(): return write_with_claimed_key() else: return db.RunInTransaction(write_with_claimed_key) return super().write(skel, **kwargs)

src/viur/core/i18n.py

sveneberth · 2025-02-25T13:37:26Z

src/viur/core/modules/user.py

@@ -799,7 +799,7 @@ def start(self):
        return self._user_module.render.edit(
            self.OtpSkel(),
            params={
-                "name": i18n.translate(self.NAME),
+                "name": i18n.translate("viur.core.modules.user." + self.NAME),


Suggested change

"name": i18n.translate("viur.core.modules.user." + self.NAME),

"name": i18n.translate(f"viur.core.modules.user.{self.NAME}"),

and for others please use f-string too

sveneberth · 2025-02-25T13:39:53Z

src/viur/core/i18n.py

-                )
+            if self.key not in systemTranslations:
+                # either the translate()-object has add_missing set
+                if not (add_missing := self.add_missing):


it seems we have different use cases for this and other expections (you want an or logic, I an and). Let's talk about this in a meet.

Co-authored-by: Sven Eberth <[email protected]>

phorward added bug(fix) Something isn't working or address a specific issue or vulnerability annoying labels Jan 9, 2025

phorward added this to the ViUR-core v3.7 milestone Jan 9, 2025

phorward commented Jan 9, 2025

View reviewed changes

src/viur/core/i18n.py Outdated Show resolved Hide resolved

phorward commented Jan 9, 2025

View reviewed changes

src/viur/core/bones/select.py Outdated Show resolved Hide resolved

phorward commented Jan 9, 2025

View reviewed changes

src/viur/core/modules/translation.py Outdated Show resolved Hide resolved

phorward added 3 commits January 9, 2025 20:11

feat: Replace former get_public() by universal dump()

adf1330

fix linter issues

9ce7c94

fixed comment

13c3efc

phorward mentioned this pull request Jan 14, 2025

fix: determine a better path for a new TranslateSkel #1367

Merged

Improved add_missing_translations

16426e8

- SelectBone now allowes for a separate `add_missing_translations`-parameter - `conf.i18n.add_missing_translations` can now be also an iterable of fnmatch-patterns - `translate`-class allowes for individual `add_missing`-flag

phorward added the Priority: High After critical issues are fixed, these should be dealt with before any further issues. label Jan 23, 2025

sveneberth added 2 commits January 25, 2025 04:26

Merge branch 'main' into fix-translations

68a421b

Conflicts: src/viur/core/i18n.py

feat: Implement default_variables; Resolves viur-framework#1379

048ae0e

sveneberth reviewed Jan 25, 2025

View reviewed changes

sveneberth linked an issue Jan 25, 2025 that may be closed by this pull request

Implement default_variables in i18n.translate #1379

Open

Merge branch 'main' into fix-translations

6fd306b

phorward added Priority: Medium This issue may be useful, and needs some attention. and removed Priority: High After critical issues are fixed, these should be dealt with before any further issues. labels Feb 5, 2025

sveneberth and others added 9 commits February 7, 2025 21:36

Merge remote-tracking branch 'viur/main' into fix-translations

111a7a7

Conflicts: src/viur/core/modules/translation.py

feat: Add default_variables

032624c

Merge branch 'main' into fix-translations

d00560f

chore: necessary fixes

d5c2281

Merge branch 'fix-translations' of github.com:phorward/viur-core into…

f1be994

… fix-translations

feat: Allow to specify multiple patterns to Translate.dump()

a411e6c

chore: Rename "tr_key" into "name", inject "name" into "key"

424517b

feat: Additional Translation views for "viur.*" and public=True

bb77473

fix: Improved check using the translation key as key

f959f27

phorward removed the Priority: Medium This issue may be useful, and needs some attention. label Feb 17, 2025

phorward added proposal Priority: High After critical issues are fixed, these should be dealt with before any further issues. refactoring Pull requests that refactor code but do not change its behavior. labels Feb 17, 2025

phorward added 2 commits February 17, 2025 20:16

Merge branch 'main' into fix-translations

8d518a9

feat: dump_can_view-callback for translation.dump()

07b898c

phorward requested a review from sveneberth February 17, 2025 20:28

phorward marked this pull request as ready for review February 17, 2025 20:28

sveneberth and others added 3 commits February 22, 2025 01:47

Merge branch 'main' into fix-translations

68d607e

fix: Don't add missing translations for errors

6ecdfa8

For non-static errors with variables like ```py raise errors.NotFound(f"No valid root node found for {node=}") ``` a new `TranslationSkel` for each `node`-key would be added.

fix: Avoid to use a translated string as translate key

5fc22d9

sveneberth requested changes Feb 25, 2025

View reviewed changes

phorward and others added 2 commits February 26, 2025 14:44

Currently disable direct key retrieval

9f99d63

Co-authored-by: Sven Eberth <[email protected]>

feat: Implement and AddMissing type

58d2e30

phorward added the viur-meeting Issues to discuss in the next ViUR meeting label Feb 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: General fixing and clean-up on `translate()` #1364

fix: General fixing and clean-up on `translate()` #1364

phorward commented Jan 9, 2025 •

edited

Loading

sveneberth Jan 25, 2025

phorward Feb 17, 2025

sveneberth Feb 25, 2025 •

edited

Loading

sveneberth Feb 26, 2025

phorward commented Feb 17, 2025 •

edited by sveneberth

Loading

phorward commented Feb 24, 2025

sveneberth Feb 25, 2025

sveneberth Feb 26, 2025

sveneberth Feb 25, 2025

sveneberth Feb 25, 2025 •

edited

Loading

	"name": i18n.translate("viur.core.modules.user." + self.NAME),
	"name": i18n.translate(f"viur.core.modules.user.{self.NAME}"),

fix: General fixing and clean-up on translate() #1364

Are you sure you want to change the base?

fix: General fixing and clean-up on translate() #1364

Conversation

phorward commented Jan 9, 2025 • edited Loading

sveneberth Jan 25, 2025

Choose a reason for hiding this comment

phorward Feb 17, 2025

Choose a reason for hiding this comment

sveneberth Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

sveneberth Feb 26, 2025

Choose a reason for hiding this comment

phorward commented Feb 17, 2025 • edited by sveneberth Loading

phorward commented Feb 24, 2025

sveneberth Feb 25, 2025

Choose a reason for hiding this comment

sveneberth Feb 26, 2025

Choose a reason for hiding this comment

sveneberth Feb 25, 2025

Choose a reason for hiding this comment

sveneberth Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

fix: General fixing and clean-up on `translate()` #1364

fix: General fixing and clean-up on `translate()` #1364

phorward commented Jan 9, 2025 •

edited

Loading

sveneberth Feb 25, 2025 •

edited

Loading

phorward commented Feb 17, 2025 •

edited by sveneberth

Loading

sveneberth Feb 25, 2025 •

edited

Loading