Skip to content

Commit

Permalink
Fix EZP-24457: Change Request injection to RequestStack
Browse files Browse the repository at this point in the history
  • Loading branch information
lolautruche committed Jun 9, 2015
1 parent 4e99f5e commit ceec45f
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 28 deletions.
14 changes: 4 additions & 10 deletions Controller/CommentsRendererController.php
Expand Up @@ -24,32 +24,26 @@ class CommentsRendererController
/** @var \eZ\Publish\API\Repository\ContentService */
private $contentService;

/** @var \Symfony\Component\HttpFoundation\Request */
private $request;

public function __construct( ProviderInterface $commentsRenderer, ContentService $contentService )
{
$this->commentsRenderer = $commentsRenderer;
$this->contentService = $contentService;
}

public function setRequest( Request $request = null )
{
$this->request = $request;
}

/**
* Renders the comments list for content with id $contentId
* Comment form might also be included
*
* @param mixed $contentId
*
* @return Response
*/
public function renderForContentAction( $contentId )
public function renderForContentAction( $contentId, Request $request )
{
return new Response(
$this->commentsRenderer->renderForContent(
$this->contentService->loadContentInfo( $contentId ),
$this->request
$request
)
);
}
Expand Down
4 changes: 1 addition & 3 deletions Resources/config/services.yml
Expand Up @@ -19,7 +19,7 @@ services:
class: %ez_comments.twig.extension.class%
arguments: [@ez_comments.renderer]
calls:
- [setRequest, ['@?request=']]
- [setRequestStack, [@request_stack]]
tags:
- { name: twig.extension }

Expand Down Expand Up @@ -55,5 +55,3 @@ services:
ez_comments.controller.comments_renderer:
class: %ez_comments.controller.comments_renderer.class%
arguments: [@ez_comments.renderer, @ezpublish.api.service.content]
calls:
- [setRequest, [@?request=]]
23 changes: 9 additions & 14 deletions Twig/Extension/CommentsExtension.php
Expand Up @@ -10,6 +10,7 @@
namespace EzSystems\CommentsBundle\Twig\Extension;

use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\Core\MVC\Symfony\RequestStackAware;
use EzSystems\CommentsBundle\Comments\ProviderInterface;
use Symfony\Component\HttpFoundation\Request;
use Twig_Extension;
Expand All @@ -18,16 +19,13 @@

class CommentsExtension extends Twig_Extension
{
use RequestStackAware;

/**
* @var \EzSystems\CommentsBundle\Comments\ProviderInterface
*/
private $commentsRenderer;

/**
* @var \Symfony\Component\HttpFoundation\Request
*/
private $request;

public function __construct( ProviderInterface $commentsRenderer )
{
$this->commentsRenderer = $commentsRenderer;
Expand Down Expand Up @@ -59,11 +57,6 @@ public function getFunctions()
);
}

public function setRequest( Request $request = null )
{
$this->request = $request;
}

/**
* Triggers comments rendering
*
Expand All @@ -81,12 +74,13 @@ public function render( array $options = array(), $provider = null )
$options['provider'] = $provider;
}

if ( !isset( $this->request ) )
$request = $this->getCurrentRequest();
if ( !isset( $request ) )
{
throw new RuntimeException( 'Comments rendering needs the Request.' );
}

return $this->commentsRenderer->render( $this->request, $options );
return $this->commentsRenderer->render( $request, $options );
}

/**
Expand All @@ -106,11 +100,12 @@ public function renderForContent( ContentInfo $contentInfo, array $options = arr
$options['provider'] = $provider;
}

if ( !isset( $this->request ) )
$request = $this->getCurrentRequest();
if ( !isset( $request ) )
{
throw new RuntimeException( 'Comments rendering needs the Request.' );
}

return $this->commentsRenderer->renderForContent( $contentInfo, $this->request, $options );
return $this->commentsRenderer->renderForContent( $contentInfo, $request, $options );
}
}
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -13,7 +13,7 @@
}
],
"require": {
"ezsystems/ezpublish-kernel": "*",
"ezsystems/ezpublish-kernel": "dev-master",

This comment has been minimized.

Copy link
@andrerom

andrerom Jun 9, 2015

Contributor

breaks the behat testing in kernel, and will break for stable releases unless we remember to change on every single tag. wouldn't it be more correct to specify you require symfony 2.4 or higher or something for this change? (well 2.6 considering the other change maybe)

This comment has been minimized.

Copy link
@lolautruche

lolautruche Jun 10, 2015

Author Contributor

I know, I'll try to find a solution. Like said elsewhere, I think the best would be to generalize branch alias usage.

wouldn't it be more correct to specify you require symfony 2.4 or higher or something for this change? (well 2.6 considering the other change maybe)

How would it fix the problem? We still need ezpublish-kernel here.

This comment has been minimized.

Copy link
@andrerom

andrerom Jun 10, 2015

Contributor

I missed the RequestStackAware part and assumed you did not have need for a specific kernel version. Branch alias it is then, lets keep the discussion going in the other place, and soon PR's.

"symfony-cmf/routing": "~1.1"
},
"require-dev": {
Expand Down

0 comments on commit ceec45f

Please sign in to comment.