From 4468fd7cdf883b4ab6936d33bf603fd717f7e73a Mon Sep 17 00:00:00 2001
From: Matthieu <mtholot19@gmail.com>
Date: Tue, 24 Mar 2026 08:36:59 +0100
Subject: [PATCH] fix(custom-fields) : match by orderIndex to prevent value
 loss on rename

When a ModelType's custom field was renamed without sending the field ID,
the service would create a new CustomField instead of reusing the existing
one, orphaning all CustomFieldValues. Now matches by orderIndex as fallback
before name, preserving the link to existing values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 src/Service/SkeletonStructureService.php | 20 +++++++++---
 tests/AbstractApiTestCase.php            |  8 +++++
 tests/Api/Entity/ModelTypeTest.php       | 40 ++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/src/Service/SkeletonStructureService.php b/src/Service/SkeletonStructureService.php
index cd9b8e9..c58d177 100644
--- a/src/Service/SkeletonStructureService.php
+++ b/src/Service/SkeletonStructureService.php
@@ -192,12 +192,14 @@ class SkeletonStructureService
             ['orderIndex' => 'ASC']
         );
 
-        // Index existing by ID and by name for matching
-        $existingById   = [];
-        $existingByName = [];
+        // Index existing by ID, name and orderIndex for matching
+        $existingById         = [];
+        $existingByName       = [];
+        $existingByOrderIndex = [];
         foreach ($existingFields as $cf) {
-            $existingById[$cf->getId()]     = $cf;
-            $existingByName[$cf->getName()] = $cf;
+            $existingById[$cf->getId()]                   = $cf;
+            $existingByName[$cf->getName()]               = $cf;
+            $existingByOrderIndex[$cf->getOrderIndex()][] = $cf;
         }
 
         $processedIds = [];
@@ -211,6 +213,14 @@ class SkeletonStructureService
             $fieldId       = $fieldData['customFieldId'] ?? $fieldData['id'] ?? null;
             if ($fieldId && isset($existingById[$fieldId])) {
                 $existingField = $existingById[$fieldId];
+            } elseif (!empty($existingByOrderIndex[$normalized['orderIndex']])) {
+                foreach ($existingByOrderIndex[$normalized['orderIndex']] as $candidate) {
+                    if (!isset($processedIds[$candidate->getId()])) {
+                        $existingField = $candidate;
+
+                        break;
+                    }
+                }
             } elseif (isset($existingByName[$normalized['name']]) && !isset($processedIds[$existingByName[$normalized['name']]->getId()])) {
                 $existingField = $existingByName[$normalized['name']];
             }
diff --git a/tests/AbstractApiTestCase.php b/tests/AbstractApiTestCase.php
index 2732158..567290e 100644
--- a/tests/AbstractApiTestCase.php
+++ b/tests/AbstractApiTestCase.php
@@ -382,6 +382,8 @@ abstract class AbstractApiTestCase extends ApiTestCase
         string $value = 'test value',
         ?Machine $machine = null,
         ?Composant $composant = null,
+        ?Piece $piece = null,
+        ?Product $product = null,
     ): CustomFieldValue {
         $cfv = new CustomFieldValue();
         $cfv->setValue($value);
@@ -392,6 +394,12 @@ abstract class AbstractApiTestCase extends ApiTestCase
         if (null !== $composant) {
             $cfv->setComposant($composant);
         }
+        if (null !== $piece) {
+            $cfv->setPiece($piece);
+        }
+        if (null !== $product) {
+            $cfv->setProduct($product);
+        }
 
         $em = $this->getEntityManager();
         $em->persist($cfv);
diff --git a/tests/Api/Entity/ModelTypeTest.php b/tests/Api/Entity/ModelTypeTest.php
index 50283fe..82d4d0b 100644
--- a/tests/Api/Entity/ModelTypeTest.php
+++ b/tests/Api/Entity/ModelTypeTest.php
@@ -4,6 +4,8 @@ declare(strict_types=1);
 
 namespace App\Tests\Api\Entity;
 
+use App\Entity\CustomField;
+use App\Entity\CustomFieldValue;
 use App\Enum\ModelCategory;
 use App\Tests\AbstractApiTestCase;
 
@@ -280,4 +282,42 @@ class ModelTypeTest extends AbstractApiTestCase
         $this->assertSame($productType->getId(), $data['structure']['products'][0]['typeProductId']);
         $this->assertSame('GR', $data['structure']['products'][0]['familyCode']);
     }
+
+    public function testPatchPieceStructureRenameKeepsCustomFieldValuesWhenIdIsMissing(): void
+    {
+        $type  = $this->createModelType('Arbre', 'ARB-001', ModelCategory::PIECE);
+        $piece = $this->createPiece('Arbre de test', 'ARB-TEST', $type);
+        $field = $this->createCustomField('diamètre', 'text', typePiece: $type, orderIndex: 0);
+        $this->createCustomFieldValue($field, '35 mm', piece: $piece);
+
+        $client = $this->createGestionnaireClient();
+        $client->request('PATCH', self::iri('model_types', $type->getId()), [
+            'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            'json'    => [
+                'structure' => [
+                    'customFields' => [[
+                        'name'       => 'Diamètre',
+                        'type'       => 'text',
+                        'required'   => false,
+                        'orderIndex' => 0,
+                    ]],
+                ],
+            ],
+        ]);
+
+        $this->assertResponseIsSuccessful();
+
+        $em = $this->getEntityManager();
+        $em->clear();
+
+        $fields = $em->getRepository(CustomField::class)->findBy(['typePiece' => $type], ['orderIndex' => 'ASC']);
+        $this->assertCount(1, $fields);
+        $this->assertSame($field->getId(), $fields[0]->getId(), 'Renaming must reuse the existing CustomField');
+        $this->assertSame('Diamètre', $fields[0]->getName());
+
+        $values = $em->getRepository(CustomFieldValue::class)->findBy(['piece' => $piece]);
+        $this->assertCount(1, $values);
+        $this->assertSame('35 mm', $values[0]->getValue(), 'Existing custom field value must be preserved');
+        $this->assertSame($field->getId(), $values[0]->getCustomField()->getId());
+    }
 }