From 227606193a597cca33e7378a4c0b966fb55dbe8e Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 1 Sep 2020 21:02:58 +0200 Subject: [PATCH] Add deprecation if label is null --- src/DependencyInjection/Configuration.php | 46 +++++++---- .../AddDependencyCallsCompilerPassTest.php | 6 -- .../DependencyInjection/ConfigurationTest.php | 82 +++++++++++++++---- 3 files changed, 95 insertions(+), 39 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 8264afe1d5..ee752d7136 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -252,24 +252,33 @@ public function getConfigTreeBuilder() ->ifArray() ->then(static function ($items) { foreach ($items as $key => $item) { - if (\is_array($item)) { - if (!\array_key_exists('label', $item) || !\array_key_exists('route', $item)) { - throw new \InvalidArgumentException('Expected either parameters "route" and "label" for array items'); - } - - if (!\array_key_exists('route_params', $item)) { - $items[$key]['route_params'] = []; - } - - $items[$key]['admin'] = ''; - } else { - $items[$key] = [ - 'admin' => $item, - 'label' => '', - 'route' => '', - 'route_params' => [], - 'route_absolute' => false, - ]; + if (!\is_array($item)) { + $item = ['admin' => $item]; + $items[$key] = $item; + + continue; + } + + // NEXT_MAJOR: Use !isset() instead and remove the elseif part. + if (!\array_key_exists('route', $item)) { + throw new \InvalidArgumentException('Expected parameter "route" for array items'); + } elseif (null === $items[$key]['route']) { + @trigger_error( + 'Passing a null route is deprecated since sonata-project/admin-bundle 3.x.', + E_USER_DEPRECATED + ); + } + + // NEXT_MAJOR: Use !isset() instead and remove the elseif part. + if (!\array_key_exists('label', $item)) { + throw new \InvalidArgumentException('Expected parameter "label" for array items'); + } elseif (null === $items[$key]['label']) { + @trigger_error( + 'Passing a null label is deprecated since sonata-project/admin-bundle 3.x.', + E_USER_DEPRECATED + ); + + $items[$key]['label'] = ''; } } @@ -289,6 +298,7 @@ public function getConfigTreeBuilder() ->end() ->arrayNode('route_params') ->prototype('scalar')->end() + ->defaultValue([]) ->end() ->booleanNode('route_absolute') ->info('Whether the generated url should be absolute') diff --git a/tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php b/tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php index 8a5f095fae..e72986fdbc 100644 --- a/tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php @@ -115,27 +115,21 @@ public function testProcessParsingFullValidConfig(): void $this->assertTrue($dashboardGroupsSettings['sonata_group_three']['on_top']); $this->assertFalse($dashboardGroupsSettings['sonata_group_one']['keep_open']); $this->assertArrayHasKey('admin', $dashboardGroupsSettings['sonata_group_one']['items'][0]); - $this->assertArrayHasKey('route', $dashboardGroupsSettings['sonata_group_one']['items'][0]); - $this->assertArrayHasKey('label', $dashboardGroupsSettings['sonata_group_one']['items'][0]); $this->assertArrayHasKey('route_params', $dashboardGroupsSettings['sonata_group_one']['items'][0]); $this->assertContains('sonata_post_admin', $dashboardGroupsSettings['sonata_group_one']['items'][0]); - $this->assertArrayHasKey('admin', $dashboardGroupsSettings['sonata_group_one']['items'][1]); $this->assertArrayHasKey('route', $dashboardGroupsSettings['sonata_group_one']['items'][1]); $this->assertArrayHasKey('label', $dashboardGroupsSettings['sonata_group_one']['items'][1]); $this->assertArrayHasKey('route_params', $dashboardGroupsSettings['sonata_group_one']['items'][1]); $this->assertContains('blog_name', $dashboardGroupsSettings['sonata_group_one']['items'][1]); $this->assertContains('Blog', $dashboardGroupsSettings['sonata_group_one']['items'][1]); - $this->assertSame('', $dashboardGroupsSettings['sonata_group_one']['items'][1]['admin']); $this->assertSame('blog_name', $dashboardGroupsSettings['sonata_group_one']['items'][1]['route']); $this->assertSame('Blog', $dashboardGroupsSettings['sonata_group_one']['items'][1]['label']); $this->assertSame([], $dashboardGroupsSettings['sonata_group_one']['items'][1]['route_params']); - $this->assertArrayHasKey('admin', $dashboardGroupsSettings['sonata_group_one']['items'][2]); $this->assertArrayHasKey('route', $dashboardGroupsSettings['sonata_group_one']['items'][2]); $this->assertArrayHasKey('label', $dashboardGroupsSettings['sonata_group_one']['items'][2]); $this->assertArrayHasKey('route_params', $dashboardGroupsSettings['sonata_group_one']['items'][2]); $this->assertContains('blog_article', $dashboardGroupsSettings['sonata_group_one']['items'][2]); $this->assertContains('Article', $dashboardGroupsSettings['sonata_group_one']['items'][2]); - $this->assertSame('', $dashboardGroupsSettings['sonata_group_one']['items'][2]['admin']); $this->assertSame('blog_article', $dashboardGroupsSettings['sonata_group_one']['items'][2]['route']); $this->assertSame('Article', $dashboardGroupsSettings['sonata_group_one']['items'][2]['label']); $this->assertSame(['articleId' => 3], $dashboardGroupsSettings['sonata_group_one']['items'][2]['route_params']); diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 3d5a92d0a3..d12ce44c69 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -15,11 +15,14 @@ use PHPUnit\Framework\TestCase; use Sonata\AdminBundle\DependencyInjection\Configuration; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Config\Definition\Exception\InvalidTypeException; use Symfony\Component\Config\Definition\Processor; class ConfigurationTest extends TestCase { + use ExpectDeprecationTrait; + public function testOptions(): void { $config = $this->process([]); @@ -153,22 +156,18 @@ public function testDashboardGroups(): void $config['dashboard']['groups']['bar']['items'][0], [ 'admin' => 'item1', - 'label' => '', - 'route' => '', + 'roles' => [], 'route_params' => [], 'route_absolute' => false, - 'roles' => [], ] ); $this->assertSame( $config['dashboard']['groups']['bar']['items'][1], [ 'admin' => 'item2', - 'label' => '', - 'route' => '', + 'roles' => [], 'route_params' => [], 'route_absolute' => false, - 'roles' => [], ] ); $this->assertSame( @@ -178,7 +177,6 @@ public function testDashboardGroups(): void 'route' => 'fooRoute', 'route_params' => ['bar' => 'foo'], 'route_absolute' => true, - 'admin' => '', 'roles' => [], ] ); @@ -187,18 +185,56 @@ public function testDashboardGroups(): void [ 'label' => 'barLabel', 'route' => 'barRoute', + 'roles' => [], 'route_params' => [], - 'admin' => '', + 'route_absolute' => false, + ] + ); + } + + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ + public function testDashboardGroupsWithNullLabel(): void + { + $this->expectDeprecation('Passing a null label is deprecated since sonata-project/admin-bundle 3.x.'); + + $config = $this->process([[ + 'dashboard' => [ + 'groups' => [ + 'bar' => [ + 'label' => 'foo', + 'icon' => '', + 'items' => [ + [ + 'label' => null, + 'route' => 'barRoute', + ], + ], + ], + ], + ], + ]]); + + $this->assertCount(1, $config['dashboard']['groups']['bar']['items']); + $this->assertSame( + $config['dashboard']['groups']['bar']['items'][0], + [ + 'label' => '', + 'route' => 'barRoute', 'roles' => [], + 'route_params' => [], 'route_absolute' => false, ] ); } - public function testDashboardGroupsWithBadItemsParams(): void + public function testDashboardGroupsWithNoRoute(): void { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('Expected either parameters "route" and "label" for array items'); + $this->expectExceptionMessage('Expected parameter "route" for array items'); $this->process([[ 'dashboard' => [ @@ -207,11 +243,27 @@ public function testDashboardGroupsWithBadItemsParams(): void 'label' => 'foo', 'icon' => '', 'items' => [ - 'item1', - 'item2', - [ - 'route' => 'fooRoute', - ], + ['label' => 'noRoute'], + ], + ], + ], + ], + ]]); + } + + public function testDashboardGroupsWithNoLabel(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Expected parameter "label" for array items'); + + $this->process([[ + 'dashboard' => [ + 'groups' => [ + 'bar' => [ + 'label' => 'foo', + 'icon' => '', + 'items' => [ + ['route' => 'noLabel'], ], ], ],