spatie / laravel-data

Powerful data objects for Laravel
https://spatie.be/docs/laravel-data/
MIT License
1.3k stars 211 forks source link

Not wrapping additional properties #893

Open ennorehling opened 1 month ago

ennorehling commented 1 month ago

✏️ Describe the bug When adding additional properties to a Data Object, they do not get wrapped by wrap().

↪️ To Reproduce Provide us a pest test like this one which shows the problem:


it('cannot wrap additional properties', function () {
    class UserData extends Data
    {
        public function __construct(
            public string $name,
        ) {
        }
    }

    dd(UserData::from(['name' => 'Ruben'])->additional(['role' => 'admin'])->wrap('data'));
});

Assertions aren't required, a simple dump or dd statement of what's going wrong is good enough 😄

✅ Expected behavior I'm expecting this to produce

[
    "data" => [
      "name" => "Ruben",
      "rank" => "admin",
    ],
  ]

but instead, I get [ "data" => [ "name" => "Ruben", ], "rank" => "admin", ]

A similar thing happens when I add a with() method to the object.

🖥️ Versions

Laravel: v10.48.22 Laravel Data: 4.11.0 PHP: 8.2.20

rust17 commented 3 weeks ago

@ennorehling It seems necessary to append toArray() or a similar method in the Spatie\LaravelData\Concerns\TransformableData class. After the transformation, the data will be properly wrapped.

ennorehling commented 3 weeks ago

I think my report is simply bad, owing to my trying to match the format of the reporting template.

What I'm actually doing is, I return this from a resource controller, roughly like this:

public function store(Request $request)
{
    $result = UserData::from(['name' => $request->validated('name')]);
    return $result->additional(['role' =>$request->validated('role')])->wrap('data');
}

my failing test looks something like this:

$values = ['name => 'Ruben', 'role' => 'admin'];
$this->post('/users', $values)->assertJson(['data' => $values]);

I don't see where adding a toArray() anywhere in here would make a difference.

rust17 commented 3 weeks ago

Thank you for explaining. I understand your perspective now. During detailed debugging, I discovered that the wrap behavior is designed to work as seen in this test case:

[
    "data" => [
      "name" => "Ruben",
    ],
    "rank" => "admin",
]

I then reviewed the documentation regarding wrapping and appending:

Based on this, it seems more reasonable for the wrap to behave as you expected:

[
    "data" => [
      "name" => "Ruben",
      "rank" => "admin",
    ],
]

Therefore, I opened a PR to adjust the behavior when both wrap and additional are present. Appreciate that maintainer could review and leave an opinion.