From a8a8424cedbe446c254410e71295580fb9e78f7f Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Tue, 31 Jul 2018 13:16:40 +0200 Subject: [PATCH] Added delete user feature #225 (#249) --- README.md | 2 +- UPGRADING.md | 5 + src/Controller/Admin/ActivityController.php | 4 +- src/Controller/Admin/CustomerController.php | 4 +- src/Controller/Admin/ProjectController.php | 4 +- src/Controller/Admin/UserController.php | 71 +++++++++- src/Controller/ProfileController.php | 25 +++- src/Entity/Activity.php | 8 ++ src/Entity/Timesheet.php | 2 +- src/Entity/User.php | 10 +- src/Migrations/Version20180730044139.php | 92 +++++++++++++ src/Model/TimesheetStatistic.php | 23 ++++ src/Repository/TimesheetRepository.php | 9 ++ src/Voter/UserVoter.php | 14 +- templates/admin/user.html.twig | 2 +- templates/admin/user_delete.html.twig | 22 +++ .../Admin/ActivityControllerTest.php | 64 ++++++++- .../Admin/CustomerControllerTest.php | 126 ++++++++++++++++++ .../Admin/ProjectControllerTest.php | 72 +++++++++- tests/Controller/Admin/UserControllerTest.php | 22 ++- tests/Controller/ControllerBaseTest.php | 24 ++++ tests/Controller/ProfileControllerTest.php | 12 ++ tests/DataFixtures/ActivityFixtures.php | 88 ++++++++++++ tests/DataFixtures/CustomerFixtures.php | 89 +++++++++++++ tests/DataFixtures/ProjectFixtures.php | 88 ++++++++++++ tests/DataFixtures/TimesheetFixtures.php | 24 +++- translations/messages.de.xliff | 7 + translations/messages.en.xliff | 7 + var/docs/installation.md | 8 +- 29 files changed, 879 insertions(+), 49 deletions(-) create mode 100644 src/Migrations/Version20180730044139.php create mode 100644 templates/admin/user_delete.html.twig create mode 100644 tests/DataFixtures/ActivityFixtures.php create mode 100644 tests/DataFixtures/CustomerFixtures.php create mode 100644 tests/DataFixtures/ProjectFixtures.php diff --git a/README.md b/README.md index 8cee3f4c..a9a08501 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ Our roadmap is open for changes and input from the community, please [sent us](i There are [further infos about installation](var/docs/installation.md) if you have to use FTP or want to develop with Kimai. -If you want to install Kimai v2 in your production environment, then SSH into your server and change to your webserevr root. +If you want to install Kimai 2 in your production environment, then SSH into your server and change to your webserver root. You need to install Git and [Composer](https://getcomposer.org/doc/00-intro.md) if you haven't already. First clone this repo: diff --git a/UPGRADING.md b/UPGRADING.md index 1b95000c..05d05b5c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,5 +1,10 @@ # Upgrading Kimai 2 +Database upgrades are currently ONLY provided for MySQL/MariaDB and SQLite. + +If you plan on using e.g. PostgreSQL, please read more about the `bin/console doctrine:migrations:diff` and +`bin/console doctrine:migrations:migrate` commands and contact us, so we can integrate them into the official releases. + ## [0.3](https://github.com/kevinpapst/kimai2/releases/tag/0.3) (2018-07-22) **Update from 0.2:** diff --git a/src/Controller/Admin/ActivityController.php b/src/Controller/Admin/ActivityController.php index 108c729f..8afc5902 100644 --- a/src/Controller/Admin/ActivityController.php +++ b/src/Controller/Admin/ActivityController.php @@ -122,7 +122,7 @@ class ActivityController extends AbstractController $this->flashSuccess('action.deleted_successfully'); - return $this->redirectToRoute('admin_activity', ['id' => $activity->getId()]); + return $this->redirectToRoute('admin_activity'); } return $this->render( @@ -160,7 +160,7 @@ class ActivityController extends AbstractController $editForm->get('create_more')->setData(true); $activity = $newActivity; } else { - return $this->redirectToRoute('admin_activity', ['id' => $activity->getId()]); + return $this->redirectToRoute('admin_activity'); } } diff --git a/src/Controller/Admin/CustomerController.php b/src/Controller/Admin/CustomerController.php index b5b62298..c3931695 100644 --- a/src/Controller/Admin/CustomerController.php +++ b/src/Controller/Admin/CustomerController.php @@ -101,7 +101,7 @@ class CustomerController extends AbstractController $this->flashSuccess('action.updated_successfully'); - return $this->redirectToRoute('admin_customer', ['id' => $customer->getId()]); + return $this->redirectToRoute('admin_customer'); } return $this->render('admin/customer_edit.html.twig', [ @@ -139,7 +139,7 @@ class CustomerController extends AbstractController $this->flashSuccess('action.deleted_successfully'); - return $this->redirectToRoute('admin_customer', ['id' => $customer->getId()]); + return $this->redirectToRoute('admin_customer'); } return $this->render('admin/customer_delete.html.twig', [ diff --git a/src/Controller/Admin/ProjectController.php b/src/Controller/Admin/ProjectController.php index 673b8e64..6b272676 100644 --- a/src/Controller/Admin/ProjectController.php +++ b/src/Controller/Admin/ProjectController.php @@ -116,7 +116,7 @@ class ProjectController extends AbstractController $this->flashSuccess('action.deleted_successfully'); - return $this->redirectToRoute('admin_project', ['id' => $project->getId()]); + return $this->redirectToRoute('admin_project'); } return $this->render('admin/project_delete.html.twig', [ @@ -151,7 +151,7 @@ class ProjectController extends AbstractController $editForm->get('create_more')->setData(true); $project = $newProject; } else { - return $this->redirectToRoute('admin_project', ['id' => $project->getId()]); + return $this->redirectToRoute('admin_project'); } } diff --git a/src/Controller/Admin/UserController.php b/src/Controller/Admin/UserController.php index 4961150f..7403917f 100644 --- a/src/Controller/Admin/UserController.php +++ b/src/Controller/Admin/UserController.php @@ -10,6 +10,7 @@ namespace App\Controller\Admin; use App\Controller\AbstractController; +use App\Entity\Timesheet; use App\Entity\User; use App\Form\Toolbar\UserToolbarForm; use App\Form\UserCreateType; @@ -19,6 +20,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; /** * Controller used to manage users in the admin part of the site. @@ -29,6 +31,27 @@ use Symfony\Component\HttpFoundation\Request; */ class UserController extends AbstractController { + /** + * @var UserPasswordEncoderInterface + */ + protected $encoder; + + /** + * @param UserPasswordEncoderInterface $encoder + */ + public function __construct(UserPasswordEncoderInterface $encoder) + { + $this->encoder = $encoder; + } + + /** + * @return \App\Repository\UserRepository + */ + protected function getRepository() + { + return $this->getDoctrine()->getRepository(User::class); + } + /** * @Route("/", defaults={"page": 1}, name="admin_user") * @Route("/page/{page}", requirements={"page": "[1-9]\d*"}, name="admin_user_paginated") @@ -47,7 +70,7 @@ class UserController extends AbstractController } /* @var $entries Pagerfanta */ - $entries = $this->getDoctrine()->getRepository(User::class)->findByQuery($query); + $entries = $this->getRepository()->findByQuery($query); return $this->render('admin/user.html.twig', [ 'entries' => $entries, @@ -69,8 +92,7 @@ class UserController extends AbstractController $editForm->handleRequest($request); if ($editForm->isSubmitted() && $editForm->isValid()) { - $password = $this->get('security.password_encoder') - ->encodePassword($user, $user->getPlainPassword()); + $password = $this->encoder->encodePassword($user, $user->getPlainPassword()); $user->setPassword($password); $user->setEnabled(true); $user->setRoles([User::DEFAULT_ROLE]); @@ -99,6 +121,49 @@ class UserController extends AbstractController ); } + /** + * The route to delete an existing user. + * + * @Route("/{id}/delete", name="admin_user_delete") + * @Method({"GET", "POST"}) + * @Security("is_granted('delete', userToDelete)") + * + * @param User $userToDelete + * @param Request $request + * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response + * @throws \Doctrine\ORM\NonUniqueResultException + */ + public function deleteAction(User $userToDelete, Request $request) + { + $stats = $this->getDoctrine()->getRepository(Timesheet::class)->getUserStatistics($userToDelete); + + $deleteForm = $this->createFormBuilder() + ->setAction($this->generateUrl('admin_user_delete', ['id' => $userToDelete->getId()])) + ->setMethod('POST') + ->getForm(); + + $deleteForm->handleRequest($request); + + if (0 == $stats->getRecordsTotal() || ($deleteForm->isSubmitted() && $deleteForm->isValid())) { + $entityManager = $this->getDoctrine()->getManager(); + $entityManager->remove($userToDelete); + $entityManager->flush(); + + $this->flashSuccess('action.deleted_successfully'); + + return $this->redirectToRoute('admin_user'); + } + + return $this->render( + 'admin/user_delete.html.twig', + [ + 'user' => $userToDelete, + 'stats' => $stats, + 'form' => $deleteForm->createView(), + ] + ); + } + /** * @param UserQuery $query * @return \Symfony\Component\Form\FormInterface diff --git a/src/Controller/ProfileController.php b/src/Controller/ProfileController.php index 9ea2001a..4a9b1881 100644 --- a/src/Controller/ProfileController.php +++ b/src/Controller/ProfileController.php @@ -22,6 +22,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Component\Form\Form; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; /** * User profile controller @@ -31,6 +32,19 @@ use Symfony\Component\HttpFoundation\Request; */ class ProfileController extends AbstractController { + /** + * @var UserPasswordEncoderInterface + */ + protected $encoder; + + /** + * @param UserPasswordEncoderInterface $encoder + */ + public function __construct(UserPasswordEncoderInterface $encoder) + { + $this->encoder = $encoder; + } + /** * @Route("/{username}", name="user_profile") * @Method("GET") @@ -75,8 +89,7 @@ class ProfileController extends AbstractController $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - $password = $this->get('security.password_encoder') - ->encodePassword($profile, $profile->getPlainPassword()); + $password = $this->encoder->encodePassword($profile, $profile->getPlainPassword()); $profile->setPassword($password); $entityManager = $this->getDoctrine()->getManager(); @@ -147,11 +160,9 @@ class ProfileController extends AbstractController } } - foreach ($preferences as $preference) { - $preference->setUser($profile); - $entityManager->persist($preference); - $entityManager->flush(); - } + $profile->setPreferences($preferences); + $entityManager->persist($profile); + $entityManager->flush(); $this->flashSuccess('action.updated_successfully'); diff --git a/src/Entity/Activity.php b/src/Entity/Activity.php index 590bdf0e..5febc090 100644 --- a/src/Entity/Activity.php +++ b/src/Entity/Activity.php @@ -69,6 +69,14 @@ class Activity */ private $timesheets; + /** + * @return Timesheet[] + */ + public function getTimesheets(): array + { + return $this->timesheets; + } + /** * @return Project */ diff --git a/src/Entity/Timesheet.php b/src/Entity/Timesheet.php index 3b00499d..b2a1a424 100644 --- a/src/Entity/Timesheet.php +++ b/src/Entity/Timesheet.php @@ -64,7 +64,7 @@ class Timesheet * @var User * * @ORM\ManyToOne(targetEntity="App\Entity\User") - * @ORM\JoinColumn(name="user", referencedColumnName="id") + * @ORM\JoinColumn(name="user", referencedColumnName="id", onDelete="CASCADE") * @Assert\NotNull() */ private $user; diff --git a/src/Entity/User.php b/src/Entity/User.php index 52b2f448..23b30ba7 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -191,12 +191,13 @@ class User extends BaseUser implements UserInterface * @param UserPreference[]|Collection $preferences * @return User */ - public function setPreferences(array $preferences) + public function setPreferences($preferences) { - if (!($preferences instanceof Collection) && is_array($preferences)) { - $preferences = new ArrayCollection($preferences); + $this->preferences = new ArrayCollection(); + + foreach ($preferences as $preference) { + $this->addPreference($preference); } - $this->preferences = $preferences; return $this; } @@ -238,6 +239,7 @@ class User extends BaseUser implements UserInterface public function addPreference(UserPreference $preference) { $this->preferences->add($preference); + $preference->setUser($this); return $this; } diff --git a/src/Migrations/Version20180730044139.php b/src/Migrations/Version20180730044139.php new file mode 100644 index 00000000..bea885c6 --- /dev/null +++ b/src/Migrations/Version20180730044139.php @@ -0,0 +1,92 @@ +getPlatform(); + + if (!in_array($platform, ['sqlite', 'mysql'])) { + $this->abortIf(true, 'Unsupported database platform: ' . $platform); + } + + $timesheet = $this->getTableName('timesheet'); + $user = $this->getTableName('users'); + $activity = $this->getTableName('activities'); + + if ($platform === 'sqlite') { + $this->addSql('DROP INDEX IDX_4F60C6B181C06096'); + $this->addSql('DROP INDEX IDX_4F60C6B18D93D649'); + $this->addSql('CREATE TEMPORARY TABLE __temp__' . $timesheet . ' AS SELECT id, user, activity_id, start_time, end_time, duration, description, rate FROM ' . $timesheet); + $this->addSql('DROP TABLE ' . $timesheet); + $this->addSql('CREATE TABLE ' . $timesheet . ' (id INTEGER NOT NULL, user INTEGER DEFAULT NULL, activity_id INTEGER DEFAULT NULL, start_time DATETIME NOT NULL, end_time DATETIME DEFAULT NULL, duration INTEGER DEFAULT NULL, description CLOB DEFAULT NULL COLLATE BINARY, rate NUMERIC(10, 2) NOT NULL, PRIMARY KEY(id), CONSTRAINT FK_4F60C6B18D93D649 FOREIGN KEY (user) REFERENCES ' . $user . ' (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT FK_4F60C6B181C06096 FOREIGN KEY (activity_id) REFERENCES ' . $activity . ' (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE)'); + $this->addSql('INSERT INTO ' . $timesheet . ' (id, user, activity_id, start_time, end_time, duration, description, rate) SELECT id, user, activity_id, start_time, end_time, duration, description, rate FROM __temp__' . $timesheet); + $this->addSql('DROP TABLE __temp__' . $timesheet); + $this->addSql('CREATE INDEX IDX_4F60C6B181C06096 ON ' . $timesheet . ' (activity_id)'); + $this->addSql('CREATE INDEX IDX_4F60C6B18D93D649 ON ' . $timesheet . ' (user)'); + } else { + $this->addSql('ALTER TABLE ' . $timesheet . ' DROP FOREIGN KEY FK_4F60C6B18D93D649'); + $this->addSql('ALTER TABLE ' . $timesheet . ' ADD CONSTRAINT FK_4F60C6B18D93D649 FOREIGN KEY (user) REFERENCES ' . $user . ' (id) ON DELETE CASCADE'); + } + } + + /** + * @param Schema $schema + * @throws \Doctrine\DBAL\DBALException + * @throws \Doctrine\DBAL\Migrations\AbortMigrationException + */ + public function down(Schema $schema): void + { + $platform = $this->getPlatform(); + + if (!in_array($platform, ['sqlite', 'mysql'])) { + $this->abortIf(true, 'Unsupported database platform: ' . $platform); + } + + $timesheet = $this->getTableName('timesheet'); + $user = $this->getTableName('user'); + + if ($platform === 'sqlite') { + $this->addSql('DROP INDEX IDX_4F60C6B18D93D649'); + $this->addSql('DROP INDEX IDX_4F60C6B181C06096'); + $this->addSql('CREATE TEMPORARY TABLE __temp__' . $timesheet . ' AS SELECT id, user, activity_id, start_time, end_time, duration, description, rate FROM ' . $timesheet); + $this->addSql('DROP TABLE ' . $timesheet); + $this->addSql('CREATE TABLE ' . $timesheet . ' (id INTEGER NOT NULL, user INTEGER DEFAULT NULL, activity_id INTEGER DEFAULT NULL, start_time DATETIME NOT NULL, end_time DATETIME DEFAULT NULL, duration INTEGER DEFAULT NULL, description CLOB DEFAULT NULL, rate NUMERIC(10, 2) NOT NULL, PRIMARY KEY(id))'); + $this->addSql('INSERT INTO ' . $timesheet . ' (id, user, activity_id, start_time, end_time, duration, description, rate) SELECT id, user, activity_id, start_time, end_time, duration, description, rate FROM __temp__' . $timesheet); + $this->addSql('DROP TABLE __temp__' . $timesheet); + $this->addSql('CREATE INDEX IDX_4F60C6B18D93D649 ON ' . $timesheet . ' (user)'); + $this->addSql('CREATE INDEX IDX_4F60C6B181C06096 ON ' . $timesheet . ' (activity_id)'); + } else { + $this->addSql('ALTER TABLE ' . $timesheet . ' DROP FOREIGN KEY FK_4F60C6B18D93D649'); + $this->addSql('ALTER TABLE ' . $timesheet . ' ADD CONSTRAINT FK_4F60C6B18D93D649 FOREIGN KEY (user) REFERENCES ' . $user . ' (id)'); + } + } +} diff --git a/src/Model/TimesheetStatistic.php b/src/Model/TimesheetStatistic.php index d3defd52..62ebbc9f 100644 --- a/src/Model/TimesheetStatistic.php +++ b/src/Model/TimesheetStatistic.php @@ -36,6 +36,10 @@ class TimesheetStatistic * @var \DateTime */ protected $firstEntry; + /** + * @var int + */ + protected $recordsTotal = 0; /** * @return int @@ -116,4 +120,23 @@ class TimesheetStatistic { $this->firstEntry = $firstEntry; } + + /** + * @return int + */ + public function getRecordsTotal(): int + { + return $this->recordsTotal; + } + + /** + * @param int $recordsTotal + * @return TimesheetStatistic + */ + public function setRecordsTotal(int $recordsTotal) + { + $this->recordsTotal = $recordsTotal; + + return $this; + } } diff --git a/src/Repository/TimesheetRepository.php b/src/Repository/TimesheetRepository.php index bb5301dd..bcb8ba23 100644 --- a/src/Repository/TimesheetRepository.php +++ b/src/Repository/TimesheetRepository.php @@ -121,6 +121,10 @@ class TimesheetRepository extends AbstractRepository ->createQuery('SELECT SUM(t.duration) FROM ' . Timesheet::class . ' t WHERE t.user = :user') ->setParameter('user', $user) ->getSingleScalarResult(); + $recordsTotal = $this->getEntityManager() + ->createQuery('SELECT COUNT(t.id) FROM ' . Timesheet::class . ' t WHERE t.user = :user') + ->setParameter('user', $user) + ->getSingleScalarResult(); $rateTotal = $this->getEntityManager() ->createQuery('SELECT SUM(t.rate) FROM ' . Timesheet::class . ' t WHERE t.user = :user') ->setParameter('user', $user) @@ -142,6 +146,7 @@ class TimesheetRepository extends AbstractRepository $stats->setAmountThisMonth($amountMonth); $stats->setDurationThisMonth($durationMonth); $stats->setFirstEntry(new DateTime($firstEntry)); + $stats->setRecordsTotal($recordsTotal); return $stats; } @@ -203,6 +208,9 @@ class TimesheetRepository extends AbstractRepository $durationTotal = $this->getEntityManager() ->createQuery('SELECT SUM(t.duration) FROM ' . Timesheet::class . ' t') ->getSingleScalarResult(); + $recordsTotal = $this->getEntityManager() + ->createQuery('SELECT COUNT(t.id) FROM ' . Timesheet::class . ' t') + ->getSingleScalarResult(); $rateTotal = $this->getEntityManager() ->createQuery('SELECT SUM(t.rate) FROM ' . Timesheet::class . ' t') ->getSingleScalarResult(); @@ -228,6 +236,7 @@ class TimesheetRepository extends AbstractRepository $stats->setActiveThisMonth($activeMonth); $stats->setAmountThisMonth($amountMonth); $stats->setDurationThisMonth($durationMonth); + $stats->setRecordsTotal($recordsTotal); return $stats; } diff --git a/src/Voter/UserVoter.php b/src/Voter/UserVoter.php index 48f1141a..0d595cfe 100644 --- a/src/Voter/UserVoter.php +++ b/src/Voter/UserVoter.php @@ -93,7 +93,7 @@ class UserVoter extends AbstractVoter */ protected function canEditPreferences(User $profile, User $user, TokenInterface $token) { - return $profile->getId() == $user->getId(); + return $profile->getId() === $user->getId(); } /** @@ -107,7 +107,7 @@ class UserVoter extends AbstractVoter return true; } - return $profile->getId() == $user->getId(); + return $profile->getId() === $user->getId(); } /** @@ -121,7 +121,7 @@ class UserVoter extends AbstractVoter return true; } - return $profile->getId() == $user->getId(); + return $profile->getId() === $user->getId(); } /** @@ -131,7 +131,11 @@ class UserVoter extends AbstractVoter */ protected function canDelete(User $profile, User $user, TokenInterface $token) { - return false; + if (!$this->canAdminUsers($token)) { + return false; + } + + return $profile->getId() !== $user->getId(); } /** @@ -140,6 +144,6 @@ class UserVoter extends AbstractVoter */ protected function canAdminUsers(TokenInterface $token) { - return $this->isFullyAuthenticated($token) && $this->hasRole('ROLE_SUPER_ADMIN', $token); + return $this->isFullyAuthenticated($token) && $this->hasRole(User::ROLE_SUPER_ADMIN, $token); } } diff --git a/templates/admin/user.html.twig b/templates/admin/user.html.twig index b2a2cf80..4be20fcc 100644 --- a/templates/admin/user.html.twig +++ b/templates/admin/user.html.twig @@ -50,7 +50,7 @@ {% set actionButtons = {'edit': path('user_profile', {'username' : entry.username})}|merge(actionButtons) %} {% endif %} {% if is_granted('delete', entry) %} - {% set actionButtons = actionButtons|merge({'trash': '#'}) %} + {% set actionButtons = actionButtons|merge({'trash': path('admin_user_delete', {'id': entry.id})}) %} {% endif %} {{ widgets.button_group(actionButtons) }} diff --git a/templates/admin/user_delete.html.twig b/templates/admin/user_delete.html.twig new file mode 100644 index 00000000..d440a54f --- /dev/null +++ b/templates/admin/user_delete.html.twig @@ -0,0 +1,22 @@ +{% extends 'base.html.twig' %} +{% import "macros/widgets.html.twig" as widgets %} +{% import "macros/datatables.html.twig" as tables %} + +{% block page_title %}{{ 'admin_user.title'|trans }}{% endblock %} +{% block page_subtitle %}{{ 'admin_user.subtitle'|trans }}{% endblock %} + +{% block main %} + + {% set params = { + '%user%': '' ~ widgets.username(user) ~ '', + '%records%': '' ~ stats.recordsTotal ~ '', + '%duration%': '' ~ stats.durationTotal|duration ~ '' + } %} + + {{ include('default/_form_delete.html.twig', { + 'message': "admin_user.delete_confirm"|trans(params)|raw, + 'form': form, + 'back': path('admin_activity') + }) }} + +{% endblock %} diff --git a/tests/Controller/Admin/ActivityControllerTest.php b/tests/Controller/Admin/ActivityControllerTest.php index 1bf6073b..9497c244 100644 --- a/tests/Controller/Admin/ActivityControllerTest.php +++ b/tests/Controller/Admin/ActivityControllerTest.php @@ -11,6 +11,8 @@ namespace App\Tests\Controller\Admin; use App\Entity\User; use App\Tests\Controller\ControllerBaseTest; +use App\Tests\DataFixtures\ProjectFixtures; +use App\Tests\DataFixtures\TimesheetFixtures; /** * @coversDefaultClass \App\Controller\Admin\ActivityController @@ -44,7 +46,7 @@ class ActivityControllerTest extends ControllerBaseTest 'name' => 'Test 2', ] ]); - $this->assertTrue($client->getResponse()->isRedirect()); + $this->assertIsRedirect($client, $this->createUrl('/admin/activity/')); $client->followRedirect(); $this->assertHasDataTable($client); } @@ -52,14 +54,26 @@ class ActivityControllerTest extends ControllerBaseTest public function testCreateActionWithCreateMore() { $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new ProjectFixtures(); + $fixture->setAmount(10); + $this->importFixture($em, $fixture); + $this->assertAccessIsGranted($client, '/admin/activity/create'); $form = $client->getCrawler()->filter('form[name=activity_edit_form]')->form(); $this->assertTrue($form->has('activity_edit_form[create_more]')); + + /** @var \Symfony\Component\DomCrawler\Field\ChoiceFormField $project */ + $project = $form->get('activity_edit_form[project]'); + $options = $project->availableOptionValues(); + $selectedProject = $options[array_rand($options)]; + $client->submit($form, [ 'activity_edit_form' => [ 'name' => 'Test create more', 'create_more' => true, - // TODO select random project + 'project' => $selectedProject, ] ]); $this->assertFalse($client->getResponse()->isRedirect()); @@ -67,7 +81,7 @@ class ActivityControllerTest extends ControllerBaseTest $form = $client->getCrawler()->filter('form[name=activity_edit_form]')->form(); $this->assertTrue($form->has('activity_edit_form[create_more]')); $this->assertEquals(1, $form->get('activity_edit_form[create_more]')->getValue()); - // TODO test that project is pre-selected + $this->assertEquals($selectedProject, $form->get('activity_edit_form[project]')->getValue()); } public function testEditAction() @@ -80,7 +94,7 @@ class ActivityControllerTest extends ControllerBaseTest $client->submit($form, [ 'activity_edit_form' => ['name' => 'Test 2'] ]); - $this->assertTrue($client->getResponse()->isRedirect()); + $this->assertIsRedirect($client, $this->createUrl('/admin/activity/')); $client->followRedirect(); $this->assertHasDataTable($client); $this->request($client, '/admin/activity/1/edit'); @@ -88,6 +102,48 @@ class ActivityControllerTest extends ControllerBaseTest $this->assertEquals('Test 2', $editForm->get('activity_edit_form[name]')->getValue()); } + public function testDeleteAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->request($client, '/admin/activity/1/edit'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $this->request($client, '/admin/activity/1/delete'); + $this->assertIsRedirect($client, $this->createUrl('/admin/activity/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/activity/1/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + + public function testDeleteActionWithTimesheetEntries() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new TimesheetFixtures(); + $fixture->setUser($this->getUserByRole($em, User::ROLE_USER)); + $fixture->setAmount(10); + $this->importFixture($em, $fixture); + + $this->request($client, '/admin/activity/1/delete'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=form]')->form(); + $this->assertStringEndsWith($this->createUrl('/admin/activity/1/delete'), $form->getUri()); + $client->submit($form); + + $this->assertIsRedirect($client, $this->createUrl('/admin/activity/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/activity/1/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + /** * @dataProvider getValidationTestData */ diff --git a/tests/Controller/Admin/CustomerControllerTest.php b/tests/Controller/Admin/CustomerControllerTest.php index 2fc6ac25..5228e47c 100644 --- a/tests/Controller/Admin/CustomerControllerTest.php +++ b/tests/Controller/Admin/CustomerControllerTest.php @@ -11,10 +11,13 @@ namespace App\Tests\Controller\Admin; use App\Entity\User; use App\Tests\Controller\ControllerBaseTest; +use App\Tests\DataFixtures\CustomerFixtures; +use App\Tests\DataFixtures\TimesheetFixtures; /** * @coversDefaultClass \App\Controller\Admin\CustomerController * @group integration + * @group legacy */ class CustomerControllerTest extends ControllerBaseTest { @@ -30,4 +33,127 @@ class CustomerControllerTest extends ControllerBaseTest $this->assertAccessIsGranted($client, '/admin/customer/'); $this->assertHasDataTable($client); } + + public function testCreateAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->assertAccessIsGranted($client, '/admin/customer/create'); + $form = $client->getCrawler()->filter('form[name=customer_edit_form]')->form(); + $client->submit($form, [ + 'customer_edit_form' => [ + 'name' => 'Test Customer', + ] + ]); + $this->assertIsRedirect($client, $this->createUrl('/admin/customer/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + } + + public function testEditAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->assertAccessIsGranted($client, '/admin/customer/1/edit'); + $form = $client->getCrawler()->filter('form[name=customer_edit_form]')->form(); + $this->assertFalse($form->has('customer_edit_form[create_more]')); + $this->assertEquals('Test', $form->get('customer_edit_form[name]')->getValue()); + $client->submit($form, [ + 'customer_edit_form' => [ + 'name' => 'Test Customer 2' + ] + ]); + $this->assertIsRedirect($client, $this->createUrl('/admin/customer/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->request($client, '/admin/customer/1/edit'); + $editForm = $client->getCrawler()->filter('form[name=customer_edit_form]')->form(); + $this->assertEquals('Test Customer 2', $editForm->get('customer_edit_form[name]')->getValue()); + } + + public function testDeleteAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new CustomerFixtures(); + $fixture->setAmount(1); + $this->importFixture($em, $fixture); + + $this->request($client, '/admin/customer/2/edit'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $this->request($client, '/admin/customer/2/delete'); + $this->assertIsRedirect($client, $this->createUrl('/admin/customer/')); + + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/customer/2/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + + public function testDeleteActionWithTimesheetEntries() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new TimesheetFixtures(); + $fixture->setUser($this->getUserByRole($em, User::ROLE_USER)); + $fixture->setAmount(10); + $this->importFixture($em, $fixture); + + $this->request($client, '/admin/customer/1/delete'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=form]')->form(); + $this->assertStringEndsWith($this->createUrl('/admin/customer/1/delete'), $form->getUri()); + $client->submit($form); + + $this->assertIsRedirect($client, $this->createUrl('/admin/customer/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/customer/1/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + + /** + * @dataProvider getValidationTestData + */ + public function testValidationForCreateAction(array $formData, array $validationFields) + { + $this->assertFormHasValidationError( + User::ROLE_ADMIN, + '/admin/customer/create', + 'form[name=customer_edit_form]', + $formData, + $validationFields + ); + } + + public function getValidationTestData() + { + return [ + [ + [ + 'customer_edit_form' => [ + 'name' => '', + 'visible' => 3, + 'country' => '00', // TODO why does it not fail? + 'currency' => '00', // TODO why does it not fail? + 'timezone' => 'XXX' + ] + ], + [ + '#customer_edit_form_name', + '#customer_edit_form_visible', + //'#customer_edit_form_country', + //'#customer_edit_form_currency', + '#customer_edit_form_timezone', + ] + ], + ]; + } } diff --git a/tests/Controller/Admin/ProjectControllerTest.php b/tests/Controller/Admin/ProjectControllerTest.php index b8c325a9..3e93e008 100644 --- a/tests/Controller/Admin/ProjectControllerTest.php +++ b/tests/Controller/Admin/ProjectControllerTest.php @@ -11,6 +11,9 @@ namespace App\Tests\Controller\Admin; use App\Entity\User; use App\Tests\Controller\ControllerBaseTest; +use App\Tests\DataFixtures\CustomerFixtures; +use App\Tests\DataFixtures\ProjectFixtures; +use App\Tests\DataFixtures\TimesheetFixtures; /** * @coversDefaultClass \App\Controller\Admin\ProjectController @@ -44,22 +47,35 @@ class ProjectControllerTest extends ControllerBaseTest 'name' => 'Test 2', ] ]); - $this->assertTrue($client->getResponse()->isRedirect()); + $this->assertIsRedirect($client, $this->createUrl('/admin/project/')); $client->followRedirect(); $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); } public function testCreateActionWithCreateMore() { $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new CustomerFixtures(); + $fixture->setAmount(10); + $this->importFixture($em, $fixture); + $this->assertAccessIsGranted($client, '/admin/project/create'); $form = $client->getCrawler()->filter('form[name=project_edit_form]')->form(); $this->assertTrue($form->has('project_edit_form[create_more]')); + + /** @var \Symfony\Component\DomCrawler\Field\ChoiceFormField $customer */ + $customer = $form->get('project_edit_form[customer]'); + $options = $customer->availableOptionValues(); + $selectedCustomer = $options[array_rand($options)]; + $client->submit($form, [ 'project_edit_form' => [ 'name' => 'Test create more', 'create_more' => true, - // TODO select random customer + 'customer' => $selectedCustomer ] ]); $this->assertFalse($client->getResponse()->isRedirect()); @@ -67,7 +83,7 @@ class ProjectControllerTest extends ControllerBaseTest $form = $client->getCrawler()->filter('form[name=project_edit_form]')->form(); $this->assertTrue($form->has('project_edit_form[create_more]')); $this->assertEquals(1, $form->get('project_edit_form[create_more]')->getValue()); - // TODO test that customer is pre-selected + $this->assertEquals($selectedCustomer, $form->get('project_edit_form[customer]')->getValue()); } public function testEditAction() @@ -80,7 +96,7 @@ class ProjectControllerTest extends ControllerBaseTest $client->submit($form, [ 'project_edit_form' => ['name' => 'Test 2'] ]); - $this->assertTrue($client->getResponse()->isRedirect()); + $this->assertIsRedirect($client, $this->createUrl('/admin/project/')); $client->followRedirect(); $this->assertHasDataTable($client); $this->request($client, '/admin/project/1/edit'); @@ -88,6 +104,54 @@ class ProjectControllerTest extends ControllerBaseTest $this->assertEquals('Test 2', $editForm->get('project_edit_form[name]')->getValue()); } + public function testDeleteAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new ProjectFixtures(); + $fixture->setAmount(1); + $this->importFixture($em, $fixture); + + $this->request($client, '/admin/project/2/edit'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $this->request($client, '/admin/project/2/delete'); + $this->assertIsRedirect($client, $this->createUrl('/admin/project/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/project/2/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + + public function testDeleteActionWithTimesheetEntries() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + $fixture = new TimesheetFixtures(); + $fixture->setUser($this->getUserByRole($em, User::ROLE_USER)); + $fixture->setAmount(10); + $this->importFixture($em, $fixture); + + $this->request($client, '/admin/project/1/delete'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=form]')->form(); + $this->assertStringEndsWith($this->createUrl('/admin/project/1/delete'), $form->getUri()); + $client->submit($form); + + $this->assertIsRedirect($client, $this->createUrl('/admin/project/')); + $client->followRedirect(); + $this->assertHasDataTable($client); + $this->assertHasFlashSuccess($client); + + $this->request($client, '/admin/project/1/edit'); + $this->assertFalse($client->getResponse()->isSuccessful()); + } + /** * @dataProvider getValidationTestData */ diff --git a/tests/Controller/Admin/UserControllerTest.php b/tests/Controller/Admin/UserControllerTest.php index 532fc9af..08d52be0 100644 --- a/tests/Controller/Admin/UserControllerTest.php +++ b/tests/Controller/Admin/UserControllerTest.php @@ -34,6 +34,7 @@ class UserControllerTest extends ControllerBaseTest public function testCreateAction() { + $username = '亚历山德拉'; $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN); $this->assertAccessIsGranted($client, '/admin/user/create'); $form = $client->getCrawler()->filter('form[name=user_create]')->form(); @@ -41,15 +42,30 @@ class UserControllerTest extends ControllerBaseTest $this->assertNull($form->get('user_create[create_more]')->getValue()); $client->submit($form, [ 'user_create' => [ - 'username' => 'foobar@example.com', + 'username' => $username, + 'alias' => $username, 'plainPassword' => ['first' => 'abcdef', 'second' => 'abcdef'], 'email' => 'foobar@example.com', 'enabled' => 1, ] ]); - $this->assertTrue($client->getResponse()->isRedirect($this->createUrl('/profile/foobar@example.com/edit'))); + $this->assertIsRedirect($client, $this->createUrl('/profile/' . urlencode($username) . '/edit')); $client->followRedirect(); - // TODO test that this is the users profile + + $tabs = $client->getCrawler()->filter('div.nav-tabs-custom ul.nav-tabs li'); + $this->assertEquals(4, $tabs->count()); + $expectedTabs = ['#charts', '#settings', '#password', '#roles']; + $foundTabs = []; + foreach ($tabs->filter('a') as $tab) { + $name = $tab->getAttribute('href'); + if (in_array($name, $expectedTabs)) { + $foundTabs[] = $name; + } + } + $this->assertEmpty(array_diff($expectedTabs, $foundTabs)); + + $form = $client->getCrawler()->filter('form[name=user_edit]')->form(); + $this->assertEquals($username, $form->get('user_edit[alias]')->getValue()); } public function testCreateActionWithCreateMore() diff --git a/tests/Controller/ControllerBaseTest.php b/tests/Controller/ControllerBaseTest.php index 700c1bc1..3fc80771 100644 --- a/tests/Controller/ControllerBaseTest.php +++ b/tests/Controller/ControllerBaseTest.php @@ -258,4 +258,28 @@ abstract class ControllerBaseTest extends WebTestCase return $em->getRepository(User::class)->findOneBy(['username' => $name]); } + + /** + * @param Client $client + */ + protected function assertHasFlashSuccess(Client $client) + { + $node = $client->getCrawler()->filter('div.alert.alert-success.alert-dismissible'); + $this->assertNotEmpty($node->text()); + } + + /** + * @param Client $client + * @param string $url + */ + protected function assertIsRedirect(Client $client, $url = null) + { + $this->assertTrue($client->getResponse()->isRedirect()); + if (null === $url) { + return; + } + + $this->assertTrue($client->getResponse()->headers->has('Location')); + $this->assertStringEndsWith($url, $client->getResponse()->headers->get('Location')); + } } diff --git a/tests/Controller/ProfileControllerTest.php b/tests/Controller/ProfileControllerTest.php index 5a9e5cd8..b50f47db 100644 --- a/tests/Controller/ProfileControllerTest.php +++ b/tests/Controller/ProfileControllerTest.php @@ -27,6 +27,18 @@ class ProfileControllerTest extends ControllerBaseTest $client = $this->getClientForAuthenticatedUser(); $this->request($client, '/profile/' . UserFixtures::USERNAME_USER); $this->assertTrue($client->getResponse()->isSuccessful()); + + $tabs = $client->getCrawler()->filter('div.nav-tabs-custom ul.nav-tabs li'); + $this->assertEquals(4, $tabs->count()); + $expectedTabs = ['#charts', '#settings', '#password', '#preferences']; + $foundTabs = []; + foreach ($tabs->filter('a') as $tab) { + $name = $tab->getAttribute('href'); + if (in_array($name, $expectedTabs)) { + $foundTabs[] = $name; + } + } + $this->assertEmpty(array_diff($expectedTabs, $foundTabs)); } public function testIndexActionWithDifferentUsername() diff --git a/tests/DataFixtures/ActivityFixtures.php b/tests/DataFixtures/ActivityFixtures.php new file mode 100644 index 00000000..f49884b5 --- /dev/null +++ b/tests/DataFixtures/ActivityFixtures.php @@ -0,0 +1,88 @@ +amount; + } + + /** + * @param int $amount + * @return ActivityFixtures + */ + public function setAmount(int $amount) + { + $this->amount = $amount; + + return $this; + } + + /** + * {@inheritdoc} + */ + public function load(ObjectManager $manager) + { + $projects = $this->getAllProjects($manager); + $faker = Factory::create(); + + // random amount of timesheet entries for every user + for ($i = 0; $i < $this->amount; $i++) { + $visible = 0 != $i % 3; + $entity = new Activity(); + $entity + ->setProject($projects[array_rand($projects)]) + ->setName($faker->bs . ($visible ? '' : ' (x)')) + ->setComment($faker->text) + ->setVisible($visible) + ; + + $manager->persist($entity); + } + + $manager->flush(); + } + + /** + * @param ObjectManager $manager + * @return Project[] + */ + protected function getAllProjects(ObjectManager $manager) + { + $all = []; + /* @var User[] $entries */ + $entries = $manager->getRepository(Project::class)->findAll(); + foreach ($entries as $temp) { + $all[$temp->getId()] = $temp; + } + + return $all; + } +} diff --git a/tests/DataFixtures/CustomerFixtures.php b/tests/DataFixtures/CustomerFixtures.php new file mode 100644 index 00000000..090baa80 --- /dev/null +++ b/tests/DataFixtures/CustomerFixtures.php @@ -0,0 +1,89 @@ +amount; + } + + /** + * @param int $amount + * @return CustomerFixtures + */ + public function setAmount(int $amount) + { + $this->amount = $amount; + + return $this; + } + + /** + * {@inheritdoc} + */ + public function load(ObjectManager $manager) + { + $faker = Factory::create(); + + for ($i = 0; $i < $this->amount; $i++) { + $visible = 0 != $i % 3; + $entity = new Customer(); + $entity + ->setCurrency($faker->currencyCode) + ->setName($faker->company . ($visible ? '' : ' (x)')) + ->setAddress($faker->address) + ->setComment($faker->text) + ->setNumber('C-' . $faker->ean8) + ->setCountry($faker->countryCode) + ->setTimezone($faker->timezone) + ->setVisible($visible) + ; + + $manager->persist($entity); + } + + $manager->flush(); + } + + /** + * @param ObjectManager $manager + * @return Customer[] + */ + protected function getAllCustomers(ObjectManager $manager) + { + $all = []; + /* @var User[] $entries */ + $entries = $manager->getRepository(Customer::class)->findAll(); + foreach ($entries as $temp) { + $all[$temp->getId()] = $temp; + } + + return $all; + } +} diff --git a/tests/DataFixtures/ProjectFixtures.php b/tests/DataFixtures/ProjectFixtures.php new file mode 100644 index 00000000..a18cbd8c --- /dev/null +++ b/tests/DataFixtures/ProjectFixtures.php @@ -0,0 +1,88 @@ +amount; + } + + /** + * @param int $amount + * @return ProjectFixtures + */ + public function setAmount(int $amount) + { + $this->amount = $amount; + + return $this; + } + + /** + * {@inheritdoc} + */ + public function load(ObjectManager $manager) + { + $customers = $this->getAllCustomers($manager); + $faker = Factory::create(); + + for ($i = 0; $i < $this->amount; $i++) { + $visible = 0 != $i % 3; + $entity = new Project(); + $entity + ->setName($faker->catchPhrase . ($visible ? '' : ' (x)')) + ->setBudget(rand(0, 10000)) + ->setComment($faker->text) + ->setCustomer($customers[array_rand($customers)]) + ->setVisible($visible) + ; + + $manager->persist($entity); + } + + $manager->flush(); + } + + /** + * @param ObjectManager $manager + * @return Customer[] + */ + protected function getAllCustomers(ObjectManager $manager) + { + $all = []; + /* @var User[] $entries */ + $entries = $manager->getRepository(Customer::class)->findAll(); + foreach ($entries as $temp) { + $all[$temp->getId()] = $temp; + } + + return $all; + } +} diff --git a/tests/DataFixtures/TimesheetFixtures.php b/tests/DataFixtures/TimesheetFixtures.php index 31d70a89..5eb3afdd 100644 --- a/tests/DataFixtures/TimesheetFixtures.php +++ b/tests/DataFixtures/TimesheetFixtures.php @@ -23,20 +23,21 @@ use Faker\Factory; class TimesheetFixtures extends Fixture { /** - * @var + * @var User */ protected $user; - /** * @var int */ protected $amount = 0; - /** * @var int */ protected $running = 0; - + /** + * @var Activity[] + */ + protected $activities = []; /** * @var string */ @@ -74,16 +75,27 @@ class TimesheetFixtures extends Fixture $this->user = $user; } + /** + * @param Activity[] $activities + */ + public function setActivities(array $activities) + { + $this->activities = $activities; + } + /** * {@inheritdoc} */ public function load(ObjectManager $manager) { - $activities = $this->getAllActivities($manager); + $activities = $this->activities; + if (empty($activities)) { + $activities = $this->getAllActivities($manager); + } + $faker = Factory::create(); $user = $this->user; - // random amount of timesheet entries for every user for ($i = 0; $i < $this->amount; $i++) { $entry = $this->createTimesheetEntry( $user, diff --git a/translations/messages.de.xliff b/translations/messages.de.xliff index afb7476b..e2b57919 100644 --- a/translations/messages.de.xliff +++ b/translations/messages.de.xliff @@ -531,6 +531,13 @@ label.roles Rolle + + admin_user.delete_confirm + + Momentan existieren für den Benutzer %user% insgesamt %records% Zeiteinträge, welche sich auf eine Gesamtdauer von %duration% belaufen. + Diese Zeiteinträge werden ebenfalls mit gelöscht! + +