-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use locale param if one is defined over accept-language header in GTFS APIs #6552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use locale param if one is defined over accept-language header in GTFS APIs #6552
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6552 +/- ##
=============================================
- Coverage 70.25% 70.24% -0.01%
+ Complexity 18387 18381 -6
=============================================
Files 2088 2088
Lines 77425 77410 -15
Branches 7843 7840 -3
=============================================
- Hits 54395 54377 -18
- Misses 20258 20264 +6
+ Partials 2772 2769 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
} | ||
|
||
return userLocale; | ||
var envLocale = environment.getLocale(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question about names:
- localContextLocale is the one passed in the GraphQL query body
- env Locale is passed the one from the HTTP header
Accept-Language
Correct?
If so, can you write a few lines in this class? It would help me understand what is going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some comments in this method that explain this. But I'll clarify the comments a bit and add javadoc to methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a few lines of Javadoc to GraphQLUtils
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is some excellent Javadoc.
Summary
Previously if both accept-language header and locale param were defined, env would be used. This changes the prioritization so that the locale param is used.
Issue
No issue
Unit tests
Update tests
Documentation
Not needed
Changelog
From title