From 08e29aceb1d20234fb132444512efba229d084b8 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Tue, 4 Oct 2016 00:38:58 +0200 Subject: [PATCH 01/49] improve request filter process --- .travis.yml | 1 - Admin/Admin.php | 52 ++++++++------ Admin/Factory/AdminFactory.php | 19 +++++- Controller/CRUDController.php | 2 +- Filter/Factory/RequestFilterFactory.php | 32 +++++++++ Filter/PagerfantaFilter.php | 31 +++------ Filter/RequestFilter.php | 67 +++++++++++++++++-- Filter/RequestFilterInterface.php | 20 ++++++ Resources/config/factories.yml | 4 ++ Tests/AdminBundle/Admin/AdminTest.php | 52 +++++++++----- .../Filter/RequestFilterFactoryTest.php | 50 ++++++++++++++ .../AdminBundle/Filter/RequestFilterTest.php | 58 ++++++++++++++++ Tests/AdminTestBase.php | 25 ++----- 13 files changed, 323 insertions(+), 90 deletions(-) create mode 100644 Filter/Factory/RequestFilterFactory.php create mode 100644 Filter/RequestFilterInterface.php create mode 100644 Tests/AdminBundle/Filter/RequestFilterFactoryTest.php create mode 100644 Tests/AdminBundle/Filter/RequestFilterTest.php diff --git a/.travis.yml b/.travis.yml index 4f6561f21..4abea13a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: php php: - - 5.5 - 5.6 - 7.0 - hhvm diff --git a/Admin/Admin.php b/Admin/Admin.php index 1814b37fc..6e62a70e9 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -13,6 +13,7 @@ use LAG\AdminBundle\Exception\AdminException; use LAG\AdminBundle\Filter\PagerfantaFilter; use LAG\AdminBundle\Filter\RequestFilter; +use LAG\AdminBundle\Filter\RequestFilterInterface; use LAG\AdminBundle\Message\MessageHandlerInterface; use LAG\AdminBundle\Pager\PagerFantaAdminAdapter; use Pagerfanta\Pagerfanta; @@ -83,6 +84,11 @@ class Admin implements AdminInterface */ protected $eventDispatcher; + /** + * @var RequestFilterInterface + */ + protected $requestFilter; + /** * Admin constructor. * @@ -91,13 +97,15 @@ class Admin implements AdminInterface * @param AdminConfiguration $configuration * @param MessageHandlerInterface $messageHandler * @param EventDispatcherInterface $eventDispatcher + * @param RequestFilterInterface $requestFilter */ public function __construct( $name, DataProviderInterface $dataProvider, AdminConfiguration $configuration, MessageHandlerInterface $messageHandler, - EventDispatcherInterface $eventDispatcher + EventDispatcherInterface $eventDispatcher, + RequestFilterInterface $requestFilter ) { $this->name = $name; $this->dataProvider = $dataProvider; @@ -105,10 +113,11 @@ public function __construct( $this->messageHandler = $messageHandler; $this->eventDispatcher = $eventDispatcher; $this->entities = new ArrayCollection(); + $this->requestFilter = $requestFilter; } /** - * Load entities and set current action according to request + * Load entities and set current action according to request. * * @param Request $request * @param null $user @@ -119,33 +128,34 @@ public function handleRequest(Request $request, $user = null) { // set current action $this->currentAction = $this->getAction($request->get('_route_params')['_action']); + // check if user is logged have required permissions to get current action $this->checkPermissions($user); - // criteria filter request - $filter = new RequestFilter($this->currentAction->getConfiguration()->getParameter('criteria')); - $criteriaFilter = $filter->filter($request); + $actionConfiguration = $this + ->currentAction + ->getConfiguration(); - // pager filter request - if ($this->currentAction->getConfiguration()->getParameter('pager') == 'pagerfanta') { - $filter = new PagerfantaFilter(); - $pagerFilter = $filter->filter($request); - } else { - // empty bag - $pagerFilter = new ParameterBag(); - } + // configure the request filter with the action and admin configured parameters + $this + ->requestFilter + ->configure( + $actionConfiguration->getParameter('criteria'), + $actionConfiguration->getParameter('order'), + $this->configuration->getParameter('max_per_page') + ); - // if load strategy is none, no entity should be loaded - if ($this->currentAction->getConfiguration()->getParameter('load_strategy') == Admin::LOAD_STRATEGY_NONE) { - return; - } + // filter the request with the configured criteria, order and max_per_page parameter + $this + ->requestFilter + ->filter($request); // load entities according to action and request $this->load( - $criteriaFilter->all(), - $pagerFilter->get('order', []), - $this->configuration->getParameter('max_per_page'), - $pagerFilter->get('page', 1) + $this->requestFilter->getCriteria(), + $this->requestFilter->getOrder(), + $this->requestFilter->getMaxPerPage(), + $this->requestFilter->getCurrentPage() ); } diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index b27322980..7d684e95c 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -15,6 +15,7 @@ use LAG\AdminBundle\DataProvider\DataProviderInterface; use LAG\AdminBundle\Admin\Event\AdminEvents; use Exception; +use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; use LAG\AdminBundle\Message\MessageHandlerInterface; use LAG\AdminBundle\Repository\RepositoryInterface; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; @@ -75,6 +76,11 @@ class AdminFactory */ protected $registry; + /** + * @var RequestFilterFactory + */ + protected $requestFilterFactory; + /** * AdminFactory constructor. * @@ -85,6 +91,7 @@ class AdminFactory * @param ActionFactory $actionFactory * @param MessageHandlerInterface $messageHandler * @param Registry $registry + * @param RequestFilterFactory $requestFilterFactory */ public function __construct( array $adminConfigurations, @@ -93,7 +100,8 @@ public function __construct( ConfigurationFactory $configurationFactory, ActionFactory $actionFactory, MessageHandlerInterface $messageHandler, - Registry $registry + Registry $registry, + RequestFilterFactory $requestFilterFactory ) { $this->eventDispatcher = $eventDispatcher; $this->entityManager = $entityManager; @@ -103,6 +111,7 @@ public function __construct( $this->messageHandler = $messageHandler; $this->dataProviders = new ParameterBag(); $this->registry = $registry; + $this->requestFilterFactory = $requestFilterFactory; } /** @@ -168,13 +177,19 @@ public function create($name, array $configuration) $adminConfiguration->getParameter('data_provider') ); + // retrieve a request filter + $requestFilter = $this + ->requestFilterFactory + ->create($adminConfiguration); + // create Admin object $admin = new Admin( $name, $dataProvider, $adminConfiguration, $this->messageHandler, - $this->eventDispatcher + $this->eventDispatcher, + $requestFilter ); // adding actions diff --git a/Controller/CRUDController.php b/Controller/CRUDController.php index 7f7f64428..758d58d81 100644 --- a/Controller/CRUDController.php +++ b/Controller/CRUDController.php @@ -51,7 +51,7 @@ public function listAction(Request $request) 'action' => $admin->getCurrentAction(), ]; - // if batch are configured, we handle a list of checboxes + // if batch are configured, we handle a list of checkboxes if ($batchActions) { // creating list form $form = $this->createForm(AdminListType::class, [ diff --git a/Filter/Factory/RequestFilterFactory.php b/Filter/Factory/RequestFilterFactory.php new file mode 100644 index 000000000..f8d62fb23 --- /dev/null +++ b/Filter/Factory/RequestFilterFactory.php @@ -0,0 +1,32 @@ +hasParameter('pager') && $configuration->getParameter('pager') === 'pagerfanta') { + // if Pagerfanta is configured, use the PagerfantaFilter + $requestFilter = new PagerfantaFilter(); + } else { + // else use the classic request filter + $requestFilter = new RequestFilter(); + } + + return $requestFilter; + } +} diff --git a/Filter/PagerfantaFilter.php b/Filter/PagerfantaFilter.php index 0f3c1220e..e6d079724 100644 --- a/Filter/PagerfantaFilter.php +++ b/Filter/PagerfantaFilter.php @@ -5,35 +5,24 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\ParameterBag; -class PagerfantaFilter +class PagerfantaFilter extends RequestFilter { + protected $currentPage; + /** * Filter request * * @param Request $request - * @return ParameterBag */ public function filter(Request $request) { - $filteredValues = new ParameterBag(); - // order column, like "name" - $order = $request->get('order'); - // sort value, asc or desc - $sort = $request->get('sort'); - // page number, like 2 - $page = $request->get('page'); - - if ($order) { - if (!$sort) { - $sort = 'asc'; - } - $filteredValues->set('order', [ - $sort => $order - ]); - } - if ($page) { - $filteredValues->set('page', $page); + if ($request->get('page')) { + $this->currentPage = $request->get('page'); } - return $filteredValues; + } + + public function getCurrentPage() + { + return $this->currentPage; } } diff --git a/Filter/RequestFilter.php b/Filter/RequestFilter.php index 6a449d866..ea168d41e 100644 --- a/Filter/RequestFilter.php +++ b/Filter/RequestFilter.php @@ -5,7 +5,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\ParameterBag; -class RequestFilter +class RequestFilter implements RequestFilterInterface { /** * @var array @@ -13,23 +13,48 @@ class RequestFilter protected $filters; /** - * RequestFilter constructor. + * @var array + */ + protected $criteria = []; + + /** + * @var array + */ + protected $orders; + + /** + * @var int + */ + protected $maxPerPage; + + /** + * @var int + */ + protected $currentPage; + + /** + * Configure the filter. * - * @param array $filters + * @param array $filters Filters for criteria parameters from configuration. Those parameters will be filtered from + * the request + * @param array $orders Default orders from configuration + * @param int $maxPerPage */ - public function __construct(array $filters = []) + public function configure(array $filters = [], array $orders = [], $maxPerPage = 1) { $this->filters = $filters; + $this->orders = $orders; + $this->maxPerPage = $maxPerPage; } /** - * Filter request values according to configured filters + * Filter request values according to configured filters. Get orders and sort parameters from the request too. * * @param Request $request - * @return ParameterBag */ public function filter(Request $request) { + // filter the request parameters with configured filters for criteria parameters $filteredValues = new ParameterBag(); foreach ($this->filters as $filter) { @@ -39,6 +64,34 @@ public function filter(Request $request) $filteredValues->set($filter, $value); } } - return $filteredValues; + $this->criteria = $filteredValues->all(); + + // filter the request parameters to find some orders parameters + if ($request->get('sort') && $request->get('order')) { + // if sort and order parameters are present in the request, we use them + $this->orders = [ + $request->get('sort') => $request->get('order') + ]; + } + } + + public function getCriteria() + { + return $this->criteria; + } + + public function getOrder() + { + return $this->orders; + } + + public function getMaxPerPage() + { + return $this->maxPerPage; + } + + public function getCurrentPage() + { + return 1; } } diff --git a/Filter/RequestFilterInterface.php b/Filter/RequestFilterInterface.php new file mode 100644 index 000000000..51d4c86d2 --- /dev/null +++ b/Filter/RequestFilterInterface.php @@ -0,0 +1,20 @@ +getFakeAdminsConfiguration(); $configuration = $configurations['full_entity']; - $adminConfiguration = $this->createAdminConfiguration($applicationConfiguration, $configuration); - $admin = $this->mockAdmin('full_entity', $adminConfiguration); + $admin = $this->createAdmin('full_entity', $configuration, $applicationConfiguration->getParameters()); $admin->addAction($this->createAction('custom_list', $admin, [ 'load_strategy' => Admin::LOAD_STRATEGY_NONE, @@ -201,7 +201,8 @@ public function testCreate() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->create(); } @@ -227,7 +228,8 @@ public function testSave() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->create(); $this->assertTrue($admin->save()); @@ -257,7 +259,8 @@ public function testSaveWithException() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->create(); $this->assertFalse($admin->save()); @@ -283,7 +286,8 @@ public function testRemove() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->create(); $this->assertTrue($admin->remove()); @@ -312,7 +316,8 @@ public function testRemoveWithException() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->create(); $this->assertFalse($admin->remove()); @@ -343,7 +348,8 @@ public function testGenerateRouteName() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $this->assertEquals('lag.test.custom_list', $admin->generateRouteName('custom_list')); $this->assertEquals('lag.test.custom_edit', $admin->generateRouteName('custom_edit')); @@ -370,7 +376,8 @@ public function testGetEntities() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -410,7 +417,8 @@ public function testGetUniqueEntity() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -440,7 +448,8 @@ public function testGetUniqueEntity() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -471,7 +480,8 @@ public function testGetUniqueEntity() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -517,7 +527,8 @@ public function testLoad() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -560,7 +571,8 @@ public function testLoadWithUniqueStrategy() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -599,7 +611,8 @@ public function testLoadWithPagerWithMultipleStrategy() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -636,7 +649,8 @@ public function testLoadWithException() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -677,7 +691,8 @@ public function testLoadWithoutPager() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $request = new Request([], [], [ '_route_params' => [ @@ -716,7 +731,8 @@ public function testGetCurrentActionException() $dataProvider, $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); $admin->addAction($this->createAction('custom_list', $admin, [ 'load_strategy' => AdminInterface::LOAD_STRATEGY_MULTIPLE, diff --git a/Tests/AdminBundle/Filter/RequestFilterFactoryTest.php b/Tests/AdminBundle/Filter/RequestFilterFactoryTest.php new file mode 100644 index 000000000..abc7df28c --- /dev/null +++ b/Tests/AdminBundle/Filter/RequestFilterFactoryTest.php @@ -0,0 +1,50 @@ +createMock(ActionConfiguration::class); + $configuration + ->method('hasParameter') + ->with('pager') + ->willReturn(false) + ; + + $factory = new RequestFilterFactory(); + $filter = $factory->create($configuration); + $this->assertInstanceOf( + RequestFilterInterface::class, + $filter + ); + } + + public function testCreateWithPager() + { + $configuration = $this->createMock(ActionConfiguration::class); + $configuration + ->method('hasParameter') + ->with('pager') + ->willReturn(true) + ; + $configuration + ->method('getParameter') + ->with('pager') + ->willReturn('pagerfanta'); + + $factory = new RequestFilterFactory(); + $filter = $factory->create($configuration); + $this->assertInstanceOf( + PagerfantaFilter::class, + $filter + ); + } +} diff --git a/Tests/AdminBundle/Filter/RequestFilterTest.php b/Tests/AdminBundle/Filter/RequestFilterTest.php new file mode 100644 index 000000000..d2f51a39d --- /dev/null +++ b/Tests/AdminBundle/Filter/RequestFilterTest.php @@ -0,0 +1,58 @@ +configure([ + 'filters' + ], [ + 'orders' + ], 50); + + $this->assertEquals([], $filter->getCriteria()); + $this->assertEquals([ + 'orders' + ], $filter->getOrder()); + $this->assertEquals(50, $filter->getMaxPerPage()); + + $request = $this->createMock(Request::class); + $request + ->method('get') + ->willReturnCallback(function ($parameter) { + if ($parameter == 'order') { + return 'asc'; + } + + if ($parameter == 'sort') { + return 'name'; + } + + if ($parameter == 'filters') { + return 'bamboo'; + } + + return null; + }); + $filter->filter($request); + + $this->assertEquals([ + 'name' => 'asc' + ], $filter->getOrder()); + $this->assertEquals([ + 'filters' => 'bamboo' + ], $filter->getCriteria()); + + function requestCallback() + { + die('ol'); + } + } +} diff --git a/Tests/AdminTestBase.php b/Tests/AdminTestBase.php index 4b8151a51..807d09aac 100644 --- a/Tests/AdminTestBase.php +++ b/Tests/AdminTestBase.php @@ -22,6 +22,8 @@ use LAG\AdminBundle\Configuration\Factory\ConfigurationFactory; use LAG\AdminBundle\DataProvider\DataProviderInterface; use LAG\AdminBundle\Field\Factory\FieldFactory; +use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; +use LAG\AdminBundle\Filter\RequestFilter; use LAG\AdminBundle\Message\MessageHandlerInterface; use LAG\AdminBundle\Repository\RepositoryInterface; use PHPUnit_Framework_MockObject_MockObject; @@ -181,7 +183,8 @@ protected function createAdmin($name, array $configuration, array $applicationCo $this->mockDataProvider(), $adminConfiguration, $this->mockMessageHandler(), - new EventDispatcher() + new EventDispatcher(), + new RequestFilter() ); } @@ -282,23 +285,6 @@ protected function mockKernel() ->getMock(); } - /** - * @param $name - * @param $configuration - * @return Admin - * @deprecated - */ - protected function mockAdmin($name, $configuration) - { - return new Admin( - $name, - $this->mockDataProvider(), - $configuration, - $this->mockMessageHandler(), - new EventDispatcher() - ); - } - /** * @param $name * @return ActionInterface | PHPUnit_Framework_MockObject_MockObject @@ -356,7 +342,8 @@ protected function createAdminFactory(array $configuration = [], EventDispatcher $this->createConfigurationFactory(), $this->mockActionFactory(), $this->mockMessageHandler(), - new \LAG\AdminBundle\Admin\Registry\Registry() + new \LAG\AdminBundle\Admin\Registry\Registry(), + new RequestFilterFactory() ); } From 3290e8572c31cf2ce174ee34616e561d74b851fd Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Fri, 7 Oct 2016 22:50:23 +0200 Subject: [PATCH 02/49] remove unused use statements --- Admin/Admin.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/Admin/Admin.php b/Admin/Admin.php index 6e62a70e9..c5db90c6e 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -11,15 +11,12 @@ use Exception; use LAG\AdminBundle\DataProvider\DataProviderInterface; use LAG\AdminBundle\Exception\AdminException; -use LAG\AdminBundle\Filter\PagerfantaFilter; -use LAG\AdminBundle\Filter\RequestFilter; use LAG\AdminBundle\Filter\RequestFilterInterface; use LAG\AdminBundle\Message\MessageHandlerInterface; use LAG\AdminBundle\Pager\PagerFantaAdminAdapter; use Pagerfanta\Pagerfanta; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Security\Core\Role\Role; From 229f978e08705864a66772ba9a83d0d15ba95db3 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 23 Oct 2016 13:15:00 +0200 Subject: [PATCH 03/49] rename method getEntityLabel to getUniqueEntityLabel to more coherent --- Admin/AdminInterface.php | 4 ++-- Admin/Behaviors/AdminTrait.php | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Admin/AdminInterface.php b/Admin/AdminInterface.php index 6d1bf5488..15ba575ad 100644 --- a/Admin/AdminInterface.php +++ b/Admin/AdminInterface.php @@ -146,10 +146,10 @@ public function isCurrentActionDefined(); public function isActionGranted($actionName, array $roles); /** - * Try to find a property to get a label from an entity. If found, it returns the property value through the + * Try to find a property to get a label from an unique entity. If found, it returns the property value through the * property accessor. * * @return string */ - public function getEntityLabel(); + public function getUniqueEntityLabel(); } diff --git a/Admin/Behaviors/AdminTrait.php b/Admin/Behaviors/AdminTrait.php index 17f6fe72f..a8dad78f5 100644 --- a/Admin/Behaviors/AdminTrait.php +++ b/Admin/Behaviors/AdminTrait.php @@ -6,9 +6,7 @@ trait AdminTrait { - use EntityLabelTrait { - getEntityLabel as parentEntityLabel; - } + use EntityLabelTrait; use TranslationKeyTrait; /** @@ -17,6 +15,8 @@ trait AdminTrait protected $pager; /** + * Return the current unique entity. + * * @return object */ public abstract function getUniqueEntity(); @@ -35,10 +35,10 @@ public function getPager() * * @return string */ - public function getEntityLabel() + public function getUniqueEntityLabel() { $entity = $this->getUniqueEntity(); - $label = $this->parentEntityLabel($entity); + $label = $this->getEntityLabel($entity); return $label; } From 73f39b515c64e4035ca6f80382515def720272b9 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 23 Oct 2016 13:41:05 +0200 Subject: [PATCH 04/49] improve request filters unit tests --- .../AdminBundle/Filter/RequestFilterTest.php | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/Tests/AdminBundle/Filter/RequestFilterTest.php b/Tests/AdminBundle/Filter/RequestFilterTest.php index d2f51a39d..fe2341b6e 100644 --- a/Tests/AdminBundle/Filter/RequestFilterTest.php +++ b/Tests/AdminBundle/Filter/RequestFilterTest.php @@ -49,10 +49,33 @@ public function testConfigure() $this->assertEquals([ 'filters' => 'bamboo' ], $filter->getCriteria()); + } + + public function testFilter() + { + $filter = new RequestFilter(); + $filter->configure([ + 'name' + ], [ + 'orders' + ], 50); + + $request = new Request([ + 'name' => 'toto', + 'page' => 53, + 'sort' => 'name', + 'order' => 'asc' + ]); - function requestCallback() - { - die('ol'); - } + $filter->filter($request); + + $this->assertEquals([ + 'name' => 'toto' + ], $filter->getCriteria()); + $this->assertEquals([ + 'name' => 'asc' + ], $filter->getOrder()); + $this->assertEquals(50, $filter->getMaxPerPage()); + $this->assertEquals(1, $filter->getCurrentPage()); } } From 06dd67872d1d6a5630609c1f7c19b46168a52202 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 23 Oct 2016 13:46:17 +0200 Subject: [PATCH 05/49] fix bug in pagerfanta filter and add corresponding tests --- Filter/PagerfantaFilter.php | 13 ++++- .../Filter/PagerfantaFilterTest.php | 54 +++++++++++++++++++ .../AdminBundle/Filter/RequestFilterTest.php | 27 ---------- 3 files changed, 66 insertions(+), 28 deletions(-) create mode 100644 Tests/AdminBundle/Filter/PagerfantaFilterTest.php diff --git a/Filter/PagerfantaFilter.php b/Filter/PagerfantaFilter.php index e6d079724..61f755de3 100644 --- a/Filter/PagerfantaFilter.php +++ b/Filter/PagerfantaFilter.php @@ -3,10 +3,14 @@ namespace LAG\AdminBundle\Filter; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\ParameterBag; class PagerfantaFilter extends RequestFilter { + /** + * The current page that should be retrieved by the pager + * + * @var int + */ protected $currentPage; /** @@ -19,8 +23,15 @@ public function filter(Request $request) if ($request->get('page')) { $this->currentPage = $request->get('page'); } + // filter normal request parameters + parent::filter($request); } + /** + * Return the current page that should be retrieved by the pager + * + * @return int + */ public function getCurrentPage() { return $this->currentPage; diff --git a/Tests/AdminBundle/Filter/PagerfantaFilterTest.php b/Tests/AdminBundle/Filter/PagerfantaFilterTest.php new file mode 100644 index 000000000..444e9f287 --- /dev/null +++ b/Tests/AdminBundle/Filter/PagerfantaFilterTest.php @@ -0,0 +1,54 @@ +configure([ + 'filters' + ], [ + 'orders' + ], 50); + + $this->assertEquals([], $filter->getCriteria()); + $this->assertEquals([ + 'orders' + ], $filter->getOrder()); + $this->assertEquals(50, $filter->getMaxPerPage()); + } + + public function testFilter() + { + $filter = new PagerfantaFilter(); + $filter->configure([ + 'name' + ], [ + 'orders' + ], 50); + + $request = new Request([ + 'name' => 'toto', + 'page' => 53, + 'sort' => 'name', + 'order' => 'asc' + ]); + + $filter->filter($request); + + $this->assertEquals([ + 'name' => 'toto' + ], $filter->getCriteria()); + $this->assertEquals([ + 'name' => 'asc' + ], $filter->getOrder()); + $this->assertEquals(50, $filter->getMaxPerPage()); + $this->assertEquals(53, $filter->getCurrentPage()); + } +} diff --git a/Tests/AdminBundle/Filter/RequestFilterTest.php b/Tests/AdminBundle/Filter/RequestFilterTest.php index fe2341b6e..2933c0185 100644 --- a/Tests/AdminBundle/Filter/RequestFilterTest.php +++ b/Tests/AdminBundle/Filter/RequestFilterTest.php @@ -22,33 +22,6 @@ public function testConfigure() 'orders' ], $filter->getOrder()); $this->assertEquals(50, $filter->getMaxPerPage()); - - $request = $this->createMock(Request::class); - $request - ->method('get') - ->willReturnCallback(function ($parameter) { - if ($parameter == 'order') { - return 'asc'; - } - - if ($parameter == 'sort') { - return 'name'; - } - - if ($parameter == 'filters') { - return 'bamboo'; - } - - return null; - }); - $filter->filter($request); - - $this->assertEquals([ - 'name' => 'asc' - ], $filter->getOrder()); - $this->assertEquals([ - 'filters' => 'bamboo' - ], $filter->getCriteria()); } public function testFilter() From 55bf538b015f11e88b777b8c6908684ce821c4ad Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 23 Oct 2016 21:14:16 +0200 Subject: [PATCH 06/49] add pagerfanta and knmenu bundles to tests kernel --- Tests/Fixtures/app/AppKernel.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/Fixtures/app/AppKernel.php b/Tests/Fixtures/app/AppKernel.php index 94069651e..90d8d3ebf 100644 --- a/Tests/Fixtures/app/AppKernel.php +++ b/Tests/Fixtures/app/AppKernel.php @@ -16,7 +16,9 @@ public function registerBundles() new Symfony\Bundle\MonologBundle\MonologBundle(), new Symfony\Bundle\WebProfilerBundle\WebProfilerBundle(), new LAG\AdminBundle\LAGAdminBundle(), - new \Test\TestBundle\TestTestBundle(), + new Knp\Bundle\MenuBundle\KnpMenuBundle(), + new WhiteOctober\PagerfantaBundle\WhiteOctoberPagerfantaBundle(), + new Test\TestBundle\TestTestBundle(), ]; } From 135540a46e1a12245d2e59a7d31204180c8b3b42 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 29 Oct 2016 16:14:47 +0200 Subject: [PATCH 07/49] update scrutinizer timeout --- .scrutinizer.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 4512c442a..a908b6c49 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -1,5 +1,6 @@ tools: - external_code_coverage: true + external_code_coverage: + timeout: 600 filter: excluded_paths: From 893206ff23ecbe5bc2f8e24c3a19a83ed87b8271 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 15:29:09 +0100 Subject: [PATCH 08/49] add profiling information for composer install in travis build --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 868a85da2..d391080a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ before_install: - cp Tests/Fixtures/app/config/parameters.yml.dist Tests/Fixtures/app/config/parameters.yml install: - - composer install + - composer install --profile --optimize-autoloader script: - bin/phpunit --coverage-text --coverage-clover=coverage.clover From 9580026cb03802980f74f3427803045dbf57c31e Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 15:39:32 +0100 Subject: [PATCH 09/49] temporary remove php 5.6 from Travis build as composer consumes too much memory and fails the build --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d391080a5..829359d17 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,8 @@ language: php php: - - 5.6 + # temporary remove php 5.6 from Travis build as composer consumes too much memory and fails the build + #- 5.6 - 7.0 - hhvm From 703bb2a8ac09ac9c3d21baa50e51a5d22fd1c6df Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 19:32:52 +0100 Subject: [PATCH 10/49] improve "order" action configuration when passing a wrong array of options --- Action/Configuration/ActionConfiguration.php | 32 +++++++++++++++++-- .../Configuration/ConfigurationException.php | 20 ++++++++++++ DataProvider/DataProvider.php | 4 ++- .../ConfigurationExceptionTest.php | 28 ++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 Action/Configuration/ConfigurationException.php create mode 100644 Tests/AdminBundle/Action/Configuration/ConfigurationExceptionTest.php diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index 31d1e5607..8bb703d26 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -102,9 +102,7 @@ public function configureOptions(OptionsResolver $resolver) // retrieved entities are sorted according to this option it should be an array indexed by the field name // with its order as a value - $resolver - ->setDefault('order', []) - ->setAllowedTypes('order', 'array'); + $this->configureOrder($resolver); // the action route should be a string $resolver @@ -249,4 +247,32 @@ public function configureOptions(OptionsResolver $resolver) }) ; } + + /** + * Configure the order option. + * + * @param OptionsResolver $resolver + */ + protected function configureOrder(OptionsResolver $resolver) + { + $resolver + ->setDefault('order', []) + ->setAllowedTypes('order', 'array') + ->setNormalizer('order', function(Options $options, $order) { + foreach ($order as $field => $sort) { + + if (is_string($sort)) { + throw new ConfigurationException( + 'Order value should be an array of string (["field" => $key]), got '.gettype($sort), + $this->actionName, + $this->admin + ); + } + // TODO add test: if the field does not exists in the target entity, an exception should be thrown + } + + return $order; + }) + ; + } } diff --git a/Action/Configuration/ConfigurationException.php b/Action/Configuration/ConfigurationException.php new file mode 100644 index 000000000..838228d78 --- /dev/null +++ b/Action/Configuration/ConfigurationException.php @@ -0,0 +1,20 @@ +getName(), + $actionName + ); + + parent::__construct($message); + } +} diff --git a/DataProvider/DataProvider.php b/DataProvider/DataProvider.php index f0898cfc1..2e8b5f845 100644 --- a/DataProvider/DataProvider.php +++ b/DataProvider/DataProvider.php @@ -86,7 +86,9 @@ public function remove($entity) */ public function create() { - $className = $this->repository->getClassName(); + $className = $this + ->repository + ->getClassName(); return new $className; } diff --git a/Tests/AdminBundle/Action/Configuration/ConfigurationExceptionTest.php b/Tests/AdminBundle/Action/Configuration/ConfigurationExceptionTest.php new file mode 100644 index 000000000..a425d5e58 --- /dev/null +++ b/Tests/AdminBundle/Action/Configuration/ConfigurationExceptionTest.php @@ -0,0 +1,28 @@ +createAdmin('test', [ + 'entity' => 'whatever', + 'form' => 'whatever' + ]) + ); + + $this->assertInstanceOf(Exception::class, $exception); + $this->assertEquals('My little message, for Admin test and action an_action', $exception->getMessage()); + } +} From a03a2539591b9e6b247059b8fe92109557f2ba84 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 19:33:09 +0100 Subject: [PATCH 11/49] add test for Action class --- README.md | 1 + Tests/AdminBundle/Action/ActionTest.php | 47 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 Tests/AdminBundle/Action/ActionTest.php diff --git a/README.md b/README.md index fce65a69b..86eab8d82 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ [![Build Status](https://travis-ci.org/larriereguichet/AdminBundle.svg?branch=master)](https://travis-ci.org/larriereguichet/AdminBundle) [![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/?branch=master) +[![Code Coverage](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/?branch=master) [![SensioLabsInsight](https://insight.sensiolabs.com/projects/c8e28654-44c7-46f3-9450-497e37bda3d0/mini.png)](https://insight.sensiolabs.com/projects/c8e28654-44c7-46f3-9450-497e37bda3d0) diff --git a/Tests/AdminBundle/Action/ActionTest.php b/Tests/AdminBundle/Action/ActionTest.php new file mode 100644 index 000000000..d552a27bc --- /dev/null +++ b/Tests/AdminBundle/Action/ActionTest.php @@ -0,0 +1,47 @@ +createAdmin('my_admin', [ + 'entity' => 'WhatEver', + 'form' => 'WhatEver', + 'actions' => [ + 'test_action' => [] + ] + ])); + $configuration->configureOptions($resolver); + $resolvedConfiguration = $resolver->resolve(); + $configuration->setParameters($resolvedConfiguration); + + $action = new Action('test_action', $configuration); + + $this->assertEquals('test_action', $action->getName()); + $this->assertEquals($configuration, $action->getConfiguration()); + $this->assertEquals([], $action->getFields()); + $this->assertEquals([], $action->getFilters()); + $this->assertEquals(['ROLE_ADMIN'], $action->getPermissions()); + $this->assertFalse($action->hasField('a field')); + + $field = new StringField(); + $field->setName('name'); + $action->setFields([ + 'name' => $field + ]); + + $this->assertEquals([ + 'name' => $field + ], $action->getFields()); + $this->assertTrue($action->hasField('name')); + } +} From b42763778cdd86f6913446943a6113fe0a98b787 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 19:33:33 +0100 Subject: [PATCH 12/49] fix possible error in TestEntity inclusion --- Tests/AdminBundle/Form/Handler/ListFormHandlerTest.php | 8 ++++---- Tests/Entity/{TestEntity.php => TestSimpleEntity.php} | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) rename Tests/Entity/{TestEntity.php => TestSimpleEntity.php} (90%) diff --git a/Tests/AdminBundle/Form/Handler/ListFormHandlerTest.php b/Tests/AdminBundle/Form/Handler/ListFormHandlerTest.php index 8305dbfe1..6c515af68 100644 --- a/Tests/AdminBundle/Form/Handler/ListFormHandlerTest.php +++ b/Tests/AdminBundle/Form/Handler/ListFormHandlerTest.php @@ -4,7 +4,7 @@ use LAG\AdminBundle\Form\Handler\ListFormHandler; use LAG\AdminBundle\Tests\AdminTestBase; -use LAG\AdminBundle\Tests\Entity\TestEntity; +use LAG\AdminBundle\Tests\Entity\TestSimpleEntity; use Symfony\Component\Form\Form; class ListFormHandlerTest extends AdminTestBase @@ -20,9 +20,9 @@ public function testHandle() ->willReturn([ 'batch_action' => [], 'entities' => [ - new TestEntity(23, 'test'), - new TestEntity(42, 'test'), - new TestEntity(64, 'test'), + new TestSimpleEntity(23, 'test'), + new TestSimpleEntity(42, 'test'), + new TestSimpleEntity(64, 'test'), ], 'batch_23' => true, 'batch_42' => true, diff --git a/Tests/Entity/TestEntity.php b/Tests/Entity/TestSimpleEntity.php similarity index 90% rename from Tests/Entity/TestEntity.php rename to Tests/Entity/TestSimpleEntity.php index abde23c0e..b944763fb 100644 --- a/Tests/Entity/TestEntity.php +++ b/Tests/Entity/TestSimpleEntity.php @@ -2,7 +2,7 @@ namespace LAG\AdminBundle\Tests\Entity; -class TestEntity +class TestSimpleEntity { protected $id; @@ -11,7 +11,7 @@ class TestEntity /** * TestEntity constructor. * - * @param null $id + * @param integer $id * @param string $name */ public function __construct($id = null, $name = '') @@ -21,7 +21,7 @@ public function __construct($id = null, $name = '') } /** - * @return null + * @return integer */ public function getId() { From 056759b6cc9039ba0a0114cc68d6fa158cb3da98 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 2 Nov 2016 19:41:16 +0100 Subject: [PATCH 13/49] improve Action unit test --- Tests/AdminBundle/Action/ActionTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Tests/AdminBundle/Action/ActionTest.php b/Tests/AdminBundle/Action/ActionTest.php index d552a27bc..bce06fd20 100644 --- a/Tests/AdminBundle/Action/ActionTest.php +++ b/Tests/AdminBundle/Action/ActionTest.php @@ -4,6 +4,7 @@ use LAG\AdminBundle\Action\Action; use LAG\AdminBundle\Action\Configuration\ActionConfiguration; +use LAG\AdminBundle\Admin\Filter; use LAG\AdminBundle\Field\Field\StringField; use LAG\AdminBundle\Tests\AdminTestBase; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -43,5 +44,17 @@ public function testAction() 'name' => $field ], $action->getFields()); $this->assertTrue($action->hasField('name')); + + $action->setFilters([ + 'test' + ]); + $this->assertEquals(['test'], $action->getFilters()); + + $filter = new Filter(); + $action->addFilter($filter); + $this->assertEquals([ + 'test', + $filter + ], $action->getFilters()); } } From a735d2bdcc7a0d15fa13f84aec6b72498245d05b Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Thu, 3 Nov 2016 18:56:47 +0100 Subject: [PATCH 14/49] add a data provider factory to manage data provider creation --- DataProvider/Factory/DataProviderFactory.php | 125 ++++++++++++++++++ .../Factory/DataProviderFactoryTest.php | 114 ++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 DataProvider/Factory/DataProviderFactory.php create mode 100644 Tests/AdminBundle/DataProvider/Factory/DataProviderFactoryTest.php diff --git a/DataProvider/Factory/DataProviderFactory.php b/DataProvider/Factory/DataProviderFactory.php new file mode 100644 index 000000000..fff7cf6e0 --- /dev/null +++ b/DataProvider/Factory/DataProviderFactory.php @@ -0,0 +1,125 @@ +entityManager = $entityManager; + } + + /** + * Add a data provider to the collection. + * + * @param string $id The data provider service id + * @param DataProviderInterface $dataProvider The data provider + * + * @throws Exception + */ + public function add($id, DataProviderInterface $dataProvider) + { + if ($this->has($id)) { + throw new Exception('Trying to add the data provider '.$id.' twice'); + } + // add the data provider to collection, indexed by ids + $this->dataProviders[$id] = $dataProvider; + } + + /** + * Return an configured data provider or try to create one for the given entity class. + * + * @param string|null id The id of an existing data provider service + * @param string|null $entityClass The class of the related entity. It will be used to find a repository in Doctrine + * registry + * + * @return DataProvider|DataProviderInterface + * + * @throws Exception + */ + public function get($id = null, $entityClass = null) + { + if (null === $id && null === $entityClass) { + throw new Exception('You should either provide an data provider or a entity class to get a data provider'); + } + + if (null !== $id && $this->has($id)) { + // a name is provided and the data provider exist, so we return the found data provider + $dataProvider = $this->dataProviders[$id]; + } else { + // no name was provided, so we try to create a generic data provider with th given entity class + $dataProvider = $this->create($entityClass); + } + + return $dataProvider; + } + + /** + * Return true if a data provider with the given id exists. + * + * @param string $id The data provider id + * + * @return bool + */ + public function has($id) + { + return array_key_exists($id, $this->dataProviders); + } + + /** + * Create a generic data provider. + * + * @param string $entityClass The class of the related entity + * @return DataProvider The created data provider + * + * @throws Exception An exception is thrown if the found repository with given entity class does not implements + * RepositoryInterface. + */ + public function create($entityClass) + { + // get the repository corresponding to the given entity class + $repository = $this + ->entityManager + ->getRepository($entityClass); + + // the repository should implements the RepositoryInterface, to ensure it has the methods create and save + if (!($repository instanceof RepositoryInterface)) { + $repositoryClass = get_class($repository); + + throw new Exception( + sprintf( + 'Repository %s should implements %s', + $repositoryClass, + RepositoryInterface::class + ) + ); + } + // create a new generic data provider from the found repository + $dataProvider = new DataProvider($repository); + + return $dataProvider; + } +} diff --git a/Tests/AdminBundle/DataProvider/Factory/DataProviderFactoryTest.php b/Tests/AdminBundle/DataProvider/Factory/DataProviderFactoryTest.php new file mode 100644 index 000000000..40932f939 --- /dev/null +++ b/Tests/AdminBundle/DataProvider/Factory/DataProviderFactoryTest.php @@ -0,0 +1,114 @@ +createMock(EntityManager::class); + $dataProvider = $this->createMock(DataProvider::class); + + $factory = new DataProviderFactory($entityManager); + $factory->add('a_data_provider', $dataProvider); + + $this->assertTrue($factory->has('a_data_provider')); + $this->assertEquals($dataProvider, $factory->get('a_data_provider')); + } + + /** + * Add method should throw an exception if we try to add a data provider twice. + */ + public function testAddException() + { + $entityManager = $this->createMock(EntityManager::class); + $dataProvider = $this->createMock(DataProvider::class); + + $factory = new DataProviderFactory($entityManager); + $factory->add('a_data_provider', $dataProvider); + + $this->assertExceptionRaised(Exception::class, function () use ($factory, $dataProvider) { + $factory->add('a_data_provider', $dataProvider); + }); + } + + /** + * Has method should return true if a data provider with the given id exists. + */ + public function testHas() + { + $entityManager = $this->createMock(EntityManager::class); + $dataProvider = $this->createMock(DataProvider::class); + + $factory = new DataProviderFactory($entityManager); + $factory->add('a_data_provider', $dataProvider); + + $this->assertTrue($factory->has('a_data_provider')); + $this->assertFalse($factory->has('no_data_provider_here')); + } + + /** + * Create method should return a generic data provider. + */ + public function testCreate() + { + $repository = $this->createMock(DoctrineRepository::class); + $entityManager = $this->createMock(EntityManager::class); + $entityManager + ->method('getRepository') + ->with('MyEntityClass') + ->willReturn($repository); + + $factory = new DataProviderFactory($entityManager); + $dataProvider = $factory->create('MyEntityClass'); + + $this->assertInstanceOf(DataProvider::class, $dataProvider); + } + + /** + * Create method should return an exception if the found repository does not implement the RepositoryInterface. + */ + public function testCreateException() + { + $repository = $this->createMock(EntityRepository::class); + $entityManager = $this->createMock(EntityManager::class); + $entityManager + ->method('getRepository') + ->with('MyEntityClass') + ->willReturn($repository); + + $factory = new DataProviderFactory($entityManager); + + $this->assertExceptionRaised(Exception::class, function () use ($factory) { + $factory->create('MyEntityClass'); + }); + } + + public function testGet() + { + $entityManager = $this->createMock(EntityManager::class); + $dataProvider = $this->createMock(DataProvider::class); + + $factory = new DataProviderFactory($entityManager); + $factory->add('a_data_provider', $dataProvider); + + $this->assertEquals($dataProvider, $factory->get('a_data_provider')); + $this->assertExceptionRaised(Exception::class, function () use ($factory) { + $factory->get('wrong_data_provider'); + }); + $this->assertExceptionRaised(Exception::class, function () use ($factory) { + $factory->get(null, null); + }); + } +} From f712c13fc61edf5f4aa66c9f3b84a10503eb295f Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Thu, 3 Nov 2016 19:00:58 +0100 Subject: [PATCH 15/49] add Scrutinizer build status to readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 86eab8d82..f33516154 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ [![Build Status](https://travis-ci.org/larriereguichet/AdminBundle.svg?branch=master)](https://travis-ci.org/larriereguichet/AdminBundle) [![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/?branch=master) [![Code Coverage](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/?branch=master) +[![Build Status](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/badges/build.png?b=master)](https://scrutinizer-ci.com/g/larriereguichet/AdminBundle/build-status/master) [![SensioLabsInsight](https://insight.sensiolabs.com/projects/c8e28654-44c7-46f3-9450-497e37bda3d0/mini.png)](https://insight.sensiolabs.com/projects/c8e28654-44c7-46f3-9450-497e37bda3d0) From e06bd5a01547cbedc000951b5069b3109af2f770 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Fri, 4 Nov 2016 00:34:28 +0100 Subject: [PATCH 16/49] add DataProviderCompilerPass test --- .../DataProviderCompilerPassTest.php | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php diff --git a/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php b/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php new file mode 100644 index 000000000..2a3605491 --- /dev/null +++ b/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php @@ -0,0 +1,72 @@ +addTag('data_provider'); + + // create the data providers factory definition + $factoryDefinition = new Definition(); + + + // add them to the container builder + $containerBuilder = new ContainerBuilder(); + $containerBuilder->addDefinitions([ + 'my_custom_provider' => $taggedServiceDefinition, + 'lag.admin.data_providers_factory' => $factoryDefinition + ]); + + // process the compiler pass + $compilerPass = new DataProviderCompilerPass(); + $compilerPass->process($containerBuilder); + + $calls = $containerBuilder->getDefinition('lag.admin.data_providers_factory')->getMethodCalls(); + + $this->assertCount(1, $calls); + $this->assertEquals('addDataProvider', $calls[0][0]); + $this->assertEquals('my_custom_provider', $calls[0][1][0]); + $this->assertInstanceOf(Reference::class, $calls[0][1][1]); + } + + /** + * Process method should not change the container builder if the admin factory definition does not exists. + */ + public function testProcessWithoutAdminFactory() + { + $containerBuilder = new ContainerBuilder(); + $compilerPass = new DataProviderCompilerPass(); + $compilerPass->process($containerBuilder); + + $this->assertCount(0, $containerBuilder->getDefinitions()); + $this->assertFalse($containerBuilder->has('lag.admin.data_providers_factory')); + } + + /** + * Process method should not change the container builder if no tagged services were found. + */ + public function testProcessWithoutTaggedServices() + { + $containerBuilder = new ContainerBuilder(); + $containerBuilder->set('lag.admin.factory', new Definition()); + + $compilerPass = new DataProviderCompilerPass(); + $compilerPass->process($containerBuilder); + + $this->assertCount(0, $containerBuilder->getDefinitions()); + $this->assertFalse($containerBuilder->has('lag.admin.data_providers_factory')); + } +} From dd8e8ad8a54a966673711b098d145507d5c73576 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:01:17 +0100 Subject: [PATCH 17/49] add DataProviderFactory to factories.yml --- Resources/config/factories.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Resources/config/factories.yml b/Resources/config/factories.yml index 15a734f36..ccc937966 100644 --- a/Resources/config/factories.yml +++ b/Resources/config/factories.yml @@ -9,12 +9,12 @@ services: arguments: - '%lag.admins%' - '@event_dispatcher' - - '@doctrine.orm.entity_manager' - '@lag.admin.configuration_factory' - '@lag.admin.action_factory' - '@lag.admin.message_handler' - '@lag.admin.registry' - '@lag.admin.request_filter_factory' + - '@lag.admin.data_providers_factory' lag.admin.action_factory: class: LAG\AdminBundle\Action\Factory\ActionFactory @@ -45,3 +45,8 @@ services: lag.admin.request_filter_factory: class: LAG\AdminBundle\Filter\Factory\RequestFilterFactory + + lag.admin.data_providers_factory: + class: LAG\AdminBundle\DataProvider\Factory\DataProviderFactory + arguments: + - '@doctrine.orm.entity_manager' From 49a10afa579599220fff9de8a334666d4beb30f0 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:01:52 +0100 Subject: [PATCH 18/49] add new AdminException class to add debug information to the exception message --- Admin/Exception/AdminException.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 Admin/Exception/AdminException.php diff --git a/Admin/Exception/AdminException.php b/Admin/Exception/AdminException.php new file mode 100644 index 000000000..daad9f432 --- /dev/null +++ b/Admin/Exception/AdminException.php @@ -0,0 +1,27 @@ +getName(), + $actionName + ); + + parent::__construct($message); + } +} From 42d3307f5b7137797586cc9684b99a3ea8d5a8ff Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:02:55 +0100 Subject: [PATCH 19/49] add test for DI Configuration --- DependencyInjection/Configuration.php | 6 ++-- .../DependencyInjection/ConfigurationTest.php | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 Tests/AdminBundle/DependencyInjection/ConfigurationTest.php diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 4cf457923..fb8dbeda2 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -34,7 +34,7 @@ public function getConfigTreeBuilder() /** * @return ArrayNodeDefinition|NodeDefinition */ - public function getAdminsConfigurationNode() + protected function getAdminsConfigurationNode() { $builder = new TreeBuilder(); $node = $builder->root('admins'); @@ -61,7 +61,7 @@ public function getAdminsConfigurationNode() /** * @return ArrayNodeDefinition|NodeDefinition */ - public function getApplicationNode() + protected function getApplicationNode() { $builder = new TreeBuilder(); $node = $builder->root('application'); @@ -103,7 +103,7 @@ public function getApplicationNode() /** * @return ArrayNodeDefinition|NodeDefinition */ - public function getMenuConfiguration() + protected function getMenuConfiguration() { $builder = new TreeBuilder(); $node = $builder->root('menus'); diff --git a/Tests/AdminBundle/DependencyInjection/ConfigurationTest.php b/Tests/AdminBundle/DependencyInjection/ConfigurationTest.php new file mode 100644 index 000000000..342863aeb --- /dev/null +++ b/Tests/AdminBundle/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,33 @@ +getConfigTreeBuilder(); + /** @var ArrayNode $arrayNode */ + $arrayNode = $tree->buildTree(); + $this->assertInstanceOf(ArrayNode::class, $arrayNode); + + $arrayConfiguration = $arrayNode->getChildren(); + + // test application configuration + $this->assertArrayHasKey('application', $arrayConfiguration); + $this->assertInstanceOf(ArrayNode::class, $arrayNode->getChildren()['application']); + $this->assertArrayHasKey('admins', $arrayConfiguration); + $this->assertInstanceOf(ArrayNode::class, $arrayNode->getChildren()['admins']); + $this->assertArrayHasKey('menus', $arrayConfiguration); + $this->assertInstanceOf(ArrayNode::class, $arrayNode->getChildren()['menus']); + } +} From 0f155496f7a54184fa0cb33c7a3b15258c67faaf Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:04:24 +0100 Subject: [PATCH 20/49] add tagged data providers to the DataProviderFactory instead of AdminFactory --- DependencyInjection/DataProviderCompilerPass.php | 9 +++++++-- .../DependencyInjection/DataProviderCompilerPassTest.php | 9 +++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/DependencyInjection/DataProviderCompilerPass.php b/DependencyInjection/DataProviderCompilerPass.php index ea5d07ae0..c07530168 100644 --- a/DependencyInjection/DataProviderCompilerPass.php +++ b/DependencyInjection/DataProviderCompilerPass.php @@ -8,13 +8,18 @@ class DataProviderCompilerPass implements CompilerPassInterface { + /** + * Add the tagged data provider to the DataProviderFactory. + * + * @param ContainerBuilder $container + */ public function process(ContainerBuilder $container) { - if (!$container->has('lag.admin.factory')) { + if (!$container->has('lag.admin.data_providers_factory')) { return; } - $definition = $container->findDefinition('lag.admin.factory'); + $definition = $container->findDefinition('lag.admin.data_providers_factory'); $taggedServices = $container->findTaggedServiceIds('data_provider'); foreach ($taggedServices as $id => $tags) { diff --git a/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php b/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php index 2a3605491..da0abff44 100644 --- a/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php +++ b/Tests/AdminBundle/DependencyInjection/DataProviderCompilerPassTest.php @@ -22,7 +22,6 @@ public function testProcess() // create the data providers factory definition $factoryDefinition = new Definition(); - // add them to the container builder $containerBuilder = new ContainerBuilder(); $containerBuilder->addDefinitions([ @@ -34,7 +33,9 @@ public function testProcess() $compilerPass = new DataProviderCompilerPass(); $compilerPass->process($containerBuilder); - $calls = $containerBuilder->getDefinition('lag.admin.data_providers_factory')->getMethodCalls(); + $calls = $containerBuilder + ->getDefinition('lag.admin.data_providers_factory') + ->getMethodCalls(); $this->assertCount(1, $calls); $this->assertEquals('addDataProvider', $calls[0][0]); @@ -61,12 +62,12 @@ public function testProcessWithoutAdminFactory() public function testProcessWithoutTaggedServices() { $containerBuilder = new ContainerBuilder(); - $containerBuilder->set('lag.admin.factory', new Definition()); + $containerBuilder->setDefinition('lag.admin.factory', new Definition()); $compilerPass = new DataProviderCompilerPass(); $compilerPass->process($containerBuilder); - $this->assertCount(0, $containerBuilder->getDefinitions()); + $this->assertCount(1, $containerBuilder->getDefinitions()); $this->assertFalse($containerBuilder->has('lag.admin.data_providers_factory')); } } From 2580511d2bc1ddf34adea15ceb2e35dfc0ebc418 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:05:01 +0100 Subject: [PATCH 21/49] add fake entity manager for tests --- Tests/Utils/FakeEntityManager.php | 536 ++++++++++++++++++++++++++++++ 1 file changed, 536 insertions(+) create mode 100644 Tests/Utils/FakeEntityManager.php diff --git a/Tests/Utils/FakeEntityManager.php b/Tests/Utils/FakeEntityManager.php new file mode 100644 index 000000000..2a00d7f19 --- /dev/null +++ b/Tests/Utils/FakeEntityManager.php @@ -0,0 +1,536 @@ + + * $qb = $em->createQueryBuilder(); + * $expr = $em->getExpressionBuilder(); + * $qb->select('u')->from('User', 'u') + * ->where($expr->orX($expr->eq('u.id', 1), $expr->eq('u.id', 2))); + * + * + * @return \Doctrine\ORM\Query\Expr + */ + public function getExpressionBuilder() + { + // TODO: Implement getExpressionBuilder() method. + } + + /** + * Starts a transaction on the underlying database connection. + * + * @return void + */ + public function beginTransaction() + { + // TODO: Implement beginTransaction() method. + } + + /** + * Executes a function in a transaction. + * + * The function gets passed this EntityManager instance as an (optional) parameter. + * + * {@link flush} is invoked prior to transaction commit. + * + * If an exception occurs during execution of the function or flushing or transaction commit, + * the transaction is rolled back, the EntityManager closed and the exception re-thrown. + * + * @param callable $func The function to execute transactionally. + * + * @return mixed The non-empty value returned from the closure or true instead. + */ + public function transactional($func) + { + // TODO: Implement transactional() method. + } + + /** + * Commits a transaction on the underlying database connection. + * + * @return void + */ + public function commit() + { + // TODO: Implement commit() method. + } + + /** + * Performs a rollback on the underlying database connection. + * + * @return void + */ + public function rollback() + { + // TODO: Implement rollback() method. + } + + /** + * Creates a new Query object. + * + * @param string $dql The DQL string. + * + * @return Query + */ + public function createQuery($dql = '') + { + // TODO: Implement createQuery() method. + } + + /** + * Creates a Query from a named query. + * + * @param string $name + * + * @return Query + */ + public function createNamedQuery($name) + { + // TODO: Implement createNamedQuery() method. + } + + /** + * Creates a native SQL query. + * + * @param string $sql + * @param ResultSetMapping $rsm The ResultSetMapping to use. + * + * @return NativeQuery + */ + public function createNativeQuery($sql, ResultSetMapping $rsm) + { + // TODO: Implement createNativeQuery() method. + } + + /** + * Creates a NativeQuery from a named native query. + * + * @param string $name + * + * @return NativeQuery + */ + public function createNamedNativeQuery($name) + { + // TODO: Implement createNamedNativeQuery() method. + } + + /** + * Create a QueryBuilder instance + * + * @return QueryBuilder + */ + public function createQueryBuilder() + { + // TODO: Implement createQueryBuilder() method. + } + + /** + * Gets a reference to the entity identified by the given type and identifier + * without actually loading it, if the entity is not yet loaded. + * + * @param string $entityName The name of the entity type. + * @param mixed $id The entity identifier. + * + * @return object The entity reference. + * + * @throws ORMException + */ + public function getReference($entityName, $id) + { + // TODO: Implement getReference() method. + } + + /** + * Gets a partial reference to the entity identified by the given type and identifier + * without actually loading it, if the entity is not yet loaded. + * + * The returned reference may be a partial object if the entity is not yet loaded/managed. + * If it is a partial object it will not initialize the rest of the entity state on access. + * Thus you can only ever safely access the identifier of an entity obtained through + * this method. + * + * The use-cases for partial references involve maintaining bidirectional associations + * without loading one side of the association or to update an entity without loading it. + * Note, however, that in the latter case the original (persistent) entity data will + * never be visible to the application (especially not event listeners) as it will + * never be loaded in the first place. + * + * @param string $entityName The name of the entity type. + * @param mixed $identifier The entity identifier. + * + * @return object The (partial) entity reference. + */ + public function getPartialReference($entityName, $identifier) + { + // TODO: Implement getPartialReference() method. + } + + /** + * Closes the EntityManager. All entities that are currently managed + * by this EntityManager become detached. The EntityManager may no longer + * be used after it is closed. + * + * @return void + */ + public function close() + { + // TODO: Implement close() method. + } + + /** + * Creates a copy of the given entity. Can create a shallow or a deep copy. + * + * @param object $entity The entity to copy. + * @param boolean $deep FALSE for a shallow copy, TRUE for a deep copy. + * + * @return object The new entity. + * + * @throws \BadMethodCallException + */ + public function copy($entity, $deep = false) + { + // TODO: Implement copy() method. + } + + /** + * Acquire a lock on the given entity. + * + * @param object $entity + * @param int $lockMode + * @param int|null $lockVersion + * + * @return void + * + * @throws OptimisticLockException + * @throws PessimisticLockException + */ + public function lock($entity, $lockMode, $lockVersion = null) + { + // TODO: Implement lock() method. + } + + /** + * Gets the EventManager used by the EntityManager. + * + * @return \Doctrine\Common\EventManager + */ + public function getEventManager() + { + // TODO: Implement getEventManager() method. + } + + /** + * Gets the Configuration used by the EntityManager. + * + * @return Configuration + */ + public function getConfiguration() + { + // TODO: Implement getConfiguration() method. + } + + /** + * Check if the Entity manager is open or closed. + * + * @return bool + */ + public function isOpen() + { + // TODO: Implement isOpen() method. + } + + /** + * Gets the UnitOfWork used by the EntityManager to coordinate operations. + * + * @return UnitOfWork + */ + public function getUnitOfWork() + { + // TODO: Implement getUnitOfWork() method. + } + + /** + * Gets a hydrator for the given hydration mode. + * + * This method caches the hydrator instances which is used for all queries that don't + * selectively iterate over the result. + * + * @deprecated + * + * @param int $hydrationMode + * + * @return \Doctrine\ORM\Internal\Hydration\AbstractHydrator + */ + public function getHydrator($hydrationMode) + { + // TODO: Implement getHydrator() method. + } + + /** + * Create a new instance for the given hydration mode. + * + * @param int $hydrationMode + * + * @return \Doctrine\ORM\Internal\Hydration\AbstractHydrator + * + * @throws ORMException + */ + public function newHydrator($hydrationMode) + { + // TODO: Implement newHydrator() method. + } + + /** + * Gets the proxy factory used by the EntityManager to create entity proxies. + * + * @return \Doctrine\ORM\Proxy\ProxyFactory + */ + public function getProxyFactory() + { + // TODO: Implement getProxyFactory() method. + } + + /** + * Gets the enabled filters. + * + * @return \Doctrine\ORM\Query\FilterCollection The active filter collection. + */ + public function getFilters() + { + // TODO: Implement getFilters() method. + } + + /** + * Checks whether the state of the filter collection is clean. + * + * @return boolean True, if the filter collection is clean. + */ + public function isFiltersStateClean() + { + // TODO: Implement isFiltersStateClean() method. + } + + /** + * Checks whether the Entity Manager has filters. + * + * @return boolean True, if the EM has a filter collection. + */ + public function hasFilters() + { + // TODO: Implement hasFilters() method. + } + + /** + * Finds an object by its identifier. + * + * This is just a convenient shortcut for getRepository($className)->find($id). + * + * @param string $className The class name of the object to find. + * @param mixed $id The identity of the object to find. + * + * @return object The found object. + */ + public function find($className, $id) + { + // TODO: Implement find() method. + } + + /** + * Tells the ObjectManager to make an instance managed and persistent. + * + * The object will be entered into the database as a result of the flush operation. + * + * NOTE: The persist operation always considers objects that are not yet known to + * this ObjectManager as NEW. Do not pass detached objects to the persist operation. + * + * @param object $object The instance to make managed and persistent. + * + * @return void + */ + public function persist($object) + { + // TODO: Implement persist() method. + } + + /** + * Removes an object instance. + * + * A removed object will be removed from the database as a result of the flush operation. + * + * @param object $object The object instance to remove. + * + * @return void + */ + public function remove($object) + { + // TODO: Implement remove() method. + } + + /** + * Merges the state of a detached object into the persistence context + * of this ObjectManager and returns the managed copy of the object. + * The object passed to merge will not become associated/managed with this ObjectManager. + * + * @param object $object + * + * @return object + */ + public function merge($object) + { + // TODO: Implement merge() method. + } + + /** + * Clears the ObjectManager. All objects that are currently managed + * by this ObjectManager become detached. + * + * @param string|null $objectName if given, only objects of this type will get detached. + * + * @return void + */ + public function clear($objectName = null) + { + // TODO: Implement clear() method. + } + + /** + * Detaches an object from the ObjectManager, causing a managed object to + * become detached. Unflushed changes made to the object if any + * (including removal of the object), will not be synchronized to the database. + * Objects which previously referenced the detached object will continue to + * reference it. + * + * @param object $object The object to detach. + * + * @return void + */ + public function detach($object) + { + // TODO: Implement detach() method. + } + + /** + * Refreshes the persistent state of an object from the database, + * overriding any local changes that have not yet been persisted. + * + * @param object $object The object to refresh. + * + * @return void + */ + public function refresh($object) + { + // TODO: Implement refresh() method. + } + + /** + * Flushes all changes to objects that have been queued up to now to the database. + * This effectively synchronizes the in-memory state of managed objects with the + * database. + * + * @return void + */ + public function flush() + { + // TODO: Implement flush() method. + } + + /** + * Gets the repository for a class. + * + * @param string $className + * + * @return \Doctrine\Common\Persistence\ObjectRepository + */ + public function getRepository($className) + { + // TODO: Implement getRepository() method. + } + + /** + * Gets the metadata factory used to gather the metadata of classes. + * + * @return \Doctrine\Common\Persistence\Mapping\ClassMetadataFactory + */ + public function getMetadataFactory() + { + // TODO: Implement getMetadataFactory() method. + } + + /** + * Helper method to initialize a lazy loading proxy or persistent collection. + * + * This method is a no-op for other objects. + * + * @param object $obj + * + * @return void + */ + public function initializeObject($obj) + { + // TODO: Implement initializeObject() method. + } + + /** + * Checks if the object is part of the current UnitOfWork and therefore managed. + * + * @param object $object + * + * @return bool + */ + public function contains($object) + { + // TODO: Implement contains() method. + } + + function __call($name, $arguments) + { + // TODO: Implement @method Mapping\ClassMetadata getClassMetadata($className) + } + + public function getClassMetadata($className) + { + + } +} From 0040da6389506b647ccacfc229967331098defe3 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:05:15 +0100 Subject: [PATCH 22/49] add test for DI Configuration --- .../FieldCompilerPassTest.php | 112 +++++++++++++ .../LAGAdminExtensionTest.php | 148 ++++++++++++++++++ 2 files changed, 260 insertions(+) create mode 100644 Tests/AdminBundle/DependencyInjection/FieldCompilerPassTest.php create mode 100644 Tests/AdminBundle/DependencyInjection/LAGAdminExtensionTest.php diff --git a/Tests/AdminBundle/DependencyInjection/FieldCompilerPassTest.php b/Tests/AdminBundle/DependencyInjection/FieldCompilerPassTest.php new file mode 100644 index 000000000..cef69b3d4 --- /dev/null +++ b/Tests/AdminBundle/DependencyInjection/FieldCompilerPassTest.php @@ -0,0 +1,112 @@ +addTag('lag.field', [ + 'type' => 'string' + ]); + + // create the data providers factory definition + $factoryDefinition = new Definition(); + + // add them to the container builder + $containerBuilder->addDefinitions([ + 'one_field' => $taggedServiceDefinition, + 'lag.admin.field_factory' => $factoryDefinition + ]); + + // process the compiler pass + $compilerPass->process($containerBuilder); + + $calls = $containerBuilder->getDefinition('lag.admin.field_factory')->getMethodCalls(); + + $this->assertCount(1, $calls); + $this->assertEquals('addFieldMapping', $calls[0][0]); + $this->assertEquals('string', $calls[0][1][0]); + $this->assertEquals('one_field', $calls[0][1][1]); + + $fieldCalls = $taggedServiceDefinition->getMethodCalls(); + $this->assertCount(1, $fieldCalls); + $this->assertEquals('setConfiguration', $fieldCalls[0][0]); + + $this->assertInstanceOf(Reference::class, $fieldCalls[0][1][0]); + $this->assertEquals('lag.admin.application', (string)$fieldCalls[0][1][0]); + + } + + public function testProcessWrongConfiguration() + { + $compilerPass = new FieldCompilerPass(); + $containerBuilder = new ContainerBuilder(); + + // create a tagged service definition + $taggedServiceDefinition = new Definition(); + $taggedServiceDefinition->addTag('lag.field'); + + // create the data providers factory definition + $factoryDefinition = new Definition(); + + // add them to the container builder + $containerBuilder->addDefinitions([ + 'my_custom_provider' => $taggedServiceDefinition, + 'lag.admin.field_factory' => $factoryDefinition + ]); + + $this->assertExceptionRaised( + InvalidConfigurationException::class, + function () use ($compilerPass, $containerBuilder) { + // process the compiler pass + $compilerPass->process($containerBuilder); + } + ); + } + + /** + * Process method should not change the container builder if the admin factory definition does not exists. + */ + public function testProcessWithoutAdminFactory() + { + $containerBuilder = new ContainerBuilder(); + $compilerPass = new FieldCompilerPass(); + $compilerPass->process($containerBuilder); + + $this->assertCount(0, $containerBuilder->getDefinitions()); + $this->assertFalse($containerBuilder->has('lag.admin.data_providers_factory')); + } + + /** + * Process method should not change the container builder if no tagged services were found. + */ + public function testProcessWithoutTaggedServices() + { + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setDefinition('lag.admin.field_factory', new Definition()); + + $compilerPass = new FieldCompilerPass(); + $compilerPass->process($containerBuilder); + + $this->assertCount(1, $containerBuilder->getDefinitions()); + $this->assertTrue($containerBuilder->hasDefinition('lag.admin.field_factory')); + } +} diff --git a/Tests/AdminBundle/DependencyInjection/LAGAdminExtensionTest.php b/Tests/AdminBundle/DependencyInjection/LAGAdminExtensionTest.php new file mode 100644 index 000000000..601e56d25 --- /dev/null +++ b/Tests/AdminBundle/DependencyInjection/LAGAdminExtensionTest.php @@ -0,0 +1,148 @@ +getWorkingContainer(); + $extension = new LAGAdminExtension(); + $extension->load([ + 'lag_admin' => [ + 'application' => [] + ] + ], $container); + $this->assertCount(28, $container->getDefinitions()); + + $eventDispatcherExtension = new FrameworkExtension(); + $eventDispatcherExtension->load([], $container); + + $twigExtension = new TwigExtension(); + $twigExtension->load([], $container); + + $knpMenuExtension = new KnpMenuExtension(); + $knpMenuExtension->load([], $container); + + // the container should compile without errors + $container->compile(); + $this->assertTrue(true); + + $this->assertServices($container); + } + + /** + * Load method should throw an exception if no application section was found. + */ + public function testLoadWithoutApplication() + { + $container = new ContainerBuilder(); + $extension = new LAGAdminExtension(); + + $this->assertExceptionRaised(InvalidConfigurationException::class, function () use ($extension, $container) { + $extension->load([], $container); + }); + } + + /** + * GetAlias method should return "lag_admin". + */ + public function testGetAlias() + { + $container = new ContainerBuilder(); + $extension = new LAGAdminExtension(); + $extension->load([ + 'lag_admin' => [ + 'application' => [] + ] + ], $container); + + $this->assertEquals('lag_admin', $extension->getAlias()); + } + + protected function assertServices(ContainerBuilder $container) + { + // assert factories are rightly instanciate + $this->assertInstanceOf(ConfigurationFactory::class, $container->get('lag.admin.configuration_factory')); + $this->assertInstanceOf(AdminFactory::class, $container->get('lag.admin.factory')); + $this->assertInstanceOf(ActionFactory::class, $container->get('lag.admin.action_factory')); + $this->assertInstanceOf(FieldFactory::class, $container->get('lag.admin.field_factory')); + $this->assertInstanceOf(FilterFactory::class, $container->get('lag.admin.filter_factory')); + $this->assertInstanceOf(MenuFactory::class, $container->get('lag.admin.menu_factory')); + $this->assertInstanceOf(RequestFilterFactory::class, $container->get('lag.admin.request_filter_factory')); + $this->assertInstanceOf(DataProviderFactory::class, $container->get('lag.admin.data_providers_factory')); + } + + /** + * Return a working container builder to compile. + * + * @return ContainerBuilder + */ + protected function getWorkingContainer() + { + $generic = new Definition(); + $generic->setClass(stdClass::class); + $fileLocator = new Definition(); + $fileLocator->setClass(FileLocator::class); + $templateNameParser = new Definition(); + $templateNameParser->setClass(TemplateNameParser::class); + $templateNameParser->addArgument($this->createMock(KernelInterface::class)); + $logger = new Definition(); + $logger->setClass(Logger::class); + $logger->addArgument('default'); + $session= new Definition(); + $session->setClass(Session::class); + + $entityManager = new Definition(); + $entityManager->setClass(FakeEntityManager::class); + + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', false); + $container->setParameter('kernel.cache_dir', sys_get_temp_dir().'/AdminBundleTests/cache'); + $container->setParameter('kernel.root_dir', realpath(__DIR__.'/../../..')); + $container->setParameter('kernel.charset', 'utf8'); + $container->setParameter('kernel.secret', 'MyLittleSecret'); + $container->setParameter('kernel.bundles', []); + $container->setParameter('kernel.environment', 'prod'); + + $container->setDefinitions([ + 'doctrine.orm.entity_manager' => $entityManager, + 'doctrine' => $generic, + 'router' => $generic, + 'logger' => $logger, + 'session' => $session, + 'form.factory' => $generic, + 'templating.locator' => $fileLocator, + 'templating.name_parser' => $templateNameParser, + ]); + + return $container; + } +} From 0a6e330893a0b4c562afe36465a04555dcdaa764 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:05:59 +0100 Subject: [PATCH 23/49] remove unused test in LAGAdminExtension --- DependencyInjection/LAGAdminExtension.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/DependencyInjection/LAGAdminExtension.php b/DependencyInjection/LAGAdminExtension.php index 149e485be..d308c9b7a 100644 --- a/DependencyInjection/LAGAdminExtension.php +++ b/DependencyInjection/LAGAdminExtension.php @@ -9,11 +9,13 @@ use Symfony\Component\HttpKernel\DependencyInjection\Extension; /** - * This is the class that loads and manages your bundle configuration. + * Load AdminBundle configuration into the container. */ class LAGAdminExtension extends Extension { /** + * Load the configuration into the container. + * * @param array $configs * @param ContainerBuilder $container */ @@ -28,9 +30,6 @@ public function load(array $configs, ContainerBuilder $container) if (!array_key_exists('application', $config)) { throw new InvalidConfigurationException('Your section "application" is not found for AdminBundle configuration'); } - if (!array_key_exists('admins', $config)) { - throw new InvalidConfigurationException('Your section "admins" is not found for AdminBundle configuration'); - } if (!array_key_exists('enable_extra_configuration', $config['application'])) { $config['application']['enable_extra_configuration'] = true; } From 24c4e91777a220ecc5d39776762c59feea8d7dbe Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:06:39 +0100 Subject: [PATCH 24/49] fix comments --- Controller/CRUDController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Controller/CRUDController.php b/Controller/CRUDController.php index 758d58d81..bf475fd01 100644 --- a/Controller/CRUDController.php +++ b/Controller/CRUDController.php @@ -121,7 +121,7 @@ public function batchAction(Request $request) * * @Template("LAGAdminBundle:CRUD:edit.html.twig") * @param Request $request - * @return array + * @return array|RedirectResponse */ public function createAction(Request $request) { From e4b9023c64da0b14a883a8aef32c31b14032baf4 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:08:22 +0100 Subject: [PATCH 25/49] - fix data provider method choosing in Admin class - move away the data provider creation from the AdminFactory - update tests in consequence (and add additional tests) --- Admin/Admin.php | 132 ++++++++++---- Admin/AdminInterface.php | 2 +- Admin/Factory/AdminFactory.php | 79 ++------- Tests/AdminBundle/Admin/AdminTest.php | 161 +++++++++++++++++- .../Admin/Behaviors/AdminTraitTest.php | 46 +++++ .../Admin/Behaviors/EntityLabelTraitTest.php | 3 +- .../Admin/Factory/AdminFactoryTest.php | 17 -- Tests/AdminTestBase.php | 5 +- 8 files changed, 322 insertions(+), 123 deletions(-) create mode 100644 Tests/AdminBundle/Admin/Behaviors/AdminTraitTest.php diff --git a/Admin/Admin.php b/Admin/Admin.php index c5db90c6e..82cc3bdf5 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -10,7 +10,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Exception; use LAG\AdminBundle\DataProvider\DataProviderInterface; -use LAG\AdminBundle\Exception\AdminException; +use LAG\AdminBundle\Admin\Exception\AdminException; use LAG\AdminBundle\Filter\RequestFilterInterface; use LAG\AdminBundle\Message\MessageHandlerInterface; use LAG\AdminBundle\Pager\PagerFantaAdminAdapter; @@ -304,7 +304,7 @@ public function generateRouteName($actionName) } /** - * Load entities manually according to criteria. + * Load entities according to the given criteria and the current action configuration. * * @param array $criteria * @param array $orderBy @@ -312,44 +312,56 @@ public function generateRouteName($actionName) * @param int $offset * @throws Exception */ - public function load(array $criteria, $orderBy = [], $limit = 25, $offset = 1) + public function load(array $criteria, array $orderBy = [], $limit = 25, $offset = 1) { - $actionConfiguration = $this - ->getCurrentAction() - ->getConfiguration(); - $pager = $actionConfiguration->getParameter('pager'); - $requirePagination = $this - ->getCurrentAction() - ->isPaginationRequired(); - - if ($pager == 'pagerfanta' && $requirePagination) { - // adapter to pagerfanta - $adapter = new PagerFantaAdminAdapter($this->dataProvider, $criteria, $orderBy); - // create pager - $this->pager = new Pagerfanta($adapter); - $this->pager->setMaxPerPage($limit); - $this->pager->setCurrentPage($offset); - - $entities = $this - ->pager - ->getCurrentPageResults(); - } else { - // if the current action should retrieve only one entity, the offset should be zero - if ($actionConfiguration->getParameter('load_strategy') !== AdminInterface::LOAD_STRATEGY_MULTIPLE) { - $offset = 0; + $currentAction = $this->getCurrentAction(); + $currentActionConfiguration = $currentAction->getConfiguration(); + + // some action, such as create, does not require the entities to be loaded + if (!$currentAction->isLoadingRequired()) { + return; + } + $pager = $currentActionConfiguration->getParameter('pager'); + + if ($currentAction->isPaginationRequired() && $pager) { + $loadStrategy = $currentActionConfiguration->getParameter('load_strategy'); + + // only pagerfanta adapter is yet supported + if ('pagerfanta' !== $pager) { + throw new AdminException( + 'Only pagerfanta value is allowed for pager parameter, given '.$pager, + $currentAction->getName(), + $this + ); } - $entities = $this - ->dataProvider - ->findBy($criteria, $orderBy, $limit, $offset); + // only load strategy multiple is allowed for pagination (ie, can not paginate if only one entity is loaded) + if (AdminInterface::LOAD_STRATEGY_MULTIPLE !== $loadStrategy) { + throw new AdminException( + 'Only "strategy_multiple" value is allowed for pager parameter, given '.$loadStrategy, + $currentAction->getName(), + $this + ); + } + // load entities using a pager + $this->loadPaginate($criteria, $orderBy, $limit, $offset); + } else { + $this->loadWithoutPagination($criteria, $orderBy, $limit, $offset); } - if (!is_array($entities) && !($entities instanceof Collection)) { - throw new Exception('The data provider should return either a collection or an array. Got '.gettype($entities).' instead'); + + // the data provider should return an array or a collection of entities. + if (!is_array($this->entities) && !$this->entities instanceof Collection) { + throw new AdminException( + 'The data provider should return either a collection or an array. Got ' + .gettype($this->entities).' instead', + $currentAction->getName(), + $this + ); } - if (is_array($entities)) { - $entities = new ArrayCollection($entities); + // if an array is provided, transform it to a collection to be more convenient + if (is_array($this->entities)) { + $this->entities = new ArrayCollection($this->entities); } - $this->entities = $entities; } /** @@ -372,10 +384,12 @@ public function getEntities() public function getUniqueEntity() { if ($this->entities->count() == 0) { - throw new Exception("Entity not found in admin \"{$this->getName()}\"."); + throw new Exception('Entity not found in admin "'.$this->getName()); } if ($this->entities->count() > 1) { - throw new Exception("Too much entities found in admin \"{$this->getName()}\"."); + throw new Exception( + 'Too much entities found in admin "{$this->getName()}" ('.$this->entities->count().').' + ); } return $this->entities->first(); } @@ -526,4 +540,50 @@ protected function generateMessageTranslationKey($message) $this->name ); } + + /** + * Load entities using PagerFanta. + * + * @param array $criteria + * @param array $orderBy + * @param int $limit + * @param int $offset + */ + protected function loadPaginate(array $criteria, array $orderBy, $limit, $offset) + { + // adapter to pagerfanta + $adapter = new PagerFantaAdminAdapter($this->dataProvider, $criteria, $orderBy); + // create pager + $this->pager = new Pagerfanta($adapter); + $this->pager->setMaxPerPage($limit); + $this->pager->setCurrentPage($offset); + + $this->entities = $this + ->pager + ->getCurrentPageResults(); + } + + /** + * Load entities using to configured data provider. + * + * @param array $criteria + * @param array $orderBy + * @param int $limit + * @param int $offset + */ + protected function loadWithoutPagination(array $criteria, $orderBy, $limit, $offset) + { + $currentAction = $this->getCurrentAction(); + $currentActionConfiguration = $currentAction->getConfiguration(); + + // if the current action should retrieve only one entity, the offset should be zero + if ($currentActionConfiguration->getParameter('load_strategy') !== AdminInterface::LOAD_STRATEGY_MULTIPLE) { + $offset = 0; + $limit = 1; + } + // load entities according to the given parameters + $this->entities = $this + ->dataProvider + ->findBy($criteria, $orderBy, $limit, $offset); + } } diff --git a/Admin/AdminInterface.php b/Admin/AdminInterface.php index 15ba575ad..90274384b 100644 --- a/Admin/AdminInterface.php +++ b/Admin/AdminInterface.php @@ -56,7 +56,7 @@ public function generateRouteName($actionName); * @param null $limit * @param null $offset */ - public function load(array $criteria, $orderBy = [], $limit = null, $offset = null); + public function load(array $criteria, array $orderBy = [], $limit = null, $offset = null); /** * Save loaded entities. diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index 7d684e95c..b24deb0eb 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -2,23 +2,21 @@ namespace LAG\AdminBundle\Admin\Factory; -use Doctrine\ORM\EntityManager; use LAG\AdminBundle\Action\Factory\ActionFactory; use LAG\AdminBundle\Admin\Admin; use LAG\AdminBundle\Admin\AdminInterface; +use LAG\AdminBundle\Admin\Configuration\AdminConfiguration; use LAG\AdminBundle\Admin\Event\AdminCreatedEvent; use LAG\AdminBundle\Admin\Event\AdminCreateEvent; use LAG\AdminBundle\Admin\Event\BeforeConfigurationEvent; use LAG\AdminBundle\Admin\Registry\Registry; use LAG\AdminBundle\Configuration\Factory\ConfigurationFactory; use LAG\AdminBundle\DataProvider\DataProvider; -use LAG\AdminBundle\DataProvider\DataProviderInterface; use LAG\AdminBundle\Admin\Event\AdminEvents; use Exception; +use LAG\AdminBundle\DataProvider\Factory\DataProviderFactory; use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; use LAG\AdminBundle\Message\MessageHandlerInterface; -use LAG\AdminBundle\Repository\RepositoryInterface; -use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -39,25 +37,13 @@ class AdminFactory */ protected $eventDispatcher; - /** - * @var EntityManager - */ - protected $entityManager; - /** * @var ConfigurationFactory */ protected $configurationFactory; /** - * User custom data provider, indexed by service id - * - * @var ParameterBagInterface - */ - protected $dataProviders; - - /** - * @var array + * @var AdminConfiguration[] */ protected $adminConfigurations; @@ -81,30 +67,34 @@ class AdminFactory */ protected $requestFilterFactory; + /** + * @var DataProviderFactory + */ + protected $dataProviderFactory; + /** * AdminFactory constructor. * * @param array $adminConfigurations * @param EventDispatcherInterface $eventDispatcher - * @param EntityManager $entityManager * @param ConfigurationFactory $configurationFactory * @param ActionFactory $actionFactory * @param MessageHandlerInterface $messageHandler * @param Registry $registry * @param RequestFilterFactory $requestFilterFactory + * @param DataProviderFactory $dataProviderFactory */ public function __construct( array $adminConfigurations, EventDispatcherInterface $eventDispatcher, - EntityManager $entityManager, ConfigurationFactory $configurationFactory, ActionFactory $actionFactory, MessageHandlerInterface $messageHandler, Registry $registry, - RequestFilterFactory $requestFilterFactory + RequestFilterFactory $requestFilterFactory, + DataProviderFactory $dataProviderFactory ) { $this->eventDispatcher = $eventDispatcher; - $this->entityManager = $entityManager; $this->configurationFactory = $configurationFactory; $this->adminConfigurations = $adminConfigurations; $this->actionFactory = $actionFactory; @@ -112,6 +102,7 @@ public function __construct( $this->dataProviders = new ParameterBag(); $this->registry = $registry; $this->requestFilterFactory = $requestFilterFactory; + $this->dataProviderFactory = $dataProviderFactory; } /** @@ -201,19 +192,6 @@ public function create($name, array $configuration) return $admin; } - /** - * Add user custom repositories (called in the repository compiler pass), to avoid injecting the service container - * - * @param string $name - * @param DataProviderInterface $dataProvider - */ - public function addDataProvider($name, DataProviderInterface $dataProvider) - { - $this - ->dataProviders - ->set($name, $dataProvider); - } - /** * @return boolean */ @@ -258,36 +236,9 @@ protected function createAction(AdminInterface $admin, $actionName, array $actio */ protected function getDataProvider($entityClass, $name = null) { - // create or get repository according to the configuration - if ($name) { - // removing arobase if exists - if (substr($name, 0, 1) == '@') { - $name = substr($name, 1); - } - - // custom data provider class must be loaded by the compiler pass - if (!$this->dataProviders->has($name)) { - throw new Exception(sprintf( - 'Data provider %s not found. Did you add the @data_provider tag in your service ?', - $name - )); - } - $dataProvider = $this - ->dataProviders - ->get($name); - } else { - // no data provider is configured, we create a new one - $repository = $this - ->entityManager - ->getRepository($entityClass); - - if (!($repository instanceof RepositoryInterface)) { - $repositoryClass = get_class($repository); - throw new Exception("Repository {$repositoryClass} should implements ".RepositoryInterface::class); - } - $dataProvider = new DataProvider($repository); - } - return $dataProvider; + return $this + ->dataProviderFactory + ->get($entityClass, $name); } } diff --git a/Tests/AdminBundle/Admin/AdminTest.php b/Tests/AdminBundle/Admin/AdminTest.php index 64dec26e7..f28c4ec0c 100644 --- a/Tests/AdminBundle/Admin/AdminTest.php +++ b/Tests/AdminBundle/Admin/AdminTest.php @@ -5,8 +5,11 @@ use Doctrine\Common\Collections\ArrayCollection; use Exception; use LAG\AdminBundle\Action\Action; +use LAG\AdminBundle\Action\ActionInterface; +use LAG\AdminBundle\Action\Configuration\ActionConfiguration; use LAG\AdminBundle\Admin\Admin; use LAG\AdminBundle\Admin\AdminInterface; +use LAG\AdminBundle\Admin\Exception\AdminException; use LAG\AdminBundle\Filter\RequestFilter; use LAG\AdminBundle\Tests\AdminTestBase; use stdClass; @@ -551,6 +554,52 @@ public function testLoad() $this->assertEquals(new ArrayCollection($testEntities), $admin->getEntities()); } + /** + * Load method should not load entities if the strategy is NONE. + */ + public function testLoadWithoutRequireLoad() + { + $testEntities = [ + new stdClass(), + new stdClass(), + ]; + $dataProvider = $this->mockDataProvider($testEntities); + + $adminConfiguration = $this + ->createAdminConfiguration( + $this->createApplicationConfiguration(), + $this->getFakeAdminsConfiguration()['full_entity'] + ); + + $admin = new Admin( + 'test', + $dataProvider, + $adminConfiguration, + $this->mockMessageHandler(), + new EventDispatcher(), + new RequestFilter() + ); + $request = new Request([], [], [ + '_route_params' => [ + '_action' => 'custom_list' + ] + ]); + $admin->addAction($this->createAction('custom_list', $admin, [ + 'load_strategy' => AdminInterface::LOAD_STRATEGY_NONE, + 'route' => '', + 'export' => '', + 'order' => [], + 'icon' => '', + 'pager' => 'pagerfanta', + 'criteria' => [], + ])); + $admin->handleRequest($request); + $admin->load([]); + + // if an array is returned from the data provider, it SHOULD wrapped into an array collection + $this->assertCount(0, $admin->getEntities()); + } + /** * load method with unique strategy SHOULD run successfully. */ @@ -632,6 +681,116 @@ public function testLoadWithPagerWithMultipleStrategy() $admin->load([]); } + /** + * load method with a wrong pager should throw an exception. + */ + public function testLoadWithWrongPager() + { + $adminConfiguration = $this + ->createAdminConfiguration( + $this->createApplicationConfiguration(), + $this->getFakeAdminsConfiguration()['full_entity'] + ); + $dataProvider = $this->mockDataProvider([ + new stdClass(), + new stdClass(), + ]); + $admin = new Admin( + 'test', + $dataProvider, + $adminConfiguration, + $this->mockMessageHandler(), + new EventDispatcher(), + new RequestFilter() + ); + $request = new Request([], [], [ + '_route_params' => [ + '_action' => 'custom_list' + ] + ]); + $actionConfiguration = new ActionConfiguration('custom_list', $admin); + $actionConfiguration->setParameters([ + 'criteria' => [], + 'order' => [], + 'pager' => 'wrong', + 'load_strategy' => AdminInterface::LOAD_STRATEGY_MULTIPLE + ]); + $action = $this->createMock(ActionInterface::class); + $action + ->method('getName') + ->willReturn('custom_list'); + $action + ->method('getConfiguration') + ->willReturn($actionConfiguration); + $action + ->method('isPaginationRequired') + ->willReturn(true); + $action + ->method('isLoadingRequired') + ->willReturn(true); + + $admin->addAction($action); + + $this->assertExceptionRaised(AdminException::class, function () use ($admin, $request) { + $admin->handleRequest($request); + }); + } + + /** + * load method with a wrong pager should throw an exception. + */ + public function testLoadWithPagerAndWrongStrategy() + { + $adminConfiguration = $this + ->createAdminConfiguration( + $this->createApplicationConfiguration(), + $this->getFakeAdminsConfiguration()['full_entity'] + ); + $dataProvider = $this->mockDataProvider([ + new stdClass(), + new stdClass(), + ]); + $admin = new Admin( + 'test', + $dataProvider, + $adminConfiguration, + $this->mockMessageHandler(), + new EventDispatcher(), + new RequestFilter() + ); + $request = new Request([], [], [ + '_route_params' => [ + '_action' => 'custom_list' + ] + ]); + $actionConfiguration = new ActionConfiguration('custom_list', $admin); + $actionConfiguration->setParameters([ + 'criteria' => [], + 'order' => [], + 'pager' => 'pagerfanta', + 'load_strategy' => AdminInterface::LOAD_STRATEGY_UNIQUE + ]); + $action = $this->createMock(ActionInterface::class); + $action + ->method('getName') + ->willReturn('custom_list'); + $action + ->method('getConfiguration') + ->willReturn($actionConfiguration); + $action + ->method('isPaginationRequired') + ->willReturn(true); + $action + ->method('isLoadingRequired') + ->willReturn(true); + + $admin->addAction($action); + + $this->assertExceptionRaised(AdminException::class, function () use ($admin, $request) { + $admin->handleRequest($request); + }); + } + /** * load method SHOULD handle exceptions. */ @@ -666,7 +825,7 @@ public function testLoadWithException() 'pager' => 'pagerfanta', 'criteria' => [], ])); - $this->assertExceptionRaised(Exception::class, function () use ($admin, $request) { + $this->assertExceptionRaised(AdminException::class, function () use ($admin, $request) { $admin->handleRequest($request); $admin->load([]); }); diff --git a/Tests/AdminBundle/Admin/Behaviors/AdminTraitTest.php b/Tests/AdminBundle/Admin/Behaviors/AdminTraitTest.php new file mode 100644 index 000000000..027bac989 --- /dev/null +++ b/Tests/AdminBundle/Admin/Behaviors/AdminTraitTest.php @@ -0,0 +1,46 @@ +pager = $this->createMock(Pagerfanta::class); + $this->assertEquals($this->pager, $this->getPager()); + } + + /** + * GetUniqueEntityLabel method should return the name of an entity. + */ + public function testGetUniqueEntityLabel() + { + $this->pager = $this->createMock(Pagerfanta::class); + $this->assertEquals('my_entity', $this->getUniqueEntityLabel()); + } + + /** + * Return the current unique entity. + * + * @return object + */ + public function getUniqueEntity() + { + $entity = new stdClass(); + $entity->name = 'my_entity'; + + return $entity; + } +} diff --git a/Tests/AdminBundle/Admin/Behaviors/EntityLabelTraitTest.php b/Tests/AdminBundle/Admin/Behaviors/EntityLabelTraitTest.php index adc106a65..e84d24007 100644 --- a/Tests/AdminBundle/Admin/Behaviors/EntityLabelTraitTest.php +++ b/Tests/AdminBundle/Admin/Behaviors/EntityLabelTraitTest.php @@ -1,6 +1,6 @@ label = 'panda'; $this->assertEquals('panda', $this->getEntityLabel($entity)); - $entity = new stdClass(); $entity->title = 'panda'; $this->assertEquals('panda', $this->getEntityLabel($entity)); diff --git a/Tests/AdminBundle/Admin/Factory/AdminFactoryTest.php b/Tests/AdminBundle/Admin/Factory/AdminFactoryTest.php index 77e983de3..86c6de9e3 100644 --- a/Tests/AdminBundle/Admin/Factory/AdminFactoryTest.php +++ b/Tests/AdminBundle/Admin/Factory/AdminFactoryTest.php @@ -55,23 +55,6 @@ public function testCreate() $this->doTestAdmin($admin, $adminConfiguration, 'admin_test2'); } - /** - * AddDataProvider method must add a DataProviderInterface to the Admin. - */ - public function testAddDataProvider() - { - // test with no configuration - $adminFactory = $this->createAdminFactory(); - // unknown admin not exists, it should throw an exception - $this->assertExceptionRaised('Exception', function () use ($adminFactory) { - $adminFactory - ->getRegistry() - ->get('unknown_admin'); - }); - $dataProvider = $this->mockDataProvider(); - $adminFactory->addDataProvider('test', $dataProvider); - } - /** * Test dispatched events. */ diff --git a/Tests/AdminTestBase.php b/Tests/AdminTestBase.php index 807d09aac..6aecf09d4 100644 --- a/Tests/AdminTestBase.php +++ b/Tests/AdminTestBase.php @@ -21,6 +21,7 @@ use LAG\AdminBundle\Admin\Factory\FilterFactory; use LAG\AdminBundle\Configuration\Factory\ConfigurationFactory; use LAG\AdminBundle\DataProvider\DataProviderInterface; +use LAG\AdminBundle\DataProvider\Factory\DataProviderFactory; use LAG\AdminBundle\Field\Factory\FieldFactory; use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; use LAG\AdminBundle\Filter\RequestFilter; @@ -338,12 +339,12 @@ protected function createAdminFactory(array $configuration = [], EventDispatcher return new AdminFactory( $configuration, $eventDispatcher, - $this->mockEntityManager(), $this->createConfigurationFactory(), $this->mockActionFactory(), $this->mockMessageHandler(), new \LAG\AdminBundle\Admin\Registry\Registry(), - new RequestFilterFactory() + new RequestFilterFactory(), + new DataProviderFactory($this->mockEntityManager()) ); } From 69e818af943ef0aa357229266edee94e72044e51 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:08:55 +0100 Subject: [PATCH 26/49] add a isLoadingRequired method to the ActionInterface --- Action/Action.php | 16 +++++++++++++++- Action/ActionInterface.php | 7 +++++++ Action/Configuration/ActionConfiguration.php | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Action/Action.php b/Action/Action.php index 09b6d87e7..19cfdf5b9 100644 --- a/Action/Action.php +++ b/Action/Action.php @@ -161,6 +161,20 @@ public function getConfiguration() */ public function isPaginationRequired() { - return $this->configuration->getParameter('load_strategy') === AdminInterface::LOAD_STRATEGY_MULTIPLE; + return $this + ->configuration + ->getParameter('load_strategy') === AdminInterface::LOAD_STRATEGY_MULTIPLE; + } + + /** + * Return true if one or more entities should be loaded ini this action. + * + * @return bool + */ + public function isLoadingRequired() + { + return $this + ->configuration + ->getParameter('load_strategy') !== AdminInterface::LOAD_STRATEGY_NONE; } } diff --git a/Action/ActionInterface.php b/Action/ActionInterface.php index 6fd0ec764..06ea8e90d 100644 --- a/Action/ActionInterface.php +++ b/Action/ActionInterface.php @@ -72,4 +72,11 @@ public function addField(Field $field); * @return bool */ public function isPaginationRequired(); + + /** + * Return true if the Action requires the loading of one or more entities. + * + * @return true + */ + public function isLoadingRequired(); } diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index 8bb703d26..844b9a349 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -203,7 +203,7 @@ public function configureOptions(OptionsResolver $resolver) // batch actions $resolver - // by default, the batch actions is desactivated + // by default, the batch action is not activated ->setDefault('batch', null) ->setNormalizer('batch', function(Options $options, $batch) { From 2e29257bd2cb3193a12730f95f6fbdc6b57b6fa6 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:09:05 +0100 Subject: [PATCH 27/49] remove old AdminException --- Exception/AdminException.php | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 Exception/AdminException.php diff --git a/Exception/AdminException.php b/Exception/AdminException.php deleted file mode 100644 index e18637ec0..000000000 --- a/Exception/AdminException.php +++ /dev/null @@ -1,9 +0,0 @@ - Date: Sat, 5 Nov 2016 22:30:32 +0100 Subject: [PATCH 28/49] fix parameters orders in AdminFactory call to DataProviderFactory --- Admin/Factory/AdminFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index b24deb0eb..c19e02f50 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -238,7 +238,7 @@ protected function getDataProvider($entityClass, $name = null) { return $this ->dataProviderFactory - ->get($entityClass, $name); + ->get($name, $entityClass); } } From 8d0c2f45267bd3e616ddb2062d006b63c771bcc5 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:32:40 +0100 Subject: [PATCH 29/49] improve comments --- Admin/Factory/AdminFactory.php | 4 ++-- DataProvider/Factory/DataProviderFactory.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index c19e02f50..8a07c6c7a 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -11,9 +11,9 @@ use LAG\AdminBundle\Admin\Event\BeforeConfigurationEvent; use LAG\AdminBundle\Admin\Registry\Registry; use LAG\AdminBundle\Configuration\Factory\ConfigurationFactory; -use LAG\AdminBundle\DataProvider\DataProvider; use LAG\AdminBundle\Admin\Event\AdminEvents; use Exception; +use LAG\AdminBundle\DataProvider\DataProviderInterface; use LAG\AdminBundle\DataProvider\Factory\DataProviderFactory; use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; use LAG\AdminBundle\Message\MessageHandlerInterface; @@ -231,7 +231,7 @@ protected function createAction(AdminInterface $admin, $actionName, array $actio * * @param string $entityClass * @param string|null $name - * @return DataProvider|mixed + * @return DataProviderInterface * @throws Exception */ protected function getDataProvider($entityClass, $name = null) diff --git a/DataProvider/Factory/DataProviderFactory.php b/DataProvider/Factory/DataProviderFactory.php index fff7cf6e0..e58525bbe 100644 --- a/DataProvider/Factory/DataProviderFactory.php +++ b/DataProvider/Factory/DataProviderFactory.php @@ -56,7 +56,7 @@ public function add($id, DataProviderInterface $dataProvider) * @param string|null $entityClass The class of the related entity. It will be used to find a repository in Doctrine * registry * - * @return DataProvider|DataProviderInterface + * @return DataProviderInterface * * @throws Exception */ @@ -93,7 +93,7 @@ public function has($id) * Create a generic data provider. * * @param string $entityClass The class of the related entity - * @return DataProvider The created data provider + * @return DataProviderInterface The created data provider * * @throws Exception An exception is thrown if the found repository with given entity class does not implements * RepositoryInterface. From 028738b280548c0211338d2bbfe38463bb158edb Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:52:30 +0100 Subject: [PATCH 30/49] fix action configuration for order parameter --- Action/Configuration/ActionConfiguration.php | 4 ++-- .../Configuration/ActionConfigurationTest.php | 22 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index 844b9a349..18199574f 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -207,7 +207,7 @@ public function configureOptions(OptionsResolver $resolver) ->setDefault('batch', null) ->setNormalizer('batch', function(Options $options, $batch) { - // if batch is desactivated, no more checks should be done + // if batch is not activated, no more checks should be done if ($batch === false) { return $batch; } @@ -261,7 +261,7 @@ protected function configureOrder(OptionsResolver $resolver) ->setNormalizer('order', function(Options $options, $order) { foreach ($order as $field => $sort) { - if (is_string($sort)) { + if (!is_string($sort) || !is_string($field) || !in_array(strtolower($sort), ['asc', 'desc'])) { throw new ConfigurationException( 'Order value should be an array of string (["field" => $key]), got '.gettype($sort), $this->actionName, diff --git a/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php b/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php index 488da73da..3984a578e 100644 --- a/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php +++ b/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php @@ -2,13 +2,31 @@ namespace LAG\AdminBundle\Tests\AdminBundle\Action\Configuration; -use LAG\AdminBundle\Field\Field; +use LAG\AdminBundle\Action\Configuration\ActionConfiguration; use LAG\AdminBundle\Tests\AdminTestBase; +use Symfony\Component\OptionsResolver\OptionsResolver; class ActionConfigurationTest extends AdminTestBase { public function testActionConfiguration() { - + $admin = $this->createAdmin('an_admin', [ + 'entity' => 'WhatEver', + 'form' => 'AForm', + 'actions' => [ + 'an_action' => [] + ] + ]); + + $resolver = new OptionsResolver(); + + $configuration = new ActionConfiguration('an_action', $admin); + $configuration->configureOptions($resolver); + + $resolver->resolve([ + 'order' => [ + 'test' => 'asc' + ] + ]); } } From 76f964aa516d5fc6b1a46f4c1edadd25c501c783 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 5 Nov 2016 22:53:23 +0100 Subject: [PATCH 31/49] remove dependencies to BlueBear/BaseBundle --- Controller/CRUDController.php | 16 ++- Form/Handler/ListFormHandler.php | 3 +- Form/Type/AdminListType.php | 2 - .../DataProvider/DataProviderTest.php | 2 +- Tests/Fixtures/Entity/TestEntity.php | 119 +++++++++++++++++- .../src/Test/TestBundle/Entity/TestEntity.php | 101 ++++++++++++++- composer.json | 1 - 7 files changed, 229 insertions(+), 15 deletions(-) diff --git a/Controller/CRUDController.php b/Controller/CRUDController.php index bf475fd01..50d78fd78 100644 --- a/Controller/CRUDController.php +++ b/Controller/CRUDController.php @@ -6,7 +6,6 @@ use LAG\AdminBundle\Form\Handler\ListFormHandler; use LAG\AdminBundle\Form\Type\AdminListType; use LAG\AdminBundle\Form\Type\BatchActionType; -use BlueBear\BaseBundle\Behavior\ControllerTrait; use Exception; use LAG\AdminBundle\Form\Type\DeleteType; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template; @@ -24,8 +23,6 @@ */ class CRUDController extends Controller { - use ControllerTrait; - /** * Generic list action. * @@ -234,6 +231,19 @@ public function deleteAction(Request $request) ]; } + /** + * Throw a 404 Exception if $boolean is false or null + * + * @param $boolean + * @param string $message + */ + protected function forward404Unless($boolean, $message = '404 Not Found') + { + if (!$boolean) { + throw $this->createNotFoundException($message); + } + } + /** * Forward to 404 if user is not allowed by configuration for an action. * diff --git a/Form/Handler/ListFormHandler.php b/Form/Handler/ListFormHandler.php index 41473a728..a62c2cd9b 100644 --- a/Form/Handler/ListFormHandler.php +++ b/Form/Handler/ListFormHandler.php @@ -2,7 +2,6 @@ namespace LAG\AdminBundle\Form\Handler; -use BlueBear\BaseBundle\Entity\Behaviors\Id; use LAG\AdminBundle\Admin\Behaviors\EntityLabelTrait; use Symfony\Component\Form\FormInterface; @@ -50,7 +49,7 @@ public function handle(FormInterface $form) protected function getLabels($entities) { $labels = []; - /** @var Id $entity */ + foreach ($entities as $entity) { $labels[$entity->getId()] = $this->getEntityLabel($entity); } diff --git a/Form/Type/AdminListType.php b/Form/Type/AdminListType.php index b06ac917d..2d222a459 100644 --- a/Form/Type/AdminListType.php +++ b/Form/Type/AdminListType.php @@ -2,7 +2,6 @@ namespace LAG\AdminBundle\Form\Type; -use BlueBear\BaseBundle\Entity\Behaviors\Id; use LAG\AdminBundle\Admin\AdminInterface; use LAG\AdminBundle\Menu\Configuration\MenuItemConfiguration; use Symfony\Component\Form\AbstractType; @@ -48,7 +47,6 @@ public function buildForm(FormBuilderInterface $builder, array $options = []) $form = $event->getForm(); if (!empty($data['entities'])) { - /** @var Id $entity */ foreach ($data['entities'] as $entity) { $form->add('batch_'.$entity->getId(), CheckboxType::class, [ 'value' => $entity->getId(), diff --git a/Tests/AdminBundle/DataProvider/DataProviderTest.php b/Tests/AdminBundle/DataProvider/DataProviderTest.php index 276023dcf..3270d1bb6 100644 --- a/Tests/AdminBundle/DataProvider/DataProviderTest.php +++ b/Tests/AdminBundle/DataProvider/DataProviderTest.php @@ -1,6 +1,6 @@ id; + } + + /** + * Set entity id + * + * @param mixed $id + */ + public function setId($id) + { + $this->id = $id; + } + + /** + * @return mixed + */ + public function getName() + { + return $this->name; + } + + /** + * @param mixed $name + */ + public function setName($name) + { + $this->name = $name; + } + + /** + * @ORM\PrePersist() + * @return $this + */ + public function setCreatedAt() + { + if (!$this->createdAt) { + $this->createdAt = new DateTime(); + } + } + + /** + * Created at cannot be set. But in some case (like imports...), it is required to set created at. Use this method + * in this case + * + * @param DateTime $createdAt + */ + public function forceCreatedAt(DateTime $createdAt) + { + $this->createdAt = $createdAt; + } + + /** + * @return DateTime + */ + public function getCreatedAt() + { + return $this->createdAt; + } + + /** + * @ORM\PrePersist() + * @ORM\PreUpdate() + * @param null $value + * @return $this + */ + public function setUpdatedAt($value = null) + { + if ($value instanceof DateTime) { + $this->updatedAt = $value; + } else { + $this->updatedAt = new DateTime(); + } + return $this; + } + + /** + * @return DateTime + */ + public function getUpdatedAt() + { + return $this->updatedAt; + } } diff --git a/Tests/Fixtures/src/Test/TestBundle/Entity/TestEntity.php b/Tests/Fixtures/src/Test/TestBundle/Entity/TestEntity.php index e46f9086f..8fb1fc926 100644 --- a/Tests/Fixtures/src/Test/TestBundle/Entity/TestEntity.php +++ b/Tests/Fixtures/src/Test/TestBundle/Entity/TestEntity.php @@ -2,8 +2,7 @@ namespace Test\TestBundle\Entity; -use BlueBear\BaseBundle\Entity\Behaviors\Id; -use BlueBear\BaseBundle\Entity\Behaviors\Timestampable; +use DateTime; use Doctrine\ORM\Mapping as ORM; /** @@ -13,10 +12,52 @@ */ class TestEntity { - use Id, Timestampable; + /** + * Entity id + * + * @ORM\Id() + * @ORM\GeneratedValue(strategy="AUTO") + * @ORM\Column(type="integer") + */ + protected $id; + /** + * @var string + */ protected $name; + /** + * @var DateTime + * @ORM\Column(name="created_at", type="datetime") + */ + protected $createdAt; + + /** + * @var DateTime + * @ORM\Column(name="updated_at", type="datetime", nullable=true) + */ + protected $updatedAt; + + /** + * Return entity id + * + * @return mixed + */ + public function getId() + { + return $this->id; + } + + /** + * Set entity id + * + * @param mixed $id + */ + public function setId($id) + { + $this->id = $id; + } + /** * @return mixed */ @@ -32,4 +73,58 @@ public function setName($name) { $this->name = $name; } + + /** + * @ORM\PrePersist() + * @return $this + */ + public function setCreatedAt() + { + if (!$this->createdAt) { + $this->createdAt = new DateTime(); + } + } + + /** + * Created at cannot be set. But in some case (like imports...), it is required to set created at. Use this method + * in this case + * + * @param DateTime $createdAt + */ + public function forceCreatedAt(DateTime $createdAt) + { + $this->createdAt = $createdAt; + } + + /** + * @return DateTime + */ + public function getCreatedAt() + { + return $this->createdAt; + } + + /** + * @ORM\PrePersist() + * @ORM\PreUpdate() + * @param null $value + * @return $this + */ + public function setUpdatedAt($value = null) + { + if ($value instanceof DateTime) { + $this->updatedAt = $value; + } else { + $this->updatedAt = new DateTime(); + } + return $this; + } + + /** + * @return DateTime + */ + public function getUpdatedAt() + { + return $this->updatedAt; + } } diff --git a/composer.json b/composer.json index 4962175da..9426e40a9 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,6 @@ "doctrine/data-fixtures": "*", "doctrine/doctrine-fixtures-bundle": "*", "symfony/monolog-bundle": "~2.4", - "bluebear/basebundle": "~0.2", "white-october/pagerfanta-bundle": "~1.0", "twig/extensions": "^1.2", "gedmo/doctrine-extensions": "^2.4", From 026cd682c97348eff9ca879c46afd468ff69044a Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 6 Nov 2016 13:28:18 +0100 Subject: [PATCH 32/49] fix dependencies after removing BaseBundle --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 9426e40a9..f05bb59e4 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,8 @@ "components/font-awesome": "^4.5", "components/jquery": "^2.2", "components/jqueryui": "^1.11", - "knplabs/knp-menu-bundle": "^2.1" + "knplabs/knp-menu-bundle": "^2.1", + "doctrine/orm": "^2.5" }, "require-dev": { "phpunit/phpunit": "^4.0|^5.0", From 8f08eb2121c17d443c0f7069b8004236b52ff532 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 6 Nov 2016 13:28:47 +0100 Subject: [PATCH 33/49] fix comments return type --- Action/ActionInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Action/ActionInterface.php b/Action/ActionInterface.php index 06ea8e90d..197e36436 100644 --- a/Action/ActionInterface.php +++ b/Action/ActionInterface.php @@ -76,7 +76,7 @@ public function isPaginationRequired(); /** * Return true if the Action requires the loading of one or more entities. * - * @return true + * @return bool */ public function isLoadingRequired(); } From 40c2b9c63567f516893bda2e101d5714b37d3e8a Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 6 Nov 2016 13:33:13 +0100 Subject: [PATCH 34/49] load entities in Admin after type check --- Admin/Admin.php | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/Admin/Admin.php b/Admin/Admin.php index 82cc3bdf5..756f7254f 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -343,25 +343,29 @@ public function load(array $criteria, array $orderBy = [], $limit = 25, $offset ); } // load entities using a pager - $this->loadPaginate($criteria, $orderBy, $limit, $offset); + $entities = $this->loadPaginate($criteria, $orderBy, $limit, $offset); } else { - $this->loadWithoutPagination($criteria, $orderBy, $limit, $offset); + // load using the data provider + $entities = $this->loadWithoutPagination($criteria, $orderBy, $limit, $offset); } // the data provider should return an array or a collection of entities. - if (!is_array($this->entities) && !$this->entities instanceof Collection) { + if (!is_array($entities) && !$entities instanceof Collection) { throw new AdminException( 'The data provider should return either a collection or an array. Got ' - .gettype($this->entities).' instead', + .gettype($entities).' instead', $currentAction->getName(), $this ); } // if an array is provided, transform it to a collection to be more convenient - if (is_array($this->entities)) { - $this->entities = new ArrayCollection($this->entities); + if (is_array($entities)) { + $entities = new ArrayCollection($this->entities); } + + // load the entities into the Admin + $this->entities = $entities; } /** @@ -548,6 +552,8 @@ protected function generateMessageTranslationKey($message) * @param array $orderBy * @param int $limit * @param int $offset + * + * @return array|\Traversable */ protected function loadPaginate(array $criteria, array $orderBy, $limit, $offset) { @@ -558,7 +564,7 @@ protected function loadPaginate(array $criteria, array $orderBy, $limit, $offset $this->pager->setMaxPerPage($limit); $this->pager->setCurrentPage($offset); - $this->entities = $this + return $this ->pager ->getCurrentPageResults(); } @@ -570,6 +576,8 @@ protected function loadPaginate(array $criteria, array $orderBy, $limit, $offset * @param array $orderBy * @param int $limit * @param int $offset + * + * @return array|Collection */ protected function loadWithoutPagination(array $criteria, $orderBy, $limit, $offset) { @@ -581,8 +589,8 @@ protected function loadWithoutPagination(array $criteria, $orderBy, $limit, $off $offset = 0; $limit = 1; } - // load entities according to the given parameters - $this->entities = $this + + return $this ->dataProvider ->findBy($criteria, $orderBy, $limit, $offset); } From 5f065e0786878c6867672f2da280887e5b429718 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 6 Nov 2016 13:34:13 +0100 Subject: [PATCH 35/49] remove unused property in AdminFactory --- Admin/Factory/AdminFactory.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index 8a07c6c7a..84eceb88e 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -17,7 +17,6 @@ use LAG\AdminBundle\DataProvider\Factory\DataProviderFactory; use LAG\AdminBundle\Filter\Factory\RequestFilterFactory; use LAG\AdminBundle\Message\MessageHandlerInterface; -use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -99,7 +98,6 @@ public function __construct( $this->adminConfigurations = $adminConfigurations; $this->actionFactory = $actionFactory; $this->messageHandler = $messageHandler; - $this->dataProviders = new ParameterBag(); $this->registry = $registry; $this->requestFilterFactory = $requestFilterFactory; $this->dataProviderFactory = $dataProviderFactory; From 0002efd947e7895083aad60b3c180ccb6a10d116 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sun, 6 Nov 2016 13:36:36 +0100 Subject: [PATCH 36/49] fix ArrayCollection creation in Admin --- Admin/Admin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Admin/Admin.php b/Admin/Admin.php index 756f7254f..4491304a0 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -361,7 +361,7 @@ public function load(array $criteria, array $orderBy = [], $limit = 25, $offset // if an array is provided, transform it to a collection to be more convenient if (is_array($entities)) { - $entities = new ArrayCollection($this->entities); + $entities = new ArrayCollection($entities); } // load the entities into the Admin From 36caa6e98bd341945657e42529c6b50b4b47d94c Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 16 Nov 2016 23:31:37 +0100 Subject: [PATCH 37/49] set pager by default for admin --- Admin/Configuration/AdminConfiguration.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Admin/Configuration/AdminConfiguration.php b/Admin/Configuration/AdminConfiguration.php index c87f1c077..29163a91f 100644 --- a/Admin/Configuration/AdminConfiguration.php +++ b/Admin/Configuration/AdminConfiguration.php @@ -62,7 +62,8 @@ public function configureOptions(OptionsResolver $resolver) 'translation_pattern' => $this ->applicationConfiguration ->getParameter('translation')['pattern'], - 'form' => null + 'form' => null, + 'pager' => 'pagerfanta' ]); // required options $resolver->setRequired([ From 9d553d596f751821f6a796b8bc032f40e79b196c Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 16 Nov 2016 23:59:02 +0100 Subject: [PATCH 38/49] display errors in batch actions form --- Resources/views/CRUD/list.html.twig | 105 +++++++++++++++------------- 1 file changed, 55 insertions(+), 50 deletions(-) diff --git a/Resources/views/CRUD/list.html.twig b/Resources/views/CRUD/list.html.twig index ea1e03e07..caf4feb63 100644 --- a/Resources/views/CRUD/list.html.twig +++ b/Resources/views/CRUD/list.html.twig @@ -20,7 +20,7 @@ {% spaceless %} {% if form is defined %} -
+ {% if form.batch_action is defined %}
@@ -31,66 +31,71 @@ {{ form_widget(form.batch_submit) }}
+
+
+ {{ form_errors(form) }} +
+
{% endif %} {% endif %} -
- - +
+
+ + + {% if form is defined %} + + {% endif %} + {% for field in fields %} + + {% endfor %} + + + + {% for entity in admin.entities %} {% if form is defined %} - + {% endif %} {% for field in fields %} - + {% endfor %} - - - {% for entity in admin.entities %} - - {% if form is defined %} - - {% endif %} - {% for field in fields %} - - {% endfor %} - - {% endfor %} - - {#{% if admin.currentAction.configuration.exports | length %}#} - {##} - {##} - {##} - {##} - {##} - {#{% endif %}#} -
+ {{ form_widget(form['select_all']) }} + + {% set sortClass = getSortColumnIconClass(app.request.get('order'), field.name, app.request.get('sort')) %} + + + {{ field_title(field.name, admin.name) }} + +
- {{ form_widget(form['select_all']) }} - + {{ form_widget(form['batch_' ~ entity.id]) }} + - {% set sortClass = getSortColumnIconClass(app.request.get('order'), field.name, app.request.get('sort')) %} - - - {{ field_title(field.name, admin.name) }} - - {{ field(field, entity) | raw }}
- {{ form_widget(form['batch_' ~ entity.id]) }} - {{ field(field, entity) | raw }}
#} - {#{% for export in admin.currentAction.configuration.exports %}#} - {#{{ export }}#} - {#{% endfor %}#} - {#
+ {% endfor %} + + {#{% if admin.currentAction.configuration.exports | length %}#} + {##} + {##} + {##} + {#{% for export in admin.currentAction.configuration.exports %}#} + {#{{ export }}#} + {#{% endfor %}#} + {##} + {##} + {##} + {#{% endif %}#} + - {% if admin.pager.haveToPaginate %} -
- {{ pagerfanta(admin.pager, 'twitter_bootstrap3', {page: admin.pager.currentPage}) }} -
- {% endif %} + {% if admin.pager.haveToPaginate %} +
+ {{ pagerfanta(admin.pager, 'twitter_bootstrap3', {page: admin.pager.currentPage}) }} +
+ {% endif %} - {% if form is defined %} - {{ form_rest(form) }} - {% endif %} -
+ {% if form is defined %} + {{ form_rest(form) }} + {% endif %} +
{% endspaceless %} {% else %} From 5040f5c1f6a08e5ab0a2e2524cf5ff7577c9e9e0 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 16 Nov 2016 23:59:27 +0100 Subject: [PATCH 39/49] fix default page in pagerfanta --- Filter/PagerfantaFilter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Filter/PagerfantaFilter.php b/Filter/PagerfantaFilter.php index 61f755de3..7ddec34f6 100644 --- a/Filter/PagerfantaFilter.php +++ b/Filter/PagerfantaFilter.php @@ -11,7 +11,7 @@ class PagerfantaFilter extends RequestFilter * * @var int */ - protected $currentPage; + protected $currentPage = 1; /** * Filter request From 93508e6521e3421ae5cc59540f4bd7d873bb8bd8 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Thu, 17 Nov 2016 00:03:45 +0100 Subject: [PATCH 40/49] fix errors in batch actions --- Controller/CRUDController.php | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/Controller/CRUDController.php b/Controller/CRUDController.php index 50d78fd78..eb1f932fc 100644 --- a/Controller/CRUDController.php +++ b/Controller/CRUDController.php @@ -63,18 +63,25 @@ public function listAction(Request $request) // get ids and batch action from list form data $formHandler = new ListFormHandler(); $data = $formHandler->handle($form); - $batchForm = $this->createForm(BatchActionType::class, [ - 'batch_action' => $data['batch_action'], - 'entity_ids' => $data['ids'] - ], [ - 'labels' => $data['labels'] - ]); - // render batch view - return $this->render('LAGAdminBundle:CRUD:batch.html.twig', [ - 'admin' => $admin, - 'form' => $batchForm->createView() - ]); + if (count($data['ids'])) { + $batchForm = $this->createForm(BatchActionType::class, [ + 'batch_action' => $data['batch_action'], + 'entity_ids' => $data['ids'] + ], [ + 'labels' => $data['labels'] + ]); + + // render batch view + return $this->render('LAGAdminBundle:CRUD:batch.html.twig', [ + 'admin' => $admin, + 'form' => $batchForm->createView() + ]); + } else { + $this + ->addFlash('info', 'lag.admin.batch_no_entities'); + } + } $viewParameters['form'] = $form->createView(); } From 18bb7aca810c69cbbd0fb599c58586bc52dfd0f5 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 19 Nov 2016 17:12:56 +0100 Subject: [PATCH 41/49] improve AdminTrait code style --- Admin/Behaviors/AdminTrait.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Admin/Behaviors/AdminTrait.php b/Admin/Behaviors/AdminTrait.php index a8dad78f5..d6950559d 100644 --- a/Admin/Behaviors/AdminTrait.php +++ b/Admin/Behaviors/AdminTrait.php @@ -37,9 +37,6 @@ public function getPager() */ public function getUniqueEntityLabel() { - $entity = $this->getUniqueEntity(); - $label = $this->getEntityLabel($entity); - - return $label; + return $this->getEntityLabel($this->getUniqueEntity()); } } From e2dd469464f1f83c7e8c662076c6933dc4ed0583 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 19 Nov 2016 17:16:19 +0100 Subject: [PATCH 42/49] rename admin registry variable to be more clear --- Admin/Factory/AdminFactory.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Admin/Factory/AdminFactory.php b/Admin/Factory/AdminFactory.php index 84eceb88e..5cfdf5431 100644 --- a/Admin/Factory/AdminFactory.php +++ b/Admin/Factory/AdminFactory.php @@ -59,7 +59,7 @@ class AdminFactory /** * @var Registry */ - protected $registry; + protected $adminRegistry; /** * @var RequestFilterFactory @@ -79,7 +79,7 @@ class AdminFactory * @param ConfigurationFactory $configurationFactory * @param ActionFactory $actionFactory * @param MessageHandlerInterface $messageHandler - * @param Registry $registry + * @param Registry $adminRegistry * @param RequestFilterFactory $requestFilterFactory * @param DataProviderFactory $dataProviderFactory */ @@ -89,7 +89,7 @@ public function __construct( ConfigurationFactory $configurationFactory, ActionFactory $actionFactory, MessageHandlerInterface $messageHandler, - Registry $registry, + Registry $adminRegistry, RequestFilterFactory $requestFilterFactory, DataProviderFactory $dataProviderFactory ) { @@ -98,7 +98,7 @@ public function __construct( $this->adminConfigurations = $adminConfigurations; $this->actionFactory = $actionFactory; $this->messageHandler = $messageHandler; - $this->registry = $registry; + $this->adminRegistry = $adminRegistry; $this->requestFilterFactory = $requestFilterFactory; $this->dataProviderFactory = $dataProviderFactory; } @@ -133,7 +133,7 @@ public function init() // create Admin object and add it to the registry $admin = $this->create($name, $event->getAdminConfiguration()); $this - ->registry + ->adminRegistry ->add($admin); // dispatch post-create event @@ -203,7 +203,7 @@ public function isInit() */ public function getRegistry() { - return $this->registry; + return $this->adminRegistry; } /** From 9e2c7620ca202c793bbefd49d3c785c83b7a0c4a Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 19 Nov 2016 17:18:23 +0100 Subject: [PATCH 43/49] add missing quotes in exception message --- Admin/Admin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Admin/Admin.php b/Admin/Admin.php index 4491304a0..cc9523954 100644 --- a/Admin/Admin.php +++ b/Admin/Admin.php @@ -388,7 +388,7 @@ public function getEntities() public function getUniqueEntity() { if ($this->entities->count() == 0) { - throw new Exception('Entity not found in admin "'.$this->getName()); + throw new Exception('Entity not found in admin "'.$this->getName().'""'); } if ($this->entities->count() > 1) { throw new Exception( From b5f5d8cf76e19686576f0fae12bb404d4b82a673 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Sat, 19 Nov 2016 17:28:37 +0100 Subject: [PATCH 44/49] remove left comments --- Resources/views/CRUD/list.html.twig | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/Resources/views/CRUD/list.html.twig b/Resources/views/CRUD/list.html.twig index caf4feb63..629a476dd 100644 --- a/Resources/views/CRUD/list.html.twig +++ b/Resources/views/CRUD/list.html.twig @@ -73,17 +73,6 @@ {% endfor %} - {#{% if admin.currentAction.configuration.exports | length %}#} - {##} - {##} - {##} - {#{% for export in admin.currentAction.configuration.exports %}#} - {#{{ export }}#} - {#{% endfor %}#} - {##} - {##} - {##} - {#{% endif %}#} {% if admin.pager.haveToPaginate %} From fb66169b708a604937f43613bff4b63a884a4790 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 23 Nov 2016 21:38:10 +0100 Subject: [PATCH 45/49] set Admin nullable for ConfigurationException --- Action/Configuration/ConfigurationException.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Action/Configuration/ConfigurationException.php b/Action/Configuration/ConfigurationException.php index 838228d78..f1e041bc8 100644 --- a/Action/Configuration/ConfigurationException.php +++ b/Action/Configuration/ConfigurationException.php @@ -7,11 +7,17 @@ class ConfigurationException extends Exception { - public function __construct($message, $actionName, AdminInterface $admin) + public function __construct($message, $actionName, AdminInterface $admin = null) { + if (null !== $admin) { + $adminName = $admin->getName(); + } else { + $adminName = 'unknown'; + } + $message .= sprintf( ', for Admin %s and action %s', - $admin->getName(), + $adminName, $actionName ); From ca2f15571aec70c213f8bb1ef479a5b4a6367f19 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 23 Nov 2016 21:39:17 +0100 Subject: [PATCH 46/49] refactor ActionConfiguration class to be more readable, improve tests on this class --- Action/Configuration/ActionConfiguration.php | 478 +++++++++++------- .../Configuration/ActionConfigurationTest.php | 58 ++- 2 files changed, 346 insertions(+), 190 deletions(-) diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index 18199574f..73461c9da 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -2,10 +2,12 @@ namespace LAG\AdminBundle\Action\Configuration; +use Closure; use LAG\AdminBundle\Admin\AdminInterface; use LAG\AdminBundle\Admin\Behaviors\TranslationKeyTrait; use LAG\AdminBundle\Configuration\Configuration; use LAG\AdminBundle\Menu\Configuration\MenuConfiguration; +use Symfony\Component\DependencyInjection\Container; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Options; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -31,10 +33,10 @@ class ActionConfiguration extends Configuration /** * ActionConfiguration constructor. * - * @param $actionName - * @param AdminInterface $admin + * @param string $actionName + * @param AdminInterface|null $admin */ - public function __construct($actionName, AdminInterface $admin) + public function __construct($actionName, AdminInterface $admin = null) { parent::__construct(); @@ -49,230 +51,328 @@ public function __construct($actionName, AdminInterface $admin) */ public function configureOptions(OptionsResolver $resolver) { - // action title, default to action name key - $translationPattern = $this - ->admin - ->getConfiguration() - ->getParameter('translation_pattern'); + $this->configureDefaultOptions($resolver); + $this->configureNormalizers($resolver); - $resolver - ->setDefault('title', $this->getTranslationKey( - $translationPattern, - $this->actionName, - $this->admin->getName()) - ) - ->setAllowedTypes('title', 'string'); - - // displayed fields for this action - $resolver - ->setDefault('fields', [ - 'id' => [] - ]) - ->setAllowedTypes('fields', 'array') - ->setNormalizer('fields', function(Options $options, $fields) { - $normalizedFields = []; - - foreach ($fields as $name => $field) { - - if ($field === null) { - $field = []; - } - - $normalizedFields[$name] = $field; - } - - return $normalizedFields; - }) - ; + $this->setAllowedTypes($resolver); + $this->setAllowedValues($resolver); + } - // action permissions. By default, only admin are allowed + /** + * Configure the default options for an Action. + * + * @param OptionsResolver $resolver + */ + protected function configureDefaultOptions(OptionsResolver $resolver) + { $resolver - ->setDefault('permissions', [ - 'ROLE_ADMIN' + ->setDefaults([ + // the default action title is a translation key using the translation pattern and the action name + 'title' => $this->getDefaultTitle(), + // the default field mapping configuration: id=23 in request, getId() in the entity + 'fields' => [ + 'id' => [] + ], + // by default, only administrator can access the admin + 'permissions' => [ + 'ROLE_ADMIN' + ], + // all export are activated + 'export' => [ + 'json', + 'html', + 'csv', + 'xls' + ], + // no order + 'order' => [], + // route will be generated if empty string or null or false is provided + 'route' => '', + // route parameters will be used when using the route (for links...) + 'route_parameters' => [], + // icon in the menu + 'icon' => '', + // entities loading strategy + 'load_strategy' => null, + // pager interface, only null or pagerfanta are allowed + 'pager' => 'pagerfanta', + // default criteria used to load entities + 'criteria' => [], + 'filters' => [], + 'menu' => [], + 'batch' => false, ]); + } - // by default, all exports type are allowed + /** + * Define the allowed values. + * + * @param OptionsResolver $resolver + */ + protected function setAllowedValues(OptionsResolver $resolver) + { $resolver - ->setDefault('export', [ - 'json', - 'html', - 'csv', - 'xls' + ->setAllowedValues('load_strategy', [ + AdminInterface::LOAD_STRATEGY_NONE, + AdminInterface::LOAD_STRATEGY_UNIQUE, + AdminInterface::LOAD_STRATEGY_MULTIPLE, + null + ]) + ->setAllowedValues('pager', [ + 'pagerfanta', + false ]); + } - // retrieved entities are sorted according to this option it should be an array indexed by the field name - // with its order as a value - $this->configureOrder($resolver); - - // the action route should be a string + /** + * Define the allowed types. + * + * @param OptionsResolver $resolver + */ + protected function setAllowedTypes(OptionsResolver $resolver) + { $resolver - ->setDefault('route', '') + ->setAllowedTypes('title', 'string') + ->setAllowedTypes('fields', 'array') + ->setAllowedTypes('order', 'array') ->setAllowedTypes('route', 'string') - ->setNormalizer('route', function(Options $options, $value) { - if (!$value) { - // if no route was provided, it should be linked to an Admin - if (!$this->admin) { - throw new InvalidOptionsException('No route was provided for action : '.$this->actionName); - } - - // generate default route from admin - return $this - ->admin - ->generateRouteName($this->actionName); - } - - return $value; - }); + ->setAllowedTypes('route_parameters', 'array') + ->setAllowedTypes('icon', 'string') + ->setAllowedTypes('criteria', 'array') + ->setAllowedTypes('filters', 'array') + ->setAllowedTypes('menu', 'array') + ; + } - // action parameters should be an array + /** + * Configure the normalizers. + * + * @param OptionsResolver $resolver + */ + protected function configureNormalizers(OptionsResolver $resolver) + { $resolver - ->setDefault('route_parameters', []) - ->setAllowedTypes('route_parameters', 'array'); + ->setNormalizer('fields', $this->getFieldsNormalizer()) + ->setNormalizer('order', $this->getOrderNormalizer()) + ->setNormalizer('route', $this->getRouteNormalizer()) + ->setNormalizer('load_strategy', $this->getLoadStrategyNormalizer()) + ->setNormalizer('criteria', $this->getCriteriaNormalizer()) + ->setNormalizer('menu', $this->getMenuNormalizer()) + ->setNormalizer('batch', $this->getBatchNormalizer()) + ; + } - // font awesome icons - $resolver - ->setDefault('icon', '') - ->setAllowedTypes('icon', 'string'); + /** + * Return the field normalizer. It will transform null configuration into array to allow field type guessing + * working. + * + * @return Closure + */ + protected function getFieldsNormalizer() + { + return function (Options $options, $fields) { + $normalizedFields = []; - // load strategy : determine which method should be called in the data provider - $resolver - ->setDefault('load_strategy', null) - ->addAllowedValues('load_strategy', AdminInterface::LOAD_STRATEGY_NONE) - ->addAllowedValues('load_strategy', AdminInterface::LOAD_STRATEGY_UNIQUE) - ->addAllowedValues('load_strategy', AdminInterface::LOAD_STRATEGY_MULTIPLE) - ->addAllowedValues('load_strategy', null) - ->setNormalizer('load_strategy', function(Options $options, $value) { - - if (!$value) { - if ($this->actionName == 'create') { - $value = AdminInterface::LOAD_STRATEGY_NONE; - } else if ($this->actionName == 'list') { - $value = AdminInterface::LOAD_STRATEGY_MULTIPLE; - } else { - $value = AdminInterface::LOAD_STRATEGY_UNIQUE; - } + foreach ($fields as $name => $field) { + + if ($field === null) { + $field = []; } - return $value; - }); + $normalizedFields[$name] = $field; + } - // pagination configuration - $resolver - ->setDefault('pager', 'pagerfanta') - ->addAllowedValues('pager', 'pagerfanta') - ->addAllowedValues('pager', false) - ; + return $normalizedFields; + }; + } - // criteria used to find entity in the data provider - $resolver - ->setDefault('criteria', []) - ->setNormalizer('criteria', function(Options $options, $value) { + /** + * Return the order normalizer. It will check if the order value passed are valid. + * + * @return Closure + */ + protected function getOrderNormalizer() + { + return function (Options $options, $order) { + foreach ($order as $field => $sort) { + + if (!is_string($sort) || !is_string($field) || !in_array(strtolower($sort), ['asc', 'desc'])) { + throw new ConfigurationException( + 'Order value should be an array of string (["field" => $key]), got '.gettype($sort), + $this->actionName, + $this->admin + ); + } + } - if (!$value) { - $idActions = [ - 'edit', - 'delete' - ]; + return $order; + }; + } - if (in_array($this->actionName, $idActions)) { - $value = [ - 'id' - ]; - } + /** + * Return the route normalizer. If an empty value or null or false, it will generate the route using the Admin. + * + * @return Closure + */ + protected function getRouteNormalizer() + { + return function (Options $options, $value) { + if (!$value) { + // if no route was provided, it should be linked to an Admin + if (!$this->admin) { + throw new InvalidOptionsException('No route was provided for action : '.$this->actionName); } - return $value; - }) - ; + // generate default route from admin + return $this + ->admin + ->generateRouteName($this->actionName); + } - // filters - $resolver->setDefault('filters', []); + return $value; + }; + } - // menus - $resolver - ->setDefault('menus', []) - ->setNormalizer('menus', function(Options $options, $menus) { - // set default to an array - if ($menus === false) { - $menus = []; + /** + * Return the load strategy normalizer. It will set the default strategy according to the action name, if no value + * is provided. + * + * @return Closure + */ + protected function getLoadStrategyNormalizer() + { + return function (Options $options, $value) { + if (!$value) { + if ($this->actionName == 'create') { + $value = AdminInterface::LOAD_STRATEGY_NONE; + } else if ($this->actionName == 'list') { + $value = AdminInterface::LOAD_STRATEGY_MULTIPLE; + } else { + $value = AdminInterface::LOAD_STRATEGY_UNIQUE; } + } - return $menus; - }) - ; + return $value; + }; + } - // batch actions - $resolver - // by default, the batch action is not activated - ->setDefault('batch', null) - ->setNormalizer('batch', function(Options $options, $batch) { + /** + * Return the menu normalizer. It will transform false values into an empty array to allow default menu + * configuration working. + * + * @return Closure + */ + protected function getMenuNormalizer() + { + return function (Options $options, $menus) { + // set default to an array + if ($menus === false) { + $menus = []; + } + + return $menus; + }; + } - // if batch is not activated, no more checks should be done - if ($batch === false) { - return $batch; + /** + * Return the criteria normalizer. It will add the id parameters for the edit and delete actions if no value is + * provided. + * + * @return Closure + */ + protected function getCriteriaNormalizer() + { + return function (Options $options, $value) { + if (!$value) { + $idActions = [ + 'edit', + 'delete' + ]; + + if (in_array($this->actionName, $idActions)) { + $value = [ + 'id' + ]; } - // for list actions, we add a default configuration - if ($batch === null) { - // delete action should be allowed in order to be place in batch actions - $allowedActions = array_keys($this + } + + return $value; + }; + } + + /** + * Return the batch normalizer. If null value is provided, it will add the delete batch action if the delete + * actions is allowed . + * + * @return Closure + */ + protected function getBatchNormalizer() + { + return function (Options $options, $batch) { + // if batch is not activated, no more checks should be done + if ($batch === false) { + return $batch; + } + // for list actions, we add a default configuration + if ($batch === null) { + // delete action should be allowed in order to be place in batch actions + $allowedActions = array_keys($this + ->admin + ->getConfiguration() + ->getParameter('actions')); + + if ($this->actionName == 'list' && in_array('delete', $allowedActions)) { + $pattern = $this ->admin ->getConfiguration() - ->getParameter('actions')); - - if ($this->actionName == 'list' && in_array('delete', $allowedActions)) { - $pattern = $this - ->admin - ->getConfiguration() - ->getParameter('translation_pattern'); - - $batch = [ - 'items' => [ - 'delete' => [ - 'admin' => $this->admin->getName(), - 'action' => 'delete', - 'text' => $this->getTranslationKey($pattern, 'delete', $this->admin->getName()) - ] + ->getParameter('translation_pattern'); + + $batch = [ + 'items' => [ + 'delete' => [ + 'admin' => $this->admin->getName(), + 'action' => 'delete', + 'text' => $this->getTranslationKey($pattern, 'delete', $this->admin->getName()) ] - ]; - } else { - return $batch; - } + ] + ]; + } else { + return $batch; } - $resolver = new OptionsResolver(); - $configuration = new MenuConfiguration(); - $configuration->configureOptions($resolver); - $batch = $resolver->resolve($batch); - - return $batch; - }) - ; + } + $resolver = new OptionsResolver(); + $configuration = new MenuConfiguration(); + $configuration->configureOptions($resolver); + $batch = $resolver->resolve($batch); + + return $batch; + }; } /** - * Configure the order option. + * Return the default title using the configured translation pattern. * - * @param OptionsResolver $resolver + * @return string */ - protected function configureOrder(OptionsResolver $resolver) + protected function getDefaultTitle() { - $resolver - ->setDefault('order', []) - ->setAllowedTypes('order', 'array') - ->setNormalizer('order', function(Options $options, $order) { - foreach ($order as $field => $sort) { - - if (!is_string($sort) || !is_string($field) || !in_array(strtolower($sort), ['asc', 'desc'])) { - throw new ConfigurationException( - 'Order value should be an array of string (["field" => $key]), got '.gettype($sort), - $this->actionName, - $this->admin - ); - } - // TODO add test: if the field does not exists in the target entity, an exception should be thrown - } - - return $order; - }) - ; + if ($this->admin) { + // by default, the action title is action name using the configured translation pattern + $translationPattern = $this + ->admin + ->getConfiguration() + ->getParameter('translation_pattern'); + + $actionTitle = $this->getTranslationKey( + $translationPattern, + $this->actionName, + $this->admin->getName() + ); + } else { + // no admin was provided, we camelize the action name + $actionTitle = Container::camelize($this->actionName); + } + + return $actionTitle; } } diff --git a/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php b/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php index 3984a578e..993ffddc3 100644 --- a/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php +++ b/Tests/AdminBundle/Action/Configuration/ActionConfigurationTest.php @@ -3,7 +3,9 @@ namespace LAG\AdminBundle\Tests\AdminBundle\Action\Configuration; use LAG\AdminBundle\Action\Configuration\ActionConfiguration; +use LAG\AdminBundle\Action\Configuration\ConfigurationException; use LAG\AdminBundle\Tests\AdminTestBase; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\OptionsResolver; class ActionConfigurationTest extends AdminTestBase @@ -23,10 +25,64 @@ public function testActionConfiguration() $configuration = new ActionConfiguration('an_action', $admin); $configuration->configureOptions($resolver); - $resolver->resolve([ + $options = $resolver->resolve([ 'order' => [ 'test' => 'asc' ] ]); } + + public function testFieldNormalizer() + { + $admin = $this->createAdmin('an_admin', [ + 'entity' => 'WhatEver', + 'form' => 'AForm', + 'actions' => [ + 'an_action' => [] + ] + ]); + $resolver = new OptionsResolver(); + + $configuration = new ActionConfiguration('an_action', $admin); + $configuration->configureOptions($resolver); + + $options = $resolver->resolve([ + 'fields' => [ + // test default field mapping + 'aField' => null + ] + ]); + + $this->assertEquals([], $options['fields']['aField']); + } + + public function testRouteNormalizer() + { + $resolver = new OptionsResolver(); + $configuration = new ActionConfiguration('an_action'); + $configuration->configureOptions($resolver); + + // an exception should be thrown if no admin is passed to generate the route + $this->assertExceptionRaised(InvalidOptionsException::class, function () use ($resolver) { + $resolver->resolve([ + 'route' => '' + ]); + }); + } + + public function testOrderNormalizer() + { + $resolver = new OptionsResolver(); + $configuration = new ActionConfiguration('an_action'); + $configuration->configureOptions($resolver); + + // an exception should be thrown if an invalid order configuration is passed + $this->assertExceptionRaised(ConfigurationException::class, function () use ($resolver) { + $resolver->resolve([ + 'order' => [ + 'id' => 'ASK', + ] + ]); + }); + } } From e1e7c4dc402c1aa925425a39b79cc537542c506d Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 23 Nov 2016 23:22:38 +0100 Subject: [PATCH 47/49] fix spacing --- Action/Configuration/ActionConfiguration.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index 73461c9da..a326f25d7 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -170,7 +170,7 @@ protected function configureNormalizers(OptionsResolver $resolver) */ protected function getFieldsNormalizer() { - return function (Options $options, $fields) { + return function(Options $options, $fields) { $normalizedFields = []; foreach ($fields as $name => $field) { @@ -193,7 +193,7 @@ protected function getFieldsNormalizer() */ protected function getOrderNormalizer() { - return function (Options $options, $order) { + return function(Options $options, $order) { foreach ($order as $field => $sort) { if (!is_string($sort) || !is_string($field) || !in_array(strtolower($sort), ['asc', 'desc'])) { @@ -216,7 +216,7 @@ protected function getOrderNormalizer() */ protected function getRouteNormalizer() { - return function (Options $options, $value) { + return function(Options $options, $value) { if (!$value) { // if no route was provided, it should be linked to an Admin if (!$this->admin) { @@ -241,7 +241,7 @@ protected function getRouteNormalizer() */ protected function getLoadStrategyNormalizer() { - return function (Options $options, $value) { + return function(Options $options, $value) { if (!$value) { if ($this->actionName == 'create') { $value = AdminInterface::LOAD_STRATEGY_NONE; @@ -264,7 +264,7 @@ protected function getLoadStrategyNormalizer() */ protected function getMenuNormalizer() { - return function (Options $options, $menus) { + return function(Options $options, $menus) { // set default to an array if ($menus === false) { $menus = []; @@ -282,7 +282,7 @@ protected function getMenuNormalizer() */ protected function getCriteriaNormalizer() { - return function (Options $options, $value) { + return function(Options $options, $value) { if (!$value) { $idActions = [ 'edit', @@ -308,7 +308,7 @@ protected function getCriteriaNormalizer() */ protected function getBatchNormalizer() { - return function (Options $options, $batch) { + return function(Options $options, $batch) { // if batch is not activated, no more checks should be done if ($batch === false) { return $batch; From 56c7d6f3fabc85e9fb425b34104588cae6f27552 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 23 Nov 2016 23:37:23 +0100 Subject: [PATCH 48/49] reduce external code coverage timeout in srcutinizer configuration --- .scrutinizer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index a908b6c49..f704093ac 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -1,6 +1,6 @@ tools: external_code_coverage: - timeout: 600 + timeout: 300 filter: excluded_paths: From 606f12dff83040082db9e668715961de76b86184 Mon Sep 17 00:00:00 2001 From: JohnKrovitch Date: Wed, 23 Nov 2016 23:52:05 +0100 Subject: [PATCH 49/49] fix menus option allowed types --- Action/Configuration/ActionConfiguration.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Action/Configuration/ActionConfiguration.php b/Action/Configuration/ActionConfiguration.php index a326f25d7..24405f2a7 100644 --- a/Action/Configuration/ActionConfiguration.php +++ b/Action/Configuration/ActionConfiguration.php @@ -99,7 +99,7 @@ protected function configureDefaultOptions(OptionsResolver $resolver) // default criteria used to load entities 'criteria' => [], 'filters' => [], - 'menu' => [], + 'menus' => [], 'batch' => false, ]); } @@ -140,7 +140,10 @@ protected function setAllowedTypes(OptionsResolver $resolver) ->setAllowedTypes('icon', 'string') ->setAllowedTypes('criteria', 'array') ->setAllowedTypes('filters', 'array') - ->setAllowedTypes('menu', 'array') + ->setAllowedTypes('menus', [ + 'array', + 'boolean', + ]) ; } @@ -157,7 +160,7 @@ protected function configureNormalizers(OptionsResolver $resolver) ->setNormalizer('route', $this->getRouteNormalizer()) ->setNormalizer('load_strategy', $this->getLoadStrategyNormalizer()) ->setNormalizer('criteria', $this->getCriteriaNormalizer()) - ->setNormalizer('menu', $this->getMenuNormalizer()) + ->setNormalizer('menus', $this->getMenuNormalizer()) ->setNormalizer('batch', $this->getBatchNormalizer()) ; }