From 41d76542b4682443e37da4f5a559baf695118313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 1 Sep 2021 16:06:20 +0200 Subject: [PATCH] move validation on person into annotations --- .../DependencyInjection/Configuration.php | 32 +++---- .../ChillPersonBundle/Entity/Person.php | 84 +++++++++++++++++-- .../Person/BirthdateValidatorTest.php | 2 + .../Validator/Person/PersonValidationTest.php | 55 ++++++++++++ .../Constraints/Person/Birthdate.php | 4 +- .../Constraints/Person/BirthdateValidator.php | 1 + .../Constraints/Person/PersonHasCenter.php | 3 + .../ChillPersonBundle/config/services.yaml | 14 ++-- .../ChillPersonBundle/config/validation.yaml | 67 --------------- 9 files changed, 165 insertions(+), 97 deletions(-) create mode 100644 src/Bundle/ChillPersonBundle/Tests/Validator/Person/PersonValidationTest.php diff --git a/src/Bundle/ChillPersonBundle/DependencyInjection/Configuration.php b/src/Bundle/ChillPersonBundle/DependencyInjection/Configuration.php index 7daec4f63..bf2d18922 100644 --- a/src/Bundle/ChillPersonBundle/DependencyInjection/Configuration.php +++ b/src/Bundle/ChillPersonBundle/DependencyInjection/Configuration.php @@ -43,26 +43,26 @@ class Configuration implements ConfigurationInterface ->arrayNode('validation') ->canBeDisabled() ->children() - ->scalarNode('birthdate_not_after') - ->info($this->validationBirthdateNotAfterInfos) - ->defaultValue('P1D') - ->validate() - ->ifTrue(function($period) { - try { - $interval = new \DateInterval($period); - } catch (\Exception $ex) { - return true; - } - return false; - }) - ->thenInvalid('Invalid period for birthdate validation : "%s" ' - . 'The parameter should match duration as defined by ISO8601 : ' - . 'https://en.wikipedia.org/wiki/ISO_8601#Durations') - ->end() // birthdate_not_after, parent = children of validation ->booleanNode('center_required') ->info('Enable a center for each person entity. If disabled, you must provide your own center provider') ->defaultValue(true) ->end() + ->scalarNode('birthdate_not_after') + ->info($this->validationBirthdateNotAfterInfos) + ->defaultValue('P1D') + ->validate() + ->ifTrue(function($period) { + try { + $interval = new \DateInterval($period); + } catch (\Exception $ex) { + return true; + } + return false; + }) + ->thenInvalid('Invalid period for birthdate validation : "%s" ' + . 'The parameter should match duration as defined by ISO8601 : ' + . 'https://en.wikipedia.org/wiki/ISO_8601#Durations') + ->end() // birthdate_not_after, parent = children of validation ->end() // children for 'validation', parent = validation ->end() //validation, parent = children of root ->end() // children of root, parent = root diff --git a/src/Bundle/ChillPersonBundle/Entity/Person.php b/src/Bundle/ChillPersonBundle/Entity/Person.php index 5bfe3bd10..f42ca6f00 100644 --- a/src/Bundle/ChillPersonBundle/Entity/Person.php +++ b/src/Bundle/ChillPersonBundle/Entity/Person.php @@ -43,6 +43,11 @@ use Doctrine\Common\Collections\Criteria; use Symfony\Component\Validator\Context\ExecutionContextInterface; use Symfony\Component\Serializer\Annotation\DiscriminatorMap; use Chill\PersonBundle\Entity\Household\PersonHouseholdAddress; +use Symfony\Component\Validator\Constraints as Assert; +use Chill\PersonBundle\Validator\Constraints\Person\Birthdate; +use Chill\MainBundle\Validation\Constraint\PhonenumberConstraint; +use Chill\PersonBundle\Validator\Constraints\Person\PersonHasCenter; +use Chill\PersonBundle\Validator\Constraints\Household\HouseholdMembershipSequential; /** * Person Class @@ -57,6 +62,12 @@ use Chill\PersonBundle\Entity\Household\PersonHouseholdAddress; * @DiscriminatorMap(typeProperty="type", mapping={ * "person"=Person::class * }) + * @PersonHasCenter( + * groups={"general", "creation"} + * ) + * @HouseholdMembershipSequential( + * groups={"household_memberships"} + * ) */ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateInterface { @@ -75,6 +86,13 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="string", length=255) + * @Assert\NotBlank( + * groups={"general", "creation"} + * ) + * @Assert\Length( + * max=255, + * groups={"general", "creation"} + * ) */ private $firstName; @@ -83,6 +101,13 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="string", length=255) + * @Assert\NotBlank( + * groups={"general", "creation"} + * ) + * @Assert\Length( + * max=255, + * groups={"general", "creation"} + * ) */ private $lastName; @@ -102,6 +127,12 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var \DateTime * * @ORM\Column(type="date", nullable=true) + * @Assert\Date( + * groups={"general", "creation"} + * ) + * @Birthdate( + * groups={"general", "creation"} + * ) */ private $birthdate; @@ -110,6 +141,9 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var \DateTimeImmutable * * @ORM\Column(type="date_immutable", nullable=true) + * @Assert\Date( + * groups={"general", "creation"} + * ) */ private ?\DateTimeImmutable $deathdate; @@ -150,6 +184,9 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="string", length=9, nullable=true) + * @Assert\NotNull( + * groups={"general", "creation"} + * ) */ private $gender; @@ -179,8 +216,11 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var \DateTime * * @ORM\Column(type="date", nullable=true) + * @Assert\Date( + * groups={"general", "creation"} + * ) */ - private $maritalStatusDate; + private ?\DateTime $maritalStatusDate; /** * Comment on marital status @@ -202,6 +242,10 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="text", nullable=true) + * @Assert\Email( + * checkMX=true, + * groups={"general", "creation"} + * ) */ private $email = ''; @@ -210,6 +254,14 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="text", length=40, nullable=true) + * @Assert\Regex( + * pattern="/^([\+{1}])([0-9\s*]{4,20})$/", + * groups={"general", "creation"} + * ) + * @PhonenumberConstraint( + * type="landline", + * groups={"general", "creation"} + * ) */ private $phonenumber = ''; @@ -218,6 +270,14 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * @var string * * @ORM\Column(type="text", length=40, nullable=true) + * @Assert\Regex( + * pattern="/^([\+{1}])([0-9\s*]{4,20})$/", + * groups={"general", "creation"} + * ) + * @PhonenumberConstraint( + * type="mobile", + * groups={"general", "creation"} + * ) */ private $mobilenumber = ''; @@ -230,12 +290,13 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * cascade={"persist", "remove", "merge", "detach"}, * orphanRemoval=true * ) + * @Assert\Valid( + * traverse=true, + * groups={"general", "creation"} + * ) */ private $otherPhoneNumbers; - //TO-ADD caseOpeningDate - //TO-ADD nativeLanguag - /** * The person's spoken languages * @var ArrayCollection @@ -352,6 +413,8 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI private $addresses; /** + * fullname canonical. Read-only field, which is calculated by + * the database. * @var string * * @ORM\Column(type="text", nullable=true) @@ -372,6 +435,8 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI private array $currentHouseholdAt = []; /** + * Read-only field, computed by the database + * * @ORM\OneToMany( * targetEntity=PersonHouseholdAddress::class, * mappedBy="person" @@ -389,8 +454,6 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI /** * Person constructor. - * - * @param \DateTime|null $opening */ public function __construct() { @@ -403,6 +466,7 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI $this->householdAddresses = new ArrayCollection(); $this->genderComment = new CommentEmbeddable(); $this->maritalStatusComment = new CommentEmbeddable(); + $this->periodLocatedOn = new ArrayCollection(); } /** @@ -1200,6 +1264,10 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * Validation callback that checks if the accompanying periods are valid * * This method add violation errors. + * + * @Assert\Callback( + * groups={"accompanying_period_consistent"} + * ) */ public function isAccompanyingPeriodValid(ExecutionContextInterface $context) { @@ -1245,6 +1313,10 @@ class Person implements HasCenterInterface, TrackCreationInterface, TrackUpdateI * two addresses with the same validFrom date) * * This method add violation errors. + * + * @Assert\Callback( + * groups={"addresses_consistent"} + * ) */ public function isAddressesValid(ExecutionContextInterface $context) { diff --git a/src/Bundle/ChillPersonBundle/Tests/Validator/Person/BirthdateValidatorTest.php b/src/Bundle/ChillPersonBundle/Tests/Validator/Person/BirthdateValidatorTest.php index 2e7df4817..6c726dd1c 100644 --- a/src/Bundle/ChillPersonBundle/Tests/Validator/Person/BirthdateValidatorTest.php +++ b/src/Bundle/ChillPersonBundle/Tests/Validator/Person/BirthdateValidatorTest.php @@ -38,6 +38,7 @@ class BirthdateValidatorTest extends ConstraintValidatorTestCase $this->validator->validate($bornToday, $this->createConstraint()); $this->buildViolation('msg') ->setParameter('%date%', (new \DateTime('yesterday'))->format('d-m-Y')) + ->setCode('3f42fd96-0b2d-11ec-8cf3-0f3b1b1ca1c4') ->assertRaised(); } @@ -54,6 +55,7 @@ class BirthdateValidatorTest extends ConstraintValidatorTestCase $this->validator->validate($bornAfter, $this->createConstraint()); $this->buildViolation('msg') ->setParameter('%date%', (new \DateTime('yesterday'))->format('d-m-Y')) + ->setCode('3f42fd96-0b2d-11ec-8cf3-0f3b1b1ca1c4') ->assertRaised(); } diff --git a/src/Bundle/ChillPersonBundle/Tests/Validator/Person/PersonValidationTest.php b/src/Bundle/ChillPersonBundle/Tests/Validator/Person/PersonValidationTest.php new file mode 100644 index 000000000..8d6c930d9 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/Tests/Validator/Person/PersonValidationTest.php @@ -0,0 +1,55 @@ +validator = self::$container->get(ValidatorInterface::class); + } + + public function testFirstnameValidation() + { + $person = (new Person()) + ->setFirstname(\str_repeat('a', 500)); + $errors = $this->validator->validate($person, null, ["creation"]); + foreach ($errors->getIterator() as $error) { + if (Length::TOO_LONG_ERROR === $error->getCode()) { + $this->assertTrue(true, + "error code for firstname too long is present"); + return; + } + } + $this->assertTrue(false, + "error code for fistname too long is present"); + + } + + public function testBirthdateInFuture() + { + $person = (new Person()) + ->setBirthdate(new \Datetime('+2 months')); + $errors = $this->validator->validate($person, null, ["creation"]); + foreach ($errors->getIterator() as $error) { + if (Birthdate::BIRTHDATE_INVALID_CODE === $error->getCode()) { + $this->assertTrue(true, + "error code for birthdate invalid is present"); + return; + } + } + $this->assertTrue(false, + "error code for birthdate invalid is present"); + + } + +} diff --git a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/Birthdate.php b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/Birthdate.php index b8cf6d8c2..20f3ed97b 100644 --- a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/Birthdate.php +++ b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/Birthdate.php @@ -29,9 +29,11 @@ use Symfony\Component\Validator\Constraint; * (this interval_spec itself is based on ISO8601 : * https://en.wikipedia.org/wiki/ISO_8601#Durations) * - * @author Julien Fastré + * @Annotation */ class Birthdate extends Constraint { + public const BIRTHDATE_INVALID_CODE = '3f42fd96-0b2d-11ec-8cf3-0f3b1b1ca1c4'; + public $message = "The birthdate must be before %date%"; } diff --git a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/BirthdateValidator.php b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/BirthdateValidator.php index 5d16be230..238a3d160 100644 --- a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/BirthdateValidator.php +++ b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/BirthdateValidator.php @@ -53,6 +53,7 @@ class BirthdateValidator extends ConstraintValidator if ($limitDate < $value) { $this->context->buildViolation($constraint->message) ->setParameter('%date%', $limitDate->format('d-m-Y')) + ->setCode(Birthdate::BIRTHDATE_INVALID_CODE) ->addViolation(); } diff --git a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/PersonHasCenter.php b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/PersonHasCenter.php index c6c8c7c14..5cabd8838 100644 --- a/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/PersonHasCenter.php +++ b/src/Bundle/ChillPersonBundle/Validator/Constraints/Person/PersonHasCenter.php @@ -2,6 +2,9 @@ namespace Chill\PersonBundle\Validator\Constraints\Person; +/** + * @Annotation + */ class PersonHasCenter extends \Symfony\Component\Validator\Constraint { public string $message = "A center is required"; diff --git a/src/Bundle/ChillPersonBundle/config/services.yaml b/src/Bundle/ChillPersonBundle/config/services.yaml index c2f60a729..bf4b8ddd8 100644 --- a/src/Bundle/ChillPersonBundle/config/services.yaml +++ b/src/Bundle/ChillPersonBundle/config/services.yaml @@ -62,18 +62,18 @@ services: - { name: security.voter } - { name: chill.role } - chill.person.birthdate_validation: - class: Chill\PersonBundle\Validator\Constraints\BirthdateValidator + Chill\PersonBundle\Validator\Constraints\: + autowire: true + autoconfigure: true + resource: '../Validator/Constraints/' + + # override default config, must be loaded after resource + Chill\PersonBundle\Validator\Constraints\BirthdateValidator: arguments: - "%chill_person.validation.birtdate_not_before%" tags: - { name: validator.constraint_validator, alias: birthdate_not_before } - Chill\PersonBundle\Validator\Constraints\Household\HouseholdMembershipSequentialValidator: - autowire: true - tags: - - { name: validator.constraint_validator } - Chill\PersonBundle\Repository\: autowire: true autoconfigure: true diff --git a/src/Bundle/ChillPersonBundle/config/validation.yaml b/src/Bundle/ChillPersonBundle/config/validation.yaml index 23369a467..c41513778 100644 --- a/src/Bundle/ChillPersonBundle/config/validation.yaml +++ b/src/Bundle/ChillPersonBundle/config/validation.yaml @@ -1,70 +1,3 @@ -Chill\PersonBundle\Entity\Person: - properties: - firstName: - - NotBlank: - groups: [general, creation] - - Length: - min: 2 - max: 255 - minMessage: 'This name is too short. It must containt {{ limit }} chars' - maxMessage: 'This name is too long. It must containt {{ limit }} chars' - groups: [general, creation] - lastName: - - NotBlank: - groups: [general, creation] - - Length: - min: 2 - max: 255 - minMessage: 'This name is too short. It must containt {{ limit }} chars' - maxMessage: 'This name is too long. It must containt {{ limit }} chars' - groups: [general, creation] - birthdate: - - Date: - message: 'Birthdate not valid' - groups: [general, creation] - - Chill\PersonBundle\Validator\Constraints\Birthdate: - groups: [general, creation] - gender: - - NotNull: - groups: [general, creation] - #accompanyingPeriods: - # - Valid: - # traverse: true - email: - - Email: - groups: [general, creation] - message: 'The email is not valid' - checkMX: true - phonenumber: - - Regex: - pattern: '/^([\+{1}])([0-9\s*]{4,20})$/' - groups: [general, creation] - message: 'Invalid phone number: it should begin with the international prefix starting with "+", hold only digits and be smaller than 20 characters. Ex: +33123456789' - - Chill\MainBundle\Validation\Constraint\PhonenumberConstraint: - type: landline - groups: [ general, creation ] - mobilenumber: - - Regex: - pattern: '/^([\+{1}])([0-9\s*]{4,20})$/' - groups: [general, creation] - message: 'Invalid phone number: it should begin with the international prefix starting with "+", hold only digits and be smaller than 20 characters. Ex: +33623456789' - - Chill\MainBundle\Validation\Constraint\PhonenumberConstraint: - type: mobile - groups: [ general, creation ] - otherPhoneNumbers: - - Valid: - traverse: true - constraints: - - Callback: - callback: isAccompanyingPeriodValid - groups: [accompanying_period_consistent] - - Callback: - callback: isAddressesValid - groups: [addresses_consistent] - - Chill\PersonBundle\Validator\Constraints\Household\HouseholdMembershipSequential: - groups: [ 'household_memberships' ] - - Chill\PersonBundle\Entity\AccompanyingPeriod: properties: openingDate: