generator-jhipster
generator-jhipster copied to clipboard
Remove LocaleConfiguration
remove LocaleConfiguration due to being a leftover from angularjs
Fix #20106
Please make sure the below checklist is followed for Pull Requests.
- [x] All continuous integration tests are green
- [x] Tests are added where necessary
- [x] The JDL part is updated if necessary
- [x] jhipster-online is updated if necessary
- [x] Documentation is added/updated where necessary
- [x] Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed
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.
Not sure why it's failing
@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.
I think we should replace we our own header. Spring i18n integration seems to be broken now.
@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)
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 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
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 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 Can you please resolve conflicts in this PR?
@mraible sure, done
@shrralis Thanks for your contribution!
Bug bounty claimed https://opencollective.com/generator-jhipster/expenses/200339