spatie / laravel-data

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

->include doesn't work if the property is nullable #847

Closed genesiscz closed 3 weeks ago

genesiscz commented 2 months ago

✏️ Describe the bug I am expecting to be able to ->include or ->includePermanently() on a lazy-loaded property on a lazy-loaded collection. Instead of deeply-including the property, it just ignores the include. I am not getting an error about property not being existing, but the property is not shown in the response.

I even tried to do one step manually by creating the first depth of collection manually, but it didn't help either:

return GetReservationsResponse::from([
            'reservations' => $reservations->getCollection()->map(
                fn (Reservation $reservation) => ReservationData::from($reservation)
                    ->includePermanently("reservationProducts.employeeProduct.product")
                    ->includePermanently("reservationProducts.employeeProduct.employee"),
            ),

            'pagination' => PaginationData::fromPagination($reservations),
        ]);

What helped (and what probably caused this) was the |null in

public Lazy|ProductData|null $product, // |null causing the property not to be included
public Lazy|EmployeeData|null $employee, // |null causing the property not to be included

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


it('cannot include fields that are nullable', function () {
    class ReservationData extends Data
    {
        public function __construct(
                #[DataCollectionOf(ReservationProductData::class)]
                public Lazy|DataCollection $reservationProducts,

        ) {
        }

        public static function fromModel(Reservation $reservation): ReservationData
        {
            $data = new self(
                reservationProducts: Lazy::whenLoaded("reservationProducts", $reservation, fn () => ReservationProductData::collect($reservation->reservationProducts, DataCollection::class)),
            );

            return $data;
        }
    }

    class ReservationProductData extends Data
    {
        public function __construct(
                public int $id,
                public Lazy|ProductData|null $product, // This |null causes this bug
        ) {
        }

        public static function fromModel(ReservationProduct $reservationProduct): ReservationProductData
        {
            $data = new self(
                id: $reservationProduct->id,
                product: Lazy::create(fn() => $reservationProduct->product)         
            );
            return $data;
        }
    }

    class Product extends Data
    {
        public function __construct(
                public int $id
        ) {
        }
    }

    $reservations = ReservationData::collect(Reservation::all(), DataCollection::class)->include("reservationProduct.product");

   dd($reservations); // 
    /* Expected: [ 
            "reservations" => [ 
                [
                    "reservationProducts" => [ 
                        [
                            "id" => 69,
                            "product" => [
                                "id" => 1,
                            ]
                        ]
                    ]
                ]
        ],
        Actual: [ 
            "reservations" => [ 
                [
                    "reservationProducts" => [ 
                        [
                            "id" => 69
                        ]
                    ]
                ]
        ],

   */

});

I am sorry, I am not sure how to do the pest with my models, but I hope it helps at least as a vizualization

✅ Expected behavior In the code

🖥️ Versions

Laravel: "v10.48.20" Laravel Data: 4.5.0 (there was an issue - I don't remember why I locked to this version, but will try latest and report if it fixes anything) PHP: 8.2

rubenvanassche commented 3 weeks ago

Hi @genesiscz,

I've rewritten your test to make use of arrays instead of Models since that makes things easier and is from the package point of view the same (except the Lazy::whenLoaded but we can replace this with a conditional Lazy which is basically the same).

Could you try breaking it? Since from my point of view it is working as expected.

I'm gonna close this issue but feel free to react to it when you found a bug, you're also always welcome to sent me a demo project with models showing the issue.

 it('cannot include fields that are nullable', function () {
    class ReservationData extends Data
    {
        public function __construct(
            #[DataCollectionOf(ReservationProductData::class)]
            public Lazy|DataCollection $reservationProducts,
        ) {
        }

        public static function fromArray(array $reservation): ReservationData
        {
            return new self(
                reservationProducts: Lazy::when(
                    fn () => true,
                    fn () => ReservationProductData::collect($reservation['reservationProducts'],
                        DataCollection::class)
                ),
            );
        }
    }

    class ReservationProductData extends Data
    {
        public function __construct(
            public int $id,
            public Lazy|ProductData|null $product, // This |null causes this bug
        )
        {
        }

        public static function fromArray(array $reservationProduct): ReservationProductData
        {
            return new self(
                id: $reservationProduct['id'],
                product: Lazy::create(fn () => $reservationProduct['product'])
            );
        }
    }

    class ProductData extends Data
    {
        public function __construct(
            public int $id
        ) {
        }
    }

    $reservations = ReservationData::collect([
        [
            "reservationProducts" => [
                [
                    "id" => 69,
                    "product" => [
                        "id" => 1,
                    ],
                ],
                [
                    "id" => 70,
                    "product" => null,
                ],
            ],
        ],
    ], DataCollection::class);

    dd(
        $reservations->include("reservationProducts")->toArray(),
        $reservations->include("reservationProducts.product")->toArray(),
    ); //
    /* Expected: [
            "reservations" => [
                [
                    "reservationProducts" => [
                        [
                            "id" => 69,
                            "product" => [
                                "id" => 1,
                            ]
                        ]
                    ]
                ]
        ],
        Actual: [
            "reservations" => [
                [
                    "reservationProducts" => [
                        [
                            "id" => 69
                        ]
                    ]
                ]
        ],

   */

});