diff --git a/.gitignore b/.gitignore index 07ef76fd..fe374b4d 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ /features/fixtures/project/public/bundles /features/fixtures/project/public/uploaded /features/fixtures/project/var/cache/* -/features/fixtures/project/var/logs/* +/features/fixtures/project/var/log/* /features/fixtures/project/var/mysql/* !/features/fixtures/project/var/.gitkeep !/features/fixtures/project/var/data/.gitkeep diff --git a/Behat/Context/AdminContext.php b/Behat/Context/AdminContext.php index 2f3e2b4d..687a85a2 100644 --- a/Behat/Context/AdminContext.php +++ b/Behat/Context/AdminContext.php @@ -26,6 +26,7 @@ use FSi\Bundle\AdminSecurityBundle\Behat\Page\Login; use FSi\Bundle\AdminSecurityBundle\Behat\Page\Page; use FSi\Bundle\AdminSecurityBundle\Behat\Page\PasswordResetRequest; +use FSi\Bundle\AdminSecurityBundle\Behat\Page\UserEdit; use FSi\Bundle\AdminSecurityBundle\Behat\Page\UserList; use FSi\Bundle\AdminSecurityBundle\Security\User\UserIdentifierHelper; use InvalidArgumentException; @@ -60,6 +61,9 @@ public function getPage(string $name): Page case 'User list': $page = $this->getPageObject(UserList::class); break; + case 'User edit': + $page = $this->getPageObject(UserEdit::class); + break; case 'Password reset request': $page = $this->getPageObject(PasswordResetRequest::class); break; diff --git a/Behat/Context/AdminUserManagementContext.php b/Behat/Context/AdminUserManagementContext.php index ec79af98..dbb10d52 100644 --- a/Behat/Context/AdminUserManagementContext.php +++ b/Behat/Context/AdminUserManagementContext.php @@ -13,11 +13,11 @@ use Assert\Assertion; use Behat\Gherkin\Node\TableNode; -use Behat\Mink\Driver\BrowserKitDriver; use FriendsOfBehat\PageObjectExtension\Page\UnexpectedPageException; use FriendsOfBehat\SymfonyExtension\Driver\SymfonyDriver; use FSi\Bundle\AdminSecurityBundle\Behat\Element\BatchAction; use FSi\Bundle\AdminSecurityBundle\Behat\Element\Datagrid; +use FSi\Bundle\AdminSecurityBundle\Behat\Page\UserEdit; use FSi\Bundle\AdminSecurityBundle\Behat\Page\UserList; use function count; @@ -91,14 +91,35 @@ public function iPressLink(string $name): void $this->getUserListPage()->clickLink($name); } + /** + * @When I click :name in :nthLiteral row + */ + public function iPressButtonInRow(string $name, string $nthLiteral): void + { + $nthRow = match ($nthLiteral) { + 'first' => 1, + 'second' => 2, + default => throw new \InvalidArgumentException(sprintf('Unknown row "%s"', $nthLiteral)), + }; + $this->getUserListPage()->clickButtonInRow($name, $nthRow); + } + /** * @Then I fill form with valid user data */ public function iFillFormWithValidUserData(): void { - $userListPage = $this->getUserListPage(); - $userListPage->fillField('Email', 'new-user@fsi.pl'); - $userListPage->checkField('ROLE_ADMIN'); + $userEditPage = $this->getUserEditPage(); + $userEditPage->fillField('Email', 'new-user@fsi.pl'); + $userEditPage->checkField('ROLE_ADMIN'); + } + + /** + * @Then I check :value + */ + public function iCheckField(string $value): void + { + $this->getUserEditPage()->checkField($value); } private function performBatchAction(string $action, int $cellIndex): void @@ -139,6 +160,11 @@ private function getUserListPage(): UserList return $this->getPageObject(UserList::class); } + private function getUserEditPage(): UserEdit + { + return $this->getPageObject(UserEdit::class); + } + private function getDatagrid(): Datagrid { /** @var Datagrid $datagrid */ diff --git a/Behat/Context/MailContext.php b/Behat/Context/MailContext.php index b374442a..2836b1fa 100644 --- a/Behat/Context/MailContext.php +++ b/Behat/Context/MailContext.php @@ -51,7 +51,7 @@ public function thereShouldBeNoEmailSent(): void Assertion::count( $this->mailerCollector->getEmails(), 0, - 'There were "%s" emails sent, where none should.' + 'The email list must contain %d email addresses, but %d was provided.' ); } diff --git a/Behat/Page/UserEdit.php b/Behat/Page/UserEdit.php new file mode 100644 index 00000000..3811ba61 --- /dev/null +++ b/Behat/Page/UserEdit.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace FSi\Bundle\AdminSecurityBundle\Behat\Page; + +use FriendsOfBehat\PageObjectExtension\Page\UnexpectedPageException; + +final class UserEdit extends Page +{ + /** + * @param array $urlParameters + * @return void + * @throws UnexpectedPageException + */ + public function verify(array $urlParameters = []): void + { + parent::verify($urlParameters); + + if (false === $this->getDocument()->has('css', '#page-header:contains("Edit element")')) { + throw new UnexpectedPageException( + sprintf("%s page is missing \"Edit element\" header", $this->getUrl($urlParameters)) + ); + } + } + + /** + * @param array $urlParameters + * @return string + */ + protected function getUrl(array $urlParameters = []): string + { + return $this->getParameter('base_url') . '/admin/en/form/admin_security_user/' . $urlParameters['id']; + } +} diff --git a/Behat/Page/UserList.php b/Behat/Page/UserList.php index e0427764..d6c1df06 100644 --- a/Behat/Page/UserList.php +++ b/Behat/Page/UserList.php @@ -11,6 +11,7 @@ namespace FSi\Bundle\AdminSecurityBundle\Behat\Page; +use Assert\Assertion; use Behat\Mink\Element\NodeElement; use FriendsOfBehat\PageObjectExtension\Page\UnexpectedPageException; @@ -62,13 +63,21 @@ public function getBatchActions(): array ); } + public function clickButtonInRow(string $name, int $nth): void + { + $locator = sprintf('tbody > tr:nth-child(%d) > td a:contains("%s")', $nth, $name); + $element = $this->getElement('table')->find('css', $locator); + Assertion::notNull($element, sprintf('"%s" not found in row %d', $name, $nth)); + $element->click(); + } + /** * @return array */ protected function getDefinedElements(): array { return [ - 'table' => '#admin_security_user', + 'table' => 'table#admin_security_user', 'batch actions' => '#batch_action_action', ]; } @@ -79,6 +88,6 @@ protected function getDefinedElements(): array */ protected function getUrl(array $urlParameters = []): string { - return $this->getParameter('base_url') . '/admin/en/list/admin_security_user'; + return $this->getParameter('base_url') . '/admin/en/list/user'; } } diff --git a/EventListener/PrepareUserListener.php b/EventListener/PrepareUserListener.php index 88ae452d..f2ff6857 100644 --- a/EventListener/PrepareUserListener.php +++ b/EventListener/PrepareUserListener.php @@ -11,6 +11,7 @@ namespace FSi\Bundle\AdminSecurityBundle\EventListener; +use FSi\Bundle\AdminBundle\Doctrine\Admin\Element; use FSi\Bundle\AdminBundle\Event\FormDataPreSaveEvent; use FSi\Bundle\AdminBundle\Event\FormEvent; use FSi\Bundle\AdminSecurityBundle\Event\UserCreatedEvent; @@ -47,8 +48,13 @@ public function prepareAndDispatchUserCreated(FormEvent $event): void return; } + $element = $event->getElement(); + if (false === $element instanceof Element) { + return; + } + // do not dispatch event and do not set random password for existing users - if (null !== $entity->getPassword()) { + if (true === $element->getObjectManager()->contains($entity)) { return; } diff --git a/Resources/translations/FSiAdminSecurity.pl.yml b/Resources/translations/FSiAdminSecurity.pl.yml index a2e2a1d7..9e39ca74 100644 --- a/Resources/translations/FSiAdminSecurity.pl.yml +++ b/Resources/translations/FSiAdminSecurity.pl.yml @@ -82,4 +82,4 @@ crud: datagrid: boolean: yes: Tak - no: Nie \ No newline at end of file + no: Nie diff --git a/composer.json b/composer.json index b86d8de6..8206f6ab 100644 --- a/composer.json +++ b/composer.json @@ -55,7 +55,7 @@ "symfony/monolog-bridge": "^5.4|^6.0|^7.0", "symfony/monolog-bundle": "^3.8", "symfony/routing": "^5.4|^6.0|^7.0", - "symfony/twig-bridge": "^5.4|^6.0|^7.0", + "symfony/twig-bridge": "~5.4.48|^6.0|^7.0", "symfony/var-dumper": "^5.4|^6.0|^7.0", "symfony/var-exporter": "^5.4|^6.0|^7.0", "rize/uri-template": "^0.3.5|^0.4.0" diff --git a/features/admin/user_managment.feature b/features/admin/user_managment.feature index 2c7f4052..ab53dfce 100644 --- a/features/admin/user_managment.feature +++ b/features/admin/user_managment.feature @@ -64,3 +64,12 @@ Feature: User management | from | activation@fsi.pl | | to | new-user@fsi.pl | | reply_to | do-not-reply@example.com | + + @email + Scenario: Edit User + Given I am on the "User list" page + And I click "Edit" in second row + And I check "ROLE_ADMIN" + And I press "Save" button + And I should be redirected to "User list" page + And no emails were sent diff --git a/features/fixtures/project/config/admin_menu.yml b/features/fixtures/project/config/admin_menu.yml index 5c8192e3..d082174d 100644 --- a/features/fixtures/project/config/admin_menu.yml +++ b/features/fixtures/project/config/admin_menu.yml @@ -1,4 +1,4 @@ menu: - news - { id: page_settings, name: Page settings } - - { id: admin_security_user, name: Users } + - { id: user, name: Users } diff --git a/features/fixtures/project/config/bundles.php b/features/fixtures/project/config/bundles.php index 3d64977c..e12413cc 100644 --- a/features/fixtures/project/config/bundles.php +++ b/features/fixtures/project/config/bundles.php @@ -12,7 +12,7 @@ FSi\Component\Translatable\Integration\Symfony\TranslatableBundle::class => ['all' => true], FSi\Bundle\ResourceRepositoryBundle\FSiResourceRepositoryBundle::class => ['all' => true], FSi\Bundle\AdminBundle\FSiAdminBundle::class => ['all' => true], - FSi\FixturesBundle\FSiFixturesBundle::class => ['all' => true], FSi\Bundle\AdminSecurityBundle\FSiAdminSecurityBundle::class => ['all' => true], - FriendsOfBehat\SymfonyExtension\Bundle\FriendsOfBehatSymfonyExtensionBundle::class => ['all' => true] + FSi\FixturesBundle\FSiFixturesBundle::class => ['all' => true], + FriendsOfBehat\SymfonyExtension\Bundle\FriendsOfBehatSymfonyExtensionBundle::class => ['all' => true], ]; diff --git a/features/fixtures/project/config/packages/fsi_resource_repository.yml b/features/fixtures/project/config/packages/fsi_resource_repository.yaml similarity index 100% rename from features/fixtures/project/config/packages/fsi_resource_repository.yml rename to features/fixtures/project/config/packages/fsi_resource_repository.yaml diff --git a/features/fixtures/project/config/packages/mailer.yml b/features/fixtures/project/config/packages/mailer.yaml similarity index 100% rename from features/fixtures/project/config/packages/mailer.yml rename to features/fixtures/project/config/packages/mailer.yaml diff --git a/features/fixtures/project/config/services.php b/features/fixtures/project/config/services.php new file mode 100644 index 00000000..7b47747d --- /dev/null +++ b/features/fixtures/project/config/services.php @@ -0,0 +1,39 @@ +services(); + + $services->defaults() + ->autowire() + ->autoconfigure() + ->private() + ->bind('$session', service('behat.mink.default_session')) + ->bind('$minkParameters', service('behat.mink.parameters')); + + $services->load( + 'FSi\\Bundle\\AdminSecurityBundle\\Behat\\Context\\', + '../../../../Behat/Context' + ); + + $services->set(DataContext::class) + ->arg('$container', service('behat.driver.service_container')->nullOnInvalid()); + + $services->set(News::class); + $services->set(PageSettings::class); + $services->set(MailCollector::class); + + $services->set(Clock::class); + $services->alias(ClockInterface::class, Clock::class)->public(); +}; diff --git a/features/fixtures/project/src/Admin/User.php b/features/fixtures/project/src/Admin/User.php new file mode 100644 index 00000000..d042b00f --- /dev/null +++ b/features/fixtures/project/src/Admin/User.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace FSi\FixturesBundle\Admin; + +use FSi\Bundle\AdminSecurityBundle\Doctrine\Admin\UserElement; +use FSi\Component\DataGrid\DataGridFactoryInterface; +use FSi\Component\DataGrid\DataGridInterface; +use FSi\FixturesBundle\Entity\User as UserEntity; + +/** + * @template-extends UserElement + */ +class User extends UserElement +{ + public function getId(): string + { + return 'user'; + } + + protected function initDataGrid(DataGridFactoryInterface $factory): DataGridInterface + { + return $factory->createDataGrid('admin_security_user'); + } +} diff --git a/features/fixtures/project/src/DependencyInjection/FSiFixturesExtension.php b/features/fixtures/project/src/DependencyInjection/FSiFixturesExtension.php index cd66dd4c..d6fda30f 100644 --- a/features/fixtures/project/src/DependencyInjection/FSiFixturesExtension.php +++ b/features/fixtures/project/src/DependencyInjection/FSiFixturesExtension.php @@ -12,10 +12,8 @@ namespace FSi\FixturesBundle\DependencyInjection; use Composer\InstalledVersions; -use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; -use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -44,7 +42,5 @@ public function prepend(ContainerBuilder $container): void */ public function load(array $configs, ContainerBuilder $container): void { - $loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); - $loader->load('services.xml'); } } diff --git a/features/fixtures/project/src/Kernel.php b/features/fixtures/project/src/Kernel.php index a565a753..e806c9ca 100644 --- a/features/fixtures/project/src/Kernel.php +++ b/features/fixtures/project/src/Kernel.php @@ -11,82 +11,23 @@ namespace FSi\FixturesBundle; +use FSi\FixturesBundle\Admin\User; use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; -use Symfony\Component\Config\Loader\LoaderInterface; -use Symfony\Component\Config\Resource\FileResource; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\HttpFoundation\Session\SessionFactoryInterface; -use Symfony\Component\HttpKernel\Bundle\BundleInterface; use Symfony\Component\HttpKernel\Kernel as BaseKernel; -use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; -use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator; -use Symfony\Component\Routing\RouteCollectionBuilder; -use function interface_exists; -use function sprintf; - -class Kernel extends BaseKernel +class Kernel extends BaseKernel implements CompilerPassInterface { use MicroKernelTrait; - private const CONFIG_EXTS = '.{php,xml,yaml,yml}'; - - public function registerBundles(): iterable + public function process(ContainerBuilder $container): void { - /** @var array, array> $contents */ - $contents = require $this->getProjectDir() . '/config/bundles.php'; - foreach ($contents as $class => $envs) { - if ($envs[$this->environment] ?? $envs['all'] ?? false) { - yield new $class(); - } - } + $container->getDefinition('admin_security.admin.user')->setClass(User::class); } public function getProjectDir(): string { return dirname(__DIR__); } - - public function getCacheDir(): string - { - return $this->getProjectDir() . '/var/cache'; - } - - public function getLogDir(): string - { - return $this->getProjectDir() . '/var/logs'; - } - - protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void - { - $configDirectory = $this->getProjectDir() . '/config'; - $container->addResource(new FileResource($configDirectory . '/bundles.php')); - $container->setParameter('container.dumper.inline_class_loader', $this->debug); - $container->setParameter('container.dumper.inline_factories', true); - - $loader->load($configDirectory . '/{packages}/*' . self::CONFIG_EXTS, 'glob'); - $loader->load($configDirectory . '/{packages}/' . $this->environment . '/*' . self::CONFIG_EXTS, 'glob'); - $loader->load($configDirectory . '/{services}' . self::CONFIG_EXTS, 'glob'); - $loader->load($configDirectory . '/{services}_' . $this->environment . self::CONFIG_EXTS, 'glob'); - - $loader->load(sprintf('%s/../src/Resources/config/services.xml', __DIR__)); - } - - /** - * @param RouteCollectionBuilder|RoutingConfigurator $routes - */ - protected function configureRoutes($routes): void - { - $confDir = $this->getProjectDir() . '/config'; - - if (true === $routes instanceof RouteCollectionBuilder) { - $routes->import($confDir . '/{routes}/' . $this->environment . '/*' . self::CONFIG_EXTS, '/', 'glob'); - $routes->import($confDir . '/{routes}/*' . self::CONFIG_EXTS, '/', 'glob'); - $routes->import($confDir . '/{routes}' . self::CONFIG_EXTS, '/', 'glob'); - } else { - $routes->import($confDir . '/{routes}/' . $this->environment . '/*' . self::CONFIG_EXTS, 'glob'); - $routes->import($confDir . '/{routes}/*' . self::CONFIG_EXTS, 'glob'); - $routes->import($confDir . '/{routes}' . self::CONFIG_EXTS, 'glob'); - } - } } diff --git a/features/fixtures/project/src/Resources/config/datagrid/admin_security_user.yml b/features/fixtures/project/src/Resources/config/datagrid/admin_security_user.yml new file mode 100644 index 00000000..66ac62a8 --- /dev/null +++ b/features/fixtures/project/src/Resources/config/datagrid/admin_security_user.yml @@ -0,0 +1,55 @@ +columns: + # All columns below are copied directly from the default configuration Resources/config/datagrid/admin_security_user.yml + email: + type: text + options: + label: admin.user_list.datagrid.email + translation_domain: FSiAdminSecurity + enabled: + type: boolean + options: + label: admin.user_list.datagrid.enabled + translation_domain: FSiAdminSecurity + last_login: + type: datetime + options: + label: admin.user_list.datagrid.last_login + translation_domain: FSiAdminSecurity + password_reset_token_non_expired: + type: token_non_expired + options: + label: admin.user_list.datagrid.password_reset_token + translation_domain: FSiAdminSecurity + field_mapping: [ passwordResetToken ] + activation_token_non_expired: + type: token_non_expired + options: + label: admin.user_list.datagrid.activation_token + translation_domain: FSiAdminSecurity + field_mapping: [ activationToken ] + batch: + type: batch + options: + translation_domain: FSiAdminSecurity + actions: + delete: + label: admin.user_list.batch_action.delete + route_name: fsi_admin_batch + additional_parameters: { element: user } # Changed element to user + password_reset: + label: admin.user_list.batch_action.password_reset + element: admin_security_password_reset + resend_activation_token: + label: admin.user_list.batch_action.resend_activation_token + element: admin_security_reset_activation_token + # New column that is not present in default configuration + actions: + type: action + options: + translation_domain: FSiAdminSecurity + field_mapping: [ id ] + actions: + Edit: + route_name: fsi_admin_form + additional_parameters: { element: user } + parameters_field_mapping: { id: id } diff --git a/features/fixtures/project/src/Resources/config/services.xml b/features/fixtures/project/src/Resources/config/services.xml deleted file mode 100644 index 7be95913..00000000 --- a/features/fixtures/project/src/Resources/config/services.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/spec/FSi/Bundle/AdminSecurityBundle/EventListener/PrepareUserListenerSpec.php b/spec/FSi/Bundle/AdminSecurityBundle/EventListener/PrepareUserListenerSpec.php index a8b77730..a968a51d 100644 --- a/spec/FSi/Bundle/AdminSecurityBundle/EventListener/PrepareUserListenerSpec.php +++ b/spec/FSi/Bundle/AdminSecurityBundle/EventListener/PrepareUserListenerSpec.php @@ -11,6 +11,9 @@ namespace spec\FSi\Bundle\AdminSecurityBundle\EventListener; +use Doctrine\Persistence\ObjectManager; +use FSi\Bundle\AdminBundle\Doctrine\Admin\CRUDElement; +use FSi\Bundle\AdminBundle\Doctrine\Admin\Element; use FSi\Bundle\AdminBundle\Event\FormEvent; use FSi\Bundle\AdminSecurityBundle\Event\UserCreatedEvent; use FSi\Bundle\AdminSecurityBundle\Security\User\UserInterface; @@ -36,11 +39,17 @@ public function it_should_prepare_new_user_and_dispatch_user_created( FormEvent $event, FormInterface $form, UserInterface $user, - EventDispatcherInterface $eventDispatcher + EventDispatcherInterface $eventDispatcher, + CRUDElement $element, + ObjectManager $objectManager, ): void { + $event->getElement()->willReturn($element); $event->getForm()->willReturn($form); $form->getData()->willReturn($user); + $element->getObjectManager()->willReturn($objectManager); + $objectManager->contains($user)->willReturn(false); + $user->getPassword()->willReturn(null); $user->setPlainPassword(Argument::type('string'))->shouldBeCalled(); @@ -54,16 +63,20 @@ public function it_should_prepare_new_user_and_dispatch_user_created( $this->prepareAndDispatchUserCreated($event); } - public function it_should_do_not_set_random_password_when_user_already_exists( + public function it_should_not_preapre_existing_user( FormEvent $event, FormInterface $form, UserInterface $user, - EventDispatcherInterface $eventDispatcher + EventDispatcherInterface $eventDispatcher, + CRUDElement $element, + ObjectManager $objectManager, ): void { + $event->getElement()->willReturn($element); $event->getForm()->willReturn($form); $form->getData()->willReturn($user); - $user->getPassword()->willReturn('password-hash'); + $element->getObjectManager()->willReturn($objectManager); + $objectManager->contains($user)->willReturn(true); $user->setPlainPassword(Argument::any())->shouldNotBeCalled(); $user->setEnabled(Argument::any())->shouldNotBeCalled();