Fix issue where validation messages where ignored by collections nest…

…ed in collections (#867)
spatie · Oct 4, 2024 · ba8728a · ba8728a
1 parent acf8721
commit ba8728a
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 7 deletions.
diff --git a/src/Resolvers/DataValidationMessagesAndAttributesResolver.php b/src/Resolvers/DataValidationMessagesAndAttributesResolver.php
@@ -2,7 +2,6 @@
 
 namespace Spatie\LaravelData\Resolvers;
 
-use Illuminate\Support\Arr;
 use Spatie\LaravelData\Support\DataConfig;
 use Spatie\LaravelData\Support\Validation\ValidationPath;
 
@@ -17,6 +16,7 @@ public function execute(
         string $class,
         array $fullPayload,
         ValidationPath $path,
+        array $nestingChain = [],
     ): array {
         $dataClass = $this->dataConfig->getDataClass($class);
 
@@ -33,15 +33,16 @@ public function execute(
                 continue;
             }
 
-            if (Arr::has($fullPayload, $propertyPath->get()) === false) {
-                continue;
-            }
-
             if ($dataProperty->type->kind->isDataObject()) {
+                if (in_array($dataProperty->type->dataClass, $nestingChain)) {
+                    continue;
+                }
+
                 $nested = $this->execute(
                     $dataProperty->type->dataClass,
                     $fullPayload,
                     $propertyPath,
+                    [...$nestingChain, $dataProperty->type->dataClass],
                 );
 
                 $messages = array_merge($messages, $nested['messages']);
@@ -51,10 +52,15 @@ public function execute(
             }
 
             if ($dataProperty->type->kind->isDataCollectable()) {
+                if (in_array($dataProperty->type->dataClass, $nestingChain)) {
+                    continue;
+                }
+
                 $collected = $this->execute(
                     $dataProperty->type->dataClass,
                     $fullPayload,
                     $propertyPath->property('*'),
+                    [...$nestingChain, $dataProperty->type->dataClass],
                 );
 
                 $messages = array_merge($messages, $collected['messages']);
@@ -68,8 +74,8 @@ public function execute(
             $messages = collect(app()->call([$class, 'messages']))
                 ->keyBy(
                     fn (mixed $messages, string $key) => ! str_contains($key, '.') && is_string($messages)
-                    ? $path->property("*.{$key}")->get()
-                    : $path->property($key)->get()
+                        ? $path->property("*.{$key}")->get()
+                        : $path->property($key)->get()
                 )
                 ->merge($messages)
                 ->all();

diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php
@@ -279,7 +279,9 @@ public static function rules(): array
     DataValidationAsserter::for(new class () extends Data {
         #[RequiredUnless('is_required', false), RequiredWith('make_required')]
         public string $property;
+
         public string $make_required;
+
         public bool $is_required;
     })->assertRules([
         'property' => ['string', 'required_unless:is_required', 'required_with:make_required'],
@@ -1856,6 +1858,55 @@ public static function messages(): array
         );
 });
 
+it('can manually set messages in double nested collections (yeah this failed once)', function () {
+    class TestDoubleNestedCollectionValidationMessagesDataA extends Data
+    {
+        public string $name;
+
+        public string $song;
+
+        public static function messages(): array
+        {
+            return [
+                'name.required' => 'Fix it Rick!',
+            ];
+        }
+    }
+
+    class TestDoubleNestedCollectionValidationMessagesInitialDataA extends Data
+    {
+        #[DataCollectionOf(TestDoubleNestedCollectionValidationMessagesDataA::class)]
+        public DataCollection $nestedCollection;
+    }
+
+    DataValidationAsserter::for(new class () extends Data {
+        #[DataCollectionOf(TestDoubleNestedCollectionValidationMessagesInitialDataA::class)]
+        public DataCollection $collection;
+    })
+        ->assertMessages(
+            messages: ['collection.*.nestedCollection.*.name.required' => 'Fix it Rick!'],
+            payload: ['collection' => [['nestedCollection' => ['collection' => [['song' => 'Never Gonna Give You Up']]]]]],
+        )
+        ->assertErrors(
+            payload: [
+                'collection' => [
+                    [
+                        'nestedCollection' => [
+                            ['song' => 'Never Gonna Give You Up'],
+                            ['song' => 'Giving up on love'],
+                        ],
+                    ],
+                    ['nestedCollection' => [['song' => 'Together Forever']]],
+                ],
+            ],
+            errors: [
+                'collection.0.nestedCollection.0.name' => ['Fix it Rick!'],
+                'collection.0.nestedCollection.1.name' => ['Fix it Rick!'],
+                'collection.1.nestedCollection.0.name' => ['Fix it Rick!'],
+            ]
+        );
+});
+
 it('can resolve validation dependencies for messages', function () {
     FakeInjectable::setup('Rick Astley');