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.

Sign up for GitHub

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

IBX-6630: Allowed injecting view type into content preview controller #285

Merged
merged 5 commits into from
Nov 6, 2023
Merged

IBX-6630: Allowed injecting view type into content preview controller #285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a0169dc
Allowed injecting view type into content preview controller
alongosz Oct 19, 2023
e87f361
[Tests] Refactored PreviewControllerTest
alongosz Oct 20, 2023
22c53ea
[Tests] Added coverage for parametrized view type in PreviewController
alongosz Oct 20, 2023
d18a6ce
[PHPStan] Dropped from baseline resolved issues in PreviewController
alongosz Oct 19, 2023
37f1de0
[PHPStan] Dropped from baseline resolved issues in PreviewControllerTest
alongosz Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 0 additions & 55 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11550,31 +11550,6 @@ parameters:
count: 1
path: src/lib/MVC/Symfony/ConfigDumperInterface.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController\\:\\:previewContentAction\\(\\) has no return type specified\\.$#"
count: 1
path: src/lib/MVC/Symfony/Controller/Content/PreviewController.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController\\:\\:previewContentAction\\(\\) has parameter \\$contentId with no type specified\\.$#"
count: 1
path: src/lib/MVC/Symfony/Controller/Content/PreviewController.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController\\:\\:previewContentAction\\(\\) has parameter \\$language with no type specified\\.$#"
count: 1
path: src/lib/MVC/Symfony/Controller/Content/PreviewController.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController\\:\\:previewContentAction\\(\\) has parameter \\$siteAccessName with no type specified\\.$#"
count: 1
path: src/lib/MVC/Symfony/Controller/Content/PreviewController.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController\\:\\:previewContentAction\\(\\) has parameter \\$versionNo with no type specified\\.$#"
count: 1
path: src/lib/MVC/Symfony/Controller/Content/PreviewController.php

-
message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\QueryController\\:\\:runPagingQuery\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -46530,36 +46505,6 @@ parameters:
count: 1
path: tests/lib/MVC/Symfony/Component/Serializer/Stubs/SerializerStub.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Controller\\\\Content\\\\PreviewControllerTest\\:\\:testPreview\\(\\) has no return type specified\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Controller\\\\Content\\\\PreviewControllerTest\\:\\:testPreviewCanUserFail\\(\\) has no return type specified\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Controller\\\\Content\\\\PreviewControllerTest\\:\\:testPreviewDefaultSiteaccess\\(\\) has no return type specified\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Controller\\\\Content\\\\PreviewControllerTest\\:\\:testPreviewUnauthorized\\(\\) has no return type specified\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Parameter \\#7 \\$controllerChecker of class Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Content\\\\PreviewController constructor expects Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\View\\\\CustomLocationControllerChecker, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Symfony\\\\Component\\\\Security\\\\Core\\\\Authorization\\\\AuthorizationCheckerInterface given\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\Controller\\\\Content\\\\PreviewControllerTest\\:\\:\\$controllerChecker \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Symfony\\\\Component\\\\Security\\\\Core\\\\Authorization\\\\AuthorizationCheckerInterface\\) does not accept Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\View\\\\CustomLocationControllerChecker&PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Controller\\\\ControllerTest\\:\\:testRender\\(\\) has no return type specified\\.$#"
count: 1
Expand Down
2 changes: 1 addition & 1 deletion src/bundle/Core/Resources/config/routing/internal.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ibexa.content.view:

ibexa.version.preview:
path: /content/versionview/{contentId}/{versionNo}/{language}/site_access/{siteAccessName}
defaults: { _controller: ibexa.controller.content.preview:previewContentAction }
controller: ibexa.controller.content.preview:previewContentAction
methods: [GET]

ibexa.content.preview.default:
Expand Down
34 changes: 17 additions & 17 deletions src/lib/MVC/Symfony/Controller/Content/PreviewController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker;
use Ibexa\Core\MVC\Symfony\View\ViewManagerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
Expand Down Expand Up @@ -77,12 +78,12 @@ public function __construct(
*/
public function previewContentAction(
Request $request,
$contentId,
$versionNo,
$language,
$siteAccessName = null,
int $contentId,
int $versionNo,
string $language,
?string $siteAccessName = null,
?int $locationId = null
) {
): Response {
$this->previewHelper->setPreviewActive(true);

try {
Expand Down Expand Up @@ -112,8 +113,9 @@ public function previewContentAction(
}

try {
$viewType = $request->query->get('viewType', ViewManagerInterface::VIEW_TYPE_FULL);
$response = $this->kernel->handle(
$this->getForwardRequest($location, $content, $siteAccess, $request, $language),
$this->getForwardRequest($location, $content, $siteAccess, $request, $language, $viewType),
HttpKernelInterface::SUB_REQUEST,
false
);
Expand All @@ -140,17 +142,15 @@ public function previewContentAction(

/**
* Returns the Request object that will be forwarded to the kernel for previewing the content.
*
* @param \Ibexa\Contracts\Core\Repository\Values\Content\Location $location
* @param \Ibexa\Contracts\Core\Repository\Values\Content\Content $content
* @param \Ibexa\Core\MVC\Symfony\SiteAccess $previewSiteAccess
* @param \Symfony\Component\HttpFoundation\Request $request
* @param string $language
*
* @return \Symfony\Component\HttpFoundation\Request
*/
protected function getForwardRequest(Location $location, Content $content, SiteAccess $previewSiteAccess, Request $request, $language)
{
protected function getForwardRequest(
Location $location,
Content $content,
SiteAccess $previewSiteAccess,
Request $request,
string $language,
string $viewType = ViewManagerInterface::VIEW_TYPE_FULL
): Request {
$forwardRequestParameters = [
'_controller' => UrlAliasRouter::VIEW_ACTION,
// specify a route for RouteReference generator
Expand All @@ -161,7 +161,7 @@ protected function getForwardRequest(Location $location, Content $content, SiteA
],
'location' => $location,
'content' => $content,
'viewType' => ViewManagerInterface::VIEW_TYPE_FULL,
'viewType' => $viewType,
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, do we even need to be aware of the defaults both here and in the caller? Doesn't admin UI fall back into a full view on it's own? If so, you might want to remove the default and just pass null.

Copy link
Member Author

Choose a reason for hiding this comment

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

Out of curiosity, do we even need to be aware of the defaults both here and in the caller? Doesn't admin UI fall back into a full view on it's own? If so, you might want to remove the default and just pass null.

viewType is a core concept. Even if somehow AdminUI does this on its own, this would be a break in core behavior for anyone directly using that controller. While there's no BC promise on that, since the class is neither final nor internal, this is very low cost to at least try keeping that BC.

'layout' => true,
'params' => [
'content' => $content,
Expand Down
Loading