Skip to content

Respect currently negotiated language. #670

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

Merged
merged 18 commits into from
Sep 20, 2018

Conversation

pmelab
Copy link
Contributor

@pmelab pmelab commented Sep 19, 2018

Set global query variable to current language and use it for
contextual arguments. Add languages to cache contexts.

@codecov
Copy link

codecov bot commented Sep 19, 2018

Codecov Report

Merging #670 into 8.x-3.x will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           8.x-3.x     #670      +/-   ##
===========================================
+ Coverage    79.66%   79.72%   +0.06%     
===========================================
  Files          192      192              
  Lines         3039     3058      +19     
===========================================
+ Hits          2421     2438      +17     
- Misses         618      620       +2
Impacted Files Coverage Δ
src/GraphQLLanguageContext.php 84.21% <100%> (+1.85%) ⬆️
src/Plugin/GraphQL/Schemas/SchemaPluginBase.php 86.16% <100%> (+0.44%) ⬆️
src/GraphQL/Execution/ResolveContext.php 100% <100%> (ø) ⬆️
src/GraphQL/Execution/QueryProcessor.php 89.57% <100%> (-1.85%) ⬇️
src/Plugin/GraphQL/Fields/FieldPluginBase.php 97.77% <100%> (+0.24%) ⬆️
...GraphQL/Fields/EntityQuery/EntityQueryEntities.php 78.72% <0%> (+2.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 366b9cd...d7f87e8. Read the comment docs.

});
return new Deferred(
function () use ($result, $value, $args, $context, $info) {
if ($this->isLanguageAwareField()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to call this twice (isLanguageAwareField()). Just cache the result and re-use it here.

else {
$args[$argument] = $context->getContext($argument, $info);
}
$args[$argument] = $context->getContext($argument, $info, $context->getGlobal($argument));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need something else than getGlobal / setGlobal? Basically set the root field? Globals were rather meant as a configuration value. Instead of having to use getContext() with a default value everytime, I'd rather set a root context so getContext() automatically finds it.

$context->getContext('language', $info)
);
}
else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else statement is redundant

@fubhy fubhy merged commit 08fa8de into 8.x-3.x Sep 20, 2018
@fubhy fubhy deleted the respect-global-language-negotiation branch September 20, 2018 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants