[MURLTRANS-26] Use I18nAuthoringSupport Created: 10/Aug/22  Updated: 19/Aug/22  Resolved: 18/Aug/22

Status: Closed
Project: URL Translation
Component/s: None
Affects Version/s: 6.2.3
Fix Version/s: None

Type: Improvement Priority: Critical
Reporter: Björn Eschle Assignee: Richard Gange
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File deriveLocalisedPropertyName.png    
Issue Links:
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   
  • Use I18nAuthoringSupport.deriveLocalisedPropertyName instead of building the localized property name itself in the URLTranslator:

 

propertyName + "_" + locale.toString();
  • Use i18nContentSupport.getLocale instead of MgnlContext.getAggregationState().getLocale()
  • Make URLTranslator non static
    • Has to be done anyway to inject I18nAuthoringSupport
    • Possibility to bind custom implementation, which can be used to implement LiveCopy compatibility (fallbackLocale is always masterLocale)

 



 Comments   
Comment by Björn Eschle [ 15/Aug/22 ]

Background info:

URLTranslator needs to be bound differently in order to work for Apps using LiveCopy with sites with different defaultLocales, since LiveCopy is using the masterLocale (default locale of master site) to store the properties in the jcr (Pass master locale instead of fallback locale).

Comment by Richard Gange [ 18/Aug/22 ]

The problem here is that I18nAuthoringSupport has a dependency on the UI. It really doesn't make sense to create such a dependency on the UI simply to inject I18nAuthoringSupport. The method #deriveLocalisedPropertyName() is really just a fancy way to concatenate strings.

Therefore I will consider replacing the code:

propertyName + "_" + locale.toString();

to be

String.format("%s_%s", propertyName, locale.toString());

rather than go through the trouble of injecting a class for authoring specifically.

Other issues mentioned in the description can be spun off into new tickets.

Comment by Björn Eschle [ 19/Aug/22 ]

Hey @Richard,

I agree, it is a large dependency, that is added (although it is only UI - API). However we're overriding deriveLocalisedPropertyName to be able to configure the locales with country, but save them with only the language (not locale tag - lang-country).

I think we could work around this by removing the country from the locale, before passing it to the UrlTranslator, but I think the deriveLocalisedPropertyName should be used for all localizedPropertyName generations.

Thanks and kind regards,

Björn

Comment by Richard Gange [ 19/Aug/22 ]

I think in the long term we should strive for public instances which don't depend on the UI. Wouldn't it be nice if you could update the UI without having shut down the public instances? Or even having to shut down the author repo? If the repos themselves run in their own containers the UI webapp could run in its own container which would make it easier and faster to update the UI. For me this is the overarching goal.

Comment by Björn Eschle [ 19/Aug/22 ]

agreed, in this case we would simply need to extract this method to another interface, which is not in the UI but maybe in the core(-api) module?

Comment by Richard Gange [ 19/Aug/22 ]

It's kind of understood we don't adjust the core product to accommodate incubator modules. Hope that makes sense.

Comment by Björn Eschle [ 19/Aug/22 ]

This would not only benefit incubator modules, but all modules. Lot's of modules implement their own get localized property name method, which basically all depend on ui + other modules implementing it the same way. With an extracted interface this would be more compatible.

Comment by Richard Gange [ 19/Aug/22 ]

And all of those modules are UI modules. Here is the call hierarchy.

Generated at Mon Feb 12 11:08:52 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.