From d49058805a2690606becb98acaebccb33a8a35d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 24 Apr 2025 14:21:16 +0200 Subject: [PATCH] Refactor aggregator and filter retrieval into a new class Moved aggregator and filter retrieval logic from ExportGenerator to the newly introduced ExportConfigProcessor class. This improves separation of concerns, simplifies ExportGenerator, and enhances code maintainability and readability. Updated related tests accordingly. --- .../Export/ExportConfigProcessor.php | 49 +++++++++++++++++++ .../Export/ExportGenerator.php | 45 +++-------------- .../Tests/Export/ExportGeneratorTest.php | 28 +++++++++-- 3 files changed, 81 insertions(+), 41 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Export/ExportConfigProcessor.php diff --git a/src/Bundle/ChillMainBundle/Export/ExportConfigProcessor.php b/src/Bundle/ChillMainBundle/Export/ExportConfigProcessor.php new file mode 100644 index 000000000..d898ac1af --- /dev/null +++ b/src/Bundle/ChillMainBundle/Export/ExportConfigProcessor.php @@ -0,0 +1,49 @@ + + */ + public function retrieveUsedAggregators(mixed $data): iterable + { + if (null === $data) { + return []; + } + + foreach ($data as $alias => $aggregatorData) { + if ($this->exportManager->hasAggregator($alias) && true === $aggregatorData['enabled']) { + yield $alias => $this->exportManager->getAggregator($alias); + } + } + } + + /** + * @return iterable + */ + public function retrieveUsedFilters(mixed $data): iterable + { + if (null === $data) { + return []; + } + + foreach ($data as $alias => $filterData) { + if ($this->exportManager->hasFilter($alias) && true === $filterData['enabled']) { + yield $alias => $this->exportManager->getFilter($alias); + } + } + } +} diff --git a/src/Bundle/ChillMainBundle/Export/ExportGenerator.php b/src/Bundle/ChillMainBundle/Export/ExportGenerator.php index 02b21029d..1e865e20a 100644 --- a/src/Bundle/ChillMainBundle/Export/ExportGenerator.php +++ b/src/Bundle/ChillMainBundle/Export/ExportGenerator.php @@ -33,6 +33,7 @@ final readonly class ExportGenerator private LoggerInterface $logger, private AuthorizationHelperInterface $authorizationHelper, private CenterRegroupementResolver $centerRegroupementResolver, + private ExportConfigProcessor $exportConfigProcessor, ) {} public function generate(string $exportAlias, array $configuration, ?User $byUser = null): FormattedExportGeneration @@ -94,10 +95,10 @@ final readonly class ExportGenerator $aggregatorsData = []; if ($query instanceof QueryBuilder) { - foreach ($this->retrieveUsedAggregators($data[ExportType::AGGREGATOR_KEY]) as $alias => $aggregator) { + foreach ($this->exportConfigProcessor->retrieveUsedAggregators($data[ExportType::AGGREGATOR_KEY]) as $alias => $aggregator) { $aggregatorsData[$alias] = $data[ExportType::AGGREGATOR_KEY][$alias]['form']; } - foreach ($this->retrieveUsedFilters($data[ExportType::FILTER_KEY]) as $alias => $filter) { + foreach ($this->exportConfigProcessor->retrieveUsedFilters($data[ExportType::FILTER_KEY]) as $alias => $filter) { $filtersData[$alias] = $data[ExportType::FILTER_KEY][$alias]['form']; } } @@ -184,7 +185,7 @@ final readonly class ExportGenerator $usedTypes = []; - foreach ($this->retrieveUsedFilters($data) as $filter) { + foreach ($this->exportConfigProcessor->retrieveUsedFilters($data) as $filter) { if (!\in_array($filter->applyOn(), $usedTypes, true)) { $usedTypes[] = $filter->applyOn(); } @@ -204,7 +205,7 @@ final readonly class ExportGenerator $usedTypes = []; - foreach ($this->retrieveUsedAggregators($data) as $alias => $aggregator) { + foreach ($this->exportConfigProcessor->retrieveUsedAggregators($data) as $alias => $aggregator) { if (!\in_array($aggregator->applyOn(), $usedTypes, true)) { $usedTypes[] = $aggregator->applyOn(); } @@ -213,38 +214,6 @@ final readonly class ExportGenerator return $usedTypes; } - /** - * @return iterable - */ - private function retrieveUsedAggregators(mixed $data): iterable - { - if (null === $data) { - return []; - } - - foreach ($data as $alias => $aggregatorData) { - if ($this->exportManager->hasAggregator($alias) && true === $aggregatorData['enabled']) { - yield $alias => $this->exportManager->getAggregator($alias); - } - } - } - - /** - * @return iterable - */ - private function retrieveUsedFilters(mixed $data): iterable - { - if (null === $data) { - return []; - } - - foreach ($data as $alias => $filterData) { - if ($this->exportManager->hasFilter($alias) && true === $filterData['enabled']) { - yield $alias => $this->exportManager->getFilter($alias); - } - } - } - /** * Alter the query with selected aggregators. */ @@ -253,7 +222,7 @@ final readonly class ExportGenerator array $data, ExportGenerationContext $context, ): void { - foreach ($this->retrieveUsedAggregators($data) as $alias => $aggregator) { + foreach ($this->exportConfigProcessor->retrieveUsedAggregators($data) as $alias => $aggregator) { $formData = $data[$alias]; $aggregator->alterQuery($qb, $formData['form'], $context); } @@ -267,7 +236,7 @@ final readonly class ExportGenerator mixed $data, ExportGenerationContext $context, ): void { - foreach ($this->retrieveUsedFilters($data) as $alias => $filter) { + foreach ($this->exportConfigProcessor->retrieveUsedFilters($data) as $alias => $filter) { $formData = $data[$alias]; $filter->alterQuery($qb, $formData['form'], $context); diff --git a/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php b/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php index efdd903cc..59e9aef5c 100644 --- a/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php @@ -17,6 +17,7 @@ use Chill\MainBundle\Entity\User; use Chill\MainBundle\Export\AggregatorInterface; use Chill\MainBundle\Export\DirectExportInterface; use Chill\MainBundle\Export\ExportConfigNormalizer; +use Chill\MainBundle\Export\ExportConfigProcessor; use Chill\MainBundle\Export\ExportGenerationContext; use Chill\MainBundle\Export\ExportGenerator; use Chill\MainBundle\Export\ExportInterface; @@ -131,7 +132,14 @@ class ExportGeneratorTest extends TestCase $authorizationHelper = $this->prophesize(AuthorizationHelperInterface::class); $authorizationHelper->getReachableCenters($user, 'dummy_role')->willReturn([$centerA, $centerB]); - $generator = new ExportGenerator($exportManager->reveal(), $exportConfigNormalizer->reveal(), new NullLogger(), $authorizationHelper->reveal(), new CenterRegroupementResolver()); + $generator = new ExportGenerator( + $exportManager->reveal(), + $exportConfigNormalizer->reveal(), + new NullLogger(), + $authorizationHelper->reveal(), + new CenterRegroupementResolver(), + new ExportConfigProcessor($exportManager->reveal()) + ); $actual = $generator->generate('dummy', $initialData, $user); @@ -191,7 +199,14 @@ class ExportGeneratorTest extends TestCase $authorizationHelper = $this->prophesize(AuthorizationHelperInterface::class); $authorizationHelper->getReachableCenters($user, 'dummy_role')->willReturn([$centerA, $centerB]); - $generator = new ExportGenerator($exportManager->reveal(), $exportConfigNormalizer->reveal(), new NullLogger(), $authorizationHelper->reveal(), new CenterRegroupementResolver()); + $generator = new ExportGenerator( + $exportManager->reveal(), + $exportConfigNormalizer->reveal(), + new NullLogger(), + $authorizationHelper->reveal(), + new CenterRegroupementResolver(), + new ExportConfigProcessor($exportManager->reveal()) + ); $actual = $generator->generate('dummy', $initialData, $user); @@ -233,7 +248,14 @@ class ExportGeneratorTest extends TestCase $authorizationHelper = $this->prophesize(AuthorizationHelperInterface::class); $authorizationHelper->getReachableCenters($user, 'dummy_role')->willReturn([$centerA, $centerB]); - $generator = new ExportGenerator($exportManager->reveal(), $exportConfigNormalizer->reveal(), new NullLogger(), $authorizationHelper->reveal(), new CenterRegroupementResolver()); + $generator = new ExportGenerator( + $exportManager->reveal(), + $exportConfigNormalizer->reveal(), + new NullLogger(), + $authorizationHelper->reveal(), + new CenterRegroupementResolver(), + new ExportConfigProcessor($exportManager->reveal()), + ); $actual = $generator->generate('dummy', $initialData, $user);