diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index daaaf79..af003dd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -4,6 +4,7 @@ on: pull_request: null push: branches: + - "main" - "0.*.x" jobs: @@ -14,14 +15,12 @@ jobs: strategy: matrix: include: - - php-version: 7.3 - symfony-version: 4.4.* - - php-version: 7.3 - symfony-version: 5.2.* - php-version: 7.4 - symfony-version: 5.2.* + symfony-version: 4.4.* + sonata-version: ^4.0 - php-version: 8.0 - symfony-version: 5.2.* + symfony-version: 5.3.* + sonata-version: ^4.0 steps: - name: "Checkout" @@ -36,6 +35,7 @@ jobs: - name: "Install dependencies with composer" run: | composer require --no-update symfony/workflow:${{ matrix.symfony-version }} + composer require --no-update sonata-project/admin-bundle:${{ matrix.sonata-version }} composer update --no-interaction --no-progress --no-suggest - name: "Run tests with phpunit/phpunit" @@ -49,7 +49,7 @@ jobs: matrix: include: - php-version: 8.0 - symfony-version: 5.2.* + symfony-version: 5.3.* steps: - name: "Checkout" @@ -76,7 +76,7 @@ jobs: matrix: include: - php-version: 8.0 - symfony-version: 5.2.* + symfony-version: 5.3.* steps: - name: "Checkout" diff --git a/.gitignore b/.gitignore index 21f3b13..2c41493 100644 --- a/.gitignore +++ b/.gitignore @@ -3,5 +3,6 @@ /build/ /vendor/ /.phpcs-cache +/.phpunit.result.cache /composer.lock /docker-compose.yml diff --git a/README.md b/README.md index 6494aa9..462722b 100644 --- a/README.md +++ b/README.md @@ -207,7 +207,7 @@ class PullRequestController extends CRUDController { use WorkflowControllerTrait; - protected function preApplyTransition($object, string $transition): ?Response + protected function preApplyTransition(object $object, string $transition): ?Response { switch ($transition) { case 'start_review': diff --git a/composer.json b/composer.json index 0107063..2567479 100644 --- a/composer.json +++ b/composer.json @@ -25,12 +25,13 @@ } }, "require": { - "php": "^7.3|^8.0", - "sonata-project/admin-bundle": "^3.0", + "php": "^7.4|^8.0", + "sonata-project/admin-bundle": "^4.0", "symfony/workflow": "^4.4|^5.0" }, "require-dev": { - "phpunit/phpunit": "^8.5|^9.5", - "squizlabs/php_codesniffer": "^3.5" + "phpunit/phpunit": "^9.5", + "squizlabs/php_codesniffer": "^3.5", + "phpspec/prophecy-phpunit": "^2.0" } } diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 9f8bda0..2c2bb6e 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,6 +1,8 @@ - + bootstrap="vendor/autoload.php"> + + + ./src + + ./tests - - - - ./src - - diff --git a/src/Admin/Extension/WorkflowExtension.php b/src/Admin/Extension/WorkflowExtension.php index b037588..fa27145 100644 --- a/src/Admin/Extension/WorkflowExtension.php +++ b/src/Admin/Extension/WorkflowExtension.php @@ -7,7 +7,7 @@ use Knp\Menu\ItemInterface as MenuItemInterface; use Sonata\AdminBundle\Admin\AbstractAdminExtension; use Sonata\AdminBundle\Admin\AdminInterface; -use Sonata\AdminBundle\Route\RouteCollection; +use Sonata\AdminBundle\Route\RouteCollectionInterface; use Symfony\Component\OptionsResolver\OptionsResolver; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Workflow\Exception\InvalidArgumentException; @@ -20,20 +20,9 @@ */ class WorkflowExtension extends AbstractAdminExtension { - /** - * @var Registry - */ - private $registry; + private Registry $registry; + private array $options; - /** - * @var array - */ - private $options; - - /** - * @param Registry $registry - * @param array $options - */ public function __construct(Registry $registry, array $options = []) { $this->registry = $registry; @@ -44,7 +33,7 @@ public function __construct(Registry $registry, array $options = []) /** * @inheritdoc */ - public function configureRoutes(AdminInterface $admin, RouteCollection $collection): void + public function configureRoutes(AdminInterface $admin, RouteCollectionInterface $collection): void { $collection->add( 'workflow_apply_transition', @@ -69,18 +58,23 @@ public function alterNewInstance(AdminInterface $admin, $object): void /** * @inheritdoc */ - public function configureSideMenu( + public function configureTabMenu( AdminInterface $admin, MenuItemInterface $menu, $action, - AdminInterface $childAdmin = null + ?AdminInterface $childAdmin = null ): void { if (null !== $childAdmin || !in_array($action, $this->options['render_actions'], true)) { return; } - $subject = $admin->getSubject(); - if (null === $subject || !$this->isGrantedView($admin, $subject)) { + try { + $subject = $admin->getSubject(); + } catch (\LogicException $exception) { + return; + } + + if (!$this->isGrantedView($admin, $subject)) { return; } @@ -111,20 +105,13 @@ public function getAccessMapping(AdminInterface $admin): array } /** - * @param object $subject - * @param string|null $workflowName - * - * @return Workflow * @throws InvalidArgumentException */ - protected function getWorkflow($subject, string $workflowName = null): Workflow + protected function getWorkflow(object $subject, string $workflowName = null): Workflow { return $this->registry->get($subject, $workflowName); } - /** - * @param OptionsResolver $resolver - */ protected function configureOptions(OptionsResolver $resolver): void { $resolver @@ -155,10 +142,6 @@ protected function configureOptions(OptionsResolver $resolver): void ; } - /** - * @param MenuItemInterface $menu - * @param AdminInterface $admin - */ protected function noTransitions(MenuItemInterface $menu, AdminInterface $admin): void { if ($this->options['no_transition_display']) { @@ -175,16 +158,13 @@ protected function noTransitions(MenuItemInterface $menu, AdminInterface $admin) } /** - * @param MenuItemInterface $menu - * @param AdminInterface $admin - * @param iterable|Transition[] $transitions - * @param object $subject + * @param iterable&Transition[] $transitions */ protected function transitionsDropdown( MenuItemInterface $menu, AdminInterface $admin, iterable $transitions, - $subject + object $subject ): void { $workflowMenu = $menu->addChild($this->options['dropdown_transitions_label'], [ 'attributes' => [ @@ -201,17 +181,11 @@ protected function transitionsDropdown( } } - /** - * @param MenuItemInterface $menu - * @param AdminInterface $admin - * @param Transition $transition - * @param object $subject - */ protected function transitionsItem( MenuItemInterface $menu, AdminInterface $admin, Transition $transition, - $subject + object $subject ): void { $options = [ 'attributes' => [], @@ -234,11 +208,6 @@ protected function transitionsItem( ); } - /** - * @param Transition $transition - * - * @return string|null - */ protected function getTransitionIcon(Transition $transition): ?string { if (isset($this->options['transitions_icons'][$transition->getName()])) { @@ -248,14 +217,7 @@ protected function getTransitionIcon(Transition $transition): ?string return $this->options['transitions_default_icon']; } - /** - * @param AdminInterface $admin - * @param Transition $transition - * @param object $subject - * - * @return string - */ - protected function generateTransitionUri(AdminInterface $admin, Transition $transition, $subject): string + protected function generateTransitionUri(AdminInterface $admin, Transition $transition, object $subject): string { return $admin->generateObjectUrl( 'workflow_apply_transition', @@ -264,13 +226,7 @@ protected function generateTransitionUri(AdminInterface $admin, Transition $tran ); } - /** - * @param AdminInterface $admin - * @param object $subject - * - * @return bool - */ - protected function isGrantedView(AdminInterface $admin, $subject): bool + protected function isGrantedView(AdminInterface $admin, object $subject): bool { try { $admin->checkAccess('viewTransitions', $subject); @@ -281,13 +237,7 @@ protected function isGrantedView(AdminInterface $admin, $subject): bool return true; } - /** - * @param AdminInterface $admin - * @param object $subject - * - * @return bool - */ - protected function isGrantedApply(AdminInterface $admin, $subject): bool + protected function isGrantedApply(AdminInterface $admin, object $subject): bool { try { $admin->checkAccess('applyTransitions', $subject); diff --git a/src/Controller/WorkflowControllerTrait.php b/src/Controller/WorkflowControllerTrait.php index be77a58..725b457 100644 --- a/src/Controller/WorkflowControllerTrait.php +++ b/src/Controller/WorkflowControllerTrait.php @@ -19,30 +19,14 @@ use Symfony\Component\Workflow\Workflow; /** - * @method void addFlash(string $type, string $message) - * @method NotFoundHttpException createNotFoundException(string $message = 'Not Found', \Exception $previous = null) - * @method string escapeHtml(string $s) - * @method ContainerInterface getContainer - * @method mixed get(string $id) - * @method void handleModelManagerException(\Exception $e) - * @method bool isXmlHttpRequest - * @method Response redirectTo(object $object) - * @method Response renderJson($data, int $status = 200, array $headers = []) - * @method string trans(string $id, array $parameters = [], string $domain = null, string $locale = null) - * @property AdminInterface $admin * * @author Yann Eugoné */ trait WorkflowControllerTrait { - /** - * @var Registry - */ - private $workflowRegistry; + private Registry $workflowRegistry; /** - * @param Registry $workflowRegistry - * * @required Symfony DI autowiring */ public function setWorkflowRegistry(Registry $workflowRegistry): void @@ -50,11 +34,6 @@ public function setWorkflowRegistry(Registry $workflowRegistry): void $this->workflowRegistry = $workflowRegistry; } - /** - * @param Request $request - * - * @return Response - */ public function workflowApplyTransitionAction(Request $request): Response { $id = $request->get($this->admin->getIdParameter()); @@ -100,7 +79,7 @@ public function workflowApplyTransitionAction(Request $request): Response $workflow->apply($existingObject, $transition); $existingObject = $this->admin->update($existingObject); - if ($this->isXmlHttpRequest()) { + if ($this->isXmlHttpRequest($request)) { return $this->renderJson( [ 'result' => 'ok', @@ -146,18 +125,15 @@ public function workflowApplyTransitionAction(Request $request): Response ); } - return $this->redirectTo($existingObject); + return $this->redirectTo($request, $existingObject); } /** - * @param object $object - * - * @return Workflow * @throws InvalidArgumentException */ - protected function getWorkflow($object): Workflow + protected function getWorkflow(object $object): Workflow { - $registry = $this->workflowRegistry; + $registry = $this->workflowRegistry ?? null; if ($registry === null) { try { if (method_exists($this, 'get')) { @@ -179,13 +155,7 @@ protected function getWorkflow($object): Workflow return $registry->get($object); } - /** - * @param object $object - * @param string $transition - * - * @return null|Response - */ - protected function preApplyTransition($object, string $transition): ?Response + protected function preApplyTransition(object $object, string $transition): ?Response { return null; } diff --git a/tests/Admin/Extension/WorkflowExtensionTest.php b/tests/Admin/Extension/WorkflowExtensionTest.php index f77b931..53aeac5 100644 --- a/tests/Admin/Extension/WorkflowExtensionTest.php +++ b/tests/Admin/Extension/WorkflowExtensionTest.php @@ -8,6 +8,7 @@ use Knp\Menu\MenuFactory; use Knp\Menu\MenuItem; use PHPUnit\Framework\TestCase; +use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Sonata\AdminBundle\Admin\AdminInterface; use Sonata\AdminBundle\Route\RouteCollection; @@ -25,6 +26,8 @@ */ class WorkflowExtensionTest extends TestCase { + use ProphecyTrait; + public function testConfigureRoutes(): void { /** @var AdminInterface|ObjectProphecy $admin */ @@ -86,19 +89,19 @@ public function testAccessMapping(): void ); } - public function testConfigureSideMenuWithoutSubject(): void + public function testConfigureTabMenuWithoutSubject(): void { /** @var AdminInterface|ObjectProphecy $admin */ $admin = $this->prophesize(AdminInterface::class); - $admin->getSubject()->willReturn(null); + $admin->getSubject()->willThrow(new \LogicException()); $extension = new WorkflowExtension(new Registry()); - $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); + $extension->configureTabMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); self::assertFalse($menu->hasChildren()); } - public function testConfigureSideMenuWithoutPermission(): void + public function testConfigureTabMenuWithoutPermission(): void { /** @var AdminInterface|ObjectProphecy $admin */ $admin = $this->prophesize(AdminInterface::class); @@ -106,12 +109,12 @@ public function testConfigureSideMenuWithoutPermission(): void $admin->checkAccess('viewTransitions', $pullRequest)->willThrow(new AccessDeniedException()); $extension = new WorkflowExtension(new Registry()); - $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); + $extension->configureTabMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); self::assertFalse($menu->hasChildren()); } - public function testConfigureSideMenuWithoutWorkflow(): void + public function testConfigureTabMenuWithoutWorkflow(): void { /** @var AdminInterface|ObjectProphecy $admin */ $admin = $this->prophesize(AdminInterface::class); @@ -119,7 +122,7 @@ public function testConfigureSideMenuWithoutWorkflow(): void $admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled(); $extension = new WorkflowExtension(new Registry()); - $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); + $extension->configureTabMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); self::assertFalse($menu->hasChildren()); } @@ -127,7 +130,7 @@ public function testConfigureSideMenuWithoutWorkflow(): void /** * @dataProvider markingToTransition */ - public function testConfigureSideMenu(string $marking, array $transitions, bool $grantedApply): void + public function testConfigureTabMenu(string $marking, array $transitions, bool $grantedApply): void { $pullRequest = new PullRequest(); $pullRequest->setMarking($marking); @@ -172,7 +175,7 @@ public function testConfigureSideMenu(string $marking, array $transitions, bool 'transitions_icons' => ['merge' => 'fa fa-times'], ]; $extension = new WorkflowExtension($registry, $options); - $extension->configureSideMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); + $extension->configureTabMenu($admin->reveal(), $menu = new MenuItem('root', new MenuFactory()), 'edit'); if (count($transitions) === 0) { self::assertNull($child = $menu->getChild('workflow_transitions')); diff --git a/tests/Controller/WorkflowControllerTest.php b/tests/Controller/WorkflowControllerTest.php index 95f09b2..2b4af09 100644 --- a/tests/Controller/WorkflowControllerTest.php +++ b/tests/Controller/WorkflowControllerTest.php @@ -5,12 +5,15 @@ namespace Yokai\SonataWorkflow\Tests\Controller; use PHPUnit\Framework\TestCase; +use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Sonata\AdminBundle\Admin\AdminInterface; use Sonata\AdminBundle\Admin\Pool; use Sonata\AdminBundle\Exception\LockException; use Sonata\AdminBundle\Exception\ModelManagerException; -use Sonata\AdminBundle\Templating\TemplateRegistryInterface; +use Sonata\AdminBundle\Request\AdminFetcher; +use Sonata\AdminBundle\Templating\MutableTemplateRegistry; +use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -27,44 +30,31 @@ use Yokai\SonataWorkflow\Tests\Fixtures\StubTranslator; use Yokai\SonataWorkflow\Tests\PullRequest; use Yokai\SonataWorkflow\Tests\PullRequestWorkflowController; -use Yokai\SonataWorkflow\Tests\TestKernel; /** * @author Yann Eugoné */ class WorkflowControllerTest extends TestCase { - /** - * @var ContainerInterface|ObjectProphecy - */ - private $container; + use ProphecyTrait; - /** - * @var Request - */ - private $request; + private ContainerInterface $container; + private Request $request; + private Registry $registry; + private FlashBag $flashBag; /** * @var AdminInterface|ObjectProphecy */ private $admin; - /** - * @var Registry - */ - private $registry; - - /** - * @var FlashBag - */ - private $flashBag; - protected function setUp(): void { parent::setUp(); - $this->container = $this->prophesize(ContainerInterface::class); $this->admin = $this->prophesize(AdminInterface::class); + + $this->container = new Container(); $this->registry = new Registry(); $this->flashBag = new FlashBag(); $translator = new StubTranslator(); @@ -74,27 +64,25 @@ protected function setUp(): void $this->request->query->set('id', 42); $this->request->attributes->set('_sonata_admin', 'admin.pull_request'); + $this->request->setSession(new Session(new MockArraySessionStorage(), null, $this->flashBag)); + + $pool = new Pool($this->container, ['admin.pull_request']); - $pool = new Pool($this->container->reveal(), 'phpunit', 'phpunit'); - $pool->setAdminServiceIds(['admin.pull_request']); - - $this->container->get('request_stack')->willReturn($stack); - $this->container->get('sonata.admin.pool')->willReturn($pool); - $this->container->get('admin.pull_request')->willReturn($this->admin->reveal()); - $this->container->get('workflow.registry')->willReturn($this->registry); - $this->container->get('kernel')->willReturn(new TestKernel()); - $this->container->has('session')->willReturn(true); - $this->container->get('session') - ->willReturn(new Session(new MockArraySessionStorage(), null, $this->flashBag)); - $this->container->get('translator')->willReturn($translator); - $this->container->has('logger')->willReturn(false); - $this->container->get('admin.pull_request.template_registry') - ->willReturn($this->prophesize(TemplateRegistryInterface::class)->reveal()); + $this->container->getParameterBag()->set('kernel.debug', true); + $this->container->set('request_stack', $stack); + $this->container->set('session', $this->request->getSession()); + $this->container->set('parameter_bag', $this->container->getParameterBag()); + $this->container->set('admin.pull_request', $this->admin->reveal()); + $this->container->set('workflow.registry', $this->registry); + $this->container->set('translator', $translator); + $this->container->set('sonata.admin.request.fetcher', new AdminFetcher($pool)); $this->admin->isChild()->willReturn(false); - $this->admin->setRequest($this->request)->willReturn(null); + $this->admin->setRequest($this->request)->shouldBeCalled(); $this->admin->getIdParameter()->willReturn('id'); $this->admin->getCode()->willReturn('admin.pull_request'); + $this->admin->hasTemplateRegistry()->willReturn(true); + $this->admin->getTemplateRegistry()->willReturn(new MutableTemplateRegistry()); } public function testWorkflowApplyTransitionActionObjectNotFound(): void @@ -322,7 +310,8 @@ private function controller(): PullRequestWorkflowController { $controller = new PullRequestWorkflowController(); - $controller->setContainer($this->container->reveal()); + $controller->setContainer($this->container); + $controller->configureAdmin($this->request); return $controller; } diff --git a/tests/PullRequest.php b/tests/PullRequest.php index 9e8adb0..a6349f4 100644 --- a/tests/PullRequest.php +++ b/tests/PullRequest.php @@ -11,7 +11,7 @@ class PullRequest { - private $marking; + private ?string $marking = null; public function getMarking(): ?string { diff --git a/tests/PullRequestWorkflowController.php b/tests/PullRequestWorkflowController.php index 9237cd5..de3a541 100644 --- a/tests/PullRequestWorkflowController.php +++ b/tests/PullRequestWorkflowController.php @@ -9,7 +9,7 @@ class PullRequestWorkflowController extends WorkflowController { - protected function preApplyTransition($object, string $transition): ?Response + protected function preApplyTransition(object $object, string $transition): ?Response { if ($transition === 'merge') { return new Response('merge');