tristan
6cf5ef4cfc
All checks were successful
Auto Tag Develop / tag (push) Successful in 6s
| Numéro du ticket | Titre du ticket | |------------------|-----------------| | | | ## Description de la PR ## Modification du .env ## Check list - [x] Pas de régression - [x] TU/TI/TF rédigée - [x] TU/TI/TF OK - [ ] CHANGELOG modifié Co-authored-by: Matthieu <mtholot19@gmail.com> Reviewed-on: #8 Co-authored-by: tristan <tristan@yuno.malio.fr> Co-committed-by: tristan <tristan@yuno.malio.fr>
532 lines
33 KiB
Markdown
532 lines
33 KiB
Markdown
# Ticket #04 — 4/4 — Outillage opt-in « site-scoped » pour modules métier
|
|
|
|
## 1. Objectif
|
|
|
|
Ce ticket livre l'outillage qui permettra aux modules metier (Commercial, Stock, Production, etc.) de declarer leurs entites comme **scopees par site** : une fois l'adoption effectuee, un utilisateur ne verra en lecture que les lignes dont `site_id` correspond a son site courant, et les creations/editions injectent automatiquement ce site courant si le payload ne le precise pas.
|
|
|
|
Le ticket est volontairement **strictement infrastructurel** : il n'adopte le pattern sur aucune entite existante. Aucun module metier n'est modifie, aucune migration n'est jouee sur des tables deja en place. Les tickets futurs (ou des tickets annexes par module) adopteront l'interface au cas par cas apres arbitrage metier.
|
|
|
|
Le ticket livre aussi une documentation developpeur (`docs/modules/site-aware.md`) qui explique comment et quand adopter le pattern, et quelles entites **ne doivent pas** l'adopter (roles, permissions, users, catalogues globaux, etc.).
|
|
|
|
## 2. Périmètre
|
|
|
|
### IN
|
|
|
|
- Creer le contrat `App\Shared\Domain\Contract\SiteAwareInterface` : interface minimale `getSite(): ?Site` / `setSite(Site $site): void`, place dans `Shared/Domain/Contract/` pour que les modules metier en dependent **sans** importer le module Sites.
|
|
- Creer `CurrentSiteProvider` (module Sites, couche Application) qui resout le site courant a partir de `Security::getUser()` + `User::getCurrentSite()`, et renvoie `null` si : pas d'user authentifie, `currentSite` null, **ou** module Sites inactif dans `config/modules.php`.
|
|
- Creer `SiteScopedQueryExtension` (module Sites, Infrastructure API Platform) implementant `QueryCollectionExtensionInterface` et `QueryItemExtensionInterface` : ajoute la clause `WHERE <alias>.site = :currentSite` quand la resource cible implemente `SiteAwareInterface`, le provider retourne un site, et l'user n'a pas `sites.bypass_scope`.
|
|
- Creer `SiteAwareInjectionProcessor` (module Sites, decorator de `api_platform.doctrine.orm.state.persist_processor`) : avant de deleguer la persistance, si `$data` est une instance de `SiteAwareInterface` et n'a pas deja de site positionne, injecte le `currentSite` fourni par le provider.
|
|
- Declarer la permission `sites.bypass_scope` dans `SitesModule::permissions()`. Admin ou user avec cette permission → le filtre Query Extension saute, visibilite globale.
|
|
- Ecrire `docs/modules/site-aware.md` : guide developpeur complet (cf. section 10).
|
|
- Tests PHPUnit avec une entite fictive `FakeSiteAwareEntity` declaree uniquement dans la suite de tests (jamais en production) pour prouver que le filtrage et l'injection automatique fonctionnent bout en bout.
|
|
- Tests du cas "Sites desactive" : desactiver `SitesModule::class` dans `config/modules.php` avant la suite, re-sync, verifier que l'outillage est no-op et qu'aucun test existant ne casse.
|
|
|
|
### OUT
|
|
|
|
- Adoption du pattern sur une entite metier reelle (ex: `Supplier`, `Client`, etc.) : **hors scope**. C'est aux tickets annexes ou aux tickets de feature de l'adopter quand necessaire, en suivant la doc.
|
|
- Migration de donnees "legacy" : ce ticket documente le piege (entites existantes sans `site_id`) mais ne livre aucune migration par module.
|
|
- Support CLI / commandes console : le filtre est uniquement actif dans le contexte API Platform (via les extensions). Une commande batch lira toutes les lignes sans filtre, comportement attendu pour les taches admin. Une eventuelle reimplementation via un Doctrine SQL Filter generique est citee en alternative non retenue (cf. Risque 4).
|
|
- Double-ecriture avec un Doctrine `SQLFilter` : non retenu dans ce ticket. Le filtre via extension API Platform couvre 100% des usages HTTP, qui est le seul contexte ou le site courant a un sens metier (user authentifie). Les commandes CLI doivent gerer la portee explicitement.
|
|
- Changement du comportement cote front : aucun. Le filtrage est transparent, le front continue de faire `GET /api/suppliers` et recoit une collection pre-filtree. Si une entite est adoptee au ticket futur, la page existante continue de fonctionner sans modification.
|
|
- Support d'entites "partiellement site-aware" (colonne site_id nullable, certaines lignes globales partagees) : non retenu. Une entite est soit SiteAware, soit globale. Si un module a besoin de la semantique hybride, il devra creer deux entites distinctes.
|
|
|
|
## 3. Fichiers à créer
|
|
|
|
### Shared — Contrat
|
|
|
|
- `/home/m-tristan/workspace/Coltura/src/Shared/Domain/Contract/SiteAwareInterface.php` : interface minimale. Depends uniquement du type `App\Module\Sites\Domain\Entity\Site`, qui est deja couple cote Core depuis le ticket 2 — le placement dans Shared n'introduit pas de nouvelle dependance transversale non souhaitee.
|
|
|
|
### Module Sites — Application
|
|
|
|
- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Application/Service/CurrentSiteProvider.php` : service injecte partout ou le site courant doit etre lu (extensions, processor, futurs voters). Gere les trois cas de retour `null` : pas d'user, `currentSite` null, module desactive.
|
|
|
|
### Module Sites — Infrastructure
|
|
|
|
- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteScopedQueryExtension.php` : une seule classe, implementant a la fois `QueryCollectionExtensionInterface` et `QueryItemExtensionInterface`. Le comportement est identique pour les deux, modulo que l'item manque retourne 404 (API Platform converti un `getOneOrNullResult` null en 404).
|
|
- `/home/m-tristan/workspace/Coltura/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php` : decorator sur le persist processor Doctrine. Injecte le site courant sur `$data` si applicable, puis delegue a `$persistProcessor`.
|
|
|
|
### Documentation
|
|
|
|
- `/home/m-tristan/workspace/Coltura/docs/modules/site-aware.md` : guide developpeur (cf. contenu section 10).
|
|
|
|
### Tests
|
|
|
|
- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteScopedQueryExtensionTest.php` : tests d'integration (`KernelTestCase`) avec l'entite `FakeSiteAwareEntity` (declaree uniquement dans le dossier de tests). Verifie :
|
|
- Le filtre s'applique sur une resource `SiteAware` quand le provider retourne un site.
|
|
- Le filtre est no-op si `SiteAware` mais provider null.
|
|
- Le filtre est no-op si resource non `SiteAware`.
|
|
- Le filtre est no-op si user a `sites.bypass_scope`.
|
|
- `totalItems` Hydra reflete bien le filtrage.
|
|
- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php` : tests unitaires (`TestCase` pur) avec mocks. Verifie :
|
|
- `$data` SiteAware sans site → injection du site courant.
|
|
- `$data` SiteAware avec site deja positionne → pas d'overwrite.
|
|
- `$data` non-SiteAware → delegation directe sans modification.
|
|
- Provider retourne null (module off ou user sans site) ET `$data` SiteAware sans site → BadRequestHttpException (400) "aucun site selectionne".
|
|
- `/home/m-tristan/workspace/Coltura/tests/Module/Sites/Application/Service/CurrentSiteProviderTest.php` : tests unitaires `TestCase`. Couvre :
|
|
- User authentifie avec currentSite → retourne le Site.
|
|
- User authentifie sans currentSite → null.
|
|
- Pas d'user → null.
|
|
- Module desactive dans config/modules.php de test → null meme si user.currentSite existe.
|
|
- `/home/m-tristan/workspace/Coltura/tests/Fixtures/SiteAware/FakeSiteAwareEntity.php` : entite Doctrine minimale (`id`, `name`, `site`) utilisee **uniquement** en tests. Mapping Doctrine declare via un `#[ORM\Entity]` mais la table n'existe jamais en prod car la fixture n'est jamais chargee hors tests. **Alternative** : utiliser un schema DB dedie au dossier de tests, cree a la volee par un helper setUp. A trancher a l'implementation.
|
|
|
|
## 4. Fichiers à modifier
|
|
|
|
- `/home/m-tristan/workspace/Coltura/src/Module/Sites/SitesModule.php` : ajouter la permission `sites.bypass_scope` dans `permissions()` :
|
|
```php
|
|
['code' => 'sites.bypass_scope', 'label' => 'Voir les donnees site-scoped de tous les sites (bypass du filtrage)'],
|
|
```
|
|
**Note importante** : la methode `permissions()` signale l'existence de la permission mais c'est la commande `app:sync-permissions` (inchangee) qui la positionne en base.
|
|
- `/home/m-tristan/workspace/Coltura/config/services.yaml` : aucun changement requis. `SiteScopedQueryExtension`, `SiteAwareInjectionProcessor` et `CurrentSiteProvider` sont autoconfigures via les `_defaults` du module. Le decorator du persist processor est declare via `#[AsDecorator]` ou via tag (cf. section 8).
|
|
- `/home/m-tristan/workspace/Coltura/phpunit.dist.xml` : aucune modification requise si la config des fixtures de tests est autonome. Si `FakeSiteAwareEntity` necessite un mapping dedie, l'option la plus propre est un `doctrine.yaml.test` ajoute via `when@test`, sans polluer la config dev/prod (cf. Risque 3).
|
|
|
|
## 5. Contrat `SiteAwareInterface`
|
|
|
|
```php
|
|
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Shared\Domain\Contract;
|
|
|
|
use App\Module\Sites\Domain\Entity\Site;
|
|
|
|
/**
|
|
* Contrat opt-in pour les entites dont la visibilite est scopee par site.
|
|
*
|
|
* Une entite implementant cette interface sera :
|
|
* - filtree en lecture par SiteScopedQueryExtension (collection + item)
|
|
* selon le site courant de l'utilisateur authentifie ;
|
|
* - alimentee automatiquement en POST/PATCH par SiteAwareInjectionProcessor
|
|
* si le payload ne precise pas de site.
|
|
*
|
|
* L'implementation concrete doit :
|
|
* - Declarer une relation ManyToOne(Site::class) avec colonne `site_id` NOT NULL.
|
|
* - Indexer `site_id` en base (sinon le filtre WHERE genere un full-scan).
|
|
*
|
|
* Ne PAS implementer cette interface pour :
|
|
* - Des entites globales (catalogue partage, roles, permissions, users).
|
|
* - Des entites dont le scope est "par tenant" plus large que le site
|
|
* (utiliser TenantAwareInterface le cas echeant).
|
|
* - Des entites transversales references par plusieurs sites.
|
|
*
|
|
* Voir `docs/modules/site-aware.md` pour le guide d'adoption complet.
|
|
*/
|
|
interface SiteAwareInterface
|
|
{
|
|
public function getSite(): ?Site;
|
|
|
|
public function setSite(Site $site): void;
|
|
}
|
|
```
|
|
|
|
### Remarque sur le typage du getter
|
|
|
|
`getSite(): ?Site` retourne nullable pour deux raisons :
|
|
- Coherence avec des entites en cours de construction (pre-persist, avant injection).
|
|
- Compat avec des colonnes qui deviendraient nullable lors d'une migration progressive (ex: deploiement en 2 etapes avec backfill).
|
|
|
|
En regime nominal, apres persistance, `getSite()` ne doit jamais etre null. Un `assert($entity->getSite() !== null)` dans du code sensible est acceptable.
|
|
|
|
## 6. Service `CurrentSiteProvider`
|
|
|
|
### Responsabilite
|
|
|
|
Expose **une seule** methode `get(): ?Site`. Resout le site courant selon la chaine :
|
|
1. Si `SitesModule::class` n'est pas present dans `config/modules.php` → `null`.
|
|
2. Sinon, si `Security::getUser()` est null → `null`.
|
|
3. Sinon, si `$user->getCurrentSite()` est null → `null`.
|
|
4. Sinon → retourne le Site.
|
|
|
|
### Detection d'activation du module
|
|
|
|
Deux strategies possibles :
|
|
|
|
**Strategie A — lire `config/modules.php` au boot du service** (pattern `ModulesProvider`) :
|
|
```php
|
|
public function __construct(
|
|
private readonly Security $security,
|
|
#[Autowire(param: 'kernel.project_dir')]
|
|
string $projectDir,
|
|
) {
|
|
$moduleClasses = require $projectDir.'/config/modules.php';
|
|
$this->sitesActive = in_array(SitesModule::class, $moduleClasses, true);
|
|
}
|
|
```
|
|
|
|
**Strategie B — extraire un service `ActiveModulesRegistry`** partage entre `ModulesProvider` et `CurrentSiteProvider` (refactor mineur).
|
|
|
|
**Recommandation** : strategie A dans ce ticket pour rester minimal. Si un troisieme consommateur apparait (probable), extraire le registry dans un ticket dedie.
|
|
|
|
### Contrat complet
|
|
|
|
```php
|
|
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Module\Sites\Application\Service;
|
|
|
|
use App\Module\Core\Domain\Entity\User;
|
|
use App\Module\Sites\Domain\Entity\Site;
|
|
use App\Module\Sites\SitesModule;
|
|
use Symfony\Bundle\SecurityBundle\Security;
|
|
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
|
|
|
final class CurrentSiteProvider
|
|
{
|
|
private readonly bool $sitesActive;
|
|
|
|
public function __construct(
|
|
private readonly Security $security,
|
|
#[Autowire(param: 'kernel.project_dir')]
|
|
string $projectDir,
|
|
) {
|
|
$moduleClasses = require $projectDir.'/config/modules.php';
|
|
$this->sitesActive = in_array(SitesModule::class, $moduleClasses, true);
|
|
}
|
|
|
|
public function get(): ?Site
|
|
{
|
|
if (!$this->sitesActive) {
|
|
return null;
|
|
}
|
|
|
|
$user = $this->security->getUser();
|
|
if (!$user instanceof User) {
|
|
return null;
|
|
}
|
|
|
|
return $user->getCurrentSite();
|
|
}
|
|
}
|
|
```
|
|
|
|
## 7. Extensions API Platform
|
|
|
|
### `SiteScopedQueryExtension`
|
|
|
|
Implemente a la fois `QueryCollectionExtensionInterface` et `QueryItemExtensionInterface`. La logique est commune et factorisee dans une methode privee `applyScope()`.
|
|
|
|
```php
|
|
public function applyToCollection(
|
|
QueryBuilder $queryBuilder,
|
|
QueryNameGeneratorInterface $queryNameGenerator,
|
|
string $resourceClass,
|
|
?Operation $operation = null,
|
|
array $context = [],
|
|
): void {
|
|
$this->applyScope($queryBuilder, $queryNameGenerator, $resourceClass);
|
|
}
|
|
|
|
public function applyToItem(
|
|
QueryBuilder $queryBuilder,
|
|
QueryNameGeneratorInterface $queryNameGenerator,
|
|
string $resourceClass,
|
|
array $identifiers,
|
|
?Operation $operation = null,
|
|
array $context = [],
|
|
): void {
|
|
$this->applyScope($queryBuilder, $queryNameGenerator, $resourceClass);
|
|
}
|
|
|
|
private function applyScope(
|
|
QueryBuilder $queryBuilder,
|
|
QueryNameGeneratorInterface $queryNameGenerator,
|
|
string $resourceClass,
|
|
): void {
|
|
// 1) Resource SiteAware ?
|
|
if (!is_subclass_of($resourceClass, SiteAwareInterface::class)) {
|
|
return;
|
|
}
|
|
|
|
// 2) Bypass admin / permission dediee ?
|
|
if ($this->security->isGranted('sites.bypass_scope')) {
|
|
return;
|
|
}
|
|
|
|
// 3) Site courant disponible ?
|
|
$currentSite = $this->currentSiteProvider->get();
|
|
if ($currentSite === null) {
|
|
// Decision assumee : no-op plutot que collection vide (cf. section 11 Risque 1).
|
|
return;
|
|
}
|
|
|
|
// 4) Applique WHERE site = :currentSite
|
|
$rootAlias = $queryBuilder->getRootAliases()[0];
|
|
$parameterName = $queryNameGenerator->generateParameterName('currentSite');
|
|
$queryBuilder
|
|
->andWhere(sprintf('%s.site = :%s', $rootAlias, $parameterName))
|
|
->setParameter($parameterName, $currentSite);
|
|
}
|
|
```
|
|
|
|
### Ordre de priorite
|
|
|
|
L'extension doit s'executer **apres** les filtres natifs API Platform (Pagination, Order, Search). Priorite par defaut (0) convient, mais si un autre filtre custom est ajoute plus tard, verifier qu'il ne court-circuite pas. Declarer la priorite explicitement via `#[AsTaggedItem(priority: -100)]` est une option pour s'executer en dernier et etre robuste a l'ordre d'ajout d'autres extensions.
|
|
|
|
### JSON-LD `totalItems`
|
|
|
|
API Platform execute un `COUNT(*)` separe pour produire le `totalItems` dans la reponse Hydra. Ce count passe par les memes extensions → le totalItems reflete automatiquement le filtrage. A verifier par un test dedie (cf. section 11).
|
|
|
|
### `applyToItem` et 404
|
|
|
|
Quand un GET `/api/suppliers/{id}` cible un supplier qui existe en base mais appartient a un autre site, la requete `SELECT ... WHERE id = :id AND site = :currentSite` retourne `null` → API Platform converti en 404. Comportement desire : l'user ne doit pas pouvoir distinguer "cet item n'existe pas" de "cet item existe mais pas dans mon site" (anti-enumeration).
|
|
|
|
## 8. Processor d'injection automatique `SiteAwareInjectionProcessor`
|
|
|
|
### Pattern decorator
|
|
|
|
Le plus propre en API Platform est de decorer le processor de persistance Doctrine :
|
|
|
|
```php
|
|
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Module\Sites\Infrastructure\ApiPlatform\State\Processor;
|
|
|
|
use ApiPlatform\Metadata\Operation;
|
|
use ApiPlatform\State\ProcessorInterface;
|
|
use App\Module\Sites\Application\Service\CurrentSiteProvider;
|
|
use App\Shared\Domain\Contract\SiteAwareInterface;
|
|
use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
|
|
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
|
|
|
#[AsDecorator('api_platform.doctrine.orm.state.persist_processor')]
|
|
final class SiteAwareInjectionProcessor implements ProcessorInterface
|
|
{
|
|
public function __construct(
|
|
private readonly ProcessorInterface $inner,
|
|
private readonly CurrentSiteProvider $currentSiteProvider,
|
|
) {}
|
|
|
|
public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
|
|
{
|
|
if ($data instanceof SiteAwareInterface && $data->getSite() === null) {
|
|
$currentSite = $this->currentSiteProvider->get();
|
|
|
|
if ($currentSite === null) {
|
|
throw new BadRequestHttpException(
|
|
'Impossible de creer l\'enregistrement : aucun site selectionne.',
|
|
);
|
|
}
|
|
|
|
$data->setSite($currentSite);
|
|
}
|
|
|
|
return $this->inner->process($data, $operation, $uriVariables, $context);
|
|
}
|
|
}
|
|
```
|
|
|
|
### Effets de bord et compatibilite
|
|
|
|
- **S'applique a TOUS les processors qui heritent du persist processor natif API Platform**. Si un processor custom (ex: `UserRbacProcessor`) delegue a `api_platform.doctrine.orm.state.persist_processor` via autowire, il passe aussi par ce decorator — transparent pour User (non SiteAware).
|
|
- **N'ecrase jamais un site deja positionne** : un admin qui POST un supplier avec `site: '/api/sites/2'` garde cette valeur, meme si son `currentSite` est 1. La regle metier "site different autorise uniquement si l'user a plusieurs sites" du ticket n'est **pas** implementee dans ce decorator : c'est au voter de securite (hors scope de ce ticket) de l'enforcer si necessaire.
|
|
- **Erreur explicite si pas de site** : BadRequestHttpException plutot qu'un `null` silencieux. Le user comprend que l'operation necessite un site actif.
|
|
|
|
### Alternative rejetee — EventListener Doctrine `prePersist`
|
|
|
|
Un listener Doctrine intercepterait toutes les persistances, y compris hors HTTP (CLI, fixtures). **Rejetee** car :
|
|
- `CurrentSiteProvider` depend de `Security`, indisponible en CLI.
|
|
- Les fixtures doivent positionner explicitement le site (cf. `AppFixtures` ticket 2), ce qui est plus correct metier.
|
|
- Les commandes batch peuvent vouloir creer des entites sans site actif (backoffice multi-sites) — un listener silencieux les bloquerait.
|
|
|
|
Le decorator HTTP-only est plus aligne avec le principe "opt-in controle".
|
|
|
|
## 9. Permission `sites.bypass_scope`
|
|
|
|
### Déclaration
|
|
|
|
Dans `SitesModule::permissions()` :
|
|
|
|
```php
|
|
public static function permissions(): array
|
|
{
|
|
return [
|
|
['code' => 'sites.view', 'label' => 'Voir les sites'],
|
|
['code' => 'sites.manage', 'label' => 'Gerer les sites (creer, editer, supprimer)'],
|
|
['code' => 'sites.bypass_scope', 'label' => 'Voir les donnees site-scoped de tous les sites (bypass du filtrage)'],
|
|
];
|
|
}
|
|
```
|
|
|
|
### Semantique
|
|
|
|
- User avec `sites.bypass_scope` → le filtre `WHERE site = :currentSite` n'est pas applique. La collection retournee est **globale** (toutes les lignes).
|
|
- User **admin** (`isAdmin = true`) → `is_granted()` retourne toujours true pour toute permission → le bypass est automatique. Pas besoin d'assignation explicite.
|
|
- Cas typique d'attribution : un admin financier qui veut consolider les suppliers a l'echelle groupe.
|
|
|
|
### Absence de bypass sur le processor
|
|
|
|
Le processor d'injection ne respecte **pas** `sites.bypass_scope` : meme un user avec bypass verra son `currentSite` injecte si le payload n'en precise pas. Justification : l'injection n'est pas une restriction, c'est un default value. Le user bypass peut toujours envoyer un site explicite different.
|
|
|
|
## 10. Documentation développeur — `docs/modules/site-aware.md`
|
|
|
|
Le fichier livre **5 sections** :
|
|
|
|
### 10.1 Quand adopter `SiteAwareInterface`
|
|
|
|
- Entite qui existe "par site" : chaque ligne appartient a un et un seul site, les users ne doivent voir que celles de leur site courant.
|
|
- Exemples : `Supplier` (chaque site a ses fournisseurs), `Order`, `StockEntry`, `Employee` (si chaque site a sa propre equipe).
|
|
|
|
### 10.2 Quand NE PAS adopter
|
|
|
|
- Entites globales : `Role`, `Permission`, `User` (les users sont transverses, rattaches a plusieurs sites).
|
|
- Catalogues partages : produits, categories, taxes — s'ils sont mutualises entre sites.
|
|
- Entites transversales : `Invoice` globale, `Contract` multi-site.
|
|
- Entites dont la portee naturelle est "par tenant" plus large que "par site" : utiliser `TenantAwareInterface` (si pertinent pour le projet multi-tenant futur).
|
|
|
|
### 10.3 Comment adopter (check-list)
|
|
|
|
1. **Entite** :
|
|
- Implementer `App\Shared\Domain\Contract\SiteAwareInterface`.
|
|
- Ajouter la relation `#[ORM\ManyToOne(targetEntity: Site::class)] #[ORM\JoinColumn(name: 'site_id', nullable: false, onDelete: 'CASCADE')] private Site $site`.
|
|
- Implementer `getSite()` et `setSite()`.
|
|
2. **Migration** :
|
|
- Creer une migration dediee au module concerne (ou racine si init critique, voir `CLAUDE.md`).
|
|
- `ALTER TABLE <table> ADD COLUMN site_id INT NOT NULL`.
|
|
- **Gestion legacy** : si des lignes existent deja, la colonne ne peut pas etre NOT NULL directement. Strategie :
|
|
1. Ajouter la colonne nullable.
|
|
2. Backfill manuel ou par script (ex: tout rattacher au site "Chatellerault" par defaut, ou laisser l'admin arbitrer).
|
|
3. Rendre la colonne NOT NULL via une seconde migration.
|
|
- **Index** : `CREATE INDEX IDX_<table>_site ON <table> (site_id)`. **Obligatoire** — le filtre `WHERE site_id = ?` genere un full-scan sinon.
|
|
3. **Serialisation** : ajouter `site` au groupe de lecture API (`#[Groups(['<resource>:read'])]`) pour que le front voie a quel site appartient la ligne.
|
|
4. **Processor custom** : si le module a deja un processor sur l'operation POST/PATCH, s'assurer qu'il delegue a `api_platform.doctrine.orm.state.persist_processor` (et non `ObjectManager::persist` direct) pour que le decorator d'injection s'applique.
|
|
|
|
### 10.4 Comportement en mode degrade
|
|
|
|
- **Module Sites desactive** (`config/modules.php`) : `CurrentSiteProvider::get()` retourne `null` → le filtre ne s'applique plus → toutes les lignes sont visibles, comme avant l'adoption. L'app reste fonctionnelle, juste sans segmentation. **Mais** : la colonne `site_id` NOT NULL reste en place, et le processor d'injection leve une 400 sur tout POST/PATCH sans site explicite. Consequence : **un module adopte ne peut pas vivre sans Sites active** pour les operations d'ecriture, sauf a envoyer systematiquement un `site` explicite dans le payload. A documenter **fortement**.
|
|
- **User sans site** (sites.length = 0, currentSite = null) : meme comportement → no-op en lecture, 400 en ecriture. Le module doit documenter l'UX degradee.
|
|
|
|
### 10.5 Gotchas et anti-patterns
|
|
|
|
- **Sous-collections** (`/api/clients/{id}/contacts`) : l'extension s'applique a la resource chargee, ici `Contact`. Si `Contact` est SiteAware, le filtre passe. Si seul `Client` est SiteAware (et Contact herite du scope via son parent), **le filtre ne se propage pas automatiquement** : il faut soit rendre Contact SiteAware aussi (redondance), soit ajouter un filtre custom qui verifie `contact.client.site == currentSite`. Ce ticket ne couvre pas le second cas.
|
|
- **Jointures** : si un repository custom fait une requete sans passer par API Platform (ex: `findByX()` appele depuis un processor), le filtre ne s'applique pas. Responsabilite du developpeur du module d'ajouter `->andWhere('x.site = :currentSite')` manuellement ou de passer par le `CurrentSiteProvider`.
|
|
- **Tests d'integration** : les tests existants d'un module adopte devront soit logger un user avec un site actif, soit utiliser `sites.bypass_scope` pour voir toute la donnee. La suite de fixtures devra positionner un site coherent sur les entites de test.
|
|
- **Cascade delete d'un site** : le ticket 2 met `user.current_site_id` a NULL si le site est supprime. Si une entite adoptee declare `onDelete: CASCADE` sur sa FK site, elle perdra toutes ses lignes au delete d'un site. Choisir explicitement : cascade (aligne sur l'invariant "une ligne SiteAware a toujours un site") ou blocage (empecher la suppression d'un site s'il reste des lignes adoptees).
|
|
|
|
## 11. Risques et points d'attention
|
|
|
|
### Risque 1 — Comportement "no-op si pas de site courant"
|
|
|
|
La spec choisit **no-op plutot que collection vide** quand `CurrentSiteProvider::get() === null`. Arbitrage :
|
|
|
|
- **No-op** (retenu) : un user sans site voit tout, un admin sans site aussi. Risque de fuite de donnees d'un site a l'autre, mais l'app reste utilisable.
|
|
- **Collection vide** : un user sans site ne voit rien. Plus strict, mais bloque un admin qui consulterait l'app avant d'avoir configure un site.
|
|
|
|
Le ticket retient **no-op** car l'app reste utilisable. La permission `sites.bypass_scope` est explicite pour les admins qui veulent voir tout. Si la decision metier evolue, le changement est localise dans `SiteScopedQueryExtension::applyScope()`.
|
|
|
|
### Risque 2 — Fuite de donnees entre sites
|
|
|
|
Si un module adopte `SiteAwareInterface` mais qu'un repository custom court-circuite API Platform, le filtre ne s'applique pas. Consequence : un endpoint custom (`GET /api/suppliers/top-rated`) pourrait exposer tous les suppliers sans filtrage.
|
|
|
|
**Mitigation** : la doc insiste sur la responsabilite du developpeur d'adopter le filtre manuellement dans les repositories custom. Un test d'integration par module adopte est **fortement recommande**.
|
|
|
|
### Risque 3 — `FakeSiteAwareEntity` en tests
|
|
|
|
L'entite fictive doit etre mappee par Doctrine pour que le QueryBuilder fonctionne. Trois options :
|
|
|
|
1. **Declaration via `when@test`** : ajouter `config/packages/doctrine.yaml` dans un bloc `when@test` avec un mapping dedie pointant vers `tests/Fixtures/SiteAware/`. Propre mais ajoute un fichier de config.
|
|
2. **Attribute Doctrine dans le fichier de test** : fonctionne si le kernel de test decouvre le namespace. Pas elegant.
|
|
3. **Mock integral du QueryBuilder** : pas d'entite reelle, on mock Doctrine. Tests plus unitaires mais moins realistes.
|
|
|
|
**Recommandation** : option 1 (mapping `when@test`). La classe reste dans `tests/` et ne pollue jamais la prod.
|
|
|
|
### Risque 4 — Pas de Doctrine SQL Filter
|
|
|
|
Un Doctrine `SQLFilter` appliquerait le filtrage a **toutes** les requetes Doctrine, y compris hors API Platform (CLI, fixtures, cron, reports). Plus defensif mais plus risque :
|
|
|
|
- Les commandes batch devraient l'activer/desactiver explicitement.
|
|
- Les fixtures devraient le desactiver pour seeder plusieurs sites.
|
|
- Les tests d'integration devraient le gerer.
|
|
|
|
Le ticket retient la strategie **API Platform only** car le site courant n'a de sens que dans un contexte HTTP authentifie. Si un besoin emerge (rapport automatique scope par site, webhook multi-site, etc.), le refactor vers un SQL filter sera localise.
|
|
|
|
### Risque 5 — Priorite des extensions
|
|
|
|
Si un autre module introduit plus tard une extension avec une clause `HAVING` ou un `setMaxResults` qui suppose que le filtre de base n'est pas modifie, il peut y avoir des surprises. Declarer explicitement une priorite negative (`priority: -100`) sur `SiteScopedQueryExtension` via `#[AsTaggedItem]` la fait s'executer apres la plupart des filtres natifs, ce qui est generalement souhaitable pour un filtre applicatif.
|
|
|
|
### Risque 6 — `UserRbacProcessor` et les autres processors custom
|
|
|
|
Le decorator `SiteAwareInjectionProcessor` decore `api_platform.doctrine.orm.state.persist_processor`. Si un module declare un processor custom qui **ne delegue pas** au persist processor (ex: fait `$em->persist($data); $em->flush()` directement), l'injection de site n'a **pas** lieu. Le module doit explicitement passer par le persist processor pour beneficier du pattern.
|
|
|
|
A mitiger par un test qui genere une entite `FakeSiteAwareEntity` via un POST `api_platform.doctrine.orm.state.persist_processor` mocke et verifie que le decorator a bien injecte le site.
|
|
|
|
### Risque 7 — Performance du `require` au boot
|
|
|
|
`CurrentSiteProvider` fait un `require 'config/modules.php'` au constructeur. Le fichier est un simple `return [...]` → l'overhead est minimal et le resultat est opcache par PHP. Meme pattern que `ModulesProvider`, sans regression perf documentee.
|
|
|
|
### Risque 8 — Doc developpeur en francais vs anglais
|
|
|
|
Le fichier `docs/modules/site-aware.md` s'adresse aux developpeurs de Coltura. Il est redige en **francais**, aligne sur la convention projet (CLAUDE.md : "commentaires en francais, code en anglais"). Aucun extrait de code ne doit etre traduit, seules les explications.
|
|
|
|
## 12. Plan de tests
|
|
|
|
### Tests unitaires (`TestCase` pur)
|
|
|
|
#### `CurrentSiteProviderTest`
|
|
|
|
1. `testReturnsNullIfSitesModuleInactive` : config/modules.php de test ne contient pas SitesModule → null meme si user + site fixent.
|
|
2. `testReturnsNullIfNoUser` : Security::getUser() = null → null.
|
|
3. `testReturnsNullIfUserHasNoCurrentSite` : user.currentSite = null → null.
|
|
4. `testReturnsSiteIfAllConditionsMet` : user + currentSite set → retourne le Site.
|
|
|
|
#### `SiteAwareInjectionProcessorTest`
|
|
|
|
1. `testInjectsCurrentSiteOnNewSiteAwareData` : $data SiteAware + getSite() = null + provider retourne Site → setSite appele avec le bon site.
|
|
2. `testDoesNotOverrideExistingSite` : $data SiteAware + getSite() non-null → pas d'appel a setSite, delegation directe.
|
|
3. `testSkipsNonSiteAwareData` : $data qui n'implemente pas SiteAwareInterface → aucune modification, delegation.
|
|
4. `testThrowsBadRequestIfNoCurrentSite` : $data SiteAware + getSite() = null + provider retourne null → BadRequestHttpException 400.
|
|
5. `testDelegatesToInnerAlways` : inner->process est appele dans tous les cas (sauf quand 400 throw).
|
|
|
|
### Tests d'intégration (`KernelTestCase`)
|
|
|
|
#### `SiteScopedQueryExtensionTest`
|
|
|
|
Fixture : 2 sites (siteA, siteB), 3 FakeSiteAwareEntity (2 sur siteA, 1 sur siteB), 1 user rattache a siteA.
|
|
|
|
1. `testCollectionFilteredByCurrentSite` : user avec currentSite=siteA → collection retourne 2 entites (celles de siteA).
|
|
2. `testCollectionNotFilteredIfNoCurrentSite` : user sans currentSite → collection retourne 3 entites (no-op).
|
|
3. `testCollectionNotFilteredIfResourceNotSiteAware` : query sur une entite non SiteAware → aucune clause additionnelle.
|
|
4. `testCollectionNotFilteredIfBypassPermission` : user avec `sites.bypass_scope` → 3 entites.
|
|
5. `testCollectionNotFilteredIfSitesModuleInactive` : desactiver SitesModule → provider null → no-op, 3 entites.
|
|
6. `testItemNotFoundIfWrongSite` : GET sur un id dont le site est siteB alors que user sur siteA → 404 (ou `null` retourne par le QueryBuilder).
|
|
7. `testItemFoundIfCorrectSite` : GET sur un id du site courant → 200.
|
|
8. `testTotalItemsReflectsFilter` : collection Hydra `totalItems: 2` (et non 3) quand le filtre s'applique.
|
|
|
|
### Tests de non-régression
|
|
|
|
Apres implementation, **re-jouer toute la suite existante** en mode module Sites active et en mode module desactive. Aucun test existant ne doit changer.
|
|
|
|
## 13. Ordre d'exécution recommandé
|
|
|
|
1. **Contrat** — `SiteAwareInterface` dans `Shared/Domain/Contract/`.
|
|
2. **Provider** — `CurrentSiteProvider` + tests unitaires.
|
|
3. **Processor decorator** — `SiteAwareInjectionProcessor` + tests unitaires avec mocks.
|
|
4. **Entite de test** — `FakeSiteAwareEntity` + mapping `when@test` si retenu.
|
|
5. **Query extension** — `SiteScopedQueryExtension` + tests d'integration.
|
|
6. **Permission bypass** — ajout dans `SitesModule::permissions()`, `make sync-permissions`, verifier en base.
|
|
7. **Tests exhaustifs** — faire passer la matrice des 8 cas d'integration.
|
|
8. **Tests non-regression** — `make test` avec SitesModule actif puis inactif.
|
|
9. **Documentation** — rediger `docs/modules/site-aware.md` (5 sections).
|
|
10. **CS fixer** — `make php-cs-fixer-allow-risky`.
|
|
11. **DoD** — valider la check-list section 14.
|
|
|
|
## 14. Critères d'acceptation (DoD)
|
|
|
|
- [ ] `App\Shared\Domain\Contract\SiteAwareInterface` existe avec les deux methodes `getSite(): ?Site` et `setSite(Site $site): void`.
|
|
- [ ] `CurrentSiteProvider::get()` retourne `null` dans les 3 cas : pas d'user, pas de currentSite, module inactif. Retourne le Site sinon.
|
|
- [ ] `SiteScopedQueryExtension` applique le WHERE sur les resources SiteAware quand un site courant est resolu et que l'user n'a pas `sites.bypass_scope`.
|
|
- [ ] `SiteAwareInjectionProcessor` injecte automatiquement le site courant sur POST/PATCH d'entites SiteAware sans site explicite.
|
|
- [ ] `SiteAwareInjectionProcessor` leve une 400 si l'entite SiteAware n'a pas de site ET que le provider retourne null.
|
|
- [ ] Permission `sites.bypass_scope` declaree dans `SitesModule::permissions()` et presente en base apres `app:sync-permissions`.
|
|
- [ ] `docs/modules/site-aware.md` livre les 5 sections (quand/comment adopter, anti-patterns, degrade, gotchas).
|
|
- [ ] Tests d'integration : au moins 8 cas couvrant filtrage collection/item, no-op dans les 3 scenarios (pas de site, resource non SiteAware, bypass), et `totalItems` Hydra.
|
|
- [ ] Tests unitaires sur `CurrentSiteProvider` et `SiteAwareInjectionProcessor`.
|
|
- [ ] Aucune migration sur des tables metier existantes (`supplier`, `client`, `user`, ...) — seules les migrations du ticket 1 et 2 sont presentes. Verifier via `make migration-migrate` : aucun SQL attendu sur la suite existante.
|
|
- [ ] `make test` passe avec `SitesModule::class` actif dans `config/modules.php`.
|
|
- [ ] `make test` passe avec `SitesModule::class` desactive dans `config/modules.php`.
|
|
- [ ] `make php-cs-fixer-allow-risky` propre sur les fichiers nouveaux.
|
|
- [ ] Aucun module metier (Commercial, Core hors User, etc.) n'a ete modifie par ce ticket — diff ne touche que `src/Shared/`, `src/Module/Sites/`, `tests/`, et `docs/`.
|