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

Upgrade rest bundle #167

Merged
merged 16 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

"doctrine/doctrine-bundle": "^1.12|^2.0",
"doctrine/persistence": "^1.3",
"friendsofsymfony/rest-bundle": "^2.1",
"friendsofsymfony/rest-bundle": "^3.0",
"jms/serializer-bundle": "^3.5",
"stof/doctrine-extensions-bundle": "^1.2",
"sylius/registry": "^1.2",
Expand Down
2 changes: 2 additions & 0 deletions src/Bundle/Controller/Parameters.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class Parameters extends ParameterBag
{
/**
* {@inheritdoc}
*
* @psalm-param string $path
*/
public function get($path, $default = null)
{
Expand Down
78 changes: 30 additions & 48 deletions src/Bundle/Controller/ResourceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,17 @@ public function showAction(Request $request): Response

$this->eventDispatcher->dispatch(ResourceActions::SHOW, $configuration, $resource);

$view = View::create($resource);

if ($configuration->isHtmlRequest()) {
$view
->setTemplate($configuration->getTemplate(ResourceActions::SHOW . '.html'))
->setTemplateVar($this->metadata->getName())
->setData([
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $resource,
$this->metadata->getName() => $resource,
])
;
return $this->render($configuration->getTemplate(ResourceActions::SHOW . '.html'), [
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $resource,
$this->metadata->getName() => $resource,
]);
}

$view = View::create($resource);

return $this->viewHandler->handle($configuration, $view);
}

Expand All @@ -158,21 +154,17 @@ public function indexAction(Request $request): Response

$this->eventDispatcher->dispatchMultiple(ResourceActions::INDEX, $configuration, $resources);

$view = View::create($resources);

if ($configuration->isHtmlRequest()) {
$view
->setTemplate($configuration->getTemplate(ResourceActions::INDEX . '.html'))
->setTemplateVar($this->metadata->getPluralName())
->setData([
'configuration' => $configuration,
'metadata' => $this->metadata,
'resources' => $resources,
$this->metadata->getPluralName() => $resources,
])
;
return $this->render($configuration->getTemplate(ResourceActions::INDEX . '.html'), [
'configuration' => $configuration,
'metadata' => $this->metadata,
'resources' => $resources,
$this->metadata->getPluralName() => $resources,
]);
}

$view = View::create($resources);

return $this->viewHandler->handle($configuration, $view);
}

Expand Down Expand Up @@ -238,18 +230,13 @@ public function createAction(Request $request): Response
return $initializeEventResponse;
}

$view = View::create()
->setData([
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $newResource,
$this->metadata->getName() => $newResource,
'form' => $form->createView(),
])
->setTemplate($configuration->getTemplate(ResourceActions::CREATE . '.html'))
;

return $this->viewHandler->handle($configuration, $view);
return $this->render($configuration->getTemplate(ResourceActions::CREATE . '.html'), [
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $newResource,
$this->metadata->getName() => $newResource,
'form' => $form->createView(),
]);
}

public function updateAction(Request $request): Response
Expand Down Expand Up @@ -326,18 +313,13 @@ public function updateAction(Request $request): Response
return $initializeEventResponse;
}

$view = View::create()
->setData([
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $resource,
$this->metadata->getName() => $resource,
'form' => $form->createView(),
])
->setTemplate($configuration->getTemplate(ResourceActions::UPDATE . '.html'))
;

return $this->viewHandler->handle($configuration, $view);
return $this->render($configuration->getTemplate(ResourceActions::UPDATE . '.html'), [
'configuration' => $configuration,
'metadata' => $this->metadata,
'resource' => $resource,
$this->metadata->getName() => $resource,
'form' => $form->createView(),
]);
}

public function deleteAction(Request $request): Response
Expand Down
3 changes: 2 additions & 1 deletion src/Bundle/Controller/ResourcesCollectionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public function get(RequestConfiguration $requestConfiguration, RepositoryInterf
$requestConfiguration->getPaginationMaxPerPage(),
$paginationLimits
));
$paginator->setCurrentPage($request->query->get('page', 1));
$currentPage = (int) $request->query->get('page', '1');
Copy link
Member

Choose a reason for hiding this comment

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

Woulndn't it be better?

Suggested change
$currentPage = (int) $request->query->get('page', '1');
$currentPage = (int) $request->query->getInt('page', '1');

Copy link
Member Author

Choose a reason for hiding this comment

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

@lchrusciel Yes, but, if it returns an integer, we don't need the (int) conversion after :)

Copy link
Member Author

@loic425 loic425 Jun 23, 2020

Choose a reason for hiding this comment

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

I pushed another suggestion to remove the unnecessary int conversion in your suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lchrusciel

Suggested change
$currentPage = (int) $request->query->get('page', '1');
$currentPage = $request->query->getInt('page', '1');

$paginator->setCurrentPage($currentPage);

// This prevents Pagerfanta from querying database from a template
$paginator->getCurrentPageResults();
Expand Down
6 changes: 3 additions & 3 deletions src/Bundle/Controller/ViewHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@

namespace Sylius\Bundle\ResourceBundle\Controller;

use FOS\RestBundle\View\ConfigurableViewHandlerInterface;
use FOS\RestBundle\View\View;
use FOS\RestBundle\View\ViewHandler as RestViewHandler;
use Symfony\Component\HttpFoundation\Response;

final class ViewHandler implements ViewHandlerInterface
{
/** @var RestViewHandler */
/** @var ConfigurableViewHandlerInterface */
private $restViewHandler;

public function __construct(RestViewHandler $restViewHandler)
public function __construct(ConfigurableViewHandlerInterface $restViewHandler)
lchrusciel marked this conversation as resolved.
Show resolved Hide resolved
{
$this->restViewHandler = $restViewHandler;
}
Expand Down
9 changes: 8 additions & 1 deletion src/Bundle/DependencyInjection/SyliusResourceExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;

final class SyliusResourceExtension extends Extension
final class SyliusResourceExtension extends Extension implements PrependExtensionInterface
{
/**
* {@inheritdoc}
Expand Down Expand Up @@ -71,6 +72,12 @@ public function getConfiguration(array $config, ContainerBuilder $container): Co
return $configuration;
}

public function prepend(ContainerBuilder $container): void
{
$config = ['body_listener' => ['enabled' => true]];
$container->prependExtensionConfig('fos_rest', $config);
}

private function loadPersistence(array $drivers, array $resources, LoaderInterface $loader): void
{
$integrateDoctrine = array_reduce($drivers, function (bool $result, string $driver): bool {
Expand Down
4 changes: 3 additions & 1 deletion src/Bundle/EventListener/ORMRepositoryClassSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ private function setCustomRepositoryClass(ClassMetadata $metadata): void
}

if ($resourceMetadata->hasClass('repository')) {
$metadata->setCustomRepositoryClass($resourceMetadata->getClass('repository'));
/** @psalm-var class-string $repository */
$repository = $resourceMetadata->getClass('repository');
$metadata->setCustomRepositoryClass($repository);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public function handleRequest(FormInterface $form, $request = null)
$params = $request->request->all();
$files = $request->files->all();
} elseif ($request->request->has($name) || $request->files->has($name)) {
/** @psalm-var array|null $default */
$default = $form->getConfig()->getCompound() ? [] : null;
$params = $request->request->get($name, $default);
$files = $request->files->get($name, $default);
Expand Down
Loading