feat(commercial) : messages de validation FR sur les contraintes back + garde-fou (ERP-107) (#59)
Auto Tag Develop / tag (push) Successful in 8s
Auto Tag Develop / tag (push) Successful in 8s
## Contexte Résout **ERP-107** — pendant back du mapping d'erreur par champ front (ERP-101). Le front (`useFormErrors` / `mapViolationsToRecord`) affiche sous chaque champ le `message` renvoyé par le back. Ce ticket garantit que ces messages existent, sont en FR et rattachés au bon champ. ## Changements - **Messages FR explicites** sur toutes les contraintes `#[Assert\*]` des entités métier : `Client`, `ClientContact`, `ClientAddress`, `ClientRib`, `Category`, `Role`, `User` (Email, NotBlank, Length, Bic, Iban, PositiveOrZero, Count…). - **Contraintes `Assert\Length` manquantes ajoutées**, calées sur le `length` de la colonne ORM (téléphones `VARCHAR(20)`, `siren`, `nTva`, `accountNumber`, `username`…). Évite une erreur Postgres 500 non rattachée au champ → 422 propre. - **Locale FR globale** (`symfony/translation` + `default_locale: fr`) comme filet pour les messages natifs Symfony non surchargés. - **Garde-fou** `tests/Architecture/EntityConstraintsHaveFrenchMessageTest` : échoue si une contrainte n'a pas de message FR explicite (comparaison au défaut Symfony) ou si `Assert\Length.max` diverge du `length` ORM. Whitelist justifiée pour les formats auto-bornés (Bic/Iban/Regex CP/couleur hex). - **Test fonctionnel** du JSON 422 réel : message FR + `propertyPath` consommable par le front. - **Convention documentée** dans `.claude/rules/backend.md`. ## Décisions - Stratégie retenue : message FR **explicite sur toutes** les contraintes + locale FR en filet (les deux leviers du ticket). - Garde-fou `Length == ORM length` : **test bloquant** (anti-dérive). - RG-1.03 (distributor/broker) : pas de `Assert\Callback` ajouté — le `ClientProcessor` gère **déjà** l'exclusivité (422 + `propertyPath`). Pas de doublon. ## Hors périmètre / à suivre - **Alignement `nullable`(DB) / `NotBlank`(back) / `required`(front)** : les champs obligatoires existants ont été confirmés, mais aucun changement de nullabilité DB n'a été fait sans arbitrage métier. À recroiser avec les astérisques front (ERP-101 / PR #58) si divergence constatée. ## Vérifications - `make test` : **469 tests verts** (1793 assertions), 0 échec/erreur. - `php-cs-fixer` : 0 fichier à corriger. --------- Co-authored-by: admin malio <malio@yuno.malio.fr> Co-authored-by: Matthieu <contact@malio.fr> Reviewed-on: #59 Co-authored-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr> Co-committed-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
This commit was merged in pull request #59.
This commit is contained in:
@@ -0,0 +1,363 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Architecture;
|
||||
|
||||
use Doctrine\ORM\Mapping\Column;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use ReflectionClass;
|
||||
use ReflectionProperty;
|
||||
use Symfony\Component\Finder\Finder;
|
||||
use Symfony\Component\Serializer\Attribute\Groups;
|
||||
use Symfony\Component\Validator\Constraint;
|
||||
use Symfony\Component\Validator\Constraints as Assert;
|
||||
|
||||
use function in_array;
|
||||
use function is_string;
|
||||
use function sprintf;
|
||||
use function str_contains;
|
||||
|
||||
/**
|
||||
* Garde-fou architecture ERP-107 : toute contrainte `#[Assert\*]` portee par une
|
||||
* entite metier doit avoir un message FR EXPLICITE (et non le defaut anglais de
|
||||
* Symfony), et toute colonne string bornee writable doit avoir une `Assert\Length`
|
||||
* calee sur le `length` de la colonne ORM.
|
||||
*
|
||||
* Pourquoi (lien ERP-101) : le front (useFormErrors / mapViolationsToRecord)
|
||||
* affiche sous chaque champ le `message` renvoye par le back. Un message absent
|
||||
* = defaut anglais ; une colonne bornee sans Assert\Length = erreur Postgres
|
||||
* (500) au lieu d'une 422 propre rattachee au champ.
|
||||
*
|
||||
* Deux verifications, sur le modele de AuditableEntitiesHaveI18nLabelTest :
|
||||
* 1. MESSAGE EXPLICITE : pour chaque contrainte connue, la (ou les) propriete(s)
|
||||
* de message pertinente(s) doivent differer du defaut Symfony. La comparaison
|
||||
* au defaut (instance « nue » de la meme contrainte) evite de valider un
|
||||
* message anglais natif laisse tel quel.
|
||||
* 2. LENGTH == ORM length : toute propriete string writable avec `ORM\Column(length:)`
|
||||
* doit porter `Assert\Length(max:)` egal a ce length — sauf si le format est
|
||||
* deja borne par Bic/Iban, ou whitelistee dans EXCLUDED_LENGTH_MIRROR.
|
||||
*
|
||||
* @internal
|
||||
*/
|
||||
final class EntityConstraintsHaveFrenchMessageTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* Proprietes writable exemptees du miroir Assert\Length == ORM length, avec
|
||||
* justification. Toute entree doit citer la raison (format deja borne par une
|
||||
* autre contrainte). Cle : "<ClasseCourte>::<propriete>".
|
||||
*
|
||||
* @var array<string, string>
|
||||
*/
|
||||
private const array EXCLUDED_LENGTH_MIRROR = [
|
||||
// Le Regex /^[0-9]{4,5}$/ borne deja la longueur a 5 caracteres (< 20).
|
||||
'ClientAddress::postalCode' => 'Regex {4,5} borne deja la longueur.',
|
||||
// Le Regex /^#[0-9A-Fa-f]{6}$/ borne la longueur a exactement 7 caracteres.
|
||||
'Site::color' => 'Regex code hex #RRGGBB borne deja la longueur.',
|
||||
];
|
||||
|
||||
/**
|
||||
* Mapping contrainte -> proprietes de message a verifier. Une contrainte
|
||||
* absente de ce mapping (hors Callback) fait ECHOUER le test : il faut
|
||||
* l'ajouter explicitement (anti faux positif vert sur une contrainte inconnue).
|
||||
*
|
||||
* Pour Length / Count, la liste est calculee dynamiquement (minMessage si
|
||||
* `min` est pose, maxMessage si `max` est pose).
|
||||
*
|
||||
* @var list<class-string<Constraint>>
|
||||
*/
|
||||
private const array SIMPLE_MESSAGE_CONSTRAINTS = [
|
||||
Assert\NotBlank::class,
|
||||
Assert\NotNull::class,
|
||||
Assert\Email::class,
|
||||
Assert\Regex::class,
|
||||
Assert\Bic::class,
|
||||
Assert\Iban::class,
|
||||
Assert\PositiveOrZero::class,
|
||||
Assert\Positive::class,
|
||||
Assert\NegativeOrZero::class,
|
||||
Assert\Negative::class,
|
||||
];
|
||||
|
||||
public function testEveryConstraintHasAnExplicitFrenchMessage(): void
|
||||
{
|
||||
$checked = 0;
|
||||
|
||||
foreach ($this->entityProperties() as [$shortClass, $property]) {
|
||||
foreach ($property->getAttributes() as $attribute) {
|
||||
$name = $attribute->getName();
|
||||
if (!is_subclass_of($name, Constraint::class)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Les Callback portent leur message dans la closure : hors scope.
|
||||
if (Assert\Callback::class === $name) {
|
||||
continue;
|
||||
}
|
||||
|
||||
/** @var Constraint $constraint */
|
||||
$constraint = $attribute->newInstance();
|
||||
$messageProps = $this->messagePropertiesFor($constraint);
|
||||
|
||||
self::assertNotNull(
|
||||
$messageProps,
|
||||
sprintf(
|
||||
'Contrainte non geree par le garde-fou : %s sur %s::$%s. '
|
||||
.'Ajouter sa classe au mapping de EntityConstraintsHaveFrenchMessageTest.',
|
||||
$name,
|
||||
$shortClass,
|
||||
$property->getName(),
|
||||
),
|
||||
);
|
||||
|
||||
foreach ($messageProps as $prop) {
|
||||
$actual = $constraint->{$prop} ?? null;
|
||||
$default = $this->defaultMessageFor($name, $prop);
|
||||
|
||||
self::assertTrue(
|
||||
is_string($actual) && '' !== $actual && $actual !== $default,
|
||||
sprintf(
|
||||
'La contrainte %s sur %s::$%s n\'a pas de %s FR explicite '
|
||||
.'(message absent ou laisse au defaut anglais). Cf. ERP-107.',
|
||||
$name,
|
||||
$shortClass,
|
||||
$property->getName(),
|
||||
$prop,
|
||||
),
|
||||
);
|
||||
++$checked;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self::assertGreaterThan(0, $checked, 'Aucune contrainte verifiee : detection d\'attributs cassee ?');
|
||||
}
|
||||
|
||||
public function testBoundedStringColumnsHaveMatchingLength(): void
|
||||
{
|
||||
$checked = 0;
|
||||
|
||||
foreach ($this->entityProperties() as [$shortClass, $property]) {
|
||||
$column = $this->ormColumn($property);
|
||||
if (null === $column || null === $column->length) {
|
||||
continue;
|
||||
}
|
||||
// Colonnes non-string (text, decimal, date...) : pas de length scalaire a calquer.
|
||||
if (null !== $column->type && 'string' !== $column->type) {
|
||||
continue;
|
||||
}
|
||||
// Le miroir ne protege que la saisie utilisateur (champs writable).
|
||||
if (!$this->isPropertyWritable($property)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$constraints = $this->constraintsOf($property);
|
||||
|
||||
// Format deja borne par Bic/Iban : longueur garantie cote contrainte.
|
||||
if ($this->hasAnyConstraint($constraints, [Assert\Bic::class, Assert\Iban::class])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$excludeKey = $shortClass.'::'.$property->getName();
|
||||
if (isset(self::EXCLUDED_LENGTH_MIRROR[$excludeKey])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$length = null;
|
||||
foreach ($constraints as $c) {
|
||||
if ($c instanceof Assert\Length) {
|
||||
$length = $c->max;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
self::assertNotNull(
|
||||
$length,
|
||||
sprintf(
|
||||
'%s::$%s est une colonne string bornee (length=%d) writable sans Assert\Length : '
|
||||
.'risque d\'erreur Postgres 500. Ajouter Assert\Length(max: %d) ou whitelister. Cf. ERP-107.',
|
||||
$shortClass,
|
||||
$property->getName(),
|
||||
$column->length,
|
||||
$column->length,
|
||||
),
|
||||
);
|
||||
self::assertSame(
|
||||
$column->length,
|
||||
$length,
|
||||
sprintf(
|
||||
'Derive Assert\Length.max (%s) != ORM length (%d) sur %s::$%s. '
|
||||
.'Le max doit refleter le length de la colonne (anti-derive ERP-107).',
|
||||
(string) $length,
|
||||
$column->length,
|
||||
$shortClass,
|
||||
$property->getName(),
|
||||
),
|
||||
);
|
||||
++$checked;
|
||||
}
|
||||
|
||||
self::assertGreaterThan(0, $checked, 'Aucune colonne string bornee verifiee : scan casse ?');
|
||||
}
|
||||
|
||||
/**
|
||||
* Itere (classe courte, ReflectionProperty) sur toutes les entites metier
|
||||
* sous src/Module/<m>/Domain/Entity/.
|
||||
*
|
||||
* @return iterable<array{0: string, 1: ReflectionProperty}>
|
||||
*/
|
||||
private function entityProperties(): iterable
|
||||
{
|
||||
$finder = new Finder()
|
||||
->files()
|
||||
->in(__DIR__.'/../../src/Module')
|
||||
->path('Domain/Entity')
|
||||
->name('*.php')
|
||||
;
|
||||
|
||||
self::assertNotEmpty(iterator_to_array($finder), 'Aucune entite scannee : chemin src/Module invalide ?');
|
||||
|
||||
foreach ($finder as $file) {
|
||||
$fqcn = $this->extractFqcn($file->getRealPath());
|
||||
if (null === $fqcn) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$reflection = new ReflectionClass($fqcn);
|
||||
if ($reflection->isAbstract()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach ($reflection->getProperties() as $property) {
|
||||
yield [$reflection->getShortName(), $property];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Liste des proprietes de message a verifier pour une contrainte donnee, ou
|
||||
* null si la contrainte n'est pas geree (le test echoue alors explicitement).
|
||||
*
|
||||
* @return list<string>|null
|
||||
*/
|
||||
private function messagePropertiesFor(Constraint $constraint): ?array
|
||||
{
|
||||
if ($constraint instanceof Assert\Length) {
|
||||
$props = [];
|
||||
if (null !== $constraint->min) {
|
||||
$props[] = 'minMessage';
|
||||
}
|
||||
if (null !== $constraint->max) {
|
||||
$props[] = 'maxMessage';
|
||||
}
|
||||
|
||||
return $props;
|
||||
}
|
||||
|
||||
if ($constraint instanceof Assert\Count) {
|
||||
$props = [];
|
||||
if (null !== $constraint->min) {
|
||||
$props[] = 'minMessage';
|
||||
}
|
||||
if (null !== $constraint->max) {
|
||||
$props[] = 'maxMessage';
|
||||
}
|
||||
|
||||
return $props;
|
||||
}
|
||||
|
||||
if (in_array($constraint::class, self::SIMPLE_MESSAGE_CONSTRAINTS, true)) {
|
||||
return ['message'];
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Message par defaut d'une contrainte (instance « nue ») pour la propriete
|
||||
* demandee. Sert de reference pour detecter un message laisse au defaut.
|
||||
*/
|
||||
private function defaultMessageFor(string $class, string $prop): ?string
|
||||
{
|
||||
$bare = match ($class) {
|
||||
Assert\Length::class => new Assert\Length(max: 1),
|
||||
Assert\Count::class => new Assert\Count(min: 1),
|
||||
Assert\Regex::class => new Assert\Regex(pattern: '/^x$/'),
|
||||
default => new $class(),
|
||||
};
|
||||
|
||||
$value = $bare->{$prop} ?? null;
|
||||
|
||||
return is_string($value) ? $value : null;
|
||||
}
|
||||
|
||||
private function ormColumn(ReflectionProperty $property): ?Column
|
||||
{
|
||||
$attrs = $property->getAttributes(Column::class);
|
||||
|
||||
return [] === $attrs ? null : $attrs[0]->newInstance();
|
||||
}
|
||||
|
||||
/** @return list<Constraint> */
|
||||
private function constraintsOf(ReflectionProperty $property): array
|
||||
{
|
||||
$out = [];
|
||||
foreach ($property->getAttributes() as $attribute) {
|
||||
if (is_subclass_of($attribute->getName(), Constraint::class)) {
|
||||
$out[] = $attribute->newInstance();
|
||||
}
|
||||
}
|
||||
|
||||
return $out;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<Constraint> $constraints
|
||||
* @param list<class-string<Constraint>> $classes
|
||||
*/
|
||||
private function hasAnyConstraint(array $constraints, array $classes): bool
|
||||
{
|
||||
foreach ($constraints as $c) {
|
||||
if (in_array($c::class, $classes, true)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private function isPropertyWritable(ReflectionProperty $property): bool
|
||||
{
|
||||
$attrs = $property->getAttributes(Groups::class);
|
||||
if ([] === $attrs) {
|
||||
return false;
|
||||
}
|
||||
|
||||
/** @var Groups $groups */
|
||||
$groups = $attrs[0]->newInstance();
|
||||
foreach ($groups->groups as $group) {
|
||||
if (is_string($group) && str_contains($group, 'write')) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private function extractFqcn(string $path): ?string
|
||||
{
|
||||
$source = file_get_contents($path);
|
||||
if (false === $source) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (
|
||||
1 !== preg_match('/^namespace\s+([^;]+);/m', $source, $nsMatch)
|
||||
|| 1 !== preg_match('/^(?:final\s+|abstract\s+|readonly\s+)*class\s+(\w+)/m', $source, $classMatch)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return trim($nsMatch[1]).'\\'.$classMatch[1];
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user