Refactored Admin Page controllers a bit...

src/Controller/AdminPages/AttachmentTypeController.php

@@ -44,6 +44,7 @@ namespace App\Controller\AdminPages;
 
 use App\Entity\Attachments\AttachmentType;
 use App\Entity\Attachments\AttachmentTypeAttachment;
+use App\Entity\Base\AbstractNamedDBElement;
 use App\Entity\Parameters\AttachmentTypeParameter;
 use App\Form\AdminPages\AttachmentTypeAdminForm;
 use App\Services\EntityExporter;
@@ -119,4 +120,15 @@ class AttachmentTypeController extends BaseAdminController
     {
         return $this->_exportEntity($entity, $exporter, $request);
     }
+
+    protected function deleteCheck(AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof AttachmentType) {
+            if ($entity->getAttachmentsForType()->count() > 0) {
+                $this->addFlash('error', 'entity.delete.must_not_contain_attachments');
+                return false;
+            }
+        }
+        return true;
+    }
 }

src/Controller/AdminPages/BaseAdminController.php

@@ -44,6 +44,7 @@ namespace App\Controller\AdminPages;
 
 use App\DataTables\LogDataTable;
 use App\Entity\Attachments\AttachmentType;
+use App\Entity\Base\AbstractDBElement;
 use App\Entity\Base\AbstractNamedDBElement;
 use App\Entity\Base\AbstractPartsContainingDBElement;
 use App\Entity\Base\AbstractStructuralDBElement;
@@ -139,11 +140,8 @@ abstract class BaseAdminController extends AbstractController
         $this->entityManager = $entityManager;
     }
 
-    protected function _edit(AbstractNamedDBElement $entity, Request $request, EntityManagerInterface $em, ?string $timestamp = null): Response
+    protected function revertElementIfNeeded(AbstractDBElement $entity, ?string $timestamp): ?\DateTime
     {
-        $this->denyAccessUnlessGranted('read', $entity);
-
-        $timeTravel_timestamp = null;
         if (null !== $timestamp) {
             $this->denyAccessUnlessGranted('@tools.timetravel');
             $this->denyAccessUnlessGranted('show_history', $entity);
@@ -155,7 +153,27 @@ abstract class BaseAdminController extends AbstractController
                 $timeTravel_timestamp = new \DateTime($timestamp);
             }
             $this->timeTravel->revertEntityToTimestamp($entity, $timeTravel_timestamp);
+
+            return $timeTravel_timestamp;
         }
