From fc4c26f494f3c55454db2a942dc75d6e52705a58 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Sun, 6 Dec 2020 01:23:54 +0100 Subject: [PATCH 1/4] Fix array shapes with groups --- src/Admin/Pool.php | 48 +++++++++++++++++++++++++-- src/Twig/Extension/GroupExtension.php | 13 +++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/Admin/Pool.php b/src/Admin/Pool.php index 8e3a66984b..6457b501dc 100644 --- a/src/Admin/Pool.php +++ b/src/Admin/Pool.php @@ -38,6 +38,23 @@ class Pool /** * @var array + * @phpstan-var array, + * route?: string, + * router_absolute: bool, + * route_params: array + * }>, + * keep_open: bool, + * on_top: bool, + * roles: list + * }> */ protected $adminGroups = []; @@ -136,9 +153,6 @@ public function __construct( $this->propertyAccessor = $propertyAccessor; } - /** - * @return array> - */ public function getGroups() { $groups = $this->adminGroups; @@ -166,6 +180,16 @@ public function hasGroup($group) /** * @return array + * @phpstan-return array, + * keep_open: bool, + * on_top: bool, + * roles: list + * }> */ public function getDashboardGroups() { @@ -449,6 +473,24 @@ public function getContainer() } /** + * @phpstan-param array, + * route?: string, + * router_absolute: bool, + * route_params: array + * }>, + * keep_open: bool, + * on_top: bool, + * roles: list + * }> $adminGroups + * * @return void */ public function setAdminGroups(array $adminGroups) diff --git a/src/Twig/Extension/GroupExtension.php b/src/Twig/Extension/GroupExtension.php index f2dfbb360a..bac816e304 100644 --- a/src/Twig/Extension/GroupExtension.php +++ b/src/Twig/Extension/GroupExtension.php @@ -44,13 +44,16 @@ public function getFunctions(): array } /** - * @phpstan-return array{array{ - * roles: list, - * icon: string, + * @phpstan-return array + * }> */ public function getDashboardGroupsWithCreatableAdmins(): array { From afde2f4471679e40bcc7d7dc6a152a488206105d Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Sun, 6 Dec 2020 21:21:13 +0100 Subject: [PATCH 2/4] Deprecate Pool::getGroups This function does not work and it throws a RuntimeError because the internal the array property adminGroups changed some time ago its content, but this function was not updated. --- UPGRADE-3.x.md | 1 + src/Admin/Pool.php | 12 ++++++++++++ tests/Admin/PoolTest.php | 10 ++++++++++ 3 files changed, 23 insertions(+) diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index a65e1749c9..645e9b21a3 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -28,6 +28,7 @@ Argument 2 of `Sonata\AdminBundle\Model\ModelManagerInterface::createQuery()` me Use `Sonata\AdminBundle\SonataConfiguration::getLogo()` instead. - `Sonata\AdminBundle\Admin\Pool::getOption()` method has been deprecated. Use `Sonata\AdminBundle\SonataConfiguration::getOption()` instead. +- `Sonata\AdminBundle\Admin\Pool::getGroups()` method has been deprecated. ### Sonata\AdminBundle\Admin\FieldDescriptionInterface diff --git a/src/Admin/Pool.php b/src/Admin/Pool.php index 6457b501dc..e20682e7e4 100644 --- a/src/Admin/Pool.php +++ b/src/Admin/Pool.php @@ -153,8 +153,20 @@ public function __construct( $this->propertyAccessor = $propertyAccessor; } + /** + * NEXT_MAJOR: Remove this method. + * + * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0. + * + * @return array + */ public function getGroups() { + @trigger_error(sprintf( + 'Method "%s()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + $groups = $this->adminGroups; foreach ($this->adminGroups as $name => $adminGroup) { diff --git a/tests/Admin/PoolTest.php b/tests/Admin/PoolTest.php index 5f507651ba..3b42256153 100644 --- a/tests/Admin/PoolTest.php +++ b/tests/Admin/PoolTest.php @@ -18,11 +18,14 @@ use Sonata\AdminBundle\Admin\AdminInterface; use Sonata\AdminBundle\Admin\Pool; use Sonata\AdminBundle\Templating\MutableTemplateRegistryInterface; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerInterface; class PoolTest extends TestCase { + use ExpectDeprecationTrait; + /** * @var Container */ @@ -40,6 +43,11 @@ protected function setUp(): void $this->pool = new Pool($this->container, 'Sonata Admin', '/path/to/pic.png', ['foo' => 'bar']); } + /** + * NEXT_MAJOR: Remove this method. + * + * @group legacy + */ public function testGetGroups(): void { $this->container->set('sonata.user.admin.group1', $this->createMock(AdminInterface::class)); @@ -50,6 +58,8 @@ public function testGetGroups(): void 'adminGroup1' => ['sonata.user.admin.group1' => []], ]); + $this->expectDeprecation('Method "Sonata\AdminBundle\Admin\Pool::getGroups()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.'); + $result = $this->pool->getGroups(); $this->assertArrayHasKey('adminGroup1', $result); $this->assertArrayHasKey('sonata.user.admin.group1', $result['adminGroup1']); From af44c9f4f850a6fd9bcf110f914cbe07170b34cb Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Sun, 6 Dec 2020 21:39:39 +0100 Subject: [PATCH 3/4] Use psalm-type to avoid repetition --- src/Admin/Pool.php | 56 ++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/src/Admin/Pool.php b/src/Admin/Pool.php index e20682e7e4..a860bd3d7f 100644 --- a/src/Admin/Pool.php +++ b/src/Admin/Pool.php @@ -20,6 +20,24 @@ use Symfony\Component\PropertyAccess\PropertyAccessorInterface; /** + * @psalm-type Group = array{ + * label: string, + * label_catalogue: string, + * icon: string, + * item_adds: array, + * items: array, + * route?: string, + * router_absolute: bool, + * route_params: array + * }>, + * keep_open: bool, + * on_top: bool, + * roles: list + * } + * * @final since sonata-project/admin-bundle 3.52 * * @author Thomas Rabaix @@ -38,23 +56,8 @@ class Pool /** * @var array - * @phpstan-var array, - * route?: string, - * router_absolute: bool, - * route_params: array - * }>, - * keep_open: bool, - * on_top: bool, - * roles: list - * }> + * @phpstan-var array> + * @psalm-var array */ protected $adminGroups = []; @@ -485,23 +488,8 @@ public function getContainer() } /** - * @phpstan-param array, - * route?: string, - * router_absolute: bool, - * route_params: array - * }>, - * keep_open: bool, - * on_top: bool, - * roles: list - * }> $adminGroups + * @phpstan-param array> $adminGroups + * @psalm-param array $adminGroups * * @return void */ From 728f533770172fcd3a292d95b4479101263daeec Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Mon, 7 Dec 2020 09:09:01 +0100 Subject: [PATCH 4/4] Deprecate Pool::hasGroup and Pool::getAdminsByGroup They were added in https://github.com/sonata-project/SonataAdminBundle/pull/1465, but they have never been used in our code. --- UPGRADE-3.x.md | 2 ++ src/Admin/Pool.php | 18 ++++++++++++++++++ tests/Admin/PoolTest.php | 30 ++++++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index 645e9b21a3..47ef97f71b 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -29,6 +29,8 @@ Argument 2 of `Sonata\AdminBundle\Model\ModelManagerInterface::createQuery()` me - `Sonata\AdminBundle\Admin\Pool::getOption()` method has been deprecated. Use `Sonata\AdminBundle\SonataConfiguration::getOption()` instead. - `Sonata\AdminBundle\Admin\Pool::getGroups()` method has been deprecated. +- `Sonata\AdminBundle\Admin\Pool::hasGroup()` method has been deprecated. +- `Sonata\AdminBundle\Admin\Pool::getAdminsByGroup()` method has been deprecated. ### Sonata\AdminBundle\Admin\FieldDescriptionInterface diff --git a/src/Admin/Pool.php b/src/Admin/Pool.php index a860bd3d7f..9f74409649 100644 --- a/src/Admin/Pool.php +++ b/src/Admin/Pool.php @@ -182,6 +182,10 @@ public function getGroups() } /** + * NEXT_MAJOR: Remove this method. + * + * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0. + * * Returns whether an admin group exists or not. * * @param string $group @@ -190,6 +194,11 @@ public function getGroups() */ public function hasGroup($group) { + @trigger_error(sprintf( + 'Method "%s()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + return isset($this->adminGroups[$group]); } @@ -237,6 +246,10 @@ public function getDashboardGroups() } /** + * NEXT_MAJOR: Remove this method. + * + * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0. + * * Returns all admins related to the given $group. * * @param string $group @@ -247,6 +260,11 @@ public function getDashboardGroups() */ public function getAdminsByGroup($group) { + @trigger_error(sprintf( + 'Method "%s()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + if (!isset($this->adminGroups[$group])) { throw new \InvalidArgumentException(sprintf('Group "%s" not found in admin pool.', $group)); } diff --git a/tests/Admin/PoolTest.php b/tests/Admin/PoolTest.php index 3b42256153..9b4dcccc09 100644 --- a/tests/Admin/PoolTest.php +++ b/tests/Admin/PoolTest.php @@ -65,12 +65,19 @@ public function testGetGroups(): void $this->assertArrayHasKey('sonata.user.admin.group1', $result['adminGroup1']); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ public function testHasGroup(): void { $this->pool->setAdminGroups([ 'adminGroup1' => [], ]); + $this->expectDeprecation('Method "Sonata\AdminBundle\Admin\Pool::hasGroup()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.'); + $this->assertTrue($this->pool->hasGroup('adminGroup1')); $this->assertFalse($this->pool->hasGroup('adminGroup2')); } @@ -113,26 +120,43 @@ public function testGetDashboardGroups(): void $this->assertSame($adminGroup1, $groups['adminGroup1']['items']['itemKey']); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ public function testGetAdminsByGroupWhenGroupNotSet(): void { - $this->expectException(\InvalidArgumentException::class); - $this->pool->setAdminGroups([ 'adminGroup1' => [], ]); + $this->expectException(\InvalidArgumentException::class); + $this->pool->getAdminsByGroup('adminGroup2'); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ public function testGetAdminsByGroupWhenGroupIsEmpty(): void { $this->pool->setAdminGroups([ 'adminGroup1' => [], ]); + $this->expectDeprecation('Method "Sonata\AdminBundle\Admin\Pool::getAdminsByGroup()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.'); + $this->assertSame([], $this->pool->getAdminsByGroup('adminGroup1')); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ public function testGetAdminsByGroup(): void { $this->container->set('sonata.admin1', $this->createMock(AdminInterface::class)); @@ -153,6 +177,8 @@ public function testGetAdminsByGroup(): void ], ]); + $this->expectDeprecation('Method "Sonata\AdminBundle\Admin\Pool::getAdminsByGroup()" is deprecated since sonata-project/admin-bundle 3.x and will be removed in version 4.0.'); + $this->assertCount(2, $this->pool->getAdminsByGroup('adminGroup1')); $this->assertCount(1, $this->pool->getAdminsByGroup('adminGroup2')); }