Skip to content

Conversation

drupov
Copy link

@drupov drupov commented Feb 11, 2020

Currently the logic for resolving translated entity references is loading the translated versions in order to check the access to them. But at the end the default language version is returned. This PR makes sure to pass the proper language version(s) into the array_filter function that strips out the ones, where access is not given.

@codecov
Copy link

codecov bot commented Feb 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (8.x-4.x@39d74e3). Click here to learn what that means.
The diff coverage is 44.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##             8.x-4.x     #974   +/-   ##
==========================================
  Coverage           ?   54.54%           
  Complexity         ?      621           
==========================================
  Files              ?      114           
  Lines              ?     1562           
  Branches           ?        0           
==========================================
  Hits               ?      852           
  Misses             ?      710           
  Partials           ?        0           
Impacted Files Coverage Δ Complexity Δ
src/Access/ExplorerAccessCheck.php 0.00% <0.00%> (ø) 2.00 <2.00> (?)
src/Access/VoyagerAccessCheck.php 0.00% <0.00%> (ø) 2.00 <2.00> (?)
src/Cache/Context/StaticCacheContext.php 0.00% <0.00%> (ø) 4.00 <4.00> (?)
src/Controller/SubrequestExtractionController.php 0.00% <0.00%> (ø) 1.00 <0.00> (?)
src/GraphQL/Buffers/EntityBuffer.php 80.76% <0.00%> (ø) 7.00 <0.00> (?)
src/GraphQL/Buffers/EntityRevisionBuffer.php 0.00% <0.00%> (ø) 7.00 <7.00> (?)
src/GraphQL/Resolver/Condition.php 0.00% <0.00%> (ø) 8.00 <8.00> (?)
src/GraphQL/Resolver/Context.php 0.00% <0.00%> (ø) 5.00 <5.00> (?)
src/GraphQL/Resolver/Map.php 0.00% <0.00%> (ø) 4.00 <4.00> (?)
src/GraphQL/Resolver/Path.php 0.00% <0.00%> (ø) 4.00 <4.00> (?)
... and 75 more

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 39d74e3...c969e25. Read the comment docs.

@joaogarin joaogarin added the 4.x label Feb 21, 2020
@fubhy
Copy link
Contributor

fubhy commented Feb 22, 2020

Thanks for this PR Dimitar! Looks good in general but I'd like to split up the function body into smaller reusable pieces and use a more descriptive function name.

@drupov drupov requested a review from fubhy March 6, 2020 20:47
@drupov
Copy link
Author

drupov commented Mar 6, 2020

Abstracted also logic inside trait and added some comments to each method.

# Conflicts:
#	src/Plugin/GraphQL/DataProducer/Field/EntityReference.php
#	src/Plugin/GraphQL/DataProducer/Field/EntityReferenceRevisions.php
@drupov drupov marked this pull request as draft February 9, 2021 22:01
@drupov drupov marked this pull request as ready for review February 9, 2021 22:01
@chrfritsch
Copy link
Contributor

The patch works nicely for me. One thing that is a bit related:
In the plugin definition language is defined as multiple. I think it shouldn't be.

@klausi
Copy link
Contributor

klausi commented Aug 16, 2021

This was merged in #1232 , thanks a lot for the upfront work here @drupov !

@klausi klausi closed this Aug 16, 2021
@drupov drupov deleted the fix-entity_reference-language branch September 30, 2021 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.