+        return null;
+    }
+
+    /**
+     * Perform some additional actions, when the form was valid, but before the entity is saved.
+     * @return bool Return true, to save entity normally, return false, to abort saving.
+     */
+    protected function additionalActionEdit(FormInterface $form, AbstractNamedDBElement $entity): bool
+    {
+        return true;
+    }
+
+    protected function _edit(AbstractNamedDBElement $entity, Request $request, EntityManagerInterface $em, ?string $timestamp = null): Response
+    {
+        $this->denyAccessUnlessGranted('read', $entity);
+
+        $timeTravel_timestamp = $this->revertElementIfNeeded($entity, $timestamp);
+
 
         if ($this->isGranted('show_history', $entity)) {
             $table = $this->dataTableFactory->createFromType(
@@ -194,42 +212,39 @@ abstract class BaseAdminController extends AbstractController
 
         $form->handleRequest($request);
         if ($form->isSubmitted() && $form->isValid()) {
-            //Check if we editing a user and if we need to change the password of it
+            if ($this->additionalActionEdit($form, $entity)) {
-            if ($entity instanceof User && ! empty($form['new_password']->getData())) {
+                //Upload passed files
-                $password = $this->passwordEncoder->encodePassword($entity, $form['new_password']->getData());
+                $attachments = $form['attachments'];
-                $entity->setPassword($password);
+                foreach ($attachments as $attachment) {
-                //By default the user must change the password afterwards
+                    /** @var FormInterface $attachment */
-                $entity->setNeedPwChange(true);
+                    $options = [
+                        'secure_attachment' => $attachment['secureFile']->getData(),
+                        'download_url' => $attachment['downloadURL']->getData(),
+                    ];
 
-                $event = new SecurityEvent($entity);
+                    try {
-                $this->eventDispatcher->dispatch($event, SecurityEvents::PASSWORD_CHANGED);
+                        $this->attachmentSubmitHandler->handleFormSubmit(
-            }
+                            $attachment->getData(),
-
+                            $attachment['file']->getData(),
-            //Upload passed files
+                            $options
-            $attachments = $form['attachments'];
+                        );
-            foreach ($attachments as $attachment) {
+                    } catch (AttachmentDownloadException $attachmentDownloadException) {
-                /** @var FormInterface $attachment */
+                        $this->addFlash(
-                $options = [
+                            'error',
-                    'secure_attachment' => $attachment['secureFile']->getData(),
+                            $this->translator->trans(
-                    'download_url' => $attachment['downloadURL']->getData(),
+                                'attachment.download_failed'
-                ];
+                            ).' '.$attachmentDownloadException->getMessage()
-
+                        );
-                try {
+                    }
-                    $this->attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData(), $options);
-                } catch (AttachmentDownloadException $attachmentDownloadException) {
-                    $this->addFlash(
-                        'error',
-                        $this->translator->trans('attachment.download_failed').' '.$attachmentDownloadException->getMessage()
-                    );
                 }
+
+                $this->commentHelper->setMessage($form['log_comment']->getData());
+
+                $em->persist($entity);
+                $em->flush();
+                $this->addFlash('success', 'entity.edit_flash');
             }
 
-            $this->commentHelper->setMessage($form['log_comment']->getData());
-
-            $em->persist($entity);
-            $em->flush();
-            $this->addFlash('success', 'entity.edit_flash');
-
             //Rebuild form, so it is based on the updated data. Important for the parent field!
             //We can not use dynamic form events here, because the parent entity list is build from database!
             $form = $this->createForm($this->form_class, $entity, [
@@ -262,6 +277,15 @@ abstract class BaseAdminController extends AbstractController
         ]);
     }
 
+    /**
+     * Perform some additional actions, when the form was valid, but before the entity is saved.
+     * @return bool Return true, to save entity normally, return false, to abort saving.
+     */
+    protected function additionalActionNew(FormInterface $form, AbstractNamedDBElement $entity): bool
+    {
+        return true;
+    }
+
     protected function _new(Request $request, EntityManagerInterface $em, EntityImporter $importer, ?AbstractNamedDBElement $entity = null)
     {
         $master_picture_backup = null;
@@ -284,39 +308,42 @@ abstract class BaseAdminController extends AbstractController
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
-            if ($new_entity instanceof User && ! empty($form['new_password']->getData())) {
-                $password = $this->passwordEncoder->encodePassword($new_entity, $form['new_password']->getData());
-                $new_entity->setPassword($password);
-                //By default the user must change the password afterwards
-                $new_entity->setNeedPwChange(true);
-            }
 
-            //Upload passed files
+            //Perform additional actions
-            $attachments = $form['attachments'];
+            if ($this->additionalActionNew($form, $new_entity)) {
-            foreach ($attachments as $attachment) {
+                //Upload passed files
-                /** @var FormInterface $attachment */
+                $attachments = $form['attachments'];
-                $options = [
+                foreach ($attachments as $attachment) {
-                    'secure_attachment' => $attachment['secureFile']->getData(),
+                    /** @var FormInterface $attachment */
-                    'download_url' => $attachment['downloadURL']->getData(),
+                    $options = [
-                ];
+                        'secure_attachment' => $attachment['secureFile']->getData(),
+                        'download_url' => $attachment['downloadURL']->getData(),
+                    ];
 
-                try {
+                    try {
-                    $this->attachmentSubmitHandler->handleFormSubmit($attachment->getData(), $attachment['file']->getData(), $options);
+                        $this->attachmentSubmitHandler->handleFormSubmit(
-                } catch (AttachmentDownloadException $attachmentDownloadException) {
+                            $attachment->getData(),
-                    $this->addFlash(
+                            $attachment['file']->getData(),
-                        'error',
+                            $options
-                        $this->translator->trans('attachment.download_failed').' '.$attachmentDownloadException->getMessage()
+                        );
-                    );
+                    } catch (AttachmentDownloadException $attachmentDownloadException) {
+                        $this->addFlash(
+                            'error',
+                            $this->translator->trans(
+                                'attachment.download_failed'
+                            ).' '.$attachmentDownloadException->getMessage()
+                        );
+                    }
                 }
+
+                $this->commentHelper->setMessage($form['log_comment']->getData());
+
+                $em->persist($new_entity);
+                $em->flush();
+                $this->addFlash('success', 'entity.created_flash');
+
+                return $this->redirectToRoute($this->route_base.'_edit', ['id' => $new_entity->getID()]);
             }
-
-            $this->commentHelper->setMessage($form['log_comment']->getData());
-
-            $em->persist($new_entity);
-            $em->flush();
-            $this->addFlash('success', 'entity.created_flash');
-
-            return $this->redirectToRoute($this->route_base.'_edit', ['id' => $new_entity->getID()]);
         }
 
         if ($form->isSubmitted() && ! $form->isValid()) {
@@ -382,6 +409,24 @@ abstract class BaseAdminController extends AbstractController
         ]);
     }
 
+    /**
+     * Performs checks if the element can be deleted safely. Otherwise an flash message is added.
+     * @param  AbstractNamedDBElement  $entity The element that should be checked.
+     * @return bool True if the the element can be deleted, false if not
+     */
+    protected function deleteCheck(AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof AbstractPartsContainingDBElement) {
+            /** @var AbstractPartsContainingRepository $repo */
+            $repo = $this->entityManager->getRepository($this->entity_class);
+            if ($repo->getPartsCount($entity) > 0) {
+                $this->addFlash('error', 'entity.delete.must_not_contain_parts');
+                return false;
+            }
+        }
+        return true;
+    }
+
     protected function _delete(Request $request, AbstractNamedDBElement $entity, StructuralElementRecursionHelper $recursionHelper): RedirectResponse
     {
         $this->denyAccessUnlessGranted('delete', $entity);
@@ -389,33 +434,8 @@ abstract class BaseAdminController extends AbstractController
         if ($this->isCsrfTokenValid('delete'.$entity->getId(), $request->request->get('_token'))) {
             $entityManager = $this->getDoctrine()->getManager();
 
-            //Check if we can delete the part (it must not contain Parts)
+            if (!$this->deleteCheck($entity)) {
-            if ($entity instanceof AbstractPartsContainingDBElement) {
+                return $this->redirectToRoute($this->route_base.'_edit', ['id' => $entity->getID()]);
-                /** @var AbstractPartsContainingRepository $repo */
-                $repo = $this->entityManager->getRepository($this->entity_class);
-                if ($repo->getPartsCount($entity) > 0) {
-                    $this->addFlash('error', 'entity.delete.must_not_contain_parts');
-                    return $this->redirectToRoute($this->route_base.'_new');
-                }
-            } elseif ($entity instanceof AttachmentType) {
-                if ($entity->getAttachmentsForType()->count() > 0) {
-                    $this->addFlash('error', 'entity.delete.must_not_contain_attachments');
-                    return $this->redirectToRoute($this->route_base.'_new');
-                }
-            } elseif ($entity instanceof Currency) {
-                if ($entity->getPricedetails()->count() > 0) {
-                    $this->addFlash('error', 'entity.delete.must_not_contain_prices');
-                    return $this->redirectToRoute($this->route_base.'_new');
-                }
-            } elseif ($entity instanceof Group) {
-                if ($entity->getUsers()->count() > 0) {
-                    $this->addFlash('error', 'entity.delete.must_not_contain_users');
-                    return $this->redirectToRoute($this->route_base.'_new');
-                }
-            } elseif ($entity instanceof User) {
-                //TODO: Find a better solution
-                $this->addFlash('error', 'Currently it is not possible to delete a user, as this would break the log... This will be implemented later...');
-                return $this->redirectToRoute($this->route_base.'_new');
             }
 
             //Check if we need to remove recursively
@@ -452,18 +472,14 @@ abstract class BaseAdminController extends AbstractController
     protected function _exportAll(EntityManagerInterface $em, EntityExporter $exporter, Request $request): Response
     {
         $entity = new $this->entity_class();
-
         $this->denyAccessUnlessGranted('read', $entity);
-
         $entities = $em->getRepository($this->entity_class)->findAll();
-
         return $exporter->exportEntityFromRequest($entities, $request);
     }
 
     protected function _exportEntity(AbstractNamedDBElement $entity, EntityExporter $exporter, Request $request): \Symfony\Component\HttpFoundation\Response
     {
         $this->denyAccessUnlessGranted('read', $entity);
-
         return $exporter->exportEntityFromRequest($entity, $request);
     }
 }

src/Controller/AdminPages/CurrencyController.php

@@ -43,6 +43,7 @@ declare(strict_types=1);
 namespace App\Controller\AdminPages;
 
 use App\Entity\Attachments\CurrencyAttachment;
+use App\Entity\Base\AbstractNamedDBElement;
 use App\Entity\Parameters\CurrencyParameter;
 use App\Entity\PriceInformations\Currency;
 use App\Form\AdminPages\CurrencyAdminForm;
@@ -121,4 +122,16 @@ class CurrencyController extends BaseAdminController
     {
         return $this->_exportEntity($entity, $exporter, $request);
     }
+
+    public function deleteCheck(AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof Currency) {
+            if ($entity->getPricedetails()->count() > 0) {
+                $this->addFlash('error', 'entity.delete.must_not_contain_prices');
+                return false;
+            }
+        }
+
+        return true;
+    }
 }

src/Controller/GroupController.php

@@ -44,6 +44,7 @@ namespace App\Controller;
 
 use App\Controller\AdminPages\BaseAdminController;
 use App\Entity\Attachments\GroupAttachment;
+use App\Entity\Base\AbstractNamedDBElement;
 use App\Entity\Parameters\GroupParameter;
 use App\Entity\UserSystem\Group;
 use App\Form\AdminPages\GroupAdminForm;
@@ -120,4 +121,16 @@ class GroupController extends BaseAdminController
     {
         return $this->_exportEntity($entity, $exporter, $request);
     }
+
+    public function deleteCheck(AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof Group) {
+            if ($entity->getUsers()->count() > 0) {
+                $this->addFlash('error', 'entity.delete.must_not_contain_users');
+                return false;
+            }
+        }
+
+        return true;
+    }
 }

src/Controller/UserController.php

@@ -44,6 +44,7 @@ namespace App\Controller;
 
 use App\DataTables\LogDataTable;
 use App\Entity\Attachments\UserAttachment;
+use App\Entity\Base\AbstractNamedDBElement;
 use App\Entity\UserSystem\User;
 use App\Events\SecurityEvent;
 use App\Events\SecurityEvents;
@@ -56,6 +57,7 @@ use Doctrine\ORM\EntityManagerInterface;
 use InvalidArgumentException;
 use Omines\DataTablesBundle\DataTableFactory;
 use Symfony\Component\Asset\Packages;
+use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
@@ -74,6 +76,21 @@ class UserController extends AdminPages\BaseAdminController
     //Just define a value here to prevent error. It is not used.
     protected $parameter_class = 'not used';
 
+    protected function additionalActionEdit(FormInterface $form, AbstractNamedDBElement $entity): bool
+    {
+        //Check if we editing a user and if we need to change the password of it
+        if ($entity instanceof User && !empty($form['new_password']->getData())) {
+            $password = $this->passwordEncoder->encodePassword($entity, $form['new_password']->getData());
+            $entity->setPassword($password);
+            //By default the user must change the password afterwards
+            $entity->setNeedPwChange(true);
+
+            $event = new SecurityEvent($entity);
+            $this->eventDispatcher->dispatch($event, SecurityEvents::PASSWORD_CHANGED);
+        }
+        return true;
+    }
+
     /**
      * @Route("/{id}/edit/{timestamp}", requirements={"id"="\d+"}, name="user_edit")
      * @Route("/{id}/", requirements={"id"="\d+"})
@@ -113,6 +130,18 @@ class UserController extends AdminPages\BaseAdminController
         return $this->_edit($entity, $request, $em, $timestamp);
     }
 
+    protected function additionalActionNew(FormInterface $form, AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof User && ! empty($form['new_password']->getData())) {
+            $password = $this->passwordEncoder->encodePassword($entity, $form['new_password']->getData());
+            $entity->setPassword($password);
+            //By default the user must change the password afterwards
+            $entity->setNeedPwChange(true);
+        }
+
+        return true;
+    }
+
     /**
      * @Route("/new", name="user_new")
      * @Route("/{id}/clone", name="user_clone")
@@ -125,6 +154,17 @@ class UserController extends AdminPages\BaseAdminController
         return $this->_new($request, $em, $importer, $entity);
     }
 
+    protected function deleteCheck(AbstractNamedDBElement $entity): bool
+    {
+        if ($entity instanceof User) {
+            //TODO: Find a better solution
+            $this->addFlash('error', 'Currently it is not possible to delete a user, as this would break the log... This will be implemented later...');
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * @Route("/{id}", name="user_delete", methods={"DELETE"}, requirements={"id"="\d+"})
      *