Skip to content
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

EZP-31118: Introduced strict types for ContenteService #87

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Nov 29, 2019

Question Answer
JIRA issue EZP-31118
Bug/Improvement yes
New feature no
Target version master
BC breaks yes
Tests pass yes
Doc needed yes

Fixed issues reported in ezsystems/ezpublish-kernel#2866 (review):

Argument 1 passed to eZ\Publish\Core\Repository\ContentService::loadContent() must be of the type int, string given, called in /home/vagrant/master/vendor/ezsystems/ezplatform-richtext/src/bundle/eZ/RichText/Renderer.php on line 130

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@adamwojs adamwojs added the Bug Something isn't working label Nov 29, 2019
@adamwojs adamwojs self-assigned this Nov 29, 2019
@adamwojs adamwojs force-pushed the content_service_typehint branch from a93af45 to 2f7afd2 Compare March 4, 2020 14:15
@@ -127,7 +127,7 @@ public function renderContentEmbed($contentId, $viewType, array $parameters, $is
/** @var \eZ\Publish\API\Repository\Values\Content\Content $content */
$content = $this->repository->sudo(
function (Repository $repository) use ($contentId) {
return $repository->getContentService()->loadContent($contentId);
return $repository->getContentService()->loadContent((int)$contentId);
Copy link
Member

Choose a reason for hiding this comment

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

TBH you should fix it here and enforce strict types in renderContentEmbed.

But can be done as a follow-up when handling renderContentEmbed by introducing strict types there and removing type cast here.

@adamwojs adamwojs force-pushed the content_service_typehint branch from 2f7afd2 to b4b1136 Compare March 13, 2020 10:50
@adamwojs adamwojs merged commit b41bd4f into master Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working QA approved
Development

Successfully merging this pull request may close these issues.

4 participants