[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: |
|
||||
| Issue Links: |
|
||||
| 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 |
propertyName + "_" + locale.toString();
|
| 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. |