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->newInstance() eror in Laravel <7.x version #8

Closed
mxdfulder opened this issue Mar 3, 2021 · 5 comments
Closed

Model->newInstance() eror in Laravel <7.x version #8

mxdfulder opened this issue Mar 3, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@mxdfulder
Copy link

Model->newInstance() uses $model->mergeCasts($this->casts); method from Illuminate\Database\Eloquent\Concerns\HasAttributes trait. But this method doesn't exist in that trait for Laravel version 5.x and 6.x (https://github.com/laravel/framework/blob/5.7/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php)
So any query request fails with allocating memory error.

@Radiergummi Radiergummi added the bug Something isn't working label Mar 3, 2021
@Radiergummi
Copy link
Member

Radiergummi commented Mar 3, 2021

We currently don't really have the bandwidth to support and test deprecated Laravel versions. I'm happy to merge any PRs fixing this behaviour though!
The current model implementation heavily borrows from Eloquent; I researched a little, and from what I can gather, it should be enough to add the method to the Elasticsearch model:

    /**
     * Merge new casts with existing casts on the model.
     *
     * @param  array  $casts
     * @return $this
     */
    public function mergeCasts($casts)
    {
        $this->casts = array_merge($this->casts, $casts);

        return $this;
    }

This should override the trait method if it exists, or serve as a stand-in if it doesn't.

Would you be able to provide a PR with a test?

@mxdfulder
Copy link
Author

Yes, I'll make PR after full test with Laravel 5.7 verstion, may be I could find some extra problems.
And I've alredy tested your suggestion with query search, and it fixes the problem.

@mxdfulder
Copy link
Author

Done: #9
But it may be a lot of compatibility problems with new features in old Laravel versions, and it's better replace all Database\Eloquent Concerns funcionality to avoid this.

@Radiergummi
Copy link
Member

Hey @mxdfulder I just released 2.3.0-beta.3, which includes all Eloquent traits as you suggested. Could you try using that release and let me know if things are working for you?

@Radiergummi
Copy link
Member

I'll consider this done for now. If there's anything lacking, please simply open a new issue 😊

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

No branches or pull requests

2 participants