diff --git a/docs/sites/ticket-01-spec.md b/docs/sites/ticket-01-spec.md new file mode 100644 index 0000000..68c43a9 --- /dev/null +++ b/docs/sites/ticket-01-spec.md @@ -0,0 +1,410 @@ +# Ticket #01 — 1/4 — Brique fondatrice du module Sites (Backend) + +## 1. Objectif + +Ce ticket livre la couche de donnees du module optionnel Sites. Il cree le bounded context, declare le module a Symfony, enregistre ses permissions RBAC, installe la table `site` en base et seed trois etablissements de demonstration utilises par les tickets suivants. + +Le resultat attendu est un socle de persistance activable par tenant via `config/modules.php`, sans UI, sans API publique, sans couplage au module Core, et sur lequel les tickets 2/3/4 pourront greffer : rattachement utilisateurs, selecteur de site dans la navbar, administration CRUD. + +## 2. Périmètre + +### IN + +- Creer le module `/home/m-tristan/workspace/Coltura/src/Module/Sites/SitesModule.php` avec `ID = 'sites'`, `LABEL = 'Sites'`, `REQUIRED = false`, et une methode statique `permissions()` declarant les deux codes RBAC `sites.view` et `sites.manage`. +- Creer l'entite Doctrine `Site` avec `id`, `name` (unique), `city`, `postalCode`, `color`, `fullAddress`, `createdAt`, `updatedAt` et les contraintes de validation applicatives associees (NotBlank, Length, Regex hex `#RRGGBB`, Regex CP FR `^\d{5}$`, UniqueEntity). +- Creer l'interface `SiteRepositoryInterface` et son implementation Doctrine `DoctrineSiteRepository`, avec un contrat CRUD complet (`findById`, `findByName`, `findAllOrderedByName`, `save`, `remove`) en anticipation du ticket 2. +- Creer une migration Doctrine creant la table `site` avec son index unique `uniq_site_name`. La migration est placee dans `/home/m-tristan/workspace/Coltura/migrations/` au namespace racine `DoctrineMigrations` conformement a l'exception documentee dans `CLAUDE.md` (bug de tri alphabetique des migrations multi-namespaces dans Doctrine Migrations 3.x). +- Creer `SitesFixtures` creant trois sites de demonstration : `Chatellerault` (`#056CF2`), `Saint-Jean` (`#10B981`), `Pommevic` (`#F59E0B`). Fixtures idempotentes via lookup par nom lorsque le purger Doctrine est desactive. +- Enregistrer `SitesModule::class` dans `/home/m-tristan/workspace/Coltura/config/modules.php` pour l'activer par defaut. +- Declarer le mapping Doctrine du module dans `/home/m-tristan/workspace/Coltura/config/packages/doctrine.yaml` (inconditionnel, le mapping reste charge meme si le module est retire de `modules.php`). +- Enregistrer l'alias service `SiteRepositoryInterface → DoctrineSiteRepository` dans `/home/m-tristan/workspace/Coltura/config/services.yaml`. +- Ajouter deux suites de tests PHPUnit : + - `SiteTest` (pure `TestCase`) pour le comportement de l'entite (constructeur, getters/setters, lifecycle `PreUpdate`). + - `SiteValidationTest` (`KernelTestCase`) pour la validation complete : regex hex, regex CP FR, NotBlank, Length, UniqueEntity via Doctrine. + +### OUT + +- Ticket `#02` : relation `User ↔ Site` (FK ou ManyToMany selon decision UX), expose les sites de l'utilisateur courant via `/api/me` et propage l'autorisation au niveau des ressources decoupees par site. +- Ticket `#03` : integration dans la navbar Coltura (selecteur de site actif, persistance du choix cote front, consommation du flux issu du ticket 2). +- Ticket `#04` : ecran d'administration CRUD des sites (page admin/sites, DataTable, drawer creation/edition, modale suppression, API Platform `Site` resource avec voters RBAC). +- Gestion des soft-deletes sur `Site` : non introduite dans ce ticket. +- Rattachement historique ou audit trail des modifications : hors scope. + +## 3. Fichiers à créer + +### Domaine — Entité + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Domain/Entity/Site.php` : entite Doctrine porteuse des attributs metier (nom unique, ville, code postal FR, couleur hex, adresse complete multi-ligne) et des timestamps auto-maintenus via lifecycle callbacks. + +### Domaine — Repository + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Domain/Repository/SiteRepositoryInterface.php` : contrat d'acces domaine a l'entite Site (CRUD applicatif ; l'acces API Platform du ticket 4 utilisera le provider Doctrine par defaut). + +### Infrastructure — Doctrine + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/Doctrine/DoctrineSiteRepository.php` : implementation Doctrine de `SiteRepositoryInterface` basee sur `ServiceEntityRepository`. + +### Infrastructure — Migration + +- `/home/m-tristan/workspace/Coltura/migrations/Version.php` : migration racine (namespace `DoctrineMigrations`) qui cree la table `site` et son index unique. Emplacement racine et non modulaire, cf. exception documentee dans `CLAUDE.md` (bug Doctrine 3.x sur le tri alphabetique des migrations multi-namespaces). + +### Infrastructure — DataFixtures + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php` : fixture Doctrine seedant les 3 sites de demonstration. Ne declare pas de `DependentFixtureInterface` (aucune dependance a AppFixtures dans ce ticket). + +### Module — Declaration + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/SitesModule.php` : marker class du module avec `ID`, `LABEL`, `REQUIRED` et `permissions()`. Meme pattern que `CoreModule`. + +### Tests + +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Domain/Entity/SiteTest.php` : tests unitaires purs (`TestCase`) couvrant constructeur, getters, setters et lifecycle `PreUpdate`. +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Domain/Entity/SiteValidationTest.php` : tests de validation (`KernelTestCase`) couvrant regex hex, regex CP FR, NotBlank, Length sur tous les champs, et `UniqueEntity` via la DB de test. + +## 4. Fichiers à modifier + +- `/home/m-tristan/workspace/Coltura/config/modules.php` : ajouter `App\Module\Sites\SitesModule::class` dans le tableau de retour. Le module est actif par defaut. Le commenter suffit a le desactiver sans autre intervention (les permissions deviendront orphelines a la prochaine sync mais la table reste). +- `/home/m-tristan/workspace/Coltura/config/packages/doctrine.yaml` : ajouter une mapping `Sites:` alignee sur le pattern du module `Core:`. Le mapping est inconditionnel : il reste declare meme si `SitesModule::class` est retire de `modules.php`. Le commentaire doit etre explicite sur cette decoupe (activation fonctionnelle via `modules.php`, structure DB via la mapping Doctrine). +- `/home/m-tristan/workspace/Coltura/config/services.yaml` : ajouter l'alias `App\Module\Sites\Domain\Repository\SiteRepositoryInterface` → `App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository`. Pattern aligne sur les trois aliases Core existants. + +## 5. Schéma cible — mapping Doctrine + +Comme pour le ticket RBAC (ticket-343), le schema est decrit par les attributs Doctrine plutot que par le SQL brut. Le fichier de migration contient le SQL final (section 6). + +### Conventions respectées + +- `declare(strict_types=1)` en tete de tous les fichiers PHP. +- Identifiants de classe et proprietes en anglais, commentaires en francais (cf. `CLAUDE.md`). +- PostgreSQL : noms de colonnes en snake_case minuscules, Doctrine les deduit des proprietes camelCase (`postalCode` → `postal_code`, `fullAddress` → `full_address`, `createdAt` → `created_at`, `updatedAt` → `updated_at`). +- Le nom de table `site` n'est pas un mot reserve PostgreSQL : pas de backtick necessaire. + +### Entité `Site` + +```php +#[ORM\Entity(repositoryClass: DoctrineSiteRepository::class)] +#[ORM\Table(name: 'site')] +#[ORM\UniqueConstraint(name: 'uniq_site_name', columns: ['name'])] +#[ORM\HasLifecycleCallbacks] +#[UniqueEntity(fields: ['name'], message: 'Un site avec ce nom existe deja.')] +class Site +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column] + private ?int $id = null; + + #[ORM\Column(length: 100)] + #[Assert\NotBlank(message: 'Le nom du site est requis.')] + #[Assert\Length(max: 100, ...)] + private string $name; + + #[ORM\Column(length: 100)] + #[Assert\NotBlank(message: 'La ville du site est requise.')] + #[Assert\Length(max: 100, ...)] + private string $city; + + #[ORM\Column(name: 'postal_code', length: 10)] + #[Assert\NotBlank(message: 'Le code postal est requis.')] + #[Assert\Length(max: 10, ...)] + #[Assert\Regex(pattern: '/^\d{5}$/', message: '...')] + private string $postalCode; + + #[ORM\Column(length: 7)] + #[Assert\NotBlank(message: 'La couleur est requise.')] + #[Assert\Regex(pattern: '/^#[0-9A-Fa-f]{6}$/', message: '...')] + private string $color; + + #[ORM\Column(name: 'full_address', type: Types::TEXT)] + #[Assert\NotBlank(message: 'L\'adresse complete est requise.')] + #[Assert\Length(max: 500, ...)] + private string $fullAddress; + + #[ORM\Column(name: 'created_at', type: Types::DATETIME_IMMUTABLE)] + private DateTimeImmutable $createdAt; + + #[ORM\Column(name: 'updated_at', type: Types::DATETIME_IMMUTABLE)] + private DateTimeImmutable $updatedAt; +} +``` + +Contraintes fonctionnelles : +- `name` est unique en base (`uniq_site_name`) et porte egalement la contrainte applicative `UniqueEntity` pour que le validator remonte une violation lisible avant d'atteindre la violation DB. +- `color` est contraint par regex a un code hex strict de 7 caracteres `#RRGGBB`, majuscules ou minuscules. La colonne `VARCHAR(7)` est dimensionnee au plus juste car la regex est exhaustive. +- `postalCode` est contraint a 5 chiffres exacts via regex (format FR). La colonne `VARCHAR(10)` est volontairement plus large que la regex pour laisser marge si le projet etend plus tard la regex a d'autres formats (UK, PT, ...). Choix assume : evite une migration DDL au ticket suivant, cout DB negligeable sur un champ court. +- `fullAddress` est de type `TEXT` (PostgreSQL) pour permettre une adresse multi-ligne, mais borne cote applicatif a 500 caracteres via `Assert\Length(max: 500)` comme garde DoS basique (une adresse FR complete tient largement dans cette enveloppe). +- `createdAt` est seede dans le constructeur et **ne change plus jamais** apres persistance. +- `updatedAt` est seede dans le constructeur a la meme valeur que `createdAt`, puis refresh a chaque update via le callback `#[ORM\PreUpdate]`. + +### Mapping Doctrine — `doctrine.yaml` + +```yaml +# Mapping inconditionnelle du module Sites : la structure DB existe meme +# si SitesModule::class est retire de config/modules.php. L'activation +# fonctionnelle (ex: exposition des permissions, futurs endpoints API) +# passe exclusivement par config/modules.php. +Sites: + type: attribute + is_bundle: false + dir: '%kernel.project_dir%/src/Module/Sites/Domain/Entity' + prefix: 'App\Module\Sites\Domain\Entity' + alias: Sites +``` + +## 6. Plan de migration Doctrine + +La migration est placee dans `/home/m-tristan/workspace/Coltura/migrations/Version.php` au namespace racine `DoctrineMigrations`, conformement a l'exception documentee dans `CLAUDE.md`. Tant que le bug de tri alphabetique des `MigrationsComparator` multi-namespaces n'est pas resolu (via un comparator custom ou un upgrade Doctrine), toute migration d'initialisation (creation de table sur base vide) reste au namespace racine. + +### `up()` — ordre des instructions + +1. Creer la table `site` avec toutes les colonnes NOT NULL : + - `id INT GENERATED BY DEFAULT AS IDENTITY NOT NULL` + - `name VARCHAR(100) NOT NULL` + - `city VARCHAR(100) NOT NULL` + - `postal_code VARCHAR(10) NOT NULL` + - `color VARCHAR(7) NOT NULL` + - `full_address TEXT NOT NULL` + - `created_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL` + - `updated_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL` + - `PRIMARY KEY (id)` +2. Creer l'index unique `uniq_site_name` sur `site(name)` pour garantir l'invariant metier "un site porte un nom unique" au niveau DB. Le validator applicatif `UniqueEntity` s'appuie dessus en lecture avant qu'une tentative d'insertion concurrente ne remonte la violation DB. + +### `down()` — rollback + +1. `DROP TABLE site`. Aucune FK n'existe depuis ou vers cette table dans ce ticket ; le rollback est donc trivial et safe. + +### Precision timestamp + +PostgreSQL `TIMESTAMP(0) WITHOUT TIME ZONE` stocke a la seconde pres. Les DateTimeImmutable PHP portent une precision microseconde mais perdent cette precision au round-trip DB. Les tests unitaires de lifecycle doivent en tenir compte (cf. section 10 — usage de reflection plutot qu'un `sleep`). + +## 7. Intégration avec sync-permissions + +Le ticket ne modifie pas `SyncPermissionsCommand`. Il exploite l'algorithme existant (cf. ticket-343 section 7) en declarant `SitesModule::permissions()` dans un format strictement conforme au contrat attendu par la commande : + +```php +public static function permissions(): array +{ + return [ + ['code' => 'sites.view', 'label' => 'Voir les sites'], + ['code' => 'sites.manage', 'label' => 'Gerer les sites (creer, editer, supprimer)'], + ]; +} +``` + +Regles de validation appliquees par `SyncPermissionsCommand` : +- Chaque entree doit contenir exactement les cles `code` et `label`. +- Le prefixe du code doit correspondre a `SitesModule::ID . '.'`, soit `sites.`. +- Ni `code` ni `label` ne peuvent etre une chaine vide. + +Comportement a attendre : +- Apres `php bin/console app:sync-permissions`, les deux lignes `sites.view` et `sites.manage` sont presentes dans la table `permission` avec `module = 'sites'` et `orphan = false`. +- Si `SitesModule::class` est retire de `config/modules.php` et la commande relancee, les deux lignes sont marquees `orphan = true` (non supprimees, pour preserver les assignations). Reactiver le module les remet a `orphan = false`. +- La cle `module` n'est **pas** presente dans le payload : elle est auto-injectee par la commande depuis `SitesModule::ID`. + +### Granularité des permissions + +`sites.manage` est une permission **composite** couvrant creation, edition et suppression. Ce choix reste simple pour un ticket fondateur, mais le ticket 4 (administration CRUD) devra arbitrer si une granularite plus fine (`sites.create`, `sites.edit`, `sites.delete`) est necessaire pour les besoins UX. Si oui, la migration de permissions se fera naturellement via la commande de sync : ajouter les trois codes dans `permissions()`, retirer `sites.manage` → la sync marque l'ancien orphelin sans casser les roles deja existants. + +## 8. Méthodes clés détaillées + +### `Site::__construct` + +Le constructeur prend les cinq champs metier obligatoires et positionne les deux timestamps a la meme valeur : + +```php +public function __construct( + string $name, + string $city, + string $postalCode, + string $color, + string $fullAddress, +) { + $this->name = $name; + $this->city = $city; + $this->postalCode = $postalCode; + $this->color = $color; + $this->fullAddress = $fullAddress; + $now = new DateTimeImmutable(); + $this->createdAt = $now; + $this->updatedAt = $now; +} +``` + +Justification : +- Tous les champs sont passes au constructeur pour forcer l'invariant "un Site instancie est toujours complet". L'alternative (setters post-new) autoriserait des etats transitoires invalides. +- `createdAt` et `updatedAt` partagent la meme valeur a l'instanciation, ce qui garantit `updated_at >= created_at` au niveau base. Le premier appel a `onPreUpdate()` fera avancer uniquement `updatedAt`. + +### `Site::onPreUpdate` + +```php +#[ORM\PreUpdate] +public function onPreUpdate(): void +{ + $this->updatedAt = new DateTimeImmutable(); +} +``` + +Justification : +- Callback Doctrine declenche **uniquement** quand Doctrine detecte au moins un changement sur l'entite en session de persistance. Pas de risque de tick silencieux sur un find pur. +- `createdAt` n'est volontairement jamais touche ici : il est immuable apres persistance. +- Pas de `#[ORM\PrePersist]` : le constructeur gere deja l'initialisation, inutile de dupliquer la logique dans un callback qui pourrait etre appele a vide. + +### `SitesFixtures::ensureSite` + +```php +private function ensureSite( + ObjectManager $manager, + string $name, + string $city, + string $postalCode, + string $color, + string $fullAddress, +): Site { + $site = $this->siteRepository->findByName($name); + + if (null === $site) { + $site = new Site($name, $city, $postalCode, $color, $fullAddress); + $manager->persist($site); + + return $site; + } + + $site->setCity($city); + $site->setPostalCode($postalCode); + $site->setColor($color); + $site->setFullAddress($fullAddress); + + return $site; +} +``` + +Contrat honnete sur l'idempotence (cf. docblock en tete de fixture) : +- **Supporte** : lookup par nom avec purger Doctrine actif (cas nominal de `doctrine:fixtures:load`). +- **Supporte** : lookup par nom hors purger si la fixture est rejouee telle quelle sur une base deja seedee → les autres champs sont re-alignes sur les valeurs de reference. +- **Non supporte** : chargement cumulatif apres qu'une autre fixture ait `persist` (sans `flush`) des Site dans la meme session → `findByName` via `findOneBy` n'inspecte pas l'unit-of-work et peut creer un doublon. +- **Non supporte** : renommage d'un site dans la fixture → le lookup par `name` rate, un nouveau site est cree, l'ancien reste en base si le purger est desactive. + +## 9. Fixtures Sites + +Trois sites de demonstration, avec des couleurs distinctes suffisamment contrastees pour un futur affichage visuel (ticket 3 — navbar) : + +| Nom | Ville | CP | Couleur | Commentaire | +|-----|-------|-----|---------|-------------| +| Chatellerault | Chatellerault | 86100 | `#056CF2` | Couleur imposee par le ticket (bleu Coltura). | +| Saint-Jean | Saint-Jean-de-Sauves | 86330 | `#10B981` | Vert emeraude (contraste avec le bleu). | +| Pommevic | Pommevic | 82400 | `#F59E0B` | Ambre (troisieme teinte nettement distincte). | + +Les adresses completes sont des chaines multi-lignes (voie + CP/ville), cas nominal d'exploitation du type `TEXT` sur `full_address`. + +### Ordre d'execution global des fixtures + +`SitesFixtures` est une `Fixture` sans dependance : elle peut s'executer dans n'importe quel ordre relatif aux autres fixtures Core (`AppFixtures`). Aucune FK inter-modules dans ce ticket. + +Le ticket 2 introduira probablement une relation `User ↔ Site` ; `SitesFixtures` devra alors etre declare comme dependance de `AppFixtures` (ou inversement, selon la direction de la FK) via `DependentFixtureInterface::getDependencies()`. + +## 10. Plan de tests PHPUnit + +Deux suites separees, motivation : +- `SiteTest` reste en `TestCase` pur (pas de kernel) pour tester le comportement mecanique de l'entite — rapide, zero dependance DB. +- `SiteValidationTest` utilise `KernelTestCase` pour avoir acces au validator applicatif, **indispensable** pour tester `UniqueEntity` dont le validator est backed par Doctrine et necessite donc un `ManagerRegistry` reel. + +### `SiteTest` — tests unitaires purs + +1. `testConstructorInitialState` : verifie que le constructeur positionne correctement les 5 champs metier et les deux timestamps (`DateTimeImmutable`). +2. `testCreatedAtAndUpdatedAtAreInitiallyEqual` : verifie l'invariant "a l'instanciation, `createdAt == updatedAt`". +3. `testOnPreUpdateAdvancesUpdatedAtOnly` : utilise `Reflection` pour forcer `updatedAt` a une valeur anterieure (`-1 hour`), appelle `onPreUpdate()`, et verifie que `updatedAt` avance strictement mais que `createdAt` reste immuable. + - **Justification reflection** : eviter un `sleep/usleep` flaky en CI et lent. +4. `testSettersMutateFields` : verifie que les setters publics modifient correctement les champs metier. + +### `SiteValidationTest` — tests d'integration validator + +Bootstrap : `self::bootKernel()` dans `setUp()`, recuperation de `ValidatorInterface` et `EntityManagerInterface` depuis le container. + +Tests de validation scalaire (via `DataProvider` PHPUnit 12+, attribut `#[DataProvider]`) : +1. `testValidSitePassesValidation` : un Site correct passe sans violation. +2. `testColorMustBeHexRrggbb` / `testValidColorsAreAccepted` : jeu de donnees invalide (`red`, `#FFF`, `FFFFFF`, `rgb(...)`, `#1234567`, `#12345G`, `""`) vs valide (`#ABCDEF`, `#abcdef`, `#0a1B2c`, `#000000`, `#FFFFFF`). +3. `testPostalCodeMustMatchFrFormat` / `testValidPostalCodesAreAccepted` : jeu de donnees invalide (`1234`, `123456`, `8610A`, `86-100`, `""`, `86 100`) vs valide (`86100`, `75001`, `97100`, `20000`). +4. `testBlankNameIsRejected`, `testBlankCityIsRejected`, `testBlankFullAddressIsRejected` : `NotBlank` sur chaque champ obligatoire. +5. `testNameLongerThan100CharsIsRejected`, `testCityLongerThan100CharsIsRejected` : `Length(max: 100)`. + +Test d'unicite : +6. `testDuplicateNameIsRejected` : **auto-suffisant** — persiste lui-meme un site porteur d'un nom unique (`Test-Duplicate-`), flush, tente de valider un second Site avec le meme nom, verifie qu'au moins une violation porte `UniqueEntity::NOT_UNIQUE_ERROR` sur la property `name`, puis supprime le site en `finally`. + - **Justification** : pas de dependance aux fixtures (robustesse, pas de couplage sur `Chatellerault`). Assertion precise sur le `code` de violation + `propertyPath`, pas sur le message (resistant aux traductions). + +### Pattern `finally` pour cleanup + +```php +try { + $duplicate = new Site($name, ...); + $violations = $this->validator->validate($duplicate); + // assertions... +} finally { + $this->em->remove($original); + $this->em->flush(); +} +``` + +Garantit le cleanup meme si une assertion rate, sans dependre d'une transaction globale de test. + +## 11. Risques et points d'attention + +### Risque 1 — Mapping Doctrine inconditionnel + +Le mapping `Sites:` est declare dans `doctrine.yaml` sans dependance a `config/modules.php`. Consequence : retirer `SitesModule::class` de `modules.php` ne desactive **pas** le mapping Doctrine ni la table `site`. + +Decision assumee et alignee avec le traitement du module `Core` : +- La structure DB est "toujours la" (migrations jouees inconditionnellement). +- L'activation fonctionnelle (exposition des permissions, futurs endpoints) passe exclusivement par `modules.php`. + +Cela doit etre **explicite dans `doctrine.yaml`** via un commentaire en tete du bloc `Sites:` pour eviter qu'un futur reviewer n'interprete le mapping comme un oubli. + +### Risque 2 — Migration racine vs migration modulaire + +La migration est placee dans `migrations/` et non dans `src/Module/Sites/Infrastructure/Doctrine/Migrations/`. C'est une exception documentee dans `CLAUDE.md` et dans le docblock de la migration elle-meme, motivee par un bug de tri alphabetique des `MigrationsComparator` en Doctrine Migrations 3.x lorsque plusieurs `migrations_paths` sont declares. + +Consequence pour les tickets futurs : +- Tant que le bug n'est pas resolu, **toute nouvelle migration d'initialisation** (creation de table sur base vide) continuera d'aller au namespace racine. +- Les migrations applicatives (ajout de colonne, backfill) qui supposent un schema deja en place peuvent vivre dans le namespace modulaire, comme prevu. +- Une fois le bug resolu (comparator custom ou upgrade Doctrine), migrer les fichiers vers le namespace modulaire sera un simple `git mv` + ajustement du namespace PHP. + +### Risque 3 — Idempotence des fixtures non cumulative + +Le docblock de `SitesFixtures` declare explicitement les cas d'idempotence supportes et non supportes (cf. section 8). Ne pas promettre une robustesse que le pattern ne tient pas : si un futur ticket introduit une fixture persistant des Site **avant** `SitesFixtures` sans flush intermediaire, un doublon peut apparaitre. Le contrat ecrit permet au reviewer de ce futur ticket de reagir. + +### Risque 4 — Regex couleur non normalisee + +La regex `/^#[0-9A-Fa-f]{6}$/` accepte majuscules et minuscules. Les fixtures utilisent des majuscules ; si l'UI du ticket 4 permet de saisir en minuscules, deux couleurs "visuellement identiques" pourront coexister en base avec casse differente, cassant toute comparaison naive (`$a->color === $b->color`). A decider au ticket 4 : normaliser en uppercase a la persistance, ou assumer le stockage tel quel et normaliser uniquement a la comparaison. + +### Risque 5 — Precision timestamp PostgreSQL TIMESTAMP(0) + +PostgreSQL `TIMESTAMP(0)` ecrete a la seconde pres. Deux updates espaces de moins d'une seconde produisent le meme `updated_at` en base. Pas un probleme pour les cas d'usage metier de ce ticket (edition manuelle), mais a garder en tete si un ticket futur introduit un `updatedAt` comme cle de tri ou de detection de version optimiste. + +## 12. Ordre d'exécution recommandé + +1. **Exploration** — Lire le module Core (`CoreModule.php`, `User.php`, `Role.php`) pour aligner le style. +2. **Module declaration** — Creer `SitesModule.php` avec `permissions()`. +3. **Entite** — Creer `Site.php` avec tous les attributs Doctrine et contraintes de validation. +4. **Repository** — Creer `SiteRepositoryInterface.php` puis `DoctrineSiteRepository.php`. +5. **Configuration** — Enregistrer le mapping dans `doctrine.yaml`, l'alias dans `services.yaml`, le module dans `modules.php`. +6. **Migration** — Generer le fichier de migration (manuellement ou via `doctrine:migrations:diff` puis ajuster), jouer `make migration-migrate`. +7. **Fixtures** — Creer `SitesFixtures.php`, jouer `make fixtures` puis `make sync-permissions`. +8. **Tests unitaires** — Ecrire `SiteTest.php` (TestCase pur). +9. **Tests validation** — Ecrire `SiteValidationTest.php` (KernelTestCase). +10. **Validation DoD** — `make test-db-setup && make test` (doit passer 148/148), verifier que designer SitesModule ne casse rien. +11. **CS fixer** — `make php-cs-fixer-allow-risky FILES="src/Module/Sites tests/Module/Sites migrations/Version.php config/..."`. + +## 13. Critères d'acceptation (DoD) + +- [ ] `SitesModule.php` existe et declare exactement 2 permissions (`sites.view`, `sites.manage`) prefixees `sites.` conformement au format attendu par `SyncPermissionsCommand`. +- [ ] `SitesModule::class` est enregistre dans `config/modules.php` et active par defaut. +- [ ] Entite `Site` creee avec tous les champs, contraintes de validation (`NotBlank`, `Length`, `Regex hex`, `Regex CP FR`, `UniqueEntity`) et timestamps auto. +- [ ] `SiteRepositoryInterface` expose au minimum `findById`, `findByName`, `findAllOrderedByName`, `save`, `remove` ; `DoctrineSiteRepository` l'implemente. +- [ ] La migration existe dans `migrations/` (namespace `DoctrineMigrations`), cree la table `site` et l'index unique `uniq_site_name`, est jouable via `make migration-migrate`. +- [ ] `SitesFixtures` cree les 3 sites avec couleurs distinctes et docblock honnete sur son idempotence. +- [ ] `make fixtures` charge les 3 sites sans erreur et est rejouable apres purge. +- [ ] Apres `app:sync-permissions`, la table `permission` contient `sites.view` et `sites.manage` avec `module = 'sites'` et `orphan = false`. +- [ ] Le mapping `Sites:` est declare dans `doctrine.yaml` avec un commentaire explicite sur son caractere inconditionnel. +- [ ] L'alias `SiteRepositoryInterface → DoctrineSiteRepository` est declare dans `services.yaml`. +- [ ] `make test` passe 148/148 tests avec `SitesModule::class` active. +- [ ] `make test` passe 148/148 tests avec `SitesModule::class` commente dans `config/modules.php`. +- [ ] `make php-cs-fixer-allow-risky` ne signale aucune correction sur les fichiers du ticket. +- [ ] Aucun import direct depuis `src/Module/Core/...` vers `src/Module/Sites/...` ni l'inverse (independance des bounded contexts). diff --git a/docs/sites/ticket-02-spec.md b/docs/sites/ticket-02-spec.md new file mode 100644 index 0000000..8c60f78 --- /dev/null +++ b/docs/sites/ticket-02-spec.md @@ -0,0 +1,592 @@ +# Ticket #02 — 2/4 — Exposition API, rattachement utilisateurs et admin CRUD + +## 1. Objectif + +Ce ticket transforme la brique de donnees du ticket 1 en module fonctionnel : il expose la ressource `Site` via API Platform (CRUD admin avec RBAC), introduit la notion de **sites autorises** et de **site courant** sur chaque utilisateur, ouvre un endpoint dedie au basculement du site courant, et livre la page d'administration `/admin/sites` ainsi que l'assignation des sites dans le drawer RBAC d'un user. + +Le resultat attendu est un module Sites utilisable de bout en bout cote admin (creer, editer, supprimer des sites et en assigner aux users), avec une API `/api/me` enrichie que le ticket 3 consommera pour alimenter le selecteur de site dans la navbar. Le ticket etablit le couplage Core → Sites **au niveau modele** (la table `user` gagne deux relations vers `site`) tout en conservant le contrat "desactiver Sites dans `config/modules.php` ne casse pas l'app" via des decisions DB/mapping assumees. + +## 2. Périmètre + +### IN + +- Exposer `Site` comme ressource API Platform avec les operations `GetCollection`, `Get`, `Post`, `Patch`, `Delete`, securisees par les permissions `sites.view` (lecture) et `sites.manage` (ecriture). +- Ajouter deux relations sur `User` (module Core) : + - `$sites` (M2M, `user_site`) : sites autorises. + - `$currentSite` (M2O nullable) : site actuellement selectionne. +- Ajouter la relation inverse `$users` sur `Site` (non exposee API). +- Generer la migration Doctrine creant la table `user_site` et la colonne `user.current_site_id` avec les bonnes strategies `ON DELETE` pour garantir les cascades attendues (suppression d'un site → `user_site` purge, `currentSite` mis a `NULL`). +- Etendre `/api/me` pour exposer `sites: Site[]` et `currentSite: Site | null` en objets serialises (pas en IRI), via les groupes `me:read` sur `User` **et** sur `Site`. +- Ajouter un endpoint dedie de switch du site courant, implemente comme une ressource API Platform virtuelle `CurrentSite` avec une operation `Patch uriTemplate: '/me/current-site'` et un processor dedie. Le processor garantit que le site cible fait partie des `sites` de l'utilisateur authentifie, sinon il leve une exception traduite en `403`. +- Etendre `UserRbacProcessor` et l'operation `PATCH /api/users/{id}/rbac` pour accepter un champ `sites: string[]` (IRIs) en plus des roles et permissions directes. Cas limite : si le `currentSite` du user cible n'est plus dans la liste, le processor le bascule a `NULL`. +- Etendre l'exception metier Core pour couvrir "site non autorise" via une nouvelle exception domaine `SiteNotAuthorizedException` placee dans le module Sites, traduite en `ForbiddenHttpException` au niveau API. +- Ajouter l'entree sidebar `sidebar.admin.sites` filtree par `module: 'sites'` + `permission: 'sites.view'` dans `config/sidebar.php`, sous la section admin Core existante. +- Livrer la page d'administration `/admin/sites` cote front (layer Nuxt `frontend/modules/sites/`) : DataTable + drawer creation/edition + modale suppression, alignee visuellement et structurellement sur `/admin/roles` et `/admin/users`. +- Etendre le drawer `UserRbacDrawer.vue` (module Core) pour afficher et editer la liste des sites autorises d'un user via un multi-select. +- Ajouter les fixtures : rattacher les 3 users existants (`admin`, `alice`, `bob`) a au moins un site et positionner un `currentSite` coherent. +- Couverture de tests PHPUnit : CRUD `/api/sites`, endpoint `/me/current-site` (cas OK + 403), extension `/api/me`, cascade DB a la suppression d'un site, extension `UserRbacProcessor` (ajout/retrait sites, auto-reset currentSite). + +### OUT + +- Ticket `#03` : selecteur de site dans la navbar, persistance du site actif cote front, integration visuelle avec la couleur du site. +- Ticket `#04` : filtrage metier par site (ex: bloquer l'acces aux ressources Commercial si l'user n'est pas rattache au site de la ressource). +- Soft-delete des sites : non introduit. +- Audit trail des modifications : hors scope. +- Color picker avance : un input hex simple avec preview de la puce suffit. +- Recherche / tri server-side sur `/api/sites` : non requis, le volume reste <20 sites par instance. +- Gestion des site "globaux" ou "par defaut" pour les nouveaux users : non introduite, les users crees via `CreateUserCommand` ou `/api/users` POST auront `sites: []` et `currentSite: null` jusqu'a rattachement explicite. + +## 3. Fichiers à créer + +### Backend — Module Sites + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php` : exception domaine levee si un user tente de switcher vers un site qui ne fait pas partie de ses sites autorises. Porte un message i18n-able et le code du site cible. +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/ApiPlatform/Resource/CurrentSiteResource.php` : ressource API Platform **virtuelle** (pas de mapping Doctrine, pas de `#[ORM\Entity]`). Sert uniquement a porter l'operation `Patch` `/me/current-site`. Expose une propriete `site: Site` en denormalisation pour recevoir l'IRI du site cible, et re-expose l'user courant en normalisation via le groupe `me:read`. +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php` : processor dedie a l'operation de switch. Valide l'appartenance du site aux `user.sites`, positionne `user.currentSite`, flush, retourne l'user. +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/ApiPlatform/EventListener/SiteNotAuthorizedExceptionListener.php` : listener Kernel qui convertit `SiteNotAuthorizedException` en `ForbiddenHttpException` (403) avec un code i18n stable (cf. pattern `SystemRoleDeletionException` du module Core dans les tickets RBAC precedents). + +### Backend — Migration + +- `/home/m-tristan/workspace/Coltura/migrations/Version.php` : migration au namespace racine `DoctrineMigrations` (cf. exception Doctrine documentee dans `CLAUDE.md`). Cree la table `user_site` et la colonne `user.current_site_id` avec les FKs et cascades appropriees. + +### Backend — Tests API + +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Api/SiteApiTest.php` : CRUD complet `/api/sites` avec matrices RBAC (admin, user avec `sites.view`, user avec `sites.manage`, user sans permission). +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php` : PATCH `/me/current-site` (OK avec site autorise, 403 avec site non autorise, 400 avec IRI invalide). +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Api/MeEndpointSitesTest.php` : `/api/me` expose bien `sites` et `currentSite` en objets. User sans site : `sites: []`, `currentSite: null`. +- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Api/SiteCascadeTest.php` : suppression d'un site `X` → toutes les lignes `user_site` referencant `X` sont supprimees, tous les users ayant `X` en `currentSite` voient leur `currentSite` repasser a `NULL`. +- `/home/m-tristan/workspace/Coltura/tests/Module/Core/Api/UserRbacSitesApiTest.php` : extension du endpoint `/api/users/{id}/rbac` — ajout de `sites: []` dans le payload, retrait du `currentSite` quand le site retire etait le courant. + +### Frontend — Module Sites (nouveau layer) + +- `/home/m-tristan/workspace/Coltura/frontend/modules/sites/nuxt.config.ts` : marker de layer Nuxt (vide). Declenche l'auto-detection par `nuxt.config.ts` racine. +- `/home/m-tristan/workspace/Coltura/frontend/modules/sites/pages/admin/sites.vue` : page `/admin/sites`. Reutilise les composants Malio UI (`MalioDataTable`, `MalioButton`, `MalioInputText`, `MalioInputTextArea`). Pattern identique a `frontend/modules/core/pages/admin/roles.vue`. +- `/home/m-tristan/workspace/Coltura/frontend/modules/sites/components/SiteDrawer.vue` : drawer creation/edition. Formulaire 5 champs (nom, ville, CP, couleur avec preview puce, adresse). Valide cote front sur le submit avant d'envoyer. +- `/home/m-tristan/workspace/Coltura/frontend/modules/sites/components/SiteDeleteModal.vue` : modale de confirmation suppression. Pattern aligne sur `RoleDeleteModal.vue`. + +### Frontend — Types partages + +- `/home/m-tristan/workspace/Coltura/frontend/shared/types/sites.ts` : types `Site`, `SiteInput`. Pattern identique a `frontend/shared/types/rbac.ts`. + +### Tests frontend (optionnels mais recommandes) + +- `/home/m-tristan/workspace/Coltura/frontend/modules/sites/pages/admin/sites.spec.ts` : smoke test Vitest (rendu + clic bouton "Nouveau site" ouvre le drawer). + +## 4. Fichiers à modifier + +### Backend — Module Core + +- `/home/m-tristan/workspace/Coltura/src/Module/Core/Domain/Entity/User.php` : + - Ajouter `private Collection $sites;` (M2M, `fetch: EAGER`, `JoinTable: user_site`), groupes `me:read`, `user:list`, `user:rbac:read`, `user:rbac:write`. + - Ajouter `private ?Site $currentSite = null;` (M2O, `fetch: EAGER`, `onDelete: 'SET NULL'`), groupe `me:read`. + - Initialiser `$this->sites = new ArrayCollection();` dans le constructeur. + - Ajouter les accesseurs `getSites()`, `addSite(Site)`, `removeSite(Site)`, `hasSite(Site)`, `getCurrentSite()`, `setCurrentSite(?Site)`. + - **Important** : `import` direct `App\Module\Sites\Domain\Entity\Site`. Ce ticket assume le couplage Core → Sites au niveau code PHP (cf. Risque 1). +- `/home/m-tristan/workspace/Coltura/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php` : + - Etendre le contrat d'entree pour accepter le champ `sites` (collection d'IRIs denormalisees en `Collection`). + - Apres l'application des roles et permissions directes, detecter si `currentSite` du user cible n'est plus dans la nouvelle collection `sites` → basculer `currentSite` a `null`. + - Conserver toutes les gardes existantes (auto-suicide admin, dernier admin global). +- `/home/m-tristan/workspace/Coltura/src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php` : + - Declarer l'implementation `DependentFixtureInterface` avec `getDependencies(): [SitesFixtures::class]` (inversion de l'ordre actuel : AppFixtures doit tourner **apres** SitesFixtures pour pouvoir reference les sites). + - Rattacher chaque user a au moins un site : `admin` a tous les sites (`Chatellerault`, `Saint-Jean`, `Pommevic`), `alice` a `Chatellerault`, `bob` a `Saint-Jean`. + - Positionner `currentSite` : `admin.currentSite = Chatellerault`, `alice.currentSite = Chatellerault`, `bob.currentSite = Saint-Jean`. + +### Backend — Module Sites + +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Domain/Entity/Site.php` : + - Ajouter les attributs `#[ApiResource]` + operations (cf. section 5 Schema). + - Ajouter les groupes de serialisation `site:read`, `site:write`, `me:read` sur les proprietes scalaires. + - Ajouter la relation inverse `private Collection $users;` (M2M mappedBy=`sites`), **sans** groupe de serialisation (pas d'exposition API cote Site). + - Initialiser `$this->users = new ArrayCollection();` dans le constructeur. + - Ajouter les accesseurs `getUsers()` pour les besoins metier (count / cascade manuel si besoin). +- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php` : aucun changement de contenu, mais verifier que la fixture n'est plus en bout de chaine de dependance (AppFixtures depend d'elle maintenant). + +### Backend — Configuration + +- `/home/m-tristan/workspace/Coltura/config/sidebar.php` : inserer l'entree `Sites` dans la section `sidebar.general.section` entre `sidebar.core.users` et `sidebar.general.logout` : + ```php + [ + 'label' => 'sidebar.core.sites', + 'to' => '/admin/sites', + 'icon' => 'mdi:domain', + 'module' => 'sites', + 'permission' => 'sites.view', + ], + ``` +- `/home/m-tristan/workspace/Coltura/config/services.yaml` : aucun changement requis. `CurrentSiteProcessor`, `SiteNotAuthorizedExceptionListener` sont autoconfigures. + +### Frontend + +- `/home/m-tristan/workspace/Coltura/frontend/modules/core/components/UserRbacDrawer.vue` : + - Charger `GET /api/sites?itemsPerPage=999` a l'ouverture du drawer (parallelement aux roles et permissions deja charges). + - Ajouter une section `sidebar.admin.usersDrawer.sitesSection` sous la section permissions directes, avec un groupe de `MalioCheckbox` par site (ou un `MalioMultiSelect` si le composant existe dans `@malio/layer-ui`). + - Etendre le payload `PATCH /api/users/{id}/rbac` avec `sites: Array` (IRIs). + - Auto-refresh de l'auth store apres save si `isSelfEdit` (deja present, conserver). +- `/home/m-tristan/workspace/Coltura/frontend/shared/types/rbac.ts` : ajouter le champ `sites: string[]` a `UserListItem` (IRIs de sites attaches). +- `/home/m-tristan/workspace/Coltura/frontend/shared/stores/auth.ts` : le store auth expose deja `user` via `/api/me`. Aucune modification requise, les nouveaux champs `sites` et `currentSite` suivent automatiquement via la typologie — a condition de mettre a jour le type `UserData` dans `shared/types/` (ajouter `sites: Site[]` et `currentSite: Site | null`). +- `/home/m-tristan/workspace/Coltura/frontend/i18n/locales/fr.json` : cles + - `sidebar.core.sites` = "Sites". + - `admin.sites.title`, `admin.sites.newSite`, `admin.sites.editSite`, `admin.sites.createSite`, `admin.sites.noSites`. + - `admin.sites.table.{name, city, postalCode, color, fullAddress}`. + - `admin.sites.form.{name, city, postalCode, color, fullAddress}`. + - `admin.sites.delete.{title, message}`. + - `admin.sites.toast.{created, updated, deleted}`. + - `admin.users.drawer.sitesSection` = "Sites autorises". + - `errors.sites.notAuthorized` = "Vous n'etes pas autorise a selectionner ce site.". + +## 5. Schéma cible — ApiResource et Doctrine + +### Entite `Site` — attributs ApiResource + +```php +#[ApiResource( + operations: [ + new GetCollection( + security: "is_granted('sites.view')", + normalizationContext: ['groups' => ['site:read']], + ), + new Get( + security: "is_granted('sites.view')", + normalizationContext: ['groups' => ['site:read']], + ), + new Post( + security: "is_granted('sites.manage')", + normalizationContext: ['groups' => ['site:read']], + denormalizationContext: ['groups' => ['site:write']], + ), + new Patch( + security: "is_granted('sites.manage')", + normalizationContext: ['groups' => ['site:read']], + denormalizationContext: ['groups' => ['site:write']], + ), + new Delete(security: "is_granted('sites.manage')"), + ], +)] +``` + +Groupes sur les proprietes de `Site` : +- `id` : `site:read`, `me:read`. +- `name`, `city`, `postalCode`, `color`, `fullAddress` : `site:read`, `site:write`, `me:read`. +- `createdAt`, `updatedAt` : `site:read` uniquement (pas exposes en embed `me:read` pour garder le payload /me leger). + +### Evolution de `User` — nouvelles relations + +```php +/** @var Collection */ +#[ORM\ManyToMany(targetEntity: Site::class, fetch: 'EAGER')] +#[ORM\JoinTable(name: 'user_site')] +#[Groups(['me:read', 'user:list', 'user:rbac:read', 'user:rbac:write'])] +private Collection $sites; + +#[ORM\ManyToOne(targetEntity: Site::class, fetch: 'EAGER')] +#[ORM\JoinColumn(name: 'current_site_id', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')] +#[Groups(['me:read'])] +private ?Site $currentSite = null; +``` + +Justification fetch=EAGER : +- Aligne sur les collections `$rbacRoles` et `$directPermissions` (cf. `User.php:87`). +- Critique pour eviter un lazy-load silencieux pendant un refresh JWT (cf. ticket 343 section 11 risque 1). +- Surcout SQL accepte a l'echelle d'un CRM PME (≤20 sites par instance). + +### Relation inverse sur `Site` + +```php +/** @var Collection */ +#[ORM\ManyToMany(targetEntity: User::class, mappedBy: 'sites')] +private Collection $users; +``` + +Pas de `#[Groups]` : la collection inverse n'est pas exposee dans la reponse API. Sa seule utilite est metier (compter les users d'un site, iterer pour un cascade applicatif si la cascade DB ne suffisait pas). + +### Ressource virtuelle `CurrentSite` + +```php +#[ApiResource( + shortName: 'CurrentSite', + operations: [ + new Patch( + uriTemplate: '/me/current-site', + security: "is_granted('ROLE_USER')", + normalizationContext: ['groups' => ['me:read']], + denormalizationContext: ['groups' => ['current-site:write']], + processor: CurrentSiteProcessor::class, + read: false, + ), + ], +)] +final class CurrentSiteResource +{ + #[Groups(['current-site:write'])] + public ?Site $site = null; +} +``` + +- `read: false` : API Platform ne tente pas de charger une entite existante via un Provider — il se contente de denormaliser le body et de passer la ressource au processor. +- `shortName: 'CurrentSite'` : evite la collision de nommage avec l'entite `Site`. +- `security: "is_granted('ROLE_USER')"` : tout user authentifie peut tenter un switch ; l'autorisation fine (appartenance du site aux `sites` du user) est verifiee par le processor, pas par la voter RBAC. + +## 6. Plan de migration Doctrine + +La migration est placee dans `/home/m-tristan/workspace/Coltura/migrations/Version.php` au namespace racine (cf. Risque 2 du ticket 1 et `CLAUDE.md`). + +### `up()` — ordre des instructions + +1. `ALTER TABLE "user" ADD current_site_id INT DEFAULT NULL` — colonne nullable, pas besoin de backfill. +2. `CREATE TABLE user_site (user_id INT NOT NULL, site_id INT NOT NULL, PRIMARY KEY (user_id, site_id))`. +3. `CREATE INDEX IDX_user_site_user ON user_site (user_id)`. +4. `CREATE INDEX IDX_user_site_site ON user_site (site_id)`. +5. `ALTER TABLE user_site ADD CONSTRAINT FK_user_site_user FOREIGN KEY (user_id) REFERENCES "user" (id) ON DELETE CASCADE`. +6. `ALTER TABLE user_site ADD CONSTRAINT FK_user_site_site FOREIGN KEY (site_id) REFERENCES site (id) ON DELETE CASCADE`. +7. `CREATE INDEX IDX_user_current_site ON "user" (current_site_id)`. +8. `ALTER TABLE "user" ADD CONSTRAINT FK_user_current_site FOREIGN KEY (current_site_id) REFERENCES site (id) ON DELETE SET NULL`. + +### `down()` — rollback + +1. `ALTER TABLE "user" DROP CONSTRAINT FK_user_current_site`. +2. `DROP INDEX IDX_user_current_site`. +3. `ALTER TABLE "user" DROP current_site_id`. +4. `ALTER TABLE user_site DROP CONSTRAINT FK_user_site_site`. +5. `ALTER TABLE user_site DROP CONSTRAINT FK_user_site_user`. +6. `DROP TABLE user_site`. + +### Comportement des cascades + +| Action | Effet | +|--------|-------| +| `DELETE FROM site WHERE id = X` | Toutes les lignes `user_site` avec `site_id = X` sont supprimees (FK `ON DELETE CASCADE`). Tous les users avec `current_site_id = X` voient leur `current_site_id` passer a `NULL` (FK `ON DELETE SET NULL`). | +| `DELETE FROM "user" WHERE id = Y` | Toutes les lignes `user_site` avec `user_id = Y` sont supprimees. Pas d'effet sur `site`. | +| `DELETE FROM user_site WHERE user_id = Y AND site_id = X` | Aucun effet auto sur `user.current_site_id` — si `X` etait le courant de `Y`, c'est le **UserRbacProcessor** qui doit le basculer a `NULL` en Php (cf. section 8). | + +**Important** : la derniere ligne du tableau est la raison pour laquelle la logique de "retirer un site qui etait le courant remet currentSite a null" vit dans `UserRbacProcessor` cote applicatif et non dans la DB via un trigger. C'est un compromis assume : la regle est metier ("retirer un droit ne doit pas laisser l'user pointer sur un site interdit"), pas purement DB. + +## 7. Algorithme du switch de site courant — `CurrentSiteProcessor` + +### Entree + +Body JSON envoye par le client : +```json +{ "site": "/api/sites/3" } +``` + +API Platform denormalise vers `CurrentSiteResource { site: Site }` en resolvant l'IRI via son `IriConverter`. + +### Algorithme + +1. Recuperer l'user authentifie via `Security::getUser()`. Si absent → `LogicException` (l'operation exige `ROLE_USER`, ne doit pas arriver). +2. Extraire `$targetSite = $resource->site`. Si `null` → `BadRequestHttpException('Le champ "site" est requis.')`. +3. Verifier `$user->hasSite($targetSite)` : + - Implementation : `$this->sites->contains($targetSite)` (comparaison par reference ; Doctrine garantit l'identite d'objet dans la meme session). + - Si `false` → throw `SiteNotAuthorizedException($targetSite->getId())`. +4. `$user->setCurrentSite($targetSite)`. +5. `$this->entityManager->flush()`. +6. Retourner `$user` — API Platform le normalise via les groupes `me:read` definis sur l'operation. + +### Mapping d'exception + +`SiteNotAuthorizedException` est convertie en `Symfony\Component\HttpKernel\Exception\HttpException` avec statut `403` par `SiteNotAuthorizedExceptionListener` (event `kernel.exception`, priority standard). Le corps de la reponse porte un code i18n-able `errors.sites.notAuthorized` pour le front. + +## 8. Évolution du `UserRbacProcessor` + +### Nouveau champ en entree + +Le payload accepte desormais : +```json +{ + "isAdmin": false, + "roles": ["/api/roles/2"], + "directPermissions": [], + "sites": ["/api/sites/1", "/api/sites/3"] +} +``` + +Le champ `sites` est optionnel : si absent, la collection n'est pas touchee (comportement PATCH standard). Si present, il remplace integralement la collection `$user->sites`. + +### Garde "currentSite coherent" + +Apres application des champs par le persist processor decore, `UserRbacProcessor` execute un controle final : + +```php +$currentSite = $data->getCurrentSite(); +if ($currentSite !== null && !$data->hasSite($currentSite)) { + $data->setCurrentSite(null); +} +``` + +Justification : si un admin retire un site qui etait le `currentSite` de la cible, le modele serait incoherent (currentSite pointant vers un site non autorise). Le processor corrige automatiquement. + +**Variante rejetee** : basculer vers "le premier site restant" plutot que `null`. Rejetee car : +- "Premier restant" n'a pas de semantique metier claire (ordre de la collection non garanti strict). +- `null` est une valeur deja supportee (user sans site courant) et explicite : le front du ticket 3 devra gerer ce cas de toute facon. + +### Ordre d'execution dans le processor + +1. Gardes auto-suicide admin + dernier admin global (code existant, inchange). +2. `$this->persistProcessor->process($data, ...)` — applique tous les champs (roles, permissions directes, **sites**). +3. Post-persist : controle coherence currentSite (code ajoute par ce ticket), flush si changement. +4. Retour du user. + +## 9. Fixtures — évolution de `AppFixtures` + +`AppFixtures` devient dependant de `SitesFixtures` (inversion du "pas de dependance dure" declare au ticket 1 — justifie par le passage fonctionnel a la relation User ↔ Site). + +```php +class AppFixtures extends Fixture implements DependentFixtureInterface +{ + public function getDependencies(): array + { + return [SitesFixtures::class]; + } + // ... +} +``` + +Dans `load()`, apres la creation des users et avant le `flush` final : + +```php +$chatellerault = $this->siteRepository->findByName('Chatellerault'); +$saintJean = $this->siteRepository->findByName('Saint-Jean'); +$pommevic = $this->siteRepository->findByName('Pommevic'); + +$admin->addSite($chatellerault); +$admin->addSite($saintJean); +$admin->addSite($pommevic); +$admin->setCurrentSite($chatellerault); + +$alice->addSite($chatellerault); +$alice->setCurrentSite($chatellerault); + +$bob->addSite($saintJean); +$bob->setCurrentSite($saintJean); +``` + +Le repository `SiteRepositoryInterface` est injecte dans le constructeur. + +**Regle** : les 3 sites sont deja en base au moment ou `AppFixtures::load()` s'execute grace a `getDependencies()`. Si `findByName` retourne `null`, c'est une misconfiguration qui doit faire echouer fort (assertion via `\assert`). + +## 10. Frontend — Page `/admin/sites` + +### Structure + +``` +frontend/modules/sites/ +├── nuxt.config.ts # marker layer Nuxt (vide) +├── pages/ +│ └── admin/ +│ └── sites.vue # page listing +└── components/ + ├── SiteDrawer.vue # creation/edition + └── SiteDeleteModal.vue # confirmation suppression +``` + +### `pages/admin/sites.vue` — pattern + +Aligne sur `frontend/modules/core/pages/admin/roles.vue` : +- En-tete : titre + bouton `Nouveau site` (visible si `can('sites.manage')`). +- `MalioDataTable` : colonnes `name`, `city`, `postalCode`, `color` (slot custom pour la puce), `fullAddress` (tronque). +- Row click → ouvre `SiteDrawer` en mode edition si `can('sites.manage')`, sinon pas de clic (row-clickable guard). +- `SiteDrawer` emet `saved` → reload de la liste, et `delete` → ouvre `SiteDeleteModal`. +- `SiteDeleteModal` → DELETE `/api/sites/{id}` + reload. + +### `components/SiteDrawer.vue` + +Formulaire a 5 champs + preview de la couleur. Pattern `RoleDrawer.vue` : +- `MalioInputText` pour `name`, `city`, `postalCode`. +- `MalioInputText` pour `color` avec preview : une puce `` 24×24 arrondie affichant la couleur en temps reel a cote du champ. Valider localement via regex avant submit (ne pas envoyer un hex invalide au backend). +- `MalioInputTextArea` pour `fullAddress`. +- Bouton save (variant primary), bouton delete (variant danger, visible uniquement en mode edition, **aucune garde system comme pour les roles** — tous les sites sont supprimables), bouton cancel (variant tertiary). + +### `components/SiteDeleteModal.vue` + +Pattern `RoleDeleteModal.vue` : +- Modal avec message "Supprimer le site {name} ? Cette action est irreversible et retirera ce site a tous les utilisateurs rattaches." +- Bouton cancel (secondary) + bouton delete (danger avec icone poubelle). +- Emet `confirm` au clic delete. + +### Extension de `UserRbacDrawer.vue` + +Ajout d'une nouvelle section entre "Permissions directes" et "Resume des permissions effectives" : + +```vue + +
+

+ {{ t('admin.users.drawer.sitesSection') }} +

+
+ +
+
+``` + +Chargement : ajout a `loadData()` d'un `api.get<{ member: Site[] }>('/sites', { itemsPerPage: 999 }, { toast: false })`. + +Le `PATCH /api/users/{id}/rbac` envoie desormais `sites: Array.from(selectedSiteIds).map(id => `/api/sites/${id}`)`. + +### Types TypeScript + +`frontend/shared/types/sites.ts` : + +```ts +export interface Site { + id: number + name: string + city: string + postalCode: string + color: string + fullAddress: string +} + +export interface SiteInput { + name: string + city: string + postalCode: string + color: string + fullAddress: string +} +``` + +`frontend/shared/types/rbac.ts` : ajouter `sites: string[]` (IRIs) dans `UserListItem`. + +`frontend/shared/types/` (fichier utilisateur courant, probablement `user.ts` ou expose dans l'auth store) : ajouter `sites: Site[]` et `currentSite: Site | null` sur le type expose via `/api/me`. + +### Sidebar + +Entree ajoutee dans `config/sidebar.php` (cf. section 4). Le `SidebarProvider` filtre deja par `module` actif et par `permission`, aucune modification backend nouvelle. + +i18n : +```json +"sidebar": { + "core": { + "sites": "Sites" + } +} +``` + +## 11. Plan de tests PHPUnit + +### `SiteApiTest` — CRUD `/api/sites` + +1. `testAdminCanListSites` : admin → 200, 3 sites. +2. `testUserWithSitesViewCanListSites` : user avec `sites.view` → 200. +3. `testUserWithoutPermissionGetsForbidden` : user sans `sites.view` → 403. +4. `testAdminCanCreateSite` : POST → 201, site present en base. +5. `testAdminCanPatchSite` : PATCH `color` → 200. +6. `testAdminCanDeleteSite` : DELETE → 204, site absent en base. +7. `testUserWithViewButNotManageCannotDelete` : user avec `sites.view` mais pas `sites.manage` → 403 sur DELETE. +8. `testCreateSiteWithDuplicateNameReturns422` : collision `uniq_site_name` → 422 avec message UniqueEntity. +9. `testCreateSiteWithInvalidColorReturns422` : validation regex → 422. + +### `CurrentSiteSwitchApiTest` — PATCH `/me/current-site` + +1. `testUserCanSwitchToAuthorizedSite` : alice a `Chatellerault` dans ses sites → PATCH OK, 200, `currentSite.name == 'Chatellerault'`. +2. `testUserCannotSwitchToUnauthorizedSite` : alice n'a pas `Pommevic` dans ses sites → PATCH → 403, pas de modification en base. +3. `testSwitchWithMissingSiteFieldReturns400` : body `{}` → 400. +4. `testSwitchWithInvalidIriReturns400` : body `{"site": "/api/sites/99999"}` (site inexistant) → 400 ou 404 (selon API Platform). +5. `testAnonymousUserCannotSwitch` : client non authentifie → 401. + +### `MeEndpointSitesTest` — extension `/api/me` + +1. `testMeExposesSitesAsObjects` : alice → `sites[0]` est un objet avec `id`, `name`, `city`, ... (pas une string IRI). +2. `testMeExposesCurrentSiteAsObject` : alice → `currentSite` est un objet, pas `null`. +3. `testUserWithoutSitesHasEmptyArrayAndNullCurrent` : creer un user jetable sans sites → `sites: []`, `currentSite: null`. + +### `SiteCascadeTest` — cascade DB a la suppression + +1. `testDeletingSitePurgesUserSiteRows` : supprimer `Chatellerault` → les users qui l'avaient dans `sites` ne l'ont plus. +2. `testDeletingSiteSetsCurrentSiteToNullOnReferencingUsers` : alice.currentSite = `Chatellerault`, supprimer `Chatellerault` → alice.currentSite = `null`. + +### `UserRbacSitesApiTest` — extension `/rbac` + +1. `testAdminCanAssignSitesToUser` : PATCH `/users/{alice}/rbac` avec `sites: ["/api/sites/2"]` → alice a desormais 1 site (`Saint-Jean`), plus `Chatellerault`. +2. `testRemovingCurrentSiteResetsCurrentSiteToNull` : alice.currentSite = `Chatellerault`, PATCH avec `sites: []` → alice.currentSite = `null`. +3. `testEmptySitesPayloadReplacesCollection` : alice avait 1 site, PATCH avec `sites: []` → 0 site. +4. `testSitesPayloadWithDuplicateIrisIsAccepted` : PATCH avec `sites: ["/api/sites/1", "/api/sites/1"]` → 1 seul site (dedoublonnage via `ArrayCollection::contains`). + +### Tests fixtures (sanity check) + +Dans `AbstractApiTestCase` ou dans un test dedie `FixturesIntegrityTest` : verifier apres `make test-db-setup` que les 3 users fixtures ont bien leurs sites attendus. Evite qu'un renommage dans la fixture passe inapercu. + +## 12. Risques et points d'attention + +### Risque 1 — Couplage Core → Sites au niveau code PHP + +L'ajout de `use App\Module\Sites\Domain\Entity\Site;` dans `User.php` introduit une dependance directe du module Core vers le module Sites. Consequence : + +- **Desactiver `SitesModule::class` dans `config/modules.php` n'empeche pas Doctrine de charger le mapping `Site` ni `User`**, grace au caractere inconditionnel des mappings declares dans `doctrine.yaml` (choix assume ticket 1). +- En revanche, la contrainte forte introduite ici est que **la table `site` doit exister** pour que la table `user` puisse etre creee (FK `user.current_site_id → site.id`). Si la migration Sites (ticket 1) n'a pas ete jouee, la migration de ce ticket echouera. +- Conclusion : Sites n'est plus "optionnel au sens strict" apres ce ticket. Le declarer `REQUIRED = false` dans `SitesModule` reste vrai du point de vue de l'activation fonctionnelle (exposer les permissions et la sidebar), mais faux du point de vue DB. **A documenter explicitement dans le docblock de `SitesModule::REQUIRED`** au moment de ce ticket. + +### Risque 2 — Cascade DB vs regle applicative + +La cascade `user_site` → `ON DELETE CASCADE` gere la suppression d'un site, mais **n'est pas triggered** quand on retire un site d'un user (DELETE d'une ligne `user_site` uniquement). Dans ce cas, `user.current_site_id` peut rester pointe vers un site que l'user n'a plus — etat incoherent qui serait masque au niveau DB mais visible a l'usage. + +La correction vit dans `UserRbacProcessor` (cf. section 8). Si un autre chemin applicatif modifie `user.sites` sans passer par ce processor (ex: une commande console custom), il devra dupliquer cette garde. **Point d'attention a consigner dans le docblock de `User::addSite()` / `User::removeSite()`** : "apres modification, verifier la coherence de `currentSite`". + +### Risque 3 — Ressource virtuelle et routing API Platform + +Le choix d'une ressource virtuelle `CurrentSite` avec `uriTemplate: '/me/current-site'` est fragile : si un futur ticket introduit une autre operation sur une URI qui commence par `/me/`, il faut verifier que le routing API Platform n'entre pas en conflit. Le pattern `priority: 1` (cf. `CLAUDE.md` section Backend) est recommande par prevention sur l'operation Patch. A valider par un test fonctionnel qui appelle explicitement `/api/me` (GET) et `/api/me/current-site` (PATCH) dans le meme scenario. + +### Risque 4 — EAGER loading et payload `/api/me` + +`User` a deja 3 collections EAGER (`$rbacRoles`, `$directPermissions`, plus les `permissions` de chaque role). Ajouter `$sites` (EAGER M2M) et `$currentSite` (EAGER M2O) augmente la taille du payload `/api/me` et le nombre de requetes SQL a chaque auth. + +Mesure : apres implementation, verifier via le profiler Symfony que le nombre de requetes SQL sur `/api/me` reste raisonnable (≤ 6-8). Si >10, envisager une projection custom (cf. ticket 343 discussion `findForSecurity`). Pas bloquant dans ce ticket, mais a reverifier. + +### Risque 5 — Tests fixtures-dependents + +Les tests API existants (`UserApiTest`, `RoleApiTest`) s'appuient sur les users fixtures. L'evolution de `AppFixtures` (ajout de sites aux 3 users) modifie l'etat initial de la DB de test. Verifier que les tests existants continuent de passer (chaines d'assertions du type "user a 1 role" ne doivent pas casser). En particulier : +- Les tests qui comptent les lignes d'une collection `member[]` sur `/api/users` peuvent voir le payload grossir (sites et currentSite ajoutes). +- Les tests qui assertent sur la forme stricte d'un user (snapshot-like) devront etre adapter. + +### Risque 6 — Serialisation infinie User ↔ Site + +`User::$sites` expose `Site` en `me:read`. `Site::$users` est la collection inverse. Si un jour `Site::$users` recevait le groupe `me:read`, la serialisation entrerait dans une boucle infinie (User → sites → users → sites → ...). **Garde** : `Site::$users` ne doit **jamais** porter de `#[Groups]`. A verifier par un test qui serialise `/api/me` et asserte qu'aucun `Site` renvoye ne contient de cle `users`. + +### Risque 7 — Pas de recours si l'utilisateur se retire tous ses sites + +Le ticket autorise un user sans sites (`sites: []`, `currentSite: null`). Mais aucune garde ne l'empeche de se retirer tous ses sites via `/api/users/{mon_id}/rbac` si il porte `sites.manage`. Consequence : l'user se retrouve bloque sur l'app si le ticket 3 rend un site actif obligatoire pour naviguer. Compromis assume pour ce ticket : on ne bloque pas l'auto-retrait (coherence avec le pattern du ticket RBAC — l'auto-retrait admin est bloque, mais pas le reste). **A reevaluer au ticket 3** si le selecteur de navbar devient bloquant. + +## 13. Ordre d'exécution recommandé + +1. **Schema backend** — modifier `User.php` (ajout `$sites`, `$currentSite`, `$users` inverse sur `Site`). Ajouter attributs `ApiResource` sur `Site`. +2. **Configuration** — aucun changement requis a `doctrine.yaml` ni `services.yaml` ni `modules.php`. +3. **Migration** — ecrire `Version.php` racine. Jouer `make migration-migrate`. +4. **Fixtures** — modifier `AppFixtures` pour dependre de `SitesFixtures` et rattacher les users. Jouer `make fixtures && make sync-permissions`. +5. **Endpoint CRUD sites** — verifier via `curl`/Postman que `GET /api/sites`, `POST /api/sites` etc. repondent avec les bonnes protections RBAC. +6. **Endpoint switch** — creer `CurrentSiteResource`, `CurrentSiteProcessor`, `SiteNotAuthorizedException`, `SiteNotAuthorizedExceptionListener`. Tester via `curl`. +7. **Extension MeProvider** — tester via `curl /api/me` que `sites` et `currentSite` apparaissent comme objets. Aucun code a changer dans `MeProvider` lui-meme, le travail est 100% fait via les groupes. +8. **Extension UserRbacProcessor** — ajouter le champ `sites` et la garde `currentSite`. Tests d'integration. +9. **Tests API** — ecrire et faire passer les 5 suites de tests decrites section 11. +10. **Sidebar** — ajouter l'entree dans `config/sidebar.php` + cle i18n. +11. **Frontend — types** — creer `shared/types/sites.ts`, etendre `shared/types/rbac.ts` et les types user. +12. **Frontend — page admin** — creer `modules/sites/nuxt.config.ts`, `pages/admin/sites.vue`, `SiteDrawer.vue`, `SiteDeleteModal.vue`. +13. **Frontend — extension UserRbacDrawer** — ajouter la section sites. +14. **Frontend — i18n** — completer `fr.json`. +15. **Validation end-to-end** — clique-droit sur chaque scenario UX : creer un site, l'editer, le supprimer, assigner sites a un user, switcher le site courant de l'user authentifie. +16. **Tests front (si Vitest du ticket)** — smoke test du rendu de `/admin/sites`. +17. **CS fixer** — `make php-cs-fixer-allow-risky` sur tous les fichiers touches. +18. **DoD** — valider les 10 criteres section 14. + +## 14. Critères d'acceptation (DoD) + +- [ ] `GET /api/sites`, `GET /api/sites/{id}` retournent 200 pour un user avec `sites.view`, 403 sinon. +- [ ] `POST /api/sites`, `PATCH /api/sites/{id}`, `DELETE /api/sites/{id}` retournent le code attendu pour un user avec `sites.manage`, 403 sinon. +- [ ] `GET /api/me` retourne `sites: Site[]` (objets complets) et `currentSite: Site | null`, avec les 3 sites pour `admin`, 1 pour `alice`, 1 pour `bob`. +- [ ] `PATCH /api/me/current-site` avec un site autorise → 200, `currentSite` mis a jour. Avec un site non autorise → 403. +- [ ] `DELETE /api/sites/{id}` cascade correctement : les lignes `user_site` sont purgees, les `current_site_id` pointant dessus repassent a `NULL`. +- [ ] `PATCH /api/users/{id}/rbac` accepte le champ `sites` ; retirer le `currentSite` de la liste le bascule a `null`. +- [ ] Page `/admin/sites` : liste, creation, edition, suppression fonctionnelles. +- [ ] `UserRbacDrawer.vue` : section "Sites autorises" visible et fonctionnelle. +- [ ] Sidebar : entree "Sites" visible pour un user avec `sites.view`, masquee sinon. Disparait si `SitesModule::class` est retire de `config/modules.php`. +- [ ] `make test` passe toutes les suites (les 5 nouvelles + les existantes ajustees aux fixtures). +- [ ] `make php-cs-fixer-allow-risky` propre sur les fichiers nouveaux et modifies. +- [ ] Desactiver `SitesModule::class` dans `config/modules.php` ne casse pas les endpoints Core (la DB reste valide, les users conservent leurs sites meme si l'UI ne les expose plus). diff --git a/docs/sites/ticket-03-spec.md b/docs/sites/ticket-03-spec.md new file mode 100644 index 0000000..7a04bc5 --- /dev/null +++ b/docs/sites/ticket-03-spec.md @@ -0,0 +1,503 @@ +# Ticket #03 — 3/4 — Barre de sélection de site (navbar horizontale) + +## 1. Objectif + +Ce ticket livre l'UI de consommation du module Sites pour l'utilisateur final : une barre horizontale en haut de l'application qui liste les sites autorises de l'utilisateur connecte, met en avant le site courant et permet de basculer d'un site a l'autre en un clic. + +Le ticket consomme la donnee posee par le ticket 2 (`/api/me` expose `sites` et `currentSite`, `PATCH /api/me/current-site` permet le switch) et s'appuie sur un nouveau composant `MalioSiteSelector` fourni par la version a jour de `@malio/layer-ui`. + +Resultat attendu : apres merge, un user avec ≥ 1 site voit une barre sous la navbar horizontale ; un clic sur un site non actif le rend actif, change l'etat global, et est persiste cote serveur. + +## 2. Périmètre + +### IN + +- **Upgrade** de `@malio/layer-ui` (actuellement `^1.3.0`) vers la version contenant `MalioSiteSelector`. La signature exacte du composant (props, slots, events) doit etre lue dans `node_modules/@malio/layer-ui/COMPONENTS.md` apres installation — la spec decrit le contrat attendu, le developpeur adapte selon l'API reelle (cf. Risque 1). +- Ajouter les champs `sites: Site[]` et `currentSite: Site | null` dans le type `UserData` (`frontend/shared/types/user-data.ts`) pour refleter le payload `/api/me` enrichi au ticket 2. +- Ajouter le type partage `Site` dans `frontend/shared/types/sites.ts` (deja cree au ticket 2, sinon a creer). +- Creer le composable `useCurrentSite()` dans `frontend/modules/sites/composables/` qui expose `currentSite`, `availableSites`, `switchSite(site)`, `resetCurrentSite()`. Pattern aligne sur `useSidebar()`. +- Creer le composable `useModules()` dans `frontend/shared/composables/` qui consomme `/api/modules` et expose `isModuleActive(id: string)`. Necessaire car `isModuleActive` est requis par le ticket mais n'existe pas encore cote front. +- Creer `SiteSelector.vue` dans `frontend/modules/sites/components/` : wrapper fin autour de `MalioSiteSelector` qui branche le composable `useCurrentSite()`, gere l'optimistic update avec rollback, emet un toast de succes/erreur. +- Integrer le selecteur dans `frontend/app/layouts/default.vue` — render conditionnel sur `isModuleActive('sites') && user.sites.length > 0`. +- Appeler `resetCurrentSite()` au logout (`frontend/modules/core/pages/logout.vue`), aligne sur `resetSidebar()` deja present. +- Gestion du **contraste automatique** : le texte du bloc passe en noir ou en blanc selon la luminance de `site.color`. Fonction utilitaire `getReadableTextColor(hex: string): 'black' | 'white'` dans `frontend/shared/utils/color.ts` (nouveau fichier utilitaire partage). +- Accessibilite : chaque bloc est un `