bumbummen99 / LaravelShoppingcart

A simple shopping cart implementation for Laravel
MIT License
505 stars 234 forks source link

undefined associatedModel function in CartItem's __get method #125

Open rucky96 opened 3 years ago

rucky96 commented 3 years ago

Hi,

I was trying to get the associated model from the cart item with $cartItem->model. However, it returns null.

I have seen that in CartItem class there is a call to the associatedModel function within __get method:

    /**
     * Get an attribute from the cart item or get the associated model.
     *
     * @param string $attribute
     *
     * @return mixed
     */
    public function __get($attribute)
    {
        if (property_exists($this, $attribute)) {
            return $this->{$attribute};
        }
        $decimals = config('cart.format.decimals', 2);

        switch ($attribute) {
            case 'model':
                if (isset($this->associatedModel)) {
                    return with(new $this->associatedModel())->find($this->id);
                }
                // no break
            case 'modelFQCN':
                if (isset($this->associatedModel)) {
                    return $this->associatedModel;
                }
                // no break
            case 'weightTotal':
                return round($this->weight * $this->qty, $decimals);
        }

        $class = new ReflectionClass(config('cart.calculator', DefaultCalculator::class));
        if (!$class->implementsInterface(Calculator::class)) {
            throw new InvalidCalculatorException('The configured Calculator seems to be invalid. Calculators have to implement the Calculator Contract.');
        }

        return call_user_func($class->getName().'::getAttribute', $attribute, $this);
    }

I have not seen where this function is defined. I have removed the '()' and now it works for me, is this a bug?

bumbummen99 commented 3 years ago

Edited your messages markdown for readability. For multi-line code blocks you have to use three ``` like this before the first and after the last line :)

hendivalon commented 3 years ago

Hi,

I was trying to get the associated model from the cart item with $cartItem->model. However, it returns null.

I have seen that in CartItem class there is a call to the associatedModel function within __get method:

    /**
     * Get an attribute from the cart item or get the associated model.
     *
     * @param string $attribute
     *
     * @return mixed
     */
    public function __get($attribute)
    {
        if (property_exists($this, $attribute)) {
            return $this->{$attribute};
        }
        $decimals = config('cart.format.decimals', 2);

        switch ($attribute) {
            case 'model':
                if (isset($this->associatedModel)) {
                    return with(new $this->associatedModel())->find($this->id);
                }
                // no break
            case 'modelFQCN':
                if (isset($this->associatedModel)) {
                    return $this->associatedModel;
                }
                // no break
            case 'weightTotal':
                return round($this->weight * $this->qty, $decimals);
        }

        $class = new ReflectionClass(config('cart.calculator', DefaultCalculator::class));
        if (!$class->implementsInterface(Calculator::class)) {
            throw new InvalidCalculatorException('The configured Calculator seems to be invalid. Calculators have to implement the Calculator Contract.');
        }

        return call_user_func($class->getName().'::getAttribute', $attribute, $this);
    }

I have not seen where this function is defined. I have removed the '()' and now it works for me, is this a bug?

i got same issue, which '()' you removed as temp solution ? can you mark it up for my reference ?

bumbummen99 commented 3 years ago

if i had to guess i think you are talking about:

if (isset($this->associatedModel)) {
    return with(new $this->associatedModel())->find($this->id);
}

And yes, associatedModel is not a function in the CartItem class. associatedModel is an attribute that should contain the FQCN of your model, so it should be a string like \\App\\Models\\Model. In PHP you can initiate a class from a SQCN like so:

$fqcn = MyClass::class;
$instance = new $fqcn()
$instance = new (MyClass::class)();
$instance = new ('\\Namespace\\MyClass')();

So the () should be valid there, the automated tests are working too. Can you please check the value of associatedModel, if it is null your model is simply not associated.

hendivalon commented 3 years ago

if i had to guess i think you are talking about:

if (isset($this->associatedModel)) {
    return with(new $this->associatedModel())->find($this->id);
}

And yes, associatedModel is not a function in the CartItem class. associatedModel is an attribute that should contain the FQCN of your model, so it should be a string like \\App\\Models\\Model. In PHP you can initiate a class from a SQCN like so:

$fqcn = MyClass::class;
$instance = new $fqcn()
$instance = new (MyClass::class)();
$instance = new ('\\Namespace\\MyClass')();

So the () should be valid there, the automated tests are working too. Can you please check the value of associatedModel, if it is null your model is simply not associated.

I'm newbie for php and laravel.

this my sample controller function store

Cart::add($item->id, $item->name, 1, $item->price, $item->weight)
            ->associate('App\Model\Product');

and this in CartItem.php

switch ($attribute) {
            case 'model':
                if (isset($this->associatedModel)) {
                    return with(new $this->associatedModel())->find($this->id);
                }
                // no break
            case 'modelFQCN':
                if (isset($this->associatedModel)) {
                    return $this->associatedModel;
                }
                // no break
            case 'weightTotal':
                return round($this->weight * $this->qty, $decimals);
        }

so where i should put these codes ?

$fqcn = MyClass::class;
$instance = new $fqcn()
$instance = new (MyClass::class)();
$instance = new ('\\Namespace\\MyClass')();
bumbummen99 commented 3 years ago

No no, this is just an example. Important for you:

MyClass::class === '\\MyClass';

So use YourModel::class or use \\ to escape the \. \ is the escape character and has always to be used with caution in strings

https://en.m.wikipedia.org/wiki/Escape_character

https://stackoverflow.com/a/25459324

So in your case you could try:

Cart::add($item->id, $item->name, 1, $item->price, $item->weight)
            ->associate('\\App\\Model\\Product');

Or, what i prefer due to better type hinting (but sometimes is not the right choice for example if Product is not available up until execution so you can not reference it):

Cart::add($item->id, $item->name, 1, $item->price, $item->weight)
            ->associate(Product::class);

@rucky96 Check modelFQCN which is associatedModel directly. If it is not null and model does return null it maybe can not find the model via the id so it maybe got (soft-)deleted.