Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model extending and attributes #1939

Open
ryanmitchell opened this issue Sep 6, 2024 · 5 comments
Open

Model extending and attributes #1939

ryanmitchell opened this issue Sep 6, 2024 · 5 comments
Labels
bug Something isn't working unconfirmed

Comments

@ryanmitchell
Copy link
Contributor

  • Lunar version: 1.0.0-beta.1
  • Laravel Version: 11.0.87
  • PHP Version: 8.3.1
  • Database Driver & Version: Mysql 8

Expected Behaviour:

When using model extending attributes would still be available on the product and product variant view.

Actual Behaviour:

When you go to the views the are no attributes.

Steps To Reproduce:

Set up lunar, set up an attribute, then after extend your model.

Internally Lunar is using product and product_variant to look them up, which no longer links with extending. When I switch them manually in the database to be \App\Models\Product (etc) they return, except that if I change them all I get no attributes as there is obviously a check somewhere to only display if there are any.

I would have expected that product and product_variant attributes were maintained.

@ryanmitchell ryanmitchell added bug Something isn't working unconfirmed labels Sep 6, 2024
@ken717w
Copy link

ken717w commented Sep 9, 2024

Things seem got buggy after upgrading from the last Alpha version to first Beta version.

Like you I also extended the product class to add custom scopes. I lookup every table and change product to App\Models\Product then it seems working again, not sure if it'd cause other issues tho.

@nathanjansen
Copy link

nathanjansen commented Sep 9, 2024

I've experienced this to. The getMorphClass method in HasModelExtending seems to be the method responsible.
I did not realise at first, but this seems related to #1930

This is how it worked in the last alpha.37

public function getMorphClass(): string
{
    $morphClass = ModelManifest::getMorphClassBaseModel(get_class($this));

    return $this->morphClass ?: ($morphClass ?? parent::getMorphClass());
}

Currently there is no getMorphClass and it uses the Laravel default way on the model. It seems we can now utilize the default Laravel way to handle the morph map https://laravel.com/docs/11.x/eloquent-relationships#custom-polymorphic-types
This change is nowhere documented and if this was intended, should be in the upgrade guide of Lunar: https://docs.lunarphp.io/core/upgrading.html#_1-0-0-beta-1

So for now, I think the best way to handle this to is define your own morph map for the models:

        Relation::enforceMorphMap([
            'product' => \Lunar\Models\Product::class,
            'product_variant' => \Lunar\Models\ProductVariant::class,
            'product_option' => \Lunar\Models\ProductOption::class,
            'product_option_value' => \Lunar\Models\ProductOptionValue::class,
            'collection' => \Lunar\Models\Collection::class,
            'customer' => \Lunar\Models\Customer::class,
            'cart' => \Lunar\Models\Cart::class,
            'cart_line' => \Lunar\Models\CartLine::class,
            'order' => \Lunar\Models\Order::class,
            'order_line' => \Lunar\Models\OrderLine::class,
        ]);

If you extend the Lunar models you should also add the morphClass to the extended model, for example:

<?php

namespace App\Models;

class Cart extends \Lunar\Models\Cart
{
    public function getMorphClass(): string
    {
        return 'cart';
    }
}

@alecritson
Copy link
Collaborator

@ryanmitchell Are able to test against the latest merge #1944 to see if this fixes the problem for you? 🙏

@ryanmitchell
Copy link
Contributor Author

@alecritson I've since updated all the data to point to our model class paths, so I can't verify it I'm afraid.

@wychoong
Copy link
Contributor

@alecritson I've since updated all the data to point to our model class paths, so I can't verify it I'm afraid.

the next release which fixed this in #1944 might break your app, you need to revert the data back to the relationship alias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unconfirmed
Projects
None yet
Development

No branches or pull requests

5 participants