Skip to content

Fix access checking.#921

Merged
fubhy merged 1 commit intodrupal-graphql:8.x-4.xfrom
rthideaway:fix/access-checking
Oct 14, 2019
Merged

Fix access checking.#921
fubhy merged 1 commit intodrupal-graphql:8.x-4.xfrom
rthideaway:fix/access-checking

Conversation

@rthideaway
Copy link
Copy Markdown
Contributor

Fixing access checking as we recently discovered it was misimplemented, see #920 (comment)

@rthideaway
Copy link
Copy Markdown
Contributor Author

@fubhy please check, these are all instances with incorrect access checking. I'm going to fix our PRs as well

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 14, 2019

Codecov Report

Merging #921 into 8.x-4.x will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             8.x-4.x     #921   +/-   ##
==========================================
  Coverage      66.43%   66.43%           
  Complexity       587      587           
==========================================
  Files             92       92           
  Lines           1299     1299           
==========================================
  Hits             863      863           
  Misses           436      436
Impacted Files Coverage Δ Complexity Δ
...GraphQL/DataProducer/Entity/EntityTranslations.php 71.42% <0%> (ø) 11 <0> (ø) ⬇️
.../GraphQL/DataProducer/Entity/EntityTranslation.php 60% <0%> (ø) 8 <0> (ø) ⬇️
...n/GraphQL/DataProducer/Entity/EntityLoadByUuid.php 57.14% <100%> (ø) 21 <0> (ø) ⬇️
.../Plugin/GraphQL/DataProducer/Entity/EntityLoad.php 95.23% <100%> (ø) 21 <0> (ø) ⬇️

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 526eeea...0921803. Read the comment docs.

@fubhy fubhy merged commit dfca1ed into drupal-graphql:8.x-4.x Oct 14, 2019
@fubhy
Copy link
Copy Markdown
Contributor

fubhy commented Oct 14, 2019

Sweet, thanks!

@klausi
Copy link
Copy Markdown
Contributor

klausi commented Oct 16, 2019

I think we should have a code comment for all those checks similar as in #920 , otherwise someone might change this back to isForbidden() in the future.

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.

3 participants