Skip to content

Commit

Permalink
Add deprecation if label is null
Browse files Browse the repository at this point in the history
  • Loading branch information
@VincentLanglet @jordisala1991
VincentLanglet authored and jordisala1991 committed Sep 29, 2020
1 parent ff2d782 commit 2276061
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 39 deletions.
46 changes: 28 additions & 18 deletions src/DependencyInjection/Configuration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'] = '';
}
}

Expand All @@ -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')
Expand Down
6 changes: 0 additions & 6 deletions tests/DependencyInjection/Compiler/AddDependencyCallsCompilerPassTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down
82 changes: 67 additions & 15 deletions tests/DependencyInjection/ConfigurationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
Expand Down Expand Up @@ -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(
Expand All @@ -178,7 +177,6 @@ public function testDashboardGroups(): void
'route' => 'fooRoute',
'route_params' => ['bar' => 'foo'],
'route_absolute' => true,
'admin' => '',
'roles' => [],
]
);
Expand All @@ -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' => '<i class="fa fa-edit"></i>',
'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' => [
Expand All @@ -207,11 +243,27 @@ public function testDashboardGroupsWithBadItemsParams(): void
'label' => 'foo',
'icon' => '<i class="fa fa-edit"></i>',
'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' => '<i class="fa fa-edit"></i>',
'items' => [
['route' => 'noLabel'],
],
],
],
Expand Down

0 comments on commit 2276061

Please sign in to comment.