[TECH SUPPORT] LPS-27239 #181

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants

Hey Máté,

This is kind of a bigger changes (including some API changes)... which is why I'm a little bit concerned when it comes to reviewing it.

6.0 didn't have localizable titles (they were not stored in XML format) and 6.1 automatically assumes that those titles belong to the system default locale instead of the article default locale.

Please let me know if you have any questions.

Daniel

Owner

danielreuther replied May 21, 2012

No, because that's the whole point of this fix. To use the article's default language and not the portal one.

do we need a whole new method overloading for this? can't we figure out the defaultLanguageId with LocaleUtil?

Owner

danielreuther replied May 21, 2012

The problem is that there's no way that this method can know the default language of a specific article. Which is why I created a new method that adds the defaultLanguage as a parameter; so that JournalArticleModelImpl can pass the article's default locale to the method.

Hi Daniel,

I've reproduced the error this fix is solving, but I had a few questions. Can you please take a look on them?

Thanks,

Máté

Hey Máté,

I tried to answer the two line comments. I actually don't think there are easier ways to achieve that; apart from maybe prepopulating the title map for all avalaible locales (which would be an option, but probably not a good one).
I definitely consider using the portal default locale there a bug; it is after all the article's language that matters.

Thanks for having had a look at it!
Daniel

Hi Máté,

Any news on that one?

Daniel

Owner

matethurzo commented May 30, 2012

Hi Daniel,

Sorry for the delay, we have received some serious customer problems, and I'm working on those escalations. I've forwarded this to @kocsisdaniel so he can review it now: KocsisDaniel#34

Thanks,

Máté

@matethurzo matethurzo closed this May 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment