From 00f38b7d4147ec8a0d576fd21aa34efc940abd88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Munsch?= Date: Mon, 31 Aug 2026 16:32:11 +0200 Subject: [PATCH] fix(employees): filter out invalid matricules and comments from Arcadia database --- .../Directories/ArcadiaEmployeeDirectory.php | 31 +++++++++++--- src/Infra/Adapters/Mappers/EmployeeMapper.php | 16 +++++++- .../Api/V1/Employees/EmployeesIndexTest.php | 41 +++++++++++++++++++ .../Employees/ArcadiaEmployeeIdTest.php | 15 ++++--- .../Adapters/Mappers/EmployeeMapperTest.php | 20 +++++++++ 5 files changed, 110 insertions(+), 13 deletions(-) diff --git a/src/Infra/Adapters/Directories/ArcadiaEmployeeDirectory.php b/src/Infra/Adapters/Directories/ArcadiaEmployeeDirectory.php index d420f17..3a85ed8 100644 --- a/src/Infra/Adapters/Directories/ArcadiaEmployeeDirectory.php +++ b/src/Infra/Adapters/Directories/ArcadiaEmployeeDirectory.php @@ -32,6 +32,7 @@ final class ArcadiaEmployeeDirectory implements EmployeeDirectory ?EmployeeFilter $filter = null, ): Paginated { $query = $this->connection->table(RuPersonneSirhTable::table($this->connection)); + $this->applyValidMatriculeFilter($query); if ($filter !== null) { if ($filter->establishment !== null) { @@ -69,7 +70,10 @@ final class ArcadiaEmployeeDirectory implements EmployeeDirectory $items = []; foreach ($rows as $row) { - $items[] = EmployeeMapper::toDomain($row); + $employee = EmployeeMapper::tryToDomain($row); + if ($employee !== null) { + $items[] = $employee; + } } /** @var Employee[] $items */ @@ -79,16 +83,33 @@ final class ArcadiaEmployeeDirectory implements EmployeeDirectory #[\Override] public function find(ArcadiaEmployeeId $id): ?Employee { - $row = $this->connection + $query = $this->connection ->table(RuPersonneSirhTable::table($this->connection)) - ->where('MATRICULE_FIC', $id->value) - ->first(); + ->where('MATRICULE_FIC', $id->value); + $this->applyValidMatriculeFilter($query); + + $row = $query->first(); if ($row === null) { return null; } - return EmployeeMapper::toDomain($row); + return EmployeeMapper::tryToDomain($row); + } + + private function applyValidMatriculeFilter(Builder $query): void + { + $driver = $this->connection->getDriverName(); + + match ($driver) { + 'oracle' => $query->whereRaw("REGEXP_LIKE(TRIM(MATRICULE_FIC), '^[0-9]{1,8}$')"), + 'pgsql' => $query->whereRaw("TRIM(MATRICULE_FIC) ~ '^[0-9]{1,8}$'"), + 'mysql' => $query->whereRaw("TRIM(MATRICULE_FIC) REGEXP '^[0-9]{1,8}$'"), + 'sqlite' => $query->whereRaw( + "TRIM(MATRICULE_FIC) != '' AND length(TRIM(MATRICULE_FIC)) <= 8 AND TRIM(MATRICULE_FIC) NOT GLOB '*[^0-9]*'" + ), + default => $query->whereRaw("MATRICULE_FIC NOT LIKE '#%'"), + }; } private function toSqlColumn(EmployeeSortColumn $sortBy): string diff --git a/src/Infra/Adapters/Mappers/EmployeeMapper.php b/src/Infra/Adapters/Mappers/EmployeeMapper.php index fc7faaa..f5a2477 100644 --- a/src/Infra/Adapters/Mappers/EmployeeMapper.php +++ b/src/Infra/Adapters/Mappers/EmployeeMapper.php @@ -13,6 +13,18 @@ use Shared\Helpers\DatesHelper; final readonly class EmployeeMapper { + /** + * @param array|object $row + */ + public static function tryToDomain(array | object $row): ?Employee + { + try { + return self::toDomain($row); + } catch (\DomainException) { + return null; + } + } + /** * @param array|object $row */ @@ -23,10 +35,10 @@ final readonly class EmployeeMapper /** @var array $data */ $data = is_object($row) ? (array) $row : $row; - $rawMatricule = self::castString( + $rawMatricule = trim(self::castString( $data['MATRICULE_FIC'] ?? $data['matricule_fic'] ?? $data['public_employee_number'] ?? null, '' - ); + )); // Garantir un format 7 ou 8 chiffres pour ArcadiaEmployeeId if (is_numeric($rawMatricule) && strlen($rawMatricule) < 7) { $rawMatricule = str_pad($rawMatricule, 7, '0', STR_PAD_LEFT); diff --git a/tests/Feature/Api/V1/Employees/EmployeesIndexTest.php b/tests/Feature/Api/V1/Employees/EmployeesIndexTest.php index f06af85..f2146cc 100644 --- a/tests/Feature/Api/V1/Employees/EmployeesIndexTest.php +++ b/tests/Feature/Api/V1/Employees/EmployeesIndexTest.php @@ -147,4 +147,45 @@ describe('Employees Index Feature API', function (): void { $response->assertStatus(200); $response->assertJsonCount(2, 'employees'); }); + + it('ignores database rows with invalid matricules like comments or alphanumeric strings', function (): void { + // GIVEN + RuPersonneSirhFactory::truncate(); + + // Valid employee + RuPersonneSirhFactory::new()->create([ + 'MATRICULE_FIC' => '01155902', + 'NOM_FIC' => 'DOE', + 'PRENOM_FIC' => 'John', + ]); + + // Invalid employees / comments in prod database + RuPersonneSirhFactory::new()->create([ + 'MATRICULE_FIC' => '# s996008', + 'NOM_FIC' => 'ARRIVE SAINT FULGENT', + 'PRENOM_FIC' => 'Baie de stockage', + ]); + RuPersonneSirhFactory::new()->create([ + 'MATRICULE_FIC' => 'd05600013', + 'NOM_FIC' => 'PYPOPS', + 'PRENOM_FIC' => 'Diana', + ]); + RuPersonneSirhFactory::new()->create([ + 'MATRICULE_FIC' => 'P99900029', + 'NOM_FIC' => 'DPE BEL', + 'PRENOM_FIC' => 'Bel', + ]); + + /** @var \Tests\TestCase $test */ + $test = $this; + + // WHEN + $response = $test->getJson('/api/v1/employees'); + + // THEN + $response->assertStatus(200); + $response->assertJsonCount(1, 'employees'); + $response->assertJsonPath('employees.0.id', '01155902'); + $response->assertJsonPath('pagination.totalItems', 1); + }); }); diff --git a/tests/Unit/Domain/Models/ValueObjects/Employees/ArcadiaEmployeeIdTest.php b/tests/Unit/Domain/Models/ValueObjects/Employees/ArcadiaEmployeeIdTest.php index adbb5de..c60bff0 100644 --- a/tests/Unit/Domain/Models/ValueObjects/Employees/ArcadiaEmployeeIdTest.php +++ b/tests/Unit/Domain/Models/ValueObjects/Employees/ArcadiaEmployeeIdTest.php @@ -61,12 +61,15 @@ describe('ArcadiaEmployeeId Value Object Instantiation', function (): void { }); describe('ArcadiaEmployeeId Value Object Domain Rules', function (): void { - it('throws a DomainException when given an invalid matricule pattern', function (): void { - // GIVEN - $invalidMatricule = '12345'; - - // WHEN & THEN + it('throws a DomainException when given an invalid matricule pattern', function (string $invalidMatricule): void { expect(fn () => new ArcadiaEmployeeId($invalidMatricule)) ->toThrow(\DomainException::class); - }); + })->with([ + 'too short' => '12345', + 'too long' => '123456789', + 'comment with hash' => '# s996008', + 'alphanumeric d prefix' => 'd05600013', + 'alphanumeric P prefix' => 'P99900029', + 'empty string' => '', + ]); }); diff --git a/tests/Unit/Infra/Adapters/Mappers/EmployeeMapperTest.php b/tests/Unit/Infra/Adapters/Mappers/EmployeeMapperTest.php index a30469d..7ed897d 100644 --- a/tests/Unit/Infra/Adapters/Mappers/EmployeeMapperTest.php +++ b/tests/Unit/Infra/Adapters/Mappers/EmployeeMapperTest.php @@ -71,4 +71,24 @@ describe('EmployeeMapper', function (): void { ->and($employee->lastName)->toBe('DUPONT') ->and($employee->firstName)->toBe('Louis'); }); + + it('returns null on tryToDomain when matricule is invalid', function (): void { + $row = [ + 'MATRICULE_FIC' => '# s996008', + 'NOM_FIC' => 'ARRIVE SAINT FULGENT', + 'PRENOM_FIC' => 'Baie de stockage', + ]; + + expect(EmployeeMapper::tryToDomain($row))->toBeNull(); + }); + + it('returns Employee on tryToDomain when matricule is valid', function (): void { + $row = [ + 'MATRICULE_FIC' => '01155902', + 'NOM_FIC' => 'DOE', + 'PRENOM_FIC' => 'John', + ]; + + expect(EmployeeMapper::tryToDomain($row))->toBeInstanceOf(Employee::class); + }); });