generator-jhipster icon indicating copy to clipboard operation
generator-jhipster copied to clipboard

Remove LocaleConfiguration

Open shrralis opened this issue 2 years ago • 10 comments

remove LocaleConfiguration due to being a leftover from angularjs

Fix #20106


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

shrralis avatar Oct 11 '23 12:10 shrralis

Not sure why it's failing

shrralis avatar Oct 11 '23 13:10 shrralis

@shrralis we are using snapshot to validate changes on file generation You need to update snapshots and it should fix the issue Also you need to add the deleted files to the cleanup list, so with the upgrade the files will be deleted too.

DanielFran avatar Oct 11 '23 14:10 DanielFran

I think we should replace we our own header. Spring i18n integration seems to be broken now.

mshima avatar Oct 11 '23 14:10 mshima

@DanielFran sorry, it is my first time doing such kind of stuff.

You need to update snapshots and it should fix the issue

Do I understand correctly I need to run npm run update-snapshots and it would apply some changes to the source files I need to commit as well?

Also you need to add the deleted files to the cleanup list

It's about this function, right? https://github.com/jhipster/generator-jhipster/blob/31dae68683e46625571326ea5118d7c381097668/generators/server/cleanup.mts#L29 If yes, then which version should I set there? Should it be under the currently latest defined 7.10.0?

@mshima I don't get what's wrong with Spring i18n. I believe it's being used by sending E-Mails only? If so, then there should be no problem since we creating a new Context(Locale.forLanguageTag(user.getLangKey())) and it's not dependent on the LocaleConfiguration we're removing (and I've re-checked, Spring i18n works as expected in E-Mails with my changes)

shrralis avatar Oct 11 '23 15:10 shrralis

Translated error messages in api responses. Would be nice to check if it resolves Accept-Language headers at least. @DanielFran what do you think?

mshima avatar Oct 11 '23 15:10 mshima

@mshima well, I could revert the change and remove only this part:

@Bean
public LocaleResolver localeResolver() {
    return new AngularCookieLocaleResolver("NG_TRANSLATE_LANG_KEY");
}

But still, I see that there's a LocaleChangeInterceptor (LocaleChangeFilter) configured which seems to be completely redundant to me because I couldn't find any usage of the language request param in the front-end part, so the locale never gets changed from what I see.

Would be nice to check if it resolves Accept-Language headers at least.

There's an AcceptHeaderLocaleResolver for that and it is being used in Spring by default:

https://github.com/spring-projects/spring-framework/blob/bb23032a789d2110a0515a8ea62550fd01b75b71/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java#L1156-L1159

shrralis avatar Oct 11 '23 17:10 shrralis

Translated error messages in api responses. Would be nice to check if it resolves Accept-Language headers at least. @DanielFran what do you think?

@mshima Not sure it is working today? @shrralis Can you check with/without your changes if it works?

DanielFran avatar Oct 12 '23 19:10 DanielFran

@DanielFran I'm not sure what I should check there. From what I saw, API error messages aren't i18n-ized and all of the i18n is being performed by frontend frameworks (except EMail sending).

shrralis avatar Oct 12 '23 20:10 shrralis

@shrralis Can you please resolve conflicts in this PR?

mraible avatar Apr 29 '24 16:04 mraible

@mraible sure, done

shrralis avatar Apr 29 '24 21:04 shrralis

@shrralis Thanks for your contribution!

mraible avatar Apr 30 '24 00:04 mraible

Bug bounty claimed https://opencollective.com/generator-jhipster/expenses/200339

shrralis avatar Apr 30 '24 06:04 shrralis