From 68b61b7d8ab829df3e813fb56b048718002aa78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 1 Apr 2025 14:19:15 +0200 Subject: [PATCH] Refactor export-related controllers and helpers Removed unused dependencies, obsolete methods, and redundant parameters across ExportController, ExportFormHelper, and SavedExportController. Simplified session handling and aligned method signatures to improve maintainability and code readability. --- phpstan-baseline-level-4.neon | 5 - ...ssociatedEntityToStoredObjectInterface.php | 6 + .../Controller/ExportController.php | 233 +----------------- .../Controller/SavedExportController.php | 18 +- .../Export/ExportFormHelper.php | 9 +- 5 files changed, 22 insertions(+), 249 deletions(-) diff --git a/phpstan-baseline-level-4.neon b/phpstan-baseline-level-4.neon index 833a55afb..a2c1e2fad 100644 --- a/phpstan-baseline-level-4.neon +++ b/phpstan-baseline-level-4.neon @@ -1800,11 +1800,6 @@ parameters: count: 1 path: src/Bundle/ChillMainBundle/Controller/PostalCodeController.php - - - message: "#^Call to an undefined method Symfony\\\\Component\\\\HttpFoundation\\\\Session\\\\SessionInterface\\:\\:getFlashBag\\(\\)\\.$#" - count: 2 - path: src/Bundle/ChillMainBundle/Controller/SavedExportController.php - - message: "#^Method Chill\\\\MainBundle\\\\Controller\\\\ScopeController\\:\\:createCreateForm\\(\\) should return Symfony\\\\Component\\\\Form\\\\Form but returns Symfony\\\\Component\\\\Form\\\\FormInterface\\.$#" count: 1 diff --git a/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php b/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php index 81d230e67..e90164a2a 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/AssociatedEntityToStoredObjectInterface.php @@ -13,7 +13,13 @@ namespace Chill\DocStoreBundle\Repository; use Chill\DocStoreBundle\Entity\StoredObject; +/** + * @template T of object + */ interface AssociatedEntityToStoredObjectInterface { + /** + * @return T|null + */ public function findAssociatedEntityToStoredObject(StoredObject $storedObject): ?object; } diff --git a/src/Bundle/ChillMainBundle/Controller/ExportController.php b/src/Bundle/ChillMainBundle/Controller/ExportController.php index bd4a946d7..04c771365 100644 --- a/src/Bundle/ChillMainBundle/Controller/ExportController.php +++ b/src/Bundle/ChillMainBundle/Controller/ExportController.php @@ -20,14 +20,10 @@ use Chill\MainBundle\Export\ExportFormHelper; use Chill\MainBundle\Export\ExportInterface; use Chill\MainBundle\Export\ExportManager; use Chill\MainBundle\Export\Messenger\ExportRequestGenerationMessage; -use Chill\MainBundle\Form\SavedExportType; use Chill\MainBundle\Form\Type\Export\ExportType; use Chill\MainBundle\Form\Type\Export\FormatterType; use Chill\MainBundle\Form\Type\Export\PickCenterType; -use Chill\MainBundle\Messenger\Stamp\AuthenticationStamp; -use Chill\MainBundle\Redis\ChillRedis; use Chill\MainBundle\Repository\SavedExportOrExportGenerationRepository; -use Chill\MainBundle\Repository\SavedExportRepositoryInterface; use Chill\MainBundle\Security\Authorization\SavedExportVoter; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; @@ -42,12 +38,9 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; -use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; -use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Security\Core\Security; -use Symfony\Contracts\Translation\TranslatorInterface; /** * Class ExportController @@ -58,15 +51,12 @@ class ExportController extends AbstractController private readonly bool $filterStatsByCenters; public function __construct( - private readonly ChillRedis $redis, private readonly ExportManager $exportManager, private readonly FormFactoryInterface $formFactory, private readonly LoggerInterface $logger, private readonly SessionInterface $session, - private readonly TranslatorInterface $translator, private readonly EntityManagerInterface $entityManager, private readonly ExportFormHelper $exportFormHelper, - private readonly SavedExportRepositoryInterface $savedExportRepository, private readonly Security $security, ParameterBagInterface $parameterBag, private readonly MessageBusInterface $messageBus, @@ -77,107 +67,6 @@ class ExportController extends AbstractController $this->filterStatsByCenters = $parameterBag->get('chill_main')['acl']['filter_stats_by_center']; } - #[Route(path: '/{_locale}/exports/download/{alias}', name: 'chill_main_export_download', methods: ['GET'])] - public function downloadResultAction(Request $request, mixed $alias) - { - $user = $this->security->getUser(); - - if (!$user instanceof User) { - throw new UnauthorizedHttpException('Only regular users can generate exports'); - } - - /** @var ExportManager $exportManager */ - $exportManager = $this->exportManager; - $export = $exportManager->getExport($alias); - $key = $request->query->get('key', null); - $savedExport = $this->getSavedExportFromRequest($request); - - [$dataCenters, $dataExport, $dataFormatter] = $this->rebuildData($key, $savedExport); - - $config = [ - 'centers' => $dataCenters, - 'export' => $dataExport['export']['export'] ?? [], - 'filters' => $dataExport['export']['filters'] ?? [], - 'aggregators' => $dataExport['export']['aggregators'] ?? [], - 'formatter' => $dataFormatter, - ]; - - $generation = new ExportGeneration('alias', $config, $this->clock->now()->add(new \DateInterval('P6M'))); - $this->entityManager->persist($generation); - $this->entityManager->flush(); - - $this->messageBus->dispatch(new ExportRequestGenerationMessage($generation, $user)); - - $formatterAlias = $exportManager->getFormatterAlias($dataExport['export']); - - if (null !== $formatterAlias) { - $formater = $exportManager->getFormatter($formatterAlias); - } else { - $formater = null; - } - - $viewVariables = [ - 'alias' => $alias, - 'export' => $export, - 'export_group' => $this->getExportGroup($export), - 'saved_export' => $savedExport, - ]; - - if ($formater instanceof \Chill\MainBundle\Export\Formatter\CSVListFormatter) { - // due to a bug in php, we add the mime type in the download view - $viewVariables['mime_type'] = 'text/csv'; - } - - return $this->render('@ChillMain/Export/download.html.twig', $viewVariables); - } - - /** - * Generate a report. - * - * This action must work with GET queries. - * - * @param string $alias - */ - #[Route(path: '/{_locale}/exports/generate/{alias}', name: 'chill_main_export_generate', methods: ['GET'])] - public function generateAction(Request $request, $alias): Response - { - /** @var ExportManager $exportManager */ - $exportManager = $this->exportManager; - $key = $request->query->get('key', null); - $savedExport = $this->getSavedExportFromRequest($request); - - [$dataCenters, $dataExport, $dataFormatter] = $this->rebuildData($key, $savedExport); - - return $exportManager->generate( - $alias, - $dataCenters['centers'], - $dataExport['export'], - null !== $dataFormatter ? $dataFormatter['formatter'] : [] - ); - } - - /** - * @throws \RedisException - */ - #[Route(path: '/{_locale}/exports/generate-from-saved/{id}', name: 'chill_main_export_generate_from_saved')] - public function generateFromSavedExport(SavedExport $savedExport): Response - { - $this->denyAccessUnlessGranted(SavedExportVoter::GENERATE, $savedExport); - - $exportGeneration = ExportGeneration::fromSavedExport($savedExport, $this->clock->now()->add(new \DateInterval('P3M'))); - $this->entityManager->persist($exportGeneration); - $this->entityManager->flush(); - - $this->messageBus->dispatch( - new Envelope( - new ExportRequestGenerationMessage($exportGeneration), - [new AuthenticationStamp($this->security->getUser())] - ) - ); - - return new Response('Ok: '.$exportGeneration->getId()->toString()); - } - /** * handle the step to build a query for an export. * @@ -214,64 +103,6 @@ class ExportController extends AbstractController }; } - #[Route(path: '/{_locale}/export/saved/update-from-key/{id}/{key}', name: 'chill_main_export_saved_edit_options_from_key')] - public function editSavedExportOptionsFromKey(SavedExport $savedExport, string $key): Response - { - $this->denyAccessUnlessGranted('ROLE_USER'); - $user = $this->getUser(); - - if (!$user instanceof User) { - throw new AccessDeniedHttpException(); - } - - $data = $this->rebuildRawData($key); - - $savedExport - ->setOptions($data); - - $this->entityManager->flush(); - - return $this->redirectToRoute('chill_main_export_saved_edit', ['id' => $savedExport->getId()]); - } - - #[Route(path: '/{_locale}/export/save-from-key/{alias}/{key}', name: 'chill_main_export_save_from_key')] - public function saveFromKey(string $alias, string $key, Request $request): Response - { - $this->denyAccessUnlessGranted('ROLE_USER'); - $user = $this->getUser(); - - if (!$user instanceof User) { - throw new AccessDeniedHttpException(); - } - - $data = $this->rebuildRawData($key); - - $savedExport = new SavedExport(); - $savedExport - ->setOptions($data) - ->setExportAlias($alias) - ->setUser($user); - - $form = $this->createForm(SavedExportType::class, $savedExport); - - $form->handleRequest($request); - - if ($form->isSubmitted() && $form->isValid()) { - $this->entityManager->persist($savedExport); - $this->entityManager->flush(); - - return $this->redirectToRoute('chill_main_export_index'); - } - - return $this->render( - '@ChillMain/SavedExport/new.html.twig', - [ - 'form' => $form->createView(), - 'saved_export' => $savedExport, - ] - ); - } - /** * create a form to show on different steps. * @@ -300,7 +131,7 @@ class ExportController extends AbstractController $defaultFormData = match ($savedExport) { null => $this->exportFormHelper->getDefaultData($step, $exportManager->getExport($alias), $options), - default => $this->exportFormHelper->savedExportDataToFormData($savedExport, $step, $options), + default => $this->exportFormHelper->savedExportDataToFormData($savedExport, $step), }; $builder = $this->formFactory @@ -478,7 +309,7 @@ class ExportController extends AbstractController $alias, $this->exportConfigNormalizer->normalizeConfig($alias, $dataToNormalize), $this->clock->now()->add(new \DateInterval('P6M')), - ) + ), ); $this->entityManager->flush(); $this->messageBus->dispatch(new ExportRequestGenerationMessage($exportGeneration, $user)); @@ -536,38 +367,6 @@ class ExportController extends AbstractController ]; } - private function rebuildData($key, ?SavedExport $savedExport) - { - $rawData = $this->rebuildRawData($key); - - $alias = $rawData['alias']; - - if ($this->filterStatsByCenters) { - $formCenters = $this->createCreateFormExport($alias, 'generate_centers', [], $savedExport); - $formCenters->submit($rawData['centers']); - $dataCenters = $formCenters->getData(); - } else { - $dataCenters = ['centers' => []]; - } - - $formExport = $this->createCreateFormExport($alias, 'generate_export', $dataCenters, $savedExport); - $formExport->submit($rawData['export']); - $dataExport = $formExport->getData(); - - if (\count($rawData['formatter']) > 0) { - $formFormatter = $this->createCreateFormExport( - $alias, - 'generate_formatter', - $dataExport, - $savedExport - ); - $formFormatter->submit($rawData['formatter']); - $dataFormatter = $formFormatter->getData(); - } - - return [$dataCenters, $dataExport, $dataFormatter ?? null]; - } - /** * @param string $alias * @@ -696,34 +495,6 @@ class ExportController extends AbstractController } } - private function rebuildRawData(?string $key): array - { - if (null === $key) { - throw $this->createNotFoundException('key does not exists'); - } - - if (1 !== $this->redis->exists($key)) { - $this->addFlash('error', $this->translator->trans('This report is not available any more')); - - throw $this->createNotFoundException('key does not exists'); - } - - $serialized = $this->redis->get($key); - - if (false === $serialized) { - throw new \LogicException('the key could not be reached from redis'); - } - - $rawData = \unserialize($serialized); - - $this->logger->notice('[export] choices for an export unserialized', [ - 'key' => $key, - 'rawData' => json_encode($rawData, JSON_THROW_ON_ERROR), - ]); - - return $rawData; - } - private function getSavedExportFromRequest(Request $request): SavedExport|ExportGeneration|null { $savedExport = match ($savedExportId = $request->query->get('from_saved', '')) { diff --git a/src/Bundle/ChillMainBundle/Controller/SavedExportController.php b/src/Bundle/ChillMainBundle/Controller/SavedExportController.php index ee2201829..b526ef798 100644 --- a/src/Bundle/ChillMainBundle/Controller/SavedExportController.php +++ b/src/Bundle/ChillMainBundle/Controller/SavedExportController.php @@ -27,7 +27,7 @@ use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -43,7 +43,6 @@ final readonly class SavedExportController private FormFactoryInterface $formFactory, private SavedExportRepositoryInterface $savedExportRepository, private Security $security, - private SessionInterface $session, private TranslatorInterface $translator, private UrlGeneratorInterface $urlGenerator, ) {} @@ -63,7 +62,10 @@ final readonly class SavedExportController $this->entityManager->remove($savedExport); $this->entityManager->flush(); - $this->session->getFlashBag()->add('success', $this->translator->trans('saved_export.Export is deleted')); + $session = $request->getSession(); + if ($session instanceof Session) { + $session->getFlashBag()->add('success', $this->translator->trans('saved_export.Export is deleted')); + } return new RedirectResponse( $this->urlGenerator->generate('chill_main_export_saved_list_my') @@ -108,7 +110,9 @@ final readonly class SavedExportController $exportGeneration->setSavedExport($savedExport); $this->entityManager->flush(); - $this->session->getFlashBag()->add('success', $this->translator->trans('saved_export.Saved export is saved!')); + if (($session = $request->getSession()) instanceof Session) { + $session->getFlashBag()->add('success', $this->translator->trans('saved_export.Saved export is saved!')); + } return new RedirectResponse( $this->urlGenerator->generate('chill_main_export_saved_list_my'), @@ -139,10 +143,12 @@ final readonly class SavedExportController if ($form->isSubmitted() && $form->isValid()) { $this->entityManager->flush(); - $this->session->getFlashBag()->add('success', $this->translator->trans('saved_export.Saved export is saved!')); + if (($session = $request->getSession()) instanceof Session) { + $session->getFlashBag()->add('success', $this->translator->trans('saved_export.Saved export is saved!')); + } return new RedirectResponse( - $this->urlGenerator->generate('chill_main_export_saved_list_my') + $this->urlGenerator->generate('chill_main_export_saved_list_my'), ); } diff --git a/src/Bundle/ChillMainBundle/Export/ExportFormHelper.php b/src/Bundle/ChillMainBundle/Export/ExportFormHelper.php index fba16f6cb..f2c015351 100644 --- a/src/Bundle/ChillMainBundle/Export/ExportFormHelper.php +++ b/src/Bundle/ChillMainBundle/Export/ExportFormHelper.php @@ -16,14 +16,12 @@ use Chill\MainBundle\Entity\SavedExport; use Chill\MainBundle\Form\Type\Export\ExportType; use Chill\MainBundle\Form\Type\Export\FilterType; use Chill\MainBundle\Security\Authorization\AuthorizationHelperForCurrentUserInterface; -use Symfony\Component\Form\FormFactoryInterface; final readonly class ExportFormHelper { public function __construct( private AuthorizationHelperForCurrentUserInterface $authorizationHelper, private ExportManager $exportManager, - private FormFactoryInterface $formFactory, private ExportConfigNormalizer $configNormalizer, ) {} @@ -92,12 +90,11 @@ final readonly class ExportFormHelper public function savedExportDataToFormData( ExportGeneration|SavedExport $savedExport, string $step, - array $formOptions = [], ): array { return match ($step) { 'centers', 'generate_centers' => $this->savedExportDataToFormDataStepCenter($savedExport), - 'export', 'generate_export' => $this->savedExportDataToFormDataStepExport($savedExport, $formOptions), - 'formatter', 'generate_formatter' => $this->savedExportDataToFormDataStepFormatter($savedExport, $formOptions), + 'export', 'generate_export' => $this->savedExportDataToFormDataStepExport($savedExport), + 'formatter', 'generate_formatter' => $this->savedExportDataToFormDataStepFormatter($savedExport), default => throw new \LogicException('this step is not allowed: '.$step), }; } @@ -113,7 +110,6 @@ final readonly class ExportFormHelper private function savedExportDataToFormDataStepExport( ExportGeneration|SavedExport $savedExport, - array $formOptions, ): array { $data = $this->configNormalizer->denormalizeConfig($savedExport->getExportAlias(), $savedExport->getOptions(), true); @@ -129,7 +125,6 @@ final readonly class ExportFormHelper private function savedExportDataToFormDataStepFormatter( ExportGeneration|SavedExport $savedExport, - array $formOptions, ): array { $data = $this->configNormalizer->denormalizeConfig($savedExport->getExportAlias(), $savedExport->getOptions(), true);