From aa64ff02ec1867c6d8facffe51f7d15359c5740b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 9 Sep 2025 22:29:56 +0200 Subject: [PATCH] Refactor `NotificationFlagDataMapper` to improve handling of null and missing data, and add comprehensive unit tests to verify behavior. --- .../DataMapper/NotificationFlagDataMapper.php | 15 +- .../NotificationFlagDataMapperTest.php | 141 ++++++++++++++++++ 2 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 src/Bundle/ChillMainBundle/Tests/Form/DataMapper/NotificationFlagDataMapperTest.php diff --git a/src/Bundle/ChillMainBundle/Form/DataMapper/NotificationFlagDataMapper.php b/src/Bundle/ChillMainBundle/Form/DataMapper/NotificationFlagDataMapper.php index d904ed5b5..0a6c553c1 100644 --- a/src/Bundle/ChillMainBundle/Form/DataMapper/NotificationFlagDataMapper.php +++ b/src/Bundle/ChillMainBundle/Form/DataMapper/NotificationFlagDataMapper.php @@ -28,13 +28,17 @@ final readonly class NotificationFlagDataMapper implements DataMapperInterface foreach ($this->notificationFlagProviders as $flagProvider) { $flag = $flagProvider->getFlag(); + $data = $viewData[$flag] ?? null; if (isset($formsArray[$flag])) { $flagForm = $formsArray[$flag]; - $immediateEmailChecked = in_array(User::NOTIF_FLAG_IMMEDIATE_EMAIL, $viewData[$flag] ?? [], true) - || !array_key_exists($flag, $viewData); - $dailyEmailChecked = in_array(User::NOTIF_FLAG_DAILY_DIGEST, $viewData[$flag] ?? [], true); + $immediateEmailChecked = + null === $data + || !array_key_exists($flag, $viewData) // true if no data given + || in_array(User::NOTIF_FLAG_IMMEDIATE_EMAIL, $data, true) // true if 'immediate-email' is present in the array + ; + $dailyEmailChecked = is_array($data) && in_array(User::NOTIF_FLAG_DAILY_DIGEST, $data, true); if ($flagForm->has('immediate_email')) { $flagForm->get('immediate_email')->setData($immediateEmailChecked); @@ -49,7 +53,6 @@ final readonly class NotificationFlagDataMapper implements DataMapperInterface public function mapFormsToData($forms, &$viewData): void { $formsArray = iterator_to_array($forms); - $viewData = []; foreach ($this->notificationFlagProviders as $flagProvider) { $flag = $flagProvider->getFlag(); @@ -65,10 +68,6 @@ final readonly class NotificationFlagDataMapper implements DataMapperInterface if (true === $flagForm['daily_email']->getData()) { $viewData[$flag][] = User::NOTIF_FLAG_DAILY_DIGEST; } - - if ([] === $viewData[$flag]) { - $viewData[$flag][] = User::NOTIF_FLAG_IMMEDIATE_EMAIL; - } } } } diff --git a/src/Bundle/ChillMainBundle/Tests/Form/DataMapper/NotificationFlagDataMapperTest.php b/src/Bundle/ChillMainBundle/Tests/Form/DataMapper/NotificationFlagDataMapperTest.php new file mode 100644 index 000000000..9880c7e94 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Form/DataMapper/NotificationFlagDataMapperTest.php @@ -0,0 +1,141 @@ +prophesize(FormInterface::class); + $formImmediateEmail = $this->prophesize(FormInterface::class); + $formImmediateEmail->setData($expectedImmediateEmailValue)->shouldBeCalled(); + $formDailyEmail = $this->prophesize(FormInterface::class); + $formDailyEmail->setData($expectedDailyEmailValue)->shouldBeCalled(); + $form->has('immediate_email')->willReturn(true); + $form->get('immediate_email')->willReturn($formImmediateEmail->reveal()); + $form->has('daily_email')->willReturn(true); + $form->get('daily_email')->willReturn($formDailyEmail->reveal()); + + $forms = ['dummy_flag' => $form->reveal()]; + + $this->buildDataMapper()->mapDataToForms($viewData, $forms); + } + + public static function provideDataMapDataToForms(): iterable + { + yield [ + ['dummy_flag' => ['immediate-email']], + true, + false, + ]; + + yield [ + ['dummy_flag' => ['daily-digest']], + false, + true, + ]; + + yield [ + ['dummy_flag' => ['immediate-email', 'daily-digest']], + true, + true, + ]; + + yield [ + ['dummy_flag' => null], + true, + false, + ]; + + yield [ + null, + true, + false, + ]; + } + + /** + * @dataProvider provideDataMapFormsToData + */ + public function testMapFormsToData(bool $immediateEmailValue, bool $dailyDigestValue, array $viewData, array $expected): void + { + $formImmediateEmail = $this->prophesize(FormInterface::class); + $formImmediateEmail->getData()->willReturn($immediateEmailValue)->shouldBeCalled(); + $formDailyEmail = $this->prophesize(FormInterface::class); + $formDailyEmail->getData()->willReturn($dailyDigestValue)->shouldBeCalled(); + + $forms = [ + 'dummy_flag' => [ + 'immediate_email' => $formImmediateEmail->reveal(), + 'daily_email' => $formDailyEmail->reveal(), + ], + ]; + + $this->buildDataMapper()->mapFormsToData($forms, $viewData); + + self::assertEqualsCanonicalizing($expected, $viewData); + } + + public static function provideDataMapFormsToData(): iterable + { + yield [ + true, true, [], ['dummy_flag' => ['immediate-email', 'daily-digest']], + ]; + + yield [ + true, false, [], ['dummy_flag' => ['immediate-email']], + ]; + + yield [ + false, true, [], ['dummy_flag' => ['daily-digest']], + ]; + + yield [ + false, false, [], ['dummy_flag' => []], + ]; + } +}