From d1378289194f0adc6e66f8fac986f6e2510cf4a2 Mon Sep 17 00:00:00 2001 From: tristan Date: Mon, 20 Apr 2026 10:09:05 +0200 Subject: [PATCH] feat(sites) : API CRUD + rattachement User<->Site + admin (ticket 2/4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exposition de Site via API Platform (5 operations RBAC sites.view/sites.manage), relation User.sites (M2M user_site EAGER) + User.currentSite (M2O nullable, ON DELETE SET NULL). Endpoint PATCH /api/me/current-site via ressource virtuelle + processor (SiteNotAuthorizedException → 403). UserRbacProcessor etendu avec gardes post-persist : auto-reset si currentSite retire, auto-select premier site si null + sites non vide. Page /admin/sites (DataTable + drawer creation/edition + modale suppression). UserRbacDrawer etendu avec section "Sites autorises". Colonne "Sites" ajoutee dans la table /admin/users (liste des noms separes par virgule). Sidebar entree Sites (module: sites, permission: sites.view). Refactor adresse : split full_address en street + complement (nullable) + getter computed Site::getFullAddress() multi-lignes. Migration ALTER dediee pour compat devs ayant deja joue le ticket 1. Fixtures avec vraies adresses (Chatellerault/Fontenet/Pommevic). Doctrine : inversedBy synchrone User.sites <-> Site.users pour maintenir la collection inverse en memoire. User::switchCurrentSite() porte la garde domaine (throw SiteNotAuthorizedException), aligne sur Role::ensureDeletable. Helper skipIfSitesModuleDisabled centralise dans AbstractApiTestCase. Tests : 182/182 (182/182 aussi module desactive, 2 skipped). 29 nouveaux tests PHPUnit (CRUD API, switch currentSite, cascade DB, /api/me enrichi, extension /rbac, gardes structurelles fullAddress/currentSite ignores, anti-cycle Site.users). 11 tests Vitest sur la validation hex couleur. Co-Authored-By: Claude Opus 4.7 (1M context) --- config/sidebar.php | 7 + frontend/i18n/locales/fr.json | 43 +++- .../core/components/UserRbacDrawer.vue | 41 ++- frontend/modules/core/pages/admin/users.vue | 31 ++- .../sites/components/SiteDeleteModal.vue | 76 ++++++ .../modules/sites/components/SiteDrawer.vue | 185 ++++++++++++++ frontend/modules/sites/nuxt.config.ts | 1 + frontend/modules/sites/pages/admin/sites.vue | 164 ++++++++++++ .../sites/utils/__tests__/color.test.ts | 50 ++++ frontend/modules/sites/utils/color.ts | 15 ++ frontend/shared/types/rbac.ts | 2 + frontend/shared/types/sites.ts | 24 ++ frontend/shared/types/user-data.ts | 6 + migrations/Version20260417120000.php | 5 + migrations/Version20260417150000.php | 88 +++++++ migrations/Version20260420130000.php | 78 ++++++ src/Module/Core/Domain/Entity/User.php | 122 +++++++++ .../State/Processor/UserRbacProcessor.php | 50 +++- .../DataFixtures/AppFixtures.php | 63 ++++- src/Module/Sites/Domain/Entity/Site.php | 223 ++++++++++++++--- .../Exception/SiteNotAuthorizedException.php | 27 ++ .../Resource/CurrentSiteResource.php | 53 ++++ .../State/Processor/CurrentSiteProcessor.php | 72 ++++++ .../DataFixtures/SitesFixtures.php | 35 +-- tests/Module/Core/Api/AbstractApiTestCase.php | 30 +++ .../Module/Core/Api/UserRbacSitesApiTest.php | 189 ++++++++++++++ .../Sites/Api/CurrentSiteSwitchApiTest.php | 92 +++++++ .../Module/Sites/Api/MeEndpointSitesTest.php | 116 +++++++++ tests/Module/Sites/Api/SiteApiTest.php | 235 ++++++++++++++++++ tests/Module/Sites/Api/SiteCascadeTest.php | 90 +++++++ tests/Module/Sites/Domain/Entity/SiteTest.php | 81 ++++-- .../Domain/Entity/SiteValidationTest.php | 94 ++++--- 32 files changed, 2271 insertions(+), 117 deletions(-) create mode 100644 frontend/modules/sites/components/SiteDeleteModal.vue create mode 100644 frontend/modules/sites/components/SiteDrawer.vue create mode 100644 frontend/modules/sites/nuxt.config.ts create mode 100644 frontend/modules/sites/pages/admin/sites.vue create mode 100644 frontend/modules/sites/utils/__tests__/color.test.ts create mode 100644 frontend/modules/sites/utils/color.ts create mode 100644 frontend/shared/types/sites.ts create mode 100644 migrations/Version20260417150000.php create mode 100644 migrations/Version20260420130000.php create mode 100644 src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php create mode 100644 src/Module/Sites/Infrastructure/ApiPlatform/Resource/CurrentSiteResource.php create mode 100644 src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php create mode 100644 tests/Module/Core/Api/UserRbacSitesApiTest.php create mode 100644 tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php create mode 100644 tests/Module/Sites/Api/MeEndpointSitesTest.php create mode 100644 tests/Module/Sites/Api/SiteApiTest.php create mode 100644 tests/Module/Sites/Api/SiteCascadeTest.php diff --git a/config/sidebar.php b/config/sidebar.php index 0b11e48..c82f02d 100644 --- a/config/sidebar.php +++ b/config/sidebar.php @@ -48,6 +48,13 @@ return [ 'module' => 'core', 'permission' => 'core.users.view', ], + [ + 'label' => 'sidebar.core.sites', + 'to' => '/admin/sites', + 'icon' => 'mdi:domain', + 'module' => 'sites', + 'permission' => 'sites.view', + ], [ 'label' => 'sidebar.general.logout', 'to' => '/logout', diff --git a/frontend/i18n/locales/fr.json b/frontend/i18n/locales/fr.json index 9acd89e..2dd1243 100644 --- a/frontend/i18n/locales/fr.json +++ b/frontend/i18n/locales/fr.json @@ -25,7 +25,8 @@ }, "core": { "roles": "Gestion des rôles", - "users": "Utilisateurs" + "users": "Utilisateurs", + "sites": "Sites" } }, "dashboard": { @@ -54,6 +55,9 @@ "put": "Erreur lors de la mise a jour", "patch": "Erreur lors de la modification", "delete": "Erreur lors de la suppression" + }, + "sites": { + "notAuthorized": "Vous n'êtes pas autorisé à sélectionner ce site." } }, "success": { @@ -102,7 +106,8 @@ "username": "Nom d'utilisateur", "admin": "Administrateur", "roles": "Roles", - "directPermissions": "Permissions directes" + "directPermissions": "Permissions directes", + "sites": "Sites" }, "drawer": { "title": "Permissions de {username}", @@ -110,6 +115,7 @@ "adminToggle": "Administrateur (bypass total)", "rolesSection": "Rôles", "directPermissionsSection": "Permissions directes", + "sitesSection": "Sites autorisés", "summarySection": "Résumé des permissions effectives", "noEffectivePermissions": "Aucune permission effective", "sourceRole": "via {role}", @@ -119,6 +125,39 @@ "toast": { "updated": "Permissions mises à jour avec succès" } + }, + "sites": { + "title": "Gestion des sites", + "newSite": "Nouveau site", + "editSite": "Modifier le site", + "createSite": "Créer un site", + "noSites": "Aucun site configuré", + "table": { + "name": "Nom", + "city": "Ville", + "postalCode": "Code postal", + "color": "Couleur", + "fullAddress": "Adresse complète" + }, + "form": { + "name": "Nom", + "street": "Rue", + "complement": "Complément d'adresse", + "complementPlaceholder": "Bâtiment, escalier, BP... (optionnel)", + "postalCode": "Code postal", + "city": "Ville", + "color": "Couleur (format #RRGGBB)", + "colorInvalid": "Format attendu : #RRGGBB (6 caractères hexadécimaux)" + }, + "delete": { + "title": "Supprimer le site", + "message": "Êtes-vous sûr de vouloir supprimer le site \"{name}\" ? Cette action est irréversible et retirera ce site à tous les utilisateurs rattachés." + }, + "toast": { + "created": "Site créé avec succès", + "updated": "Site mis à jour avec succès", + "deleted": "Site supprimé avec succès" + } } } } diff --git a/frontend/modules/core/components/UserRbacDrawer.vue b/frontend/modules/core/components/UserRbacDrawer.vue index 4a03ac9..2d6be9c 100644 --- a/frontend/modules/core/components/UserRbacDrawer.vue +++ b/frontend/modules/core/components/UserRbacDrawer.vue @@ -64,6 +64,27 @@ + +
+

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

+
+ {{ t('admin.sites.noSites') }} +
+
+ +
+
+

@@ -92,6 +113,7 @@ + + diff --git a/frontend/modules/sites/components/SiteDrawer.vue b/frontend/modules/sites/components/SiteDrawer.vue new file mode 100644 index 0000000..8d929c6 --- /dev/null +++ b/frontend/modules/sites/components/SiteDrawer.vue @@ -0,0 +1,185 @@ + + + diff --git a/frontend/modules/sites/nuxt.config.ts b/frontend/modules/sites/nuxt.config.ts new file mode 100644 index 0000000..268da7f --- /dev/null +++ b/frontend/modules/sites/nuxt.config.ts @@ -0,0 +1 @@ +export default defineNuxtConfig({}) diff --git a/frontend/modules/sites/pages/admin/sites.vue b/frontend/modules/sites/pages/admin/sites.vue new file mode 100644 index 0000000..f0cdead --- /dev/null +++ b/frontend/modules/sites/pages/admin/sites.vue @@ -0,0 +1,164 @@ + + + diff --git a/frontend/modules/sites/utils/__tests__/color.test.ts b/frontend/modules/sites/utils/__tests__/color.test.ts new file mode 100644 index 0000000..e07cad7 --- /dev/null +++ b/frontend/modules/sites/utils/__tests__/color.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from 'vitest' +import { isValidSiteColor } from '../color' + +describe('isValidSiteColor', () => { + it('accepte un hex majuscule', () => { + expect(isValidSiteColor('#ABCDEF')).toBe(true) + }) + + it('accepte un hex minuscule', () => { + expect(isValidSiteColor('#abcdef')).toBe(true) + }) + + it('accepte un hex mixte', () => { + expect(isValidSiteColor('#0a1B2c')).toBe(true) + }) + + it('accepte les couleurs fixtures du projet', () => { + expect(isValidSiteColor('#056CF2')).toBe(true) + expect(isValidSiteColor('#10B981')).toBe(true) + expect(isValidSiteColor('#F59E0B')).toBe(true) + }) + + it('rejette un nom CSS', () => { + expect(isValidSiteColor('red')).toBe(false) + }) + + it('rejette un hex court (3 caracteres)', () => { + expect(isValidSiteColor('#FFF')).toBe(false) + }) + + it('rejette un hex sans diese', () => { + expect(isValidSiteColor('FFFFFF')).toBe(false) + }) + + it('rejette un format rgb()', () => { + expect(isValidSiteColor('rgb(255, 0, 0)')).toBe(false) + }) + + it('rejette un hex trop long', () => { + expect(isValidSiteColor('#1234567')).toBe(false) + }) + + it('rejette un caractere non hex', () => { + expect(isValidSiteColor('#12345G')).toBe(false) + }) + + it('rejette une chaine vide', () => { + expect(isValidSiteColor('')).toBe(false) + }) +}) diff --git a/frontend/modules/sites/utils/color.ts b/frontend/modules/sites/utils/color.ts new file mode 100644 index 0000000..2530d30 --- /dev/null +++ b/frontend/modules/sites/utils/color.ts @@ -0,0 +1,15 @@ +/** + * Validation du format de couleur d'un site. + * + * Aligne sur la regex backend (Site entity) : seul le format #RRGGBB + * strict est accepte, avec 6 caracteres hexadecimaux apres le #. + * Tolere la casse (majuscules, minuscules, mixte). + * + * Utilise par SiteDrawer pour bloquer le submit cote front avant qu'une + * requete invalide parte au backend. + */ +const HEX_COLOR_REGEX = /^#[0-9A-Fa-f]{6}$/ + +export function isValidSiteColor(hex: string): boolean { + return HEX_COLOR_REGEX.test(hex) +} diff --git a/frontend/shared/types/rbac.ts b/frontend/shared/types/rbac.ts index d80a4d1..4b22f0c 100644 --- a/frontend/shared/types/rbac.ts +++ b/frontend/shared/types/rbac.ts @@ -21,6 +21,8 @@ export interface UserListItem { isAdmin: boolean roles: string[] directPermissions: string[] + /** IRIs des sites autorises (ticket 2 module Sites). */ + sites: string[] } export interface EffectivePermission { diff --git a/frontend/shared/types/sites.ts b/frontend/shared/types/sites.ts new file mode 100644 index 0000000..6ff1a47 --- /dev/null +++ b/frontend/shared/types/sites.ts @@ -0,0 +1,24 @@ +export interface Site { + id: number + name: string + street: string + complement: string | null + postalCode: string + city: string + color: string + /** Adresse complete reconstituee cote backend (getter computed). Lecture seule. */ + fullAddress: string +} + +/** + * Payload accepte en POST/PATCH /api/sites. Volontairement sans `fullAddress` + * (computed cote backend) ni champs read-only (id, timestamps). + */ +export interface SiteInput { + name: string + street: string + complement: string | null + postalCode: string + city: string + color: string +} diff --git a/frontend/shared/types/user-data.ts b/frontend/shared/types/user-data.ts index 8fd024e..7c5f54a 100644 --- a/frontend/shared/types/user-data.ts +++ b/frontend/shared/types/user-data.ts @@ -1,3 +1,5 @@ +import type { Site } from './sites' + export interface UserData { id: number username: string @@ -6,4 +8,8 @@ export interface UserData { isAdmin: boolean /** Codes de permission effectifs de l'utilisateur, tries alphabetiquement, sans doublon. */ effectivePermissions: string[] + /** Sites autorises pour l'utilisateur (ticket 2 module Sites). */ + sites: Site[] + /** Site actuellement selectionne par l'utilisateur, ou null si aucun. */ + currentSite: Site | null } diff --git a/migrations/Version20260417120000.php b/migrations/Version20260417120000.php index 6efd921..20d3c49 100644 --- a/migrations/Version20260417120000.php +++ b/migrations/Version20260417120000.php @@ -39,6 +39,11 @@ final class Version20260417120000 extends AbstractMigration // - `postal_code` est limite a 10 caracteres pour laisser marge a // d'eventuels formats etrangers plus tard, tout en le validant // strictement en 5 chiffres cote applicatif (format FR). + // + // Note : `full_address` est restructure au ticket 2 (migration + // Version20260420130000) en `street` + `complement` (nullable). La + // structure d'origine est conservee ici pour ne pas casser les devs + // qui ont deja joue cette migration en local. $this->addSql(<<<'SQL' CREATE TABLE site ( id INT GENERATED BY DEFAULT AS IDENTITY NOT NULL, diff --git a/migrations/Version20260417150000.php b/migrations/Version20260417150000.php new file mode 100644 index 0000000..fd8ee76 --- /dev/null +++ b/migrations/Version20260417150000.php @@ -0,0 +1,88 @@ +addSql(<<<'SQL' + CREATE TABLE user_site ( + user_id INT NOT NULL, + site_id INT NOT NULL, + PRIMARY KEY (user_id, site_id) + ) + SQL); + $this->addSql('CREATE INDEX IDX_user_site_user ON user_site (user_id)'); + $this->addSql('CREATE INDEX IDX_user_site_site ON user_site (site_id)'); + $this->addSql(<<<'SQL' + ALTER TABLE user_site + ADD CONSTRAINT FK_user_site_user + FOREIGN KEY (user_id) REFERENCES "user" (id) ON DELETE CASCADE + SQL); + $this->addSql(<<<'SQL' + ALTER TABLE user_site + ADD CONSTRAINT FK_user_site_site + FOREIGN KEY (site_id) REFERENCES site (id) ON DELETE CASCADE + SQL); + + // 2) Ajout de la colonne nullable user.current_site_id + FK SET NULL. + $this->addSql('ALTER TABLE "user" ADD current_site_id INT DEFAULT NULL'); + $this->addSql('CREATE INDEX IDX_user_current_site ON "user" (current_site_id)'); + $this->addSql(<<<'SQL' + ALTER TABLE "user" + ADD CONSTRAINT FK_user_current_site + FOREIGN KEY (current_site_id) REFERENCES site (id) ON DELETE SET NULL + SQL); + } + + public function down(Schema $schema): void + { + // Rollback en ordre inverse : enfants avant parents. + $this->addSql('ALTER TABLE "user" DROP CONSTRAINT FK_user_current_site'); + $this->addSql('DROP INDEX IDX_user_current_site'); + $this->addSql('ALTER TABLE "user" DROP current_site_id'); + + $this->addSql('ALTER TABLE user_site DROP CONSTRAINT FK_user_site_site'); + $this->addSql('ALTER TABLE user_site DROP CONSTRAINT FK_user_site_user'); + $this->addSql('DROP TABLE user_site'); + } +} diff --git a/migrations/Version20260420130000.php b/migrations/Version20260420130000.php new file mode 100644 index 0000000..3e87132 --- /dev/null +++ b/migrations/Version20260420130000.php @@ -0,0 +1,78 @@ +addSql('ALTER TABLE site ADD street VARCHAR(255) DEFAULT NULL'); + $this->addSql('ALTER TABLE site ADD complement VARCHAR(255) DEFAULT NULL'); + + // 2) Backfill : recopier full_address dans street pour ne pas perdre + // les donnees existantes. Les retours a la ligne sont preserves + // (PostgreSQL VARCHAR accepte \n) ; un admin pourra reformater + // apres coup si besoin. Cas d'adresse > 255 chars : la migration + // echoue cleanly (pas de tronquage silencieux). + $this->addSql('UPDATE site SET street = full_address'); + + // 3) Bascule street en NOT NULL une fois le backfill applique. + $this->addSql('ALTER TABLE site ALTER COLUMN street SET NOT NULL'); + + // 4) Drop de l'ancienne colonne full_address. + $this->addSql('ALTER TABLE site DROP full_address'); + } + + public function down(Schema $schema): void + { + // Recreation de full_address (NOT NULL via DEFAULT '' pour eviter + // un crash si la table a deja des lignes), puis backfill inverse, + // puis drop des nouvelles colonnes. + $this->addSql("ALTER TABLE site ADD full_address TEXT NOT NULL DEFAULT ''"); + $this->addSql("UPDATE site SET full_address = street || COALESCE(E'\\n' || complement, '')"); + $this->addSql('ALTER TABLE site ALTER COLUMN full_address DROP DEFAULT'); + + $this->addSql('ALTER TABLE site DROP street'); + $this->addSql('ALTER TABLE site DROP complement'); + } +} diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index 77a8f12..3c27d84 100644 --- a/src/Module/Core/Domain/Entity/User.php +++ b/src/Module/Core/Domain/Entity/User.php @@ -15,6 +15,8 @@ use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserProcessor; use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserRbacProcessor; use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\MeProvider; use App\Module\Core\Infrastructure\Doctrine\DoctrineUserRepository; +use App\Module\Sites\Domain\Entity\Site; +use App\Module\Sites\Domain\Exception\SiteNotAuthorizedException; use DateTimeImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; @@ -107,6 +109,39 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])] private Collection $directPermissions; + /** + * Sites autorises pour l'utilisateur (ticket 2 du module Sites). + * + * Relation ManyToMany avec table de jointure `user_site`. Fetch EAGER + * pour la meme raison que `$rbacRoles` : garantir que `/api/me` et les + * voters futurs aient toujours la collection hydratee, meme dans un + * contexte de refresh JWT hors EntityManager. Le surcout SQL reste + * negligeable (≤ quelques sites par user en pratique). + * + * @var Collection + */ + #[ORM\ManyToMany(targetEntity: Site::class, inversedBy: 'users', fetch: 'EAGER')] + #[ORM\JoinTable(name: 'user_site')] + #[Groups(['me:read', 'user:list', 'user:rbac:read', 'user:rbac:write'])] + private Collection $sites; + + /** + * Site courant selectionne par l'utilisateur (ticket 2 du module Sites). + * + * Relation ManyToOne nullable : un user peut ne pas avoir encore choisi + * de site actif (par ex. apres creation avant premier login). La FK porte + * `onDelete: SET NULL` pour que la suppression d'un site ne detruise pas + * les users qui le pointaient — ils repassent simplement a `null`. + * + * Doit TOUJOURS pointer vers un site present dans `$sites`. L'invariant + * est enforce par UserRbacProcessor qui bascule automatiquement a `null` + * si le site courant est retire des sites autorises. + */ + #[ORM\ManyToOne(targetEntity: Site::class, fetch: 'EAGER')] + #[ORM\JoinColumn(name: 'current_site_id', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')] + #[Groups(['me:read', 'user:list'])] + private ?Site $currentSite = null; + #[ORM\Column] private ?string $password = null; @@ -121,6 +156,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface $this->createdAt = new DateTimeImmutable(); $this->rbacRoles = new ArrayCollection(); $this->directPermissions = new ArrayCollection(); + $this->sites = new ArrayCollection(); } public function getId(): ?int @@ -313,4 +349,90 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface { $this->plainPassword = null; } + + /** + * @return Collection + */ + public function getSites(): Collection + { + return $this->sites; + } + + /** + * Idempotent : ajouter deux fois le meme site n'entraine pas de doublon. + * Synchronise la collection inverse Site::$users en memoire pour eviter + * un etat incoherent entre les deux cotes de la M2M dans une meme + * session Doctrine (cf. ticket 2 review point #1). + */ + public function addSite(Site $site): static + { + if (!$this->sites->contains($site)) { + $this->sites->add($site); + $site->addUser($this); + } + + return $this; + } + + /** + * Retire un site de la collection + maintient la collection inverse en + * memoire (cf. addSite). Attention : ne met PAS a jour `$currentSite` + * si le site retire en etait le courant — cet invariant est enforce + * par UserRbacProcessor (cote applicatif) ou doit etre maintenu + * explicitement par l'appelant. Voir Risque 2 du ticket 2 spec. + */ + public function removeSite(Site $site): static + { + if ($this->sites->removeElement($site)) { + $site->removeUser($this); + } + + return $this; + } + + /** + * Garde applicative rapide : teste la presence d'un site dans la + * collection autorisee, via comparaison d'identite d'objet Doctrine. + * Utilise par CurrentSiteProcessor pour valider un switch. + */ + public function hasSite(Site $site): bool + { + return $this->sites->contains($site); + } + + public function getCurrentSite(): ?Site + { + return $this->currentSite; + } + + /** + * Setter brut, sans garde. Usage interne pour les flux qui doivent + * pouvoir positionner un site arbitraire ou null (reset de coherence + * post-PATCH RBAC, fixtures, init). Pour le flux user-facing + * "selectionner un site dans la liste autorisee", utiliser + * switchCurrentSite() qui porte la garde domaine. + */ + public function setCurrentSite(?Site $currentSite): static + { + $this->currentSite = $currentSite; + + return $this; + } + + /** + * Garde domaine du switch utilisateur : refuse un site qui n'est pas + * dans la collection autorisee. Levee d'une exception domaine que le + * processor HTTP traduit en 403 (pattern aligne sur Role::ensureDeletable + * → SystemRoleDeletionException). + * + * @throws SiteNotAuthorizedException si $site n'appartient pas a $this->sites + */ + public function switchCurrentSite(Site $site): void + { + if (!$this->hasSite($site)) { + throw SiteNotAuthorizedException::forSite($site); + } + + $this->currentSite = $site; + } } diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index 9f0029b..7800cf3 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -29,6 +29,14 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; * - Dernier admin global : impossible de retirer `isAdmin` si c'est le * dernier administrateur de l'instance, meme par un tiers. Enforce via * AdminHeadcountGuardInterface. + * - Coherence currentSite (ticket 2 module Sites) : apres persist des + * sites autorises, si le `currentSite` n'est plus dans la collection, + * il est repositionne automatiquement : + * a) repasse a `null` s'il pointait vers un site retire ; + * b) est auto-selectionne sur le premier site de `sites` s'il etait + * null alors que la collection n'est pas vide (pratique pour un + * premier rattachement). + * Un second flush est emis uniquement si la coherence a du etre corrigee. * * @implements ProcessorInterface */ @@ -80,6 +88,46 @@ final class UserRbacProcessor implements ProcessorInterface } } - return $this->persistProcessor->process($data, $operation, $uriVariables, $context); + $result = $this->persistProcessor->process($data, $operation, $uriVariables, $context); + + // Garde coherence currentSite (ticket 2 module Sites). + // Post-persist : le champ `sites` a ete applique par le persist processor. + // On s'assure que `currentSite` pointe toujours vers un site present + // dans la collection ou est recale automatiquement. + $this->ensureCurrentSiteConsistency($data); + + return $result; + } + + /** + * Applique deux corrections post-persist sur `currentSite` : + * - si l'actuel n'est plus dans `sites` apres update → repasse a null ; + * - si null et `sites` non vide → auto-selectionne le premier site + * (coherent avec le choix de ne jamais laisser un user rattache a + * plusieurs sites sans contexte courant). + * + * N'emet un flush additionnel que si une correction a ete necessaire : + * pas de cout DB sur la majorite des requetes /rbac qui ne touchent pas + * aux sites. + */ + private function ensureCurrentSiteConsistency(User $user): void + { + $currentSite = $user->getCurrentSite(); + $sites = $user->getSites(); + $changed = false; + + if (null !== $currentSite && !$user->hasSite($currentSite)) { + $user->setCurrentSite(null); + $changed = true; + } + + if (null === $user->getCurrentSite() && !$sites->isEmpty()) { + $user->setCurrentSite($sites->first() ?: null); + $changed = true; + } + + if ($changed) { + $this->entityManager->flush(); + } } } diff --git a/src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php b/src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php index 855bba5..1c2be08 100644 --- a/src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php +++ b/src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php @@ -8,26 +8,50 @@ use App\Module\Core\Domain\Entity\Role; use App\Module\Core\Domain\Entity\User; use App\Module\Core\Domain\Repository\RoleRepositoryInterface; use App\Module\Core\Domain\Security\SystemRoles; +use App\Module\Sites\Domain\Entity\Site; +use App\Module\Sites\Domain\Repository\SiteRepositoryInterface; +use App\Module\Sites\Infrastructure\DataFixtures\SitesFixtures; use Doctrine\Bundle\FixturesBundle\Fixture; +use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; +use RuntimeException; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; /** * Fixtures de base du module Core : 3 utilisateurs (1 admin + 2 standards) - * rattaches aux roles systeme RBAC seedes par la migration Version20260414150034. + * rattaches aux roles systeme RBAC seedes par la migration Version20260414150034, + * puis (ticket 2 module Sites) rattaches a au moins un site avec un currentSite + * coherent. * * Note : le purger Doctrine execute avant load() supprime l'ensemble des * entites managees, ce qui inclut la table role. On re-seede donc les roles * systeme de maniere idempotente avant de rattacher les utilisateurs, afin * que le workflow "make db-reset && make fixtures" reste one-shot. + * + * Dependance explicite a SitesFixtures (ticket 2) : les 3 sites Chatellerault, + * Saint-Jean et Pommevic doivent etre presents en base avant d'etre rattaches + * aux users. L'inversion volontaire de l'ordre (AppFixtures ← SitesFixtures) + * casse l'independance declaree au ticket 1 : c'est un couplage assume car + * apres ticket 2 le modele metier exprime un besoin legitime de rattachement. */ -class AppFixtures extends Fixture +class AppFixtures extends Fixture implements DependentFixtureInterface { public function __construct( private readonly UserPasswordHasherInterface $passwordHasher, private readonly RoleRepositoryInterface $roleRepository, + private readonly SiteRepositoryInterface $siteRepository, ) {} + /** + * @return array + */ + public function getDependencies(): array + { + // SitesFixtures doit tourner AVANT AppFixtures pour que les sites + // soient disponibles au rattachement des users ci-dessous. + return [SitesFixtures::class]; + } + public function load(ObjectManager $manager): void { $adminRole = $this->ensureSystemRole( @@ -43,23 +67,43 @@ class AppFixtures extends Fixture 'Role de base sans permission specifique', ); + // Recupere les 3 sites seedes par SitesFixtures. Si absents, c'est + // une misconfiguration (fixture hors purge ou dependance ignoree) : + // on fail fort avec un message explicite plutot que de continuer + // avec des users orphelins de site. + $chatellerault = $this->requireSite('Chatellerault'); + $saintJean = $this->requireSite('Saint-Jean'); + $pommevic = $this->requireSite('Pommevic'); + $admin = new User(); $admin->setUsername('admin'); $admin->setIsAdmin(true); $admin->setPassword($this->passwordHasher->hashPassword($admin, 'admin')); $admin->addRbacRole($adminRole); + // Admin rattache aux 3 sites pour faciliter le dev / les tests manuels. + $admin->addSite($chatellerault); + $admin->addSite($saintJean); + $admin->addSite($pommevic); + $admin->setCurrentSite($chatellerault); $manager->persist($admin); $alice = new User(); $alice->setUsername('alice'); $alice->setPassword($this->passwordHasher->hashPassword($alice, 'alice')); $alice->addRbacRole($userRole); + // Alice : un seul site, site courant = ce site. + $alice->addSite($chatellerault); + $alice->setCurrentSite($chatellerault); $manager->persist($alice); $bob = new User(); $bob->setUsername('bob'); $bob->setPassword($this->passwordHasher->hashPassword($bob, 'bob')); $bob->addRbacRole($userRole); + // Bob : site different de Alice, pour prouver le filtrage par site + // dans les futurs tests (ticket 4 outillage SiteAware). + $bob->addSite($saintJean); + $bob->setCurrentSite($saintJean); $manager->persist($bob); $manager->flush(); @@ -90,4 +134,19 @@ class AppFixtures extends Fixture return $role; } + + private function requireSite(string $name): Site + { + $site = $this->siteRepository->findByName($name); + + if (null === $site) { + throw new RuntimeException(sprintf( + 'SitesFixtures doit avoir seede le site "%s" avant le chargement des users. ' + .'Verifier que SitesFixtures est bien en dependance de AppFixtures.', + $name, + )); + } + + return $site; + } } diff --git a/src/Module/Sites/Domain/Entity/Site.php b/src/Module/Sites/Domain/Entity/Site.php index 2ac11b4..da75255 100644 --- a/src/Module/Sites/Domain/Entity/Site.php +++ b/src/Module/Sites/Domain/Entity/Site.php @@ -4,22 +4,63 @@ declare(strict_types=1); namespace App\Module\Sites\Domain\Entity; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Delete; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; +use App\Module\Core\Domain\Entity\User; use App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository; use DateTimeImmutable; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity; +use Symfony\Component\Serializer\Attribute\Groups; use Symfony\Component\Validator\Constraints as Assert; /** * Site physique (usine / etablissement) appartenant a l'instance Coltura. * - * Brique fondatrice du module Sites : cette entite n'est pas exposee par - * une ressource API Platform dans ce ticket (ticket 1/4). Elle sert de socle - * de donnees aux tickets suivants (rattachement utilisateurs, affichage - * navbar, etc.). Aucune dependance dure depuis le module Core : la table - * est creee meme si le module est desactive (voir migration dediee). + * Adresse decomposee en champs structures (rue, complement, CP, ville) pour + * permettre des recherches/tris fins ulterieurs et eviter les divergences + * entre champs duplique. La methode `getFullAddress()` fournit la version + * concatenee multi-lignes pour les usages d'affichage. + * + * Expose en API Platform pour l'administration CRUD avec RBAC : + * - lecture (GET list / item) : requiert la permission `sites.view` + * - ecriture (POST / PATCH / DELETE) : requiert la permission `sites.manage` + * + * Egalement embarque dans la reponse `/api/me` (groupe `me:read`) pour que + * le frontend connaisse les sites autorises et le site courant de l'user. */ +#[ApiResource( + operations: [ + new GetCollection( + normalizationContext: ['groups' => ['site:read']], + security: "is_granted('sites.view')", + ), + new Get( + normalizationContext: ['groups' => ['site:read']], + security: "is_granted('sites.view')", + ), + new Post( + normalizationContext: ['groups' => ['site:read']], + denormalizationContext: ['groups' => ['site:write']], + security: "is_granted('sites.manage')", + ), + new Patch( + normalizationContext: ['groups' => ['site:read']], + denormalizationContext: ['groups' => ['site:write']], + security: "is_granted('sites.manage')", + ), + new Delete(security: "is_granted('sites.manage')"), + ], + normalizationContext: ['groups' => ['site:read']], + denormalizationContext: ['groups' => ['site:write']], +)] #[ORM\Entity(repositoryClass: DoctrineSiteRepository::class)] #[ORM\Table(name: 'site')] #[ORM\UniqueConstraint(name: 'uniq_site_name', columns: ['name'])] @@ -30,17 +71,27 @@ class Site #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] + #[Groups(['site:read', 'me:read'])] private ?int $id = null; #[ORM\Column(length: 100)] #[Assert\NotBlank(message: 'Le nom du site est requis.')] #[Assert\Length(max: 100, maxMessage: 'Le nom du site ne peut pas depasser {{ limit }} caracteres.')] + #[Groups(['site:read', 'site:write', 'me:read'])] private string $name; - #[ORM\Column(length: 100)] - #[Assert\NotBlank(message: 'La ville du site est requise.')] - #[Assert\Length(max: 100, maxMessage: 'La ville ne peut pas depasser {{ limit }} caracteres.')] - private string $city; + // Premiere ligne d'adresse : numero + voie. Requise. + #[ORM\Column(length: 255)] + #[Assert\NotBlank(message: 'La rue est requise.')] + #[Assert\Length(max: 255, maxMessage: 'La rue ne peut pas depasser {{ limit }} caracteres.')] + #[Groups(['site:read', 'site:write', 'me:read'])] + private string $street; + + // Complement d'adresse optionnel : batiment, escalier, BP, etc. + #[ORM\Column(length: 255, nullable: true)] + #[Assert\Length(max: 255, maxMessage: 'Le complement ne peut pas depasser {{ limit }} caracteres.')] + #[Groups(['site:read', 'site:write', 'me:read'])] + private ?string $complement = null; // Colonne mappee sur le snake_case PostgreSQL (convention projet : noms de // colonnes en minuscules dans le SQL brut). Le format est contraint au @@ -52,50 +103,71 @@ class Site pattern: '/^\d{5}$/', message: 'Le code postal doit etre compose de 5 chiffres (format FR).', )] + #[Groups(['site:read', 'site:write', 'me:read'])] private string $postalCode; + #[ORM\Column(length: 100)] + #[Assert\NotBlank(message: 'La ville du site est requise.')] + #[Assert\Length(max: 100, maxMessage: 'La ville ne peut pas depasser {{ limit }} caracteres.')] + #[Groups(['site:read', 'site:write', 'me:read'])] + private string $city; + // Couleur d'identification visuelle du site au format hex #RRGGBB (7 chars - // incluant le diese). Utilisee plus tard par la navbar (ticket 3) pour - // distinguer les sites d'un coup d'oeil. + // incluant le diese). Utilisee par la navbar (ticket 3) pour distinguer + // les sites d'un coup d'oeil. #[ORM\Column(length: 7)] #[Assert\NotBlank(message: 'La couleur est requise.')] #[Assert\Regex( pattern: '/^#[0-9A-Fa-f]{6}$/', message: 'La couleur doit etre un code hex de 7 caracteres au format #RRGGBB.', )] + #[Groups(['site:read', 'site:write', 'me:read'])] private string $color; - // Champ TEXT volontaire : l'adresse complete peut courir sur plusieurs - // lignes (voie + complement + mention particuliere). Borne haute a 500 - // caracteres : une adresse francaise complete tient tres largement dans - // cette enveloppe, et la limite applicative protege contre les payloads - // anormalement volumineux envoyes par un client (garde DoS basique). - #[ORM\Column(name: 'full_address', type: Types::TEXT)] - #[Assert\NotBlank(message: 'L\'adresse complete est requise.')] - #[Assert\Length(max: 500, maxMessage: 'L\'adresse complete ne peut pas depasser {{ limit }} caracteres.')] - private string $fullAddress; - + // createdAt / updatedAt volontairement exclus du groupe `me:read` : + // le payload `/api/me` doit rester leger, ces metadonnees ne sont utiles + // qu'a l'admin (exposees uniquement via `site:read` sur /api/sites). #[ORM\Column(name: 'created_at', type: Types::DATETIME_IMMUTABLE)] + #[Groups(['site:read'])] private DateTimeImmutable $createdAt; #[ORM\Column(name: 'updated_at', type: Types::DATETIME_IMMUTABLE)] + #[Groups(['site:read'])] private DateTimeImmutable $updatedAt; + /** + * Collection inverse des users rattaches a ce site. + * + * Volontairement SANS `#[Groups]` : la collection n'est jamais exposee via + * l'API pour deux raisons : + * - eviter une boucle de serialisation infinie User → sites → users → ... + * si un jour un developpeur ajoute `me:read` ici par megarde ; + * - l'inverse n'a de valeur qu'en interne (compter les users d'un site, + * iterer en test de cascade). + * + * @var Collection + */ + #[ORM\ManyToMany(targetEntity: User::class, mappedBy: 'sites')] + private Collection $users; + public function __construct( string $name, - string $city, + string $street, + ?string $complement, string $postalCode, + string $city, 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; + $this->name = $name; + $this->street = $street; + $this->complement = $complement; + $this->postalCode = $postalCode; + $this->city = $city; + $this->color = $color; + $now = new DateTimeImmutable(); + $this->createdAt = $now; + $this->updatedAt = $now; + $this->users = new ArrayCollection(); } /** @@ -125,14 +197,26 @@ class Site return $this; } - public function getCity(): string + public function getStreet(): string { - return $this->city; + return $this->street; } - public function setCity(string $city): static + public function setStreet(string $street): static { - $this->city = $city; + $this->street = $street; + + return $this; + } + + public function getComplement(): ?string + { + return $this->complement; + } + + public function setComplement(?string $complement): static + { + $this->complement = $complement; return $this; } @@ -149,6 +233,18 @@ class Site return $this; } + public function getCity(): string + { + return $this->city; + } + + public function setCity(string $city): static + { + $this->city = $city; + + return $this; + } + public function getColor(): string { return $this->color; @@ -161,16 +257,26 @@ class Site return $this; } + /** + * Adresse complete reconstituee : street, [complement,] {CP} {ville}, + * separes par des sauts de ligne. Methode pure, jamais persistee. + * + * Expose en lecture API (groupes site:read + me:read) pour que les + * consommateurs (frontend, exports PDF) recoivent une adresse prete a + * afficher sans dupliquer la logique de concatenation cote client. + */ + #[Groups(['site:read', 'me:read'])] public function getFullAddress(): string { - return $this->fullAddress; - } + $lines = [$this->street]; - public function setFullAddress(string $fullAddress): static - { - $this->fullAddress = $fullAddress; + if (null !== $this->complement && '' !== trim($this->complement)) { + $lines[] = $this->complement; + } - return $this; + $lines[] = sprintf('%s %s', $this->postalCode, $this->city); + + return implode("\n", $lines); } public function getCreatedAt(): DateTimeImmutable @@ -182,4 +288,39 @@ class Site { return $this->updatedAt; } + + /** + * @return Collection + */ + public function getUsers(): Collection + { + return $this->users; + } + + /** + * Synchronise la collection inverse cote Site quand User::addSite est + * appele. Idempotent. Ne re-appelle pas $user->addSite($this) pour + * eviter une recursion infinie : User::addSite est le point d'entree + * unique de la mutation. + * + * @internal Appele uniquement par User::addSite() + */ + public function addUser(User $user): static + { + if (!$this->users->contains($user)) { + $this->users->add($user); + } + + return $this; + } + + /** + * @internal Appele uniquement par User::removeSite() + */ + public function removeUser(User $user): static + { + $this->users->removeElement($user); + + return $this; + } } diff --git a/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php b/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php new file mode 100644 index 0000000..919e97e --- /dev/null +++ b/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php @@ -0,0 +1,27 @@ +getName(), + )); + } +} diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/Resource/CurrentSiteResource.php b/src/Module/Sites/Infrastructure/ApiPlatform/Resource/CurrentSiteResource.php new file mode 100644 index 0000000..510bbe6 --- /dev/null +++ b/src/Module/Sites/Infrastructure/ApiPlatform/Resource/CurrentSiteResource.php @@ -0,0 +1,53 @@ + ['me:read']], + denormalizationContext: ['groups' => ['current-site:write']], + processor: CurrentSiteProcessor::class, + read: false, + priority: 1, + ), + ], +)] +final class CurrentSiteResource +{ + /** + * Site cible du switch, denormalise depuis l'IRI envoye dans le body : + * `{ "site": "/api/sites/{id}" }`. Resolu automatiquement par + * l'IriConverter d'API Platform en instance de `Site`. + */ + #[Groups(['current-site:write'])] + public ?Site $site = null; +} diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php new file mode 100644 index 0000000..5e2579a --- /dev/null +++ b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php @@ -0,0 +1,72 @@ + + */ +final class CurrentSiteProcessor implements ProcessorInterface +{ + public function __construct( + private readonly Security $security, + private readonly EntityManagerInterface $entityManager, + ) {} + + public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed + { + if (!$data instanceof CurrentSiteResource) { + throw new LogicException(sprintf( + 'CurrentSiteProcessor attend une instance de %s, %s recu.', + CurrentSiteResource::class, + get_debug_type($data), + )); + } + + $user = $this->security->getUser(); + if (!$user instanceof User) { + // security: "is_granted('ROLE_USER')" sur l'operation doit deja + // bloquer ce cas — garde defensive si la config change. + throw new AccessDeniedHttpException('Authentification requise pour changer de site courant.'); + } + + $targetSite = $data->site; + if (null === $targetSite) { + throw new BadRequestHttpException('Le champ "site" est requis.'); + } + + try { + $user->switchCurrentSite($targetSite); + } catch (SiteNotAuthorizedException $e) { + // Traduction HTTP immediate (pas de listener kernel necessaire) : + // aligne sur le pattern RoleProcessor → SystemRoleDeletionException. + throw new AccessDeniedHttpException($e->getMessage(), $e); + } + + $this->entityManager->flush(); + + return $user; + } +} diff --git a/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php b/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php index 194fd14..2362271 100644 --- a/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php +++ b/src/Module/Sites/Infrastructure/DataFixtures/SitesFixtures.php @@ -40,38 +40,43 @@ class SitesFixtures extends Fixture $this->ensureSite( $manager, name: 'Chatellerault', - city: 'Chatellerault', + street: "14 All. d'Argenson", + complement: null, postalCode: '86100', + city: 'Châtellerault', color: '#056CF2', - fullAddress: "1 avenue de l'Europe\n86100 Chatellerault", ); // Saint-Jean : vert emeraude pour contraster avec le bleu Chatellerault. + // Note : le nom du site (identifier) ne reflete pas la ville reelle + // (Fontenet) — c'est une nomenclature interne client. $this->ensureSite( $manager, name: 'Saint-Jean', - city: 'Saint-Jean-de-Sauves', - postalCode: '86330', + street: 'Z i', + complement: null, + postalCode: '17400', + city: 'Fontenet', color: '#10B981', - fullAddress: "12 route de Poitiers\n86330 Saint-Jean-de-Sauves", ); // Pommevic : ambre pour une troisieme teinte nettement distincte. $this->ensureSite( $manager, name: 'Pommevic', - city: 'Pommevic', + street: '1 Av. Jean Duquesne', + complement: null, postalCode: '82400', + city: 'Pommevic', color: '#F59E0B', - fullAddress: "5 chemin des Peupliers\n82400 Pommevic", ); $manager->flush(); } /** - * Cree le site s'il n'existe pas encore, sinon re-aligne ville, code - * postal, couleur et adresse sur les valeurs de reference. + * Cree le site s'il n'existe pas encore, sinon re-aligne rue, complement, + * code postal, ville et couleur sur les valeurs de reference. * * Note : le nom sert de cle de lookup (il est unique en base) et n'est * donc pas resynchronise. Consequence : renommer un site dans la @@ -81,24 +86,26 @@ class SitesFixtures extends Fixture private function ensureSite( ObjectManager $manager, string $name, - string $city, + string $street, + ?string $complement, string $postalCode, + string $city, string $color, - string $fullAddress, ): Site { $site = $this->siteRepository->findByName($name); if (null === $site) { - $site = new Site($name, $city, $postalCode, $color, $fullAddress); + $site = new Site($name, $street, $complement, $postalCode, $city, $color); $manager->persist($site); return $site; } - $site->setCity($city); + $site->setStreet($street); + $site->setComplement($complement); $site->setPostalCode($postalCode); + $site->setCity($city); $site->setColor($color); - $site->setFullAddress($fullAddress); return $site; } diff --git a/tests/Module/Core/Api/AbstractApiTestCase.php b/tests/Module/Core/Api/AbstractApiTestCase.php index c4993dc..6d1b7b4 100644 --- a/tests/Module/Core/Api/AbstractApiTestCase.php +++ b/tests/Module/Core/Api/AbstractApiTestCase.php @@ -130,4 +130,34 @@ abstract class AbstractApiTestCase extends ApiTestCase return ['username' => $username, 'password' => $password]; } + + /** + * Skip le test courant si le module Sites est desactive dans + * `config/modules.php` de l'environnement de test. + * + * Mecanisme : on cherche la permission `sites.view` en base. Si le + * module Sites est desactive, `app:sync-permissions` aura marque cette + * permission comme orpheline et l'aura supprimee de la table — donc + * `findOneBy(['code' => 'sites.view'])` renvoie null. + * + * Quand utiliser ce helper : tests qui s'appuient sur + * `createUserWithPermission('sites.*')`. Les tests qui utilisent + * uniquement l'admin (qui bypass via isAdmin) n'en ont pas besoin : + * la classe Site reste mappee Doctrine et exposee via API Platform + * meme module desactive (mapping inconditionnel, decision assumee + * ticket 1). + */ + protected function skipIfSitesModuleDisabled(): void + { + if (!self::$kernel) { + self::bootKernel(); + } + $perm = $this->getEm() + ->getRepository(Permission::class) + ->findOneBy(['code' => 'sites.view']) + ; + if (null === $perm) { + self::markTestSkipped('Module Sites desactive : permission sites.view introuvable en base.'); + } + } } diff --git a/tests/Module/Core/Api/UserRbacSitesApiTest.php b/tests/Module/Core/Api/UserRbacSitesApiTest.php new file mode 100644 index 0000000..29fa75d --- /dev/null +++ b/tests/Module/Core/Api/UserRbacSitesApiTest.php @@ -0,0 +1,189 @@ +getEm(); + $saintJean = $em->getRepository(Site::class)->findOneBy(['name' => 'Saint-Jean']); + self::assertNotNull($saintJean); + + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'sites' => ['/api/sites/'.$saintJean->getId()], + ], + ]); + + self::assertResponseIsSuccessful(); + + // Verification cote base. + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertNotNull($reloaded); + self::assertCount(1, $reloaded->getSites()); + self::assertSame('Saint-Jean', $reloaded->getSites()->first()->getName()); + + // Restauration pour ne pas polluer les autres tests. + $this->restoreAliceSites(); + } + + public function testRemovingCurrentSiteResetsCurrentSiteToNullThenAutoSelectsFirst(): void + { + // alice a actuellement {Chatellerault}, currentSite=Chatellerault. + // On lui attribue {Saint-Jean} : Chatellerault disparait → currentSite + // devrait temporairement etre null, PUIS auto-select Saint-Jean (seul + // site restant). + $em = $this->getEm(); + $saintJean = $em->getRepository(Site::class)->findOneBy(['name' => 'Saint-Jean']); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'sites' => ['/api/sites/'.$saintJean->getId()], + ], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertNotNull($reloaded->getCurrentSite()); + self::assertSame('Saint-Jean', $reloaded->getCurrentSite()->getName()); + + $this->restoreAliceSites(); + } + + public function testEmptySitesPayloadResetsCurrentSiteToNull(): void + { + $em = $this->getEm(); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'sites' => [], + ], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertCount(0, $reloaded->getSites()); + self::assertNull($reloaded->getCurrentSite()); + + $this->restoreAliceSites(); + } + + public function testCurrentSiteFieldInRbacPayloadIsSilentlyIgnored(): void + { + // Garde structurelle : `currentSite` n'est pas dans le groupe + // user:rbac:write. Un client malveillant qui essaierait de set un + // currentSite arbitraire via /rbac doit etre silencieusement + // ignore (le seul flux autorise est PATCH /me/current-site). + $em = $this->getEm(); + $pommevic = $em->getRepository(Site::class)->findOneBy(['name' => 'Pommevic']); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'currentSite' => '/api/sites/'.$pommevic->getId(), + ], + ]); + + self::assertResponseIsSuccessful(); + + // alice n'a Pommevic ni dans ses sites ni en currentSite (le champ + // a ete ignore par le denormalizer). Son currentSite reste son + // Chatellerault d'origine. + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertNotNull($reloaded); + self::assertNotNull($reloaded->getCurrentSite()); + self::assertSame('Chatellerault', $reloaded->getCurrentSite()->getName()); + } + + public function testRbacPatchWithoutSitesFieldDoesNotChangeCurrentSite(): void + { + // Garde structurelle : si le payload /rbac ne contient pas le champ + // `sites`, ensureCurrentSiteConsistency ne doit pas auto-modifier + // le currentSite (alice avait deja Chatellerault). Un PATCH qui + // change uniquement isAdmin ou roles ne doit pas remuer la + // configuration site de l'user. + $em = $this->getEm(); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + $aliceId = $alice->getId(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'isAdmin' => false, + ], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + $reloaded = $em->getRepository(User::class)->find($aliceId); + self::assertNotNull($reloaded->getCurrentSite()); + self::assertSame('Chatellerault', $reloaded->getCurrentSite()->getName()); + } + + /** + * Remet alice dans l'etat des fixtures : un seul site Chatellerault, + * currentSite Chatellerault. Evite la pollution inter-tests. + */ + private function restoreAliceSites(): void + { + $em = $this->getEm(); + $chatellerault = $em->getRepository(Site::class)->findOneBy(['name' => 'Chatellerault']); + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + + // Reset complet des sites + foreach ($alice->getSites() as $existing) { + $alice->removeSite($existing); + } + $alice->addSite($chatellerault); + $alice->setCurrentSite($chatellerault); + $em->flush(); + } +} diff --git a/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php b/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php new file mode 100644 index 0000000..e8bf214 --- /dev/null +++ b/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php @@ -0,0 +1,92 @@ +getEm(); + $pommevic = $em->getRepository(Site::class)->findOneBy(['name' => 'Pommevic']); + self::assertNotNull($pommevic); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('PATCH', '/api/me/current-site', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['site' => '/api/sites/'.$pommevic->getId()], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame('Pommevic', $data['currentSite']['name']); + } + + public function testUserCannotSwitchToUnauthorizedSite(): void + { + // alice n'a que Chatellerault. Tenter Pommevic → 403. + $em = $this->getEm(); + $pommevic = $em->getRepository(Site::class)->findOneBy(['name' => 'Pommevic']); + self::assertNotNull($pommevic); + + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('PATCH', '/api/me/current-site', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['site' => '/api/sites/'.$pommevic->getId()], + ]); + + self::assertResponseStatusCodeSame(403); + } + + public function testSwitchWithMissingSiteFieldReturns400(): void + { + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('PATCH', '/api/me/current-site', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [], + ]); + + self::assertResponseStatusCodeSame(400); + } + + public function testAnonymousUserCannotSwitch(): void + { + $client = self::createClient(); + $client->request('PATCH', '/api/me/current-site', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['site' => '/api/sites/1'], + ]); + + self::assertResponseStatusCodeSame(401); + } + + public function testSwitchWithNonExistentSiteIriReturnsErrorStatus(): void + { + // IRI vers un site qui n'existe pas en base : API Platform leve un + // 400 Bad Request a la denormalisation (l'IriConverter ne peut pas + // resoudre l'IRI). On grave le code de retour reel pour eviter + // qu'une regression silencieuse passe inapercue. + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('PATCH', '/api/me/current-site', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['site' => '/api/sites/999999'], + ]); + + self::assertResponseStatusCodeSame(400); + } +} diff --git a/tests/Module/Sites/Api/MeEndpointSitesTest.php b/tests/Module/Sites/Api/MeEndpointSitesTest.php new file mode 100644 index 0000000..14452e5 --- /dev/null +++ b/tests/Module/Sites/Api/MeEndpointSitesTest.php @@ -0,0 +1,116 @@ +authenticatedClient('alice', 'alice'); + $response = $client->request('GET', '/api/me'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + + self::assertArrayHasKey('sites', $data); + self::assertIsArray($data['sites']); + self::assertCount(1, $data['sites']); + + $firstSite = $data['sites'][0]; + self::assertIsArray($firstSite, 'Un site doit etre serialise en objet, pas en IRI string.'); + self::assertArrayHasKey('id', $firstSite); + self::assertArrayHasKey('name', $firstSite); + self::assertArrayHasKey('street', $firstSite); + self::assertArrayHasKey('city', $firstSite); + self::assertArrayHasKey('color', $firstSite); + // Le getter computed est expose en lecture pour eviter au front + // de redupliquer la logique de concatenation. + self::assertArrayHasKey('fullAddress', $firstSite); + self::assertSame('Chatellerault', $firstSite['name']); + + // Garde anti-cycle (cf. Site::$users sans Groups, ticket 2 spec + // section 12 risque 6) : la collection inverse ne doit JAMAIS etre + // serialisee dans /api/me sous peine de boucle infinie + // User → sites → users → sites → ... + self::assertArrayNotHasKey( + 'users', + $firstSite, + 'Site.users ne doit JAMAIS etre serialise dans /api/me (cycle infini).', + ); + } + + public function testMeExposesCurrentSiteAsObject(): void + { + $client = $this->authenticatedClient('alice', 'alice'); + $response = $client->request('GET', '/api/me'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + + self::assertArrayHasKey('currentSite', $data); + self::assertIsArray($data['currentSite'], 'currentSite doit etre un objet, pas une IRI.'); + self::assertSame('Chatellerault', $data['currentSite']['name']); + } + + public function testAdminHasAllThreeSites(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/me'); + + $data = $response->toArray(); + self::assertCount(3, $data['sites']); + + $names = array_column($data['sites'], 'name'); + sort($names); + self::assertSame(['Chatellerault', 'Pommevic', 'Saint-Jean'], $names); + } + + public function testUserWithoutSitesHasEmptyArrayAndNullCurrent(): void + { + // Creer un user jetable sans rattachement a un site. + $em = $this->getEm(); + + $suffix = substr(bin2hex(random_bytes(4)), 0, 8); + $username = 'orphan_'.$suffix; + + $hasher = self::getContainer()->get('security.user_password_hasher'); + $user = new User(); + $user->setUsername($username); + $user->setIsAdmin(false); + $user->setPassword($hasher->hashPassword($user, 'testpass')); + $em->persist($user); + $em->flush(); + + try { + $client = $this->authenticatedClient($username, 'testpass'); + $response = $client->request('GET', '/api/me'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame([], $data['sites']); + self::assertNull($data['currentSite']); + } finally { + $em = $this->getEm(); + $reloaded = $em->getRepository(User::class)->findOneBy(['username' => $username]); + if (null !== $reloaded) { + $em->remove($reloaded); + $em->flush(); + } + } + } +} diff --git a/tests/Module/Sites/Api/SiteApiTest.php b/tests/Module/Sites/Api/SiteApiTest.php new file mode 100644 index 0000000..a643d19 --- /dev/null +++ b/tests/Module/Sites/Api/SiteApiTest.php @@ -0,0 +1,235 @@ +cleanupTestSites(); + } + + protected function tearDown(): void + { + $this->cleanupTestSites(); + parent::tearDown(); + } + + public function testAdminCanListSites(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/sites'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertGreaterThanOrEqual(3, $data['totalItems']); + } + + public function testUserWithSitesViewCanListSites(): void + { + $this->skipIfSitesModuleDisabled(); + $credentials = $this->createUserWithPermission('sites.view'); + $client = $this->authenticatedClient($credentials['username'], $credentials['password']); + $client->request('GET', '/api/sites'); + + self::assertResponseIsSuccessful(); + } + + public function testUserWithoutPermissionGetsForbidden(): void + { + // alice a la permission via son role "user" ? Non : le role user par + // defaut n'a aucune permission. Elle ne peut donc pas lister. + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('GET', '/api/sites'); + + self::assertResponseStatusCodeSame(403); + } + + public function testUnauthenticatedGetCollectionReturns401(): void + { + $client = self::createClient(); + $client->request('GET', '/api/sites'); + + self::assertResponseStatusCodeSame(401); + } + + public function testAdminCanCreateSite(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('POST', '/api/sites', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'name' => 'Test-New-Site', + 'street' => '1 rue du Test', + 'complement' => null, + 'postalCode' => '86000', + 'city' => 'Poitiers', + 'color' => '#AABBCC', + ], + ]); + + self::assertResponseStatusCodeSame(201); + $data = $response->toArray(); + self::assertSame('Test-New-Site', $data['name']); + self::assertSame('#AABBCC', $data['color']); + } + + public function testAdminCanPatchSite(): void + { + $em = $this->getEm(); + $site = new Site('Test-Patch-Site', '1 rue Test', null, '86000', 'Poitiers', '#000000'); + $em->persist($site); + $em->flush(); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('PATCH', '/api/sites/'.$site->getId(), [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['color' => '#FF0000'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame('#FF0000', $data['color']); + } + + public function testAdminCanDeleteSite(): void + { + $em = $this->getEm(); + $site = new Site('Test-Delete-Site', '1 rue Test', null, '86000', 'Poitiers', '#000000'); + $em->persist($site); + $em->flush(); + $siteId = $site->getId(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/sites/'.$siteId); + + self::assertResponseStatusCodeSame(204); + + $em->clear(); + self::assertNull($em->getRepository(Site::class)->find($siteId)); + } + + public function testUserWithViewButNotManageCannotDelete(): void + { + $em = $this->getEm(); + $site = new Site('Test-Protected', '1 rue Test', null, '86000', 'Poitiers', '#000000'); + $em->persist($site); + $em->flush(); + + $this->skipIfSitesModuleDisabled(); + $credentials = $this->createUserWithPermission('sites.view'); + $client = $this->authenticatedClient($credentials['username'], $credentials['password']); + $client->request('DELETE', '/api/sites/'.$site->getId()); + + self::assertResponseStatusCodeSame(403); + } + + public function testCreateSiteWithDuplicateNameReturns422(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/sites', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'name' => 'Chatellerault', + 'street' => 'Autre rue', + 'postalCode' => '75001', + 'city' => 'Autre ville', + 'color' => '#FF0000', + ], + ]); + + self::assertResponseStatusCodeSame(422); + } + + public function testCreateSiteWithInvalidColorReturns422(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/sites', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'name' => 'Test-Invalid-Color', + 'street' => '1 rue Test', + 'postalCode' => '86000', + 'city' => 'Poitiers', + 'color' => 'red', + ], + ]); + + self::assertResponseStatusCodeSame(422); + } + + public function testCreateSiteIgnoresFullAddressInPayload(): void + { + // Garde structurelle : `fullAddress` est un getter computed cote + // backend (Site::getFullAddress, groupe site:read uniquement). Si un + // client envoie ce champ en POST, API Platform doit l'ignorer + // silencieusement car il n'est pas dans le groupe site:write. On + // grave ce comportement pour qu'un futur dev qui ajouterait un + // setter casse ce test au lieu de casser l'invariant en silence. + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('POST', '/api/sites', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'name' => 'Test-FullAddress-Ignored', + 'street' => '1 rue Test', + 'postalCode' => '86000', + 'city' => 'Poitiers', + 'color' => '#000000', + 'fullAddress' => 'Adresse arbitraire envoyee par le client', + ], + ]); + + self::assertResponseStatusCodeSame(201); + $data = $response->toArray(); + // Le getter computed prevaut sur ce qu'envoie le client : street + // determine la 1re ligne, jamais la valeur "Adresse arbitraire...". + self::assertSame("1 rue Test\n86000 Poitiers", $data['fullAddress']); + } + + public function testCreateSiteWithInvalidPostalCodeReturns422(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/sites', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'name' => 'Test-Invalid-CP', + 'street' => '1 rue Test', + 'postalCode' => '123', + 'city' => 'Poitiers', + 'color' => '#000000', + ], + ]); + + self::assertResponseStatusCodeSame(422); + } + + private function cleanupTestSites(): void + { + if (!self::$kernel) { + self::bootKernel(); + } + $em = $this->getEm(); + $em->createQuery('DELETE FROM '.Site::class.' s WHERE s.name LIKE :prefix') + ->setParameter('prefix', self::TEST_NAME_PREFIX.'%') + ->execute() + ; + $em->clear(); + } +} diff --git a/tests/Module/Sites/Api/SiteCascadeTest.php b/tests/Module/Sites/Api/SiteCascadeTest.php new file mode 100644 index 0000000..8fec8aa --- /dev/null +++ b/tests/Module/Sites/Api/SiteCascadeTest.php @@ -0,0 +1,90 @@ +getEm(); + $site = new Site('Test-Cascade-Purge', '1 rue Test', null, '12345', 'Ville', '#000000'); + $em->persist($site); + $em->flush(); + $siteId = $site->getId(); + + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + self::assertNotNull($alice); + $alice->addSite($site); + $em->flush(); + $em->clear(); + + // Verifie presence du rattachement M2M via SQL direct (l'EM est cleared). + $connection = $this->getEm()->getConnection(); + $before = (int) $connection->fetchOne( + 'SELECT COUNT(*) FROM user_site WHERE site_id = :id', + ['id' => $siteId], + ); + self::assertSame(1, $before); + + // Admin supprime le site. + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/sites/'.$siteId); + self::assertResponseStatusCodeSame(204); + + // L'entree user_site doit avoir disparu via ON DELETE CASCADE. + $after = (int) $connection->fetchOne( + 'SELECT COUNT(*) FROM user_site WHERE site_id = :id', + ['id' => $siteId], + ); + self::assertSame(0, $after, 'Les rattachements user_site doivent etre purges en cascade.'); + } + + public function testDeletingSiteSetsCurrentSiteToNullOnReferencingUsers(): void + { + $em = $this->getEm(); + $site = new Site('Test-Cascade-Current', '1 rue Test', null, '12345', 'Ville', '#000000'); + $em->persist($site); + $em->flush(); + $siteId = $site->getId(); + + $alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']); + self::assertNotNull($alice); + $aliceId = $alice->getId(); + $alice->addSite($site); + $alice->setCurrentSite($site); + $em->flush(); + $em->clear(); + + // Admin supprime le site. + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/sites/'.$siteId); + self::assertResponseStatusCodeSame(204); + + // currentSite d'alice doit etre passe a NULL via ON DELETE SET NULL. + $em = $this->getEm(); + $em->clear(); + $reload = $em->getRepository(User::class)->find($aliceId); + self::assertNotNull($reload); + self::assertNull( + $reload->getCurrentSite(), + 'currentSite doit etre NULL apres suppression du site reference.', + ); + } +} diff --git a/tests/Module/Sites/Domain/Entity/SiteTest.php b/tests/Module/Sites/Domain/Entity/SiteTest.php index 1895d8b..1064514 100644 --- a/tests/Module/Sites/Domain/Entity/SiteTest.php +++ b/tests/Module/Sites/Domain/Entity/SiteTest.php @@ -10,9 +10,9 @@ use PHPUnit\Framework\TestCase; use ReflectionClass; /** - * Tests unitaires de comportement de l'entite Site : etat initial, setters - * et gestion des timestamps. Les contraintes de validation (regex, unicite) - * sont couvertes par SiteValidationTest. + * Tests unitaires de comportement de l'entite Site : etat initial, setters, + * gestion des timestamps et getter d'adresse complete. Les contraintes de + * validation (regex, unicite) sont couvertes par SiteValidationTest. * * @internal */ @@ -21,26 +21,28 @@ final class SiteTest extends TestCase public function testConstructorInitialState(): void { $site = new Site( - 'Chatellerault', - 'Chatellerault', - '86100', - '#056CF2', - "1 avenue de l'Europe\n86100 Chatellerault", + name: 'Chatellerault', + street: "1 avenue de l'Europe", + complement: null, + postalCode: '86100', + city: 'Chatellerault', + color: '#056CF2', ); self::assertNull($site->getId()); self::assertSame('Chatellerault', $site->getName()); - self::assertSame('Chatellerault', $site->getCity()); + self::assertSame("1 avenue de l'Europe", $site->getStreet()); + self::assertNull($site->getComplement()); self::assertSame('86100', $site->getPostalCode()); + self::assertSame('Chatellerault', $site->getCity()); self::assertSame('#056CF2', $site->getColor()); - self::assertStringContainsString('Chatellerault', $site->getFullAddress()); self::assertInstanceOf(DateTimeImmutable::class, $site->getCreatedAt()); self::assertInstanceOf(DateTimeImmutable::class, $site->getUpdatedAt()); } public function testCreatedAtAndUpdatedAtAreInitiallyEqual(): void { - $site = new Site('A', 'B', '12345', '#000000', 'Rue X'); + $site = new Site('A', 'Rue X', null, '12345', 'B', '#000000'); // A la creation, les deux timestamps sont seedes avec la meme valeur // pour garantir updated_at >= created_at au niveau base. @@ -49,7 +51,7 @@ final class SiteTest extends TestCase public function testOnPreUpdateAdvancesUpdatedAtOnly(): void { - $site = new Site('A', 'B', '12345', '#000000', 'Rue X'); + $site = new Site('A', 'Rue X', null, '12345', 'B', '#000000'); $originalCreatedAt = $site->getCreatedAt(); // On force updatedAt a une valeur strictement anterieure via reflection @@ -69,18 +71,63 @@ final class SiteTest extends TestCase public function testSettersMutateFields(): void { - $site = new Site('Old', 'OldCity', '12345', '#000000', 'Old Addr'); + $site = new Site('Old', 'Old Street', null, '12345', 'OldCity', '#000000'); $site->setName('New'); - $site->setCity('NewCity'); + $site->setStreet('New Street'); + $site->setComplement('Bat A'); $site->setPostalCode('67890'); + $site->setCity('NewCity'); $site->setColor('#ABCDEF'); - $site->setFullAddress('New Addr'); self::assertSame('New', $site->getName()); - self::assertSame('NewCity', $site->getCity()); + self::assertSame('New Street', $site->getStreet()); + self::assertSame('Bat A', $site->getComplement()); self::assertSame('67890', $site->getPostalCode()); + self::assertSame('NewCity', $site->getCity()); self::assertSame('#ABCDEF', $site->getColor()); - self::assertSame('New Addr', $site->getFullAddress()); + } + + public function testFullAddressGetterWithoutComplement(): void + { + $site = new Site( + name: 'Site1', + street: '1 avenue de l\'Europe', + complement: null, + postalCode: '86100', + city: 'Chatellerault', + color: '#000000', + ); + + self::assertSame( + "1 avenue de l'Europe\n86100 Chatellerault", + $site->getFullAddress(), + ); + } + + public function testFullAddressGetterWithComplement(): void + { + $site = new Site( + name: 'Site2', + street: '12 route de Poitiers', + complement: 'Batiment B', + postalCode: '86330', + city: 'Saint-Jean-de-Sauves', + color: '#000000', + ); + + self::assertSame( + "12 route de Poitiers\nBatiment B\n86330 Saint-Jean-de-Sauves", + $site->getFullAddress(), + ); + } + + public function testFullAddressGetterIgnoresEmptyComplement(): void + { + // Garde defensive : un complement vide ou whitespace-only ne doit + // pas creer une ligne vide visuellement disgracieuse. + $site = new Site('S', 'Rue', ' ', '12345', 'Ville', '#000000'); + + self::assertSame("Rue\n12345 Ville", $site->getFullAddress()); } } diff --git a/tests/Module/Sites/Domain/Entity/SiteValidationTest.php b/tests/Module/Sites/Domain/Entity/SiteValidationTest.php index 14a6b7f..5559999 100644 --- a/tests/Module/Sites/Domain/Entity/SiteValidationTest.php +++ b/tests/Module/Sites/Domain/Entity/SiteValidationTest.php @@ -50,9 +50,15 @@ final class SiteValidationTest extends KernelTestCase public function testValidSitePassesValidation(): void { - // Reutilise un nom deja present en fixtures (Chatellerault) impliquerait - // une collision UniqueEntity. On prend donc un nom dedie aux tests. - $site = new Site('Test-Valid-'.uniqid('', true), 'Poitiers', '86000', '#056CF2', 'Adresse valide'); + $site = $this->makeSite(); + $violations = $this->validator->validate($site); + + self::assertCount(0, $violations, (string) $violations); + } + + public function testValidSiteWithComplementPassesValidation(): void + { + $site = $this->makeSite(complement: 'Batiment C'); $violations = $this->validator->validate($site); self::assertCount(0, $violations, (string) $violations); @@ -61,8 +67,7 @@ final class SiteValidationTest extends KernelTestCase #[DataProvider('invalidColorProvider')] public function testColorMustBeHexRrggbb(string $color): void { - $site = new Site('Test-'.uniqid('', true), 'Y', '12345', $color, 'Addr'); - + $site = $this->makeSite(color: $color); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count(), sprintf('La couleur "%s" devrait etre rejetee.', $color)); @@ -91,8 +96,7 @@ final class SiteValidationTest extends KernelTestCase #[DataProvider('validColorProvider')] public function testValidColorsAreAccepted(string $color): void { - $site = new Site('Test-'.uniqid('', true), 'Y', '12345', $color, 'Addr'); - + $site = $this->makeSite(color: $color); $violations = $this->validator->validate($site); self::assertCount(0, $violations, sprintf('La couleur "%s" devrait etre acceptee.', $color)); @@ -117,8 +121,7 @@ final class SiteValidationTest extends KernelTestCase #[DataProvider('invalidPostalCodeProvider')] public function testPostalCodeMustMatchFrFormat(string $postalCode): void { - $site = new Site('Test-'.uniqid('', true), 'Y', $postalCode, '#000000', 'Addr'); - + $site = $this->makeSite(postalCode: $postalCode); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count(), sprintf('Le CP "%s" devrait etre rejete.', $postalCode)); @@ -145,8 +148,7 @@ final class SiteValidationTest extends KernelTestCase #[DataProvider('validPostalCodeProvider')] public function testValidPostalCodesAreAccepted(string $postalCode): void { - $site = new Site('Test-'.uniqid('', true), 'Y', $postalCode, '#000000', 'Addr'); - + $site = $this->makeSite(postalCode: $postalCode); $violations = $this->validator->validate($site); self::assertCount(0, $violations, (string) $violations); @@ -168,8 +170,15 @@ final class SiteValidationTest extends KernelTestCase public function testBlankNameIsRejected(): void { - $site = new Site('', 'Y', '12345', '#000000', 'Addr'); + $site = $this->makeSite(name: ''); + $violations = $this->validator->validate($site); + self::assertGreaterThan(0, $violations->count()); + } + + public function testBlankStreetIsRejected(): void + { + $site = $this->makeSite(street: ''); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count()); @@ -177,17 +186,7 @@ final class SiteValidationTest extends KernelTestCase public function testBlankCityIsRejected(): void { - $site = new Site('Test-'.uniqid('', true), '', '12345', '#000000', 'Addr'); - - $violations = $this->validator->validate($site); - - self::assertGreaterThan(0, $violations->count()); - } - - public function testBlankFullAddressIsRejected(): void - { - $site = new Site('Test-'.uniqid('', true), 'Y', '12345', '#000000', ''); - + $site = $this->makeSite(city: ''); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count()); @@ -195,8 +194,7 @@ final class SiteValidationTest extends KernelTestCase public function testNameLongerThan100CharsIsRejected(): void { - $site = new Site(str_repeat('a', 101), 'Y', '12345', '#000000', 'Addr'); - + $site = $this->makeSite(name: str_repeat('a', 101)); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count()); @@ -204,8 +202,23 @@ final class SiteValidationTest extends KernelTestCase public function testCityLongerThan100CharsIsRejected(): void { - $site = new Site('Test-'.uniqid('', true), str_repeat('a', 101), '12345', '#000000', 'Addr'); + $site = $this->makeSite(city: str_repeat('a', 101)); + $violations = $this->validator->validate($site); + self::assertGreaterThan(0, $violations->count()); + } + + public function testStreetLongerThan255CharsIsRejected(): void + { + $site = $this->makeSite(street: str_repeat('a', 256)); + $violations = $this->validator->validate($site); + + self::assertGreaterThan(0, $violations->count()); + } + + public function testComplementLongerThan255CharsIsRejected(): void + { + $site = $this->makeSite(complement: str_repeat('a', 256)); $violations = $this->validator->validate($site); self::assertGreaterThan(0, $violations->count()); @@ -223,16 +236,13 @@ final class SiteValidationTest extends KernelTestCase */ public function testDuplicateNameIsRejected(): void { - // Nom unique par execution pour eviter toute collision avec les - // fixtures (Chatellerault, Saint-Jean, Pommevic) ou des tests - // paralleles. $name = 'Test-Duplicate-'.uniqid('', true); - $original = new Site($name, 'Poitiers', '86000', '#056CF2', 'Adresse originale'); + $original = $this->makeSite(name: $name); $this->em->persist($original); $this->em->flush(); try { - $duplicate = new Site($name, 'Autre ville', '75001', '#FF0000', 'Autre adresse'); + $duplicate = $this->makeSite(name: $name, city: 'Autre'); $violations = $this->validator->validate($duplicate); self::assertGreaterThan(0, $violations->count(), 'Un site homonyme doit lever au moins une violation.'); @@ -256,4 +266,26 @@ final class SiteValidationTest extends KernelTestCase $this->em->flush(); } } + + /** + * Helper : construit un Site valide avec un nom unique, sur lequel on + * peut superposer un seul champ invalide pour tester une contrainte. + */ + private function makeSite( + ?string $name = null, + string $street = '1 rue Test', + ?string $complement = null, + string $postalCode = '12345', + string $city = 'Poitiers', + string $color = '#000000', + ): Site { + return new Site( + $name ?? 'Test-'.uniqid('', true), + $street, + $complement, + $postalCode, + $city, + $color, + ); + } }