From 00d199cf322593724147c4dd4a745b8863d366e5 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Mon, 23 Dec 2024 17:03:26 +0000 Subject: [PATCH 01/10] inspect user input and throw helpful error if not valid --- client/app/src/Form/ClientType.php | 11 ++++++-- client/app/src/Form/Report/ReportType.php | 32 +++++++++++++++-------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/client/app/src/Form/ClientType.php b/client/app/src/Form/ClientType.php index 71c1801dab..23531fe8d5 100644 --- a/client/app/src/Form/ClientType.php +++ b/client/app/src/Form/ClientType.php @@ -5,6 +5,7 @@ use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type as FormTypes; use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -30,19 +31,25 @@ public function buildForm(FormBuilderInterface $builder, array $options) ->add('address5', FormTypes\TextType::class) ->add('postcode', FormTypes\TextType::class) ->add('country', FormTypes\CountryType::class, [ - 'preferred_choices' => ['GB'], - 'placeholder' => 'country.defaultOption', + 'preferred_choices' => ['GB'], + 'placeholder' => 'country.defaultOption', ]) ->add('phone', FormTypes\TextType::class) ->add('id', FormTypes\HiddenType::class) ->add('save', FormTypes\SubmitType::class); // strip tags to prevent XSS as the name is often displayed around mixed with translation with the twig "raw" filter + // only allow user to enter a four digit year $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { $data = $event->getData(); $data['firstname'] = strip_tags($data['firstname']); $data['lastname'] = strip_tags($data['lastname']); $event->setData($data); + + if (!preg_match('/^\d{4}$/', $data['courtDate']['year'])) { + $form = $event->getForm(); + $form->addError(new FormError('Please enter a valid four-digit year.')); + } }); } diff --git a/client/app/src/Form/Report/ReportType.php b/client/app/src/Form/Report/ReportType.php index 04d44d9333..807d9ac5f9 100644 --- a/client/app/src/Form/Report/ReportType.php +++ b/client/app/src/Form/Report/ReportType.php @@ -5,6 +5,9 @@ use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type as FormTypes; use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormEvent; +use Symfony\Component\Form\FormEvents; use Symfony\Component\OptionsResolver\OptionsResolver; class ReportType extends AbstractType @@ -12,19 +15,26 @@ class ReportType extends AbstractType public function buildForm(FormBuilderInterface $builder, array $options) { $builder - ->add('id', FormTypes\HiddenType::class) - ->add('startDate', FormTypes\DateType::class, ['widget' => 'text', - 'input' => 'datetime', - 'format' => 'yyyy-MM-dd', - 'invalid_message' => 'report.startDate.invalidMessage', ]) + ->add('id', FormTypes\HiddenType::class) + ->add('startDate', FormTypes\DateType::class, ['widget' => 'text', + 'input' => 'datetime', + 'format' => 'yyyy-MM-dd', + 'invalid_message' => 'report.startDate.invalidMessage', ]) + ->add('endDate', FormTypes\DateType::class, ['widget' => 'text', + 'input' => 'datetime', + 'format' => 'yyyy-MM-dd', + 'invalid_message' => 'report.endDate.invalidMessage', + ]) + ->add('save', FormTypes\SubmitType::class); - ->add('endDate', FormTypes\DateType::class, ['widget' => 'text', - 'input' => 'datetime', - 'format' => 'yyyy-MM-dd', - 'invalid_message' => 'report.endDate.invalidMessage', - ]) + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { + $data = $event->getData(); - ->add('save', FormTypes\SubmitType::class); + if (!preg_match('/^\d{4}$/', $data['startDate']['year']) || !preg_match('/^\d{4}$/', $data['endDate']['year'])) { + $form = $event->getForm(); + $form->addError(new FormError('Please enter a valid four-digit year.')); + } + }); } public function configureOptions(OptionsResolver $resolver) From 5c704e8acff8289641d3e5b229e1f787a97c0b79 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Mon, 6 Jan 2025 14:23:30 +0000 Subject: [PATCH 02/10] add unit tests for validation against incorrect data input --- .../app/tests/phpunit/Form/ReportTypeTest.php | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 client/app/tests/phpunit/Form/ReportTypeTest.php diff --git a/client/app/tests/phpunit/Form/ReportTypeTest.php b/client/app/tests/phpunit/Form/ReportTypeTest.php new file mode 100644 index 0000000000..2c4e12690b --- /dev/null +++ b/client/app/tests/phpunit/Form/ReportTypeTest.php @@ -0,0 +1,69 @@ + '2022', + 'month' => '01', + 'day' => '02', + ]; + + $endDate = [ + 'year' => '2023', + 'month' => '01', + 'day' => '02', + ]; + + $formData = [ + 'id' => 1, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]; + + $form = $this->factory->create(ReportType::class); + + $form->submit($formData); + + $this->assertTrue($form->isSubmitted()); + $this->assertTrue($form->isValid()); + } + + public function testSubmitInvalidData() + { + $startDate = [ + 'year' => '22', + 'month' => '01', + 'day' => '02', + ]; + + $endDate = [ + 'year' => '23', + 'month' => '01', + 'day' => '02', + ]; + + $formData = [ + 'id' => 1, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]; + + $form = $this->factory->create(ReportType::class); + + $form->submit($formData); + $errors = $form->getErrors(); + + $this->assertTrue($form->isSubmitted()); + $this->assertFalse($form->isValid()); + + $this->assertCount(1, $errors); + $this->assertSame('Please enter a valid four-digit year.', $errors[0]->getMessage()); + } +} From d98f65aac843f447d5e2bbeca04f92a27900ea1b Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Mon, 6 Jan 2025 15:56:40 +0000 Subject: [PATCH 03/10] add unit tests for validation against incorrect data input --- .../app/tests/phpunit/Form/ClientTypeTest.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 client/app/tests/phpunit/Form/ClientTypeTest.php diff --git a/client/app/tests/phpunit/Form/ClientTypeTest.php b/client/app/tests/phpunit/Form/ClientTypeTest.php new file mode 100644 index 0000000000..9658a55ea0 --- /dev/null +++ b/client/app/tests/phpunit/Form/ClientTypeTest.php @@ -0,0 +1,62 @@ + '2024', + 'month' => '01', + 'day' => '01', + ]; + + $formData = [ + 'firstname' => 'Mel', + 'lastname' => 'Jones', + 'caseNumber' => '0000000', + 'courtDate' => $courtDate, + 'address' => '10 Downing Street', + 'phone' => '0123456789', + ]; + + $form = $this->factory->create(ClientType::class); + + $form->submit($formData); + + $this->assertTrue($form->isSubmitted()); + $this->assertTrue($form->isValid()); + } + + public function testSubmitInvalidYear() + { + $courtDate = [ + 'year' => '20', + 'month' => '01', + 'day' => '01', + ]; + + $formData = [ + 'firstname' => 'Mel', + 'lastname' => 'Jones', + 'caseNumber' => '0000000', + 'courtDate' => $courtDate, + 'address' => '10 Downing Street', + 'phone' => '0123456789', + ]; + + $form = $this->factory->create(ClientType::class); + + $form->submit($formData); + $errors = $form->getErrors(); + + $this->assertTrue($form->isSubmitted()); + $this->assertFalse($form->isValid()); + + $this->assertCount(1, $errors); + $this->assertSame('Please enter a valid four-digit year.', $errors[0]->getMessage()); + } +} From 5d3f5308784dd95731fbe76dafa1c65872445b89 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Tue, 7 Jan 2025 17:20:17 +0000 Subject: [PATCH 04/10] create custom symfony constraint and register as a service --- client/app/config/services.yml | 4 +++ client/app/src/Entity/Report/Report.php | 2 ++ .../Constraints/YearMustBeFourDigits.php | 26 ++++++++++++++ .../YearMustBeFourDigitsLongValidator.php | 36 +++++++++++++++++++ 4 files changed, 68 insertions(+) create mode 100644 client/app/src/Validator/Constraints/YearMustBeFourDigits.php create mode 100644 client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php diff --git a/client/app/config/services.yml b/client/app/config/services.yml index 2e9bf30e0e..45a285241a 100644 --- a/client/app/config/services.yml +++ b/client/app/config/services.yml @@ -255,6 +255,10 @@ services: tags: - { name: validator.constraint_validator, alias: email_same_domain } + App\Validator\Constraints\YearMustBeFourDigitsLongValidator: + tags: + - { name: validator.constraint_validator, alias: start_end_dates } + GuzzleHttp\Client: arguments: $config: diff --git a/client/app/src/Entity/Report/Report.php b/client/app/src/Entity/Report/Report.php index 57cb57dd9c..5175ab97ab 100644 --- a/client/app/src/Entity/Report/Report.php +++ b/client/app/src/Entity/Report/Report.php @@ -17,6 +17,8 @@ * * @AppAssert\EndDateNotBeforeStartDate(groups={"start-end-dates"}) * + * @AppAssert\YearMustBeFourDigits(groups={"start-end-dates"}) + * * @AppAssert\ProfDeputyCostsEstimate\CostBreakdownNotGreaterThanTotal(groups={"prof-deputy-estimate-costs"}) * * @Assert\Callback(callback="debtsValid", groups={"debts"}) diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigits.php b/client/app/src/Validator/Constraints/YearMustBeFourDigits.php new file mode 100644 index 0000000000..d0d2653202 --- /dev/null +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigits.php @@ -0,0 +1,26 @@ +getStartDate(), $data->getEndDate()]; + + foreach ($startAndEndDate as $date) { + if (!$date instanceof \DateTime) { + return; + } + } + + foreach ($startAndEndDate as $date) { + $year = $date->format('Y'); // extract the year from the date string + + $count = 0; + !preg_match('/^2\d{3}$/', $year) ? $count++ : $count; // if the year does not start with a 2 and is not 4 digits long + } + if ($count > 0) { + $this->context + ->buildViolation($constraint->message) + ->addViolation(); + } + } +} From d7e39e958d9fe82c79da5d79c1411b58f5f1b01a Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Wed, 8 Jan 2025 14:10:27 +0000 Subject: [PATCH 05/10] update validation to run across multiple properties, drop unnecessary registration from services.yaml --- client/app/config/services.yml | 4 -- client/app/src/Entity/Client.php | 5 +- client/app/src/Entity/Report/Report.php | 2 +- client/app/src/Form/ClientType.php | 9 +--- client/app/src/Form/Report/ReportType.php | 15 +----- ...igits.php => YearMustBeFourDigitsLong.php} | 4 +- .../YearMustBeFourDigitsLongValidator.php | 48 +++++++++++++------ 7 files changed, 43 insertions(+), 44 deletions(-) rename client/app/src/Validator/Constraints/{YearMustBeFourDigits.php => YearMustBeFourDigitsLong.php} (79%) diff --git a/client/app/config/services.yml b/client/app/config/services.yml index 45a285241a..2e9bf30e0e 100644 --- a/client/app/config/services.yml +++ b/client/app/config/services.yml @@ -255,10 +255,6 @@ services: tags: - { name: validator.constraint_validator, alias: email_same_domain } - App\Validator\Constraints\YearMustBeFourDigitsLongValidator: - tags: - - { name: validator.constraint_validator, alias: start_end_dates } - GuzzleHttp\Client: arguments: $config: diff --git a/client/app/src/Entity/Client.php b/client/app/src/Entity/Client.php index 3961367fa9..533716fa95 100644 --- a/client/app/src/Entity/Client.php +++ b/client/app/src/Entity/Client.php @@ -5,11 +5,14 @@ use App\Entity\Report\Report; use App\Entity\Traits\ActiveAudit; use App\Entity\Traits\IsSoftDeleteableEntity; -use DateTime; +use App\Validator\Constraints as AppAssert; use Doctrine\Common\Collections\ArrayCollection; use JMS\Serializer\Annotation as JMS; use Symfony\Component\Validator\Constraints as Assert; +/** + * @AppAssert\YearMustBeFourDigitsLong(groups={"client-court-date"}) + */ class Client { use IsSoftDeleteableEntity; diff --git a/client/app/src/Entity/Report/Report.php b/client/app/src/Entity/Report/Report.php index 5175ab97ab..fc88a9544b 100644 --- a/client/app/src/Entity/Report/Report.php +++ b/client/app/src/Entity/Report/Report.php @@ -17,7 +17,7 @@ * * @AppAssert\EndDateNotBeforeStartDate(groups={"start-end-dates"}) * - * @AppAssert\YearMustBeFourDigits(groups={"start-end-dates"}) + * @AppAssert\YearMustBeFourDigitsLong(groups={"start-end-dates"}) * * @AppAssert\ProfDeputyCostsEstimate\CostBreakdownNotGreaterThanTotal(groups={"prof-deputy-estimate-costs"}) * diff --git a/client/app/src/Form/ClientType.php b/client/app/src/Form/ClientType.php index 23531fe8d5..829e37bc30 100644 --- a/client/app/src/Form/ClientType.php +++ b/client/app/src/Form/ClientType.php @@ -5,7 +5,6 @@ use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type as FormTypes; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -39,17 +38,11 @@ public function buildForm(FormBuilderInterface $builder, array $options) ->add('save', FormTypes\SubmitType::class); // strip tags to prevent XSS as the name is often displayed around mixed with translation with the twig "raw" filter - // only allow user to enter a four digit year $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { $data = $event->getData(); $data['firstname'] = strip_tags($data['firstname']); $data['lastname'] = strip_tags($data['lastname']); $event->setData($data); - - if (!preg_match('/^\d{4}$/', $data['courtDate']['year'])) { - $form = $event->getForm(); - $form->addError(new FormError('Please enter a valid four-digit year.')); - } }); } @@ -57,7 +50,7 @@ public function configureOptions(OptionsResolver $resolver) { $resolver->setDefaults([ 'translation_domain' => 'registration', - 'validation_groups' => 'lay-deputy-client', + 'validation_groups' => ['lay-deputy-client', 'client-court-date'], ]); } diff --git a/client/app/src/Form/Report/ReportType.php b/client/app/src/Form/Report/ReportType.php index 807d9ac5f9..69510703ef 100644 --- a/client/app/src/Form/Report/ReportType.php +++ b/client/app/src/Form/Report/ReportType.php @@ -5,9 +5,6 @@ use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type as FormTypes; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Form\FormError; -use Symfony\Component\Form\FormEvent; -use Symfony\Component\Form\FormEvents; use Symfony\Component\OptionsResolver\OptionsResolver; class ReportType extends AbstractType @@ -19,22 +16,14 @@ public function buildForm(FormBuilderInterface $builder, array $options) ->add('startDate', FormTypes\DateType::class, ['widget' => 'text', 'input' => 'datetime', 'format' => 'yyyy-MM-dd', - 'invalid_message' => 'report.startDate.invalidMessage', ]) + 'invalid_message' => 'report.startDate.invalidMessage', + ]) ->add('endDate', FormTypes\DateType::class, ['widget' => 'text', 'input' => 'datetime', 'format' => 'yyyy-MM-dd', 'invalid_message' => 'report.endDate.invalidMessage', ]) ->add('save', FormTypes\SubmitType::class); - - $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { - $data = $event->getData(); - - if (!preg_match('/^\d{4}$/', $data['startDate']['year']) || !preg_match('/^\d{4}$/', $data['endDate']['year'])) { - $form = $event->getForm(); - $form->addError(new FormError('Please enter a valid four-digit year.')); - } - }); } public function configureOptions(OptionsResolver $resolver) diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigits.php b/client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php similarity index 79% rename from client/app/src/Validator/Constraints/YearMustBeFourDigits.php rename to client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php index d0d2653202..9186564a24 100644 --- a/client/app/src/Validator/Constraints/YearMustBeFourDigits.php +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php @@ -7,13 +7,13 @@ /** * @Annotation */ -class YearMustBeFourDigits extends Constraint +class YearMustBeFourDigitsLong extends Constraint { public string $message = 'Please enter a valid four-digit year.'; public function validatedBy() { - return 'start_end_dates'; + return static::class.'Validator'; } /** diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php b/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php index 9b46dd727a..8814604595 100644 --- a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php @@ -9,28 +9,46 @@ class YearMustBeFourDigitsLongValidator extends ConstraintValidator { public function validate($data, Constraint $constraint) { - if (!$data instanceof StartEndDateComparableInterface) { - throw new \InvalidArgumentException(sprintf('Validation data must implement %s interface', StartEndDateComparableInterface::class)); - } + $startAndEndDate = []; + $courtDate = new \DateTime(); + + if ($data instanceof StartEndDateComparableInterface) { + $startAndEndDate = [$data->getStartDate(), $data->getEndDate()]; - $startAndEndDate = [$data->getStartDate(), $data->getEndDate()]; + foreach ($startAndEndDate as $date) { + if (!$date instanceof \DateTime) { + return; + } + } + } else { + $courtDate = $data->getCourtDate(); - foreach ($startAndEndDate as $date) { - if (!$date instanceof \DateTime) { + if (!$courtDate instanceof \DateTime) { return; } } - foreach ($startAndEndDate as $date) { - $year = $date->format('Y'); // extract the year from the date string + if ($startAndEndDate) { + $count = count(array_filter($startAndEndDate, function ($date) { + $year = $date->format('Y'); - $count = 0; - !preg_match('/^2\d{3}$/', $year) ? $count++ : $count; // if the year does not start with a 2 and is not 4 digits long - } - if ($count > 0) { - $this->context - ->buildViolation($constraint->message) - ->addViolation(); + // if the year does not start with a 2 and is not 4 digits long add to count + return !preg_match('/^2\d{3}$/', $year); + })); + + if ($count > 0) { + $this->context + ->buildViolation($constraint->message) + ->addViolation(); + } + } else { + $year = $courtDate->format('Y'); + + if (!preg_match('/^2\d{3}$/', $year)) { + $this->context + ->buildViolation($constraint->message) + ->addViolation(); + } } } } From fc658c6519fd626db9570bbdfb517a80aec46243 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Thu, 9 Jan 2025 16:18:26 +0000 Subject: [PATCH 06/10] add range validation for start date and supporting unit tests --- client/app/src/Form/Report/ReportType.php | 9 +++ .../app/tests/phpunit/Form/ClientTypeTest.php | 62 ------------------- .../app/tests/phpunit/Form/ReportTypeTest.php | 32 +++++++--- client/app/translations/validators.en.yml | 1 + 4 files changed, 34 insertions(+), 70 deletions(-) delete mode 100644 client/app/tests/phpunit/Form/ClientTypeTest.php diff --git a/client/app/src/Form/Report/ReportType.php b/client/app/src/Form/Report/ReportType.php index 69510703ef..f38e99af43 100644 --- a/client/app/src/Form/Report/ReportType.php +++ b/client/app/src/Form/Report/ReportType.php @@ -6,6 +6,7 @@ use Symfony\Component\Form\Extension\Core\Type as FormTypes; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\Validator\Constraints\Range; class ReportType extends AbstractType { @@ -17,6 +18,14 @@ public function buildForm(FormBuilderInterface $builder, array $options) 'input' => 'datetime', 'format' => 'yyyy-MM-dd', 'invalid_message' => 'report.startDate.invalidMessage', + 'constraints' => [ + new Range([ + 'min' => (new \DateTime('now'))->modify('-7 years'), + 'max' => (new \DateTime('now'))->modify('+3 years'), + 'notInRangeMessage' => 'Please enter a valid year.', + 'groups' => 'start-end-dates', + ]), + ], ]) ->add('endDate', FormTypes\DateType::class, ['widget' => 'text', 'input' => 'datetime', diff --git a/client/app/tests/phpunit/Form/ClientTypeTest.php b/client/app/tests/phpunit/Form/ClientTypeTest.php deleted file mode 100644 index 9658a55ea0..0000000000 --- a/client/app/tests/phpunit/Form/ClientTypeTest.php +++ /dev/null @@ -1,62 +0,0 @@ - '2024', - 'month' => '01', - 'day' => '01', - ]; - - $formData = [ - 'firstname' => 'Mel', - 'lastname' => 'Jones', - 'caseNumber' => '0000000', - 'courtDate' => $courtDate, - 'address' => '10 Downing Street', - 'phone' => '0123456789', - ]; - - $form = $this->factory->create(ClientType::class); - - $form->submit($formData); - - $this->assertTrue($form->isSubmitted()); - $this->assertTrue($form->isValid()); - } - - public function testSubmitInvalidYear() - { - $courtDate = [ - 'year' => '20', - 'month' => '01', - 'day' => '01', - ]; - - $formData = [ - 'firstname' => 'Mel', - 'lastname' => 'Jones', - 'caseNumber' => '0000000', - 'courtDate' => $courtDate, - 'address' => '10 Downing Street', - 'phone' => '0123456789', - ]; - - $form = $this->factory->create(ClientType::class); - - $form->submit($formData); - $errors = $form->getErrors(); - - $this->assertTrue($form->isSubmitted()); - $this->assertFalse($form->isValid()); - - $this->assertCount(1, $errors); - $this->assertSame('Please enter a valid four-digit year.', $errors[0]->getMessage()); - } -} diff --git a/client/app/tests/phpunit/Form/ReportTypeTest.php b/client/app/tests/phpunit/Form/ReportTypeTest.php index 2c4e12690b..0fcf478638 100644 --- a/client/app/tests/phpunit/Form/ReportTypeTest.php +++ b/client/app/tests/phpunit/Form/ReportTypeTest.php @@ -3,20 +3,36 @@ namespace App\Form; use App\Form\Report\ReportType; +use Symfony\Component\Form\Extension\Validator\ValidatorExtension; use Symfony\Component\Form\Test\TypeTestCase; +use Symfony\Component\Validator\Validation; class ReportTypeTest extends TypeTestCase { - public function testSubmitValidData() + // ensures validation constraints are applied + protected function getExtensions(): array { + $validator = Validation::createValidator(); + + return [ + new ValidatorExtension($validator), + ]; + } + + public function testSubmitValidYear() + { + $currentDate = new \DateTime(); + $currentYear = $currentDate->format('Y'); + $followingYear = $currentDate->modify('+1 year')->format('Y'); + $startDate = [ - 'year' => '2022', + 'year' => $currentYear, 'month' => '01', 'day' => '02', ]; $endDate = [ - 'year' => '2023', + 'year' => $followingYear, 'month' => '01', 'day' => '02', ]; @@ -35,16 +51,16 @@ public function testSubmitValidData() $this->assertTrue($form->isValid()); } - public function testSubmitInvalidData() + public function testSubmitInvalidYear() { $startDate = [ - 'year' => '22', + 'year' => '2000', 'month' => '01', 'day' => '02', ]; $endDate = [ - 'year' => '23', + 'year' => '2001', 'month' => '01', 'day' => '02', ]; @@ -58,12 +74,12 @@ public function testSubmitInvalidData() $form = $this->factory->create(ReportType::class); $form->submit($formData); - $errors = $form->getErrors(); + $errors = $form['startDate']->getErrors(); $this->assertTrue($form->isSubmitted()); $this->assertFalse($form->isValid()); $this->assertCount(1, $errors); - $this->assertSame('Please enter a valid four-digit year.', $errors[0]->getMessage()); + $this->assertSame('Please enter a valid year.', $errors[0]->getMessage()); } } diff --git a/client/app/translations/validators.en.yml b/client/app/translations/validators.en.yml index 6df59bda19..e324879796 100644 --- a/client/app/translations/validators.en.yml +++ b/client/app/translations/validators.en.yml @@ -105,6 +105,7 @@ report: invalidMessage: "Enter a valid date in this format: DD/MM/YYYY" beforeStart: "Check the end date: it cannot be before the start date" greaterThan15Months: "Check the end date: your reporting period cannot be more than 15 months" + invalidYear: Please enter a valid year dueDate: notBlank: Enter the new due date invalidMessage: "Enter a valid date in this format: DD/MM/YYYY" From f049eebe3111ed7a86c45c1dd2951fa06ff8a66c Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Fri, 10 Jan 2025 11:49:02 +0000 Subject: [PATCH 07/10] create clearer naming convention, add unit tests for custom constraint and validator --- client/app/src/Entity/Client.php | 2 +- client/app/src/Entity/Report/Report.php | 2 +- client/app/src/Form/Report/ReportType.php | 2 +- ...g.php => YearMustBeFourDigitsAndValid.php} | 2 +- ...YearMustBeFourDigitsAndValidValidator.php} | 2 +- .../YearMustBeFourDigitsLongValidatorTest.php | 105 ++++++++++++++++++ 6 files changed, 110 insertions(+), 5 deletions(-) rename client/app/src/Validator/Constraints/{YearMustBeFourDigitsLong.php => YearMustBeFourDigitsAndValid.php} (88%) rename client/app/src/Validator/Constraints/{YearMustBeFourDigitsLongValidator.php => YearMustBeFourDigitsAndValidValidator.php} (95%) create mode 100644 client/app/tests/phpunit/Validator/Constraints/YearMustBeFourDigitsLongValidatorTest.php diff --git a/client/app/src/Entity/Client.php b/client/app/src/Entity/Client.php index 533716fa95..7bd888f50d 100644 --- a/client/app/src/Entity/Client.php +++ b/client/app/src/Entity/Client.php @@ -11,7 +11,7 @@ use Symfony\Component\Validator\Constraints as Assert; /** - * @AppAssert\YearMustBeFourDigitsLong(groups={"client-court-date"}) + * @AppAssert\YearMustBeFourDigitsAndValid(groups={"client-court-date"}) */ class Client { diff --git a/client/app/src/Entity/Report/Report.php b/client/app/src/Entity/Report/Report.php index fc88a9544b..9b92bccf50 100644 --- a/client/app/src/Entity/Report/Report.php +++ b/client/app/src/Entity/Report/Report.php @@ -17,7 +17,7 @@ * * @AppAssert\EndDateNotBeforeStartDate(groups={"start-end-dates"}) * - * @AppAssert\YearMustBeFourDigitsLong(groups={"start-end-dates"}) + * @AppAssert\YearMustBeFourDigitsAndValid(groups={"start-end-dates"}) * * @AppAssert\ProfDeputyCostsEstimate\CostBreakdownNotGreaterThanTotal(groups={"prof-deputy-estimate-costs"}) * diff --git a/client/app/src/Form/Report/ReportType.php b/client/app/src/Form/Report/ReportType.php index f38e99af43..c75308b1dc 100644 --- a/client/app/src/Form/Report/ReportType.php +++ b/client/app/src/Form/Report/ReportType.php @@ -22,7 +22,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) new Range([ 'min' => (new \DateTime('now'))->modify('-7 years'), 'max' => (new \DateTime('now'))->modify('+3 years'), - 'notInRangeMessage' => 'Please enter a valid year.', + 'notInRangeMessage' => 'Please enter a valid start date.', 'groups' => 'start-end-dates', ]), ], diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValid.php similarity index 88% rename from client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php rename to client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValid.php index 9186564a24..07cd82b914 100644 --- a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLong.php +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValid.php @@ -7,7 +7,7 @@ /** * @Annotation */ -class YearMustBeFourDigitsLong extends Constraint +class YearMustBeFourDigitsAndValid extends Constraint { public string $message = 'Please enter a valid four-digit year.'; diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php similarity index 95% rename from client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php rename to client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php index 8814604595..6339221092 100644 --- a/client/app/src/Validator/Constraints/YearMustBeFourDigitsLongValidator.php +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php @@ -5,7 +5,7 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; -class YearMustBeFourDigitsLongValidator extends ConstraintValidator +class YearMustBeFourDigitsAndValidValidator extends ConstraintValidator { public function validate($data, Constraint $constraint) { diff --git a/client/app/tests/phpunit/Validator/Constraints/YearMustBeFourDigitsLongValidatorTest.php b/client/app/tests/phpunit/Validator/Constraints/YearMustBeFourDigitsLongValidatorTest.php new file mode 100644 index 0000000000..ace170c257 --- /dev/null +++ b/client/app/tests/phpunit/Validator/Constraints/YearMustBeFourDigitsLongValidatorTest.php @@ -0,0 +1,105 @@ +getMockBuilder('Symfony\Component\Validator\Violation\ConstraintViolationBuilder') + ->disableOriginalConstructor() + ->setMethods(['addViolation']) + ->getMock(); + + // mock the validator context + $context = $this->getMockBuilder('Symfony\Component\Validator\Context\ExecutionContext') + ->disableOriginalConstructor() + ->setMethods(['buildViolation']) + ->getMock(); + + if ($expectedMessage) { + $builder->expects($this->once()) + ->method('addViolation'); + + $context->expects($this->once()) + ->method('buildViolation') + ->with($this->equalTo($expectedMessage)) + ->will($this->returnValue($builder)); + } else { + $context->expects($this->never()) + ->method('buildViolation'); + } + + // initialize the validator with the mocked context + $validator = new YearMustBeFourDigitsAndValidValidator(); + $validator->initialize($context); + + return $validator; + } + + /** + * Verify a constraint message is triggered when court date year is invalid. + */ + public function testValidateOnInvalidCourtDate() + { + $constraint = new YearMustBeFourDigitsAndValid(); + $validator = $this->configureValidator($constraint->message); + + $client = ClientHelpers::createClient(); + $client->setCourtDate(new \DateTime('0116-01-01')); + + $validator->validate($client, $constraint); + } + + /** + * Verify no constraint message is triggered when court date year is valid. + */ + public function testValidateOnValidCourtDate() + { + $constraint = new YearMustBeFourDigitsAndValid(); + $validator = $this->configureValidator(); + + $client = ClientHelpers::createClient(); + $client->setCourtDate(new \DateTime('today')); + + $validator->validate($client, $constraint); + } + + /** + * Verify a constraint message is triggered when reporting period year is invalid. + */ + public function testValidateOnInvalidYear() + { + $constraint = new YearMustBeFourDigitsAndValid(); + $validator = $this->configureValidator($constraint->message); + + $client = ReportHelpers::createReport(); + $client->setStartDate(new \DateTime('0116-01-02')); + $client->setEndDate(new \DateTime('0117-01-01')); + + $validator->validate($client, $constraint); + } + + /** + * Verify no constraint message is triggered when reporting period year is valid. + */ + public function testValidateOnValidYear() + { + $constraint = new YearMustBeFourDigitsAndValid(); + $validator = $this->configureValidator(); + + $client = ReportHelpers::createReport(); + $client->setStartDate(new \DateTime('today')); + $client->setEndDate((new \DateTime('today'))->modify('+1 year')); + + $validator->validate($client, $constraint); + } +} From 94ec133f1140231b13e815c21b897c7d66b5c2b9 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Mon, 13 Jan 2025 10:03:59 +0000 Subject: [PATCH 08/10] fix failing tests --- .../v2/Registration/SelfRegistrationTrait.php | 16 ++++++++-------- .../sirius-csvs/lay-2-rows-co-deputy.csv | 4 ++-- client/app/tests/phpunit/Form/ReportTypeTest.php | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/api/app/tests/Behat/bootstrap/v2/Registration/SelfRegistrationTrait.php b/api/app/tests/Behat/bootstrap/v2/Registration/SelfRegistrationTrait.php index f0fb5da380..2e2d165b6c 100644 --- a/api/app/tests/Behat/bootstrap/v2/Registration/SelfRegistrationTrait.php +++ b/api/app/tests/Behat/bootstrap/v2/Registration/SelfRegistrationTrait.php @@ -396,20 +396,20 @@ private function fillClientDetailsAndSubmit(): void $this->fillInField('client_postcode', 'NG1 2HT'); $this->fillInField('client_country', 'GB'); $this->fillInField('client_phone', '01789432876'); - $this->fillInField('client_courtDate_day', '01'); - $this->fillInField('client_courtDate_month', '01'); - $this->fillInField('client_courtDate_year', '2016'); + $this->fillInField('client_courtDate_day', '16'); + $this->fillInField('client_courtDate_month', '04'); + $this->fillInField('client_courtDate_year', '2023'); $this->pressButton('client_save'); } private function fillInReportDetailsAndSubmit(): void { - $this->fillInField('report_startDate_day', '01'); + $this->fillInField('report_startDate_day', '02'); $this->fillInField('report_startDate_month', '01'); - $this->fillInField('report_startDate_year', '2016'); - $this->fillInField('report_endDate_day', '31'); - $this->fillInField('report_endDate_month', '03'); - $this->fillInField('report_endDate_year', '2017'); + $this->fillInField('report_startDate_year', '2023'); + $this->fillInField('report_endDate_day', '01'); + $this->fillInField('report_endDate_month', '01'); + $this->fillInField('report_endDate_year', '2024'); $this->pressButton('report_save'); } diff --git a/api/app/tests/Behat/fixtures/sirius-csvs/lay-2-rows-co-deputy.csv b/api/app/tests/Behat/fixtures/sirius-csvs/lay-2-rows-co-deputy.csv index a1712eaae2..3e462de5de 100644 --- a/api/app/tests/Behat/fixtures/sirius-csvs/lay-2-rows-co-deputy.csv +++ b/api/app/tests/Behat/fixtures/sirius-csvs/lay-2-rows-co-deputy.csv @@ -1,3 +1,3 @@ Case,ClientFirstname,ClientSurname,ClientAddress1,ClientAddress2,ClientAddress3,ClientAddress4,ClientAddress5,ClientPostcode,DeputyUid,DeputyFirstname,DeputySurname,DeputyAddress1,DeputyAddress2,DeputyAddress3,DeputyAddress4,DeputyAddress5,DeputyPostcode,ReportType,MadeDate,OrderType,CoDeputy,Hybrid -1717171T,JANE,LOUIE,10 Quaker Street,Birstall,Manchester,,,M3,35672419,BRIAN,MCDUCK,Shieldaig,Northamptonshire,,,,B73,OPG102,2011-04-16,pfa,no,SINGLE -1717171T,JULIE,LOUIE,10 Quaker Street,Birstall,Manchester,,,M3,85462817,LIAM,MCQUACK,Fieldag,Southamptonshire,,,,Y73,OPG102,2011-04-16,pfa,no,SINGLE +1717171T,JANE,LOUIE,10 Quaker Street,Birstall,Manchester,,,M3,35672419,BRIAN,MCDUCK,Shieldaig,Northamptonshire,,,,B73,OPG102,2023-04-16,pfa,no,SINGLE +1717171T,JULIE,LOUIE,10 Quaker Street,Birstall,Manchester,,,M3,85462817,LIAM,MCQUACK,Fieldag,Southamptonshire,,,,Y73,OPG102,2023-04-16,pfa,no,SINGLE diff --git a/client/app/tests/phpunit/Form/ReportTypeTest.php b/client/app/tests/phpunit/Form/ReportTypeTest.php index 0fcf478638..1d087863cd 100644 --- a/client/app/tests/phpunit/Form/ReportTypeTest.php +++ b/client/app/tests/phpunit/Form/ReportTypeTest.php @@ -80,6 +80,6 @@ public function testSubmitInvalidYear() $this->assertFalse($form->isValid()); $this->assertCount(1, $errors); - $this->assertSame('Please enter a valid year.', $errors[0]->getMessage()); + $this->assertSame('Please enter a valid start date.', $errors[0]->getMessage()); } } From be9e2b703b0362d9c65e1dc70bf1bcdc85fee936 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Mon, 13 Jan 2025 11:49:12 +0000 Subject: [PATCH 09/10] correct maximum range for start date --- client/app/src/Form/Report/ReportType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/src/Form/Report/ReportType.php b/client/app/src/Form/Report/ReportType.php index c75308b1dc..83ebcbe7b9 100644 --- a/client/app/src/Form/Report/ReportType.php +++ b/client/app/src/Form/Report/ReportType.php @@ -21,7 +21,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) 'constraints' => [ new Range([ 'min' => (new \DateTime('now'))->modify('-7 years'), - 'max' => (new \DateTime('now'))->modify('+3 years'), + 'max' => (new \DateTime('now'))->modify('+2 years'), 'notInRangeMessage' => 'Please enter a valid start date.', 'groups' => 'start-end-dates', ]), From df4df9f7fc57eb3fc8cf8d10217581c553626131 Mon Sep 17 00:00:00 2001 From: Mia Gordon Date: Wed, 22 Jan 2025 15:57:43 +0000 Subject: [PATCH 10/10] expand condition to account for rare cases of older court order dates --- .../Constraints/YearMustBeFourDigitsAndValidValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php index 6339221092..810333a537 100644 --- a/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php +++ b/client/app/src/Validator/Constraints/YearMustBeFourDigitsAndValidValidator.php @@ -44,7 +44,7 @@ public function validate($data, Constraint $constraint) } else { $year = $courtDate->format('Y'); - if (!preg_match('/^2\d{3}$/', $year)) { + if (!preg_match('/^2\d{3}$/', $year) && !preg_match('/^19\d{2}$/', $year)) { $this->context ->buildViolation($constraint->message) ->addViolation();