unset _meta on save. - Githubissues

pdphilip / laravel-elasticsearch

Laravel Elasticsearch: An Elasticsearch implementation of Laravel's Eloquent ORM

MIT License

86 stars 16 forks source link

unset _meta on save. #30

Closed use-the-fork closed 1 month ago

use-the-fork commented 1 month ago

I'm not sure why but whenever I was saving it was including the _meta and saving it as part of the document. I just added an unset with the save.

Before:

My Fix:

pdphilip commented 1 month ago

Hi @use-the-fork - this is a bug, the meta data should only be available with the getMeta() method: https://elasticsearch.pdphilip.com/the-base-model#query-meta

It should not be set as model fields. Could you share the methods you used to get here so that I can recreate this issue myself?

I'm finalizing the next release and it would be good to get to the bottom of this one before that. Thank you!

use-the-fork commented 1 month ago

Yes, I have narrowed it down to when I update a model, not when I create one. I'm going to try and reproduce it in a test case. If I replicate it, I will link the branch to you.

pdphilip commented 1 month ago

Ok don't stress, I managed to find it. I thought it was a model hydration issue, but in some cases, the data is written before that point, causing this.

Your PR looks good, thank you. Please resubmit it with just the relevant fix and keep the indented blank lines as is, then I'll gladly accept the PR and bring you in as a contributor on the next release (which is good to go!)

Thanks @use-the-fork!

pdphilip commented 1 month ago

.. and just to clarify on the empty lines, I'll remove them for the entire package after bringing in your PR. I just want to track your contribution separately. Thanks

pdphilip commented 1 month ago

All good, nothing to do 😊 Thanks @use-the-fork

use-the-fork commented 1 month ago

All good, nothing to do 😊 Thanks @use-the-fork

Thanks! I’m almost done with a test suite setup and general setup using Nix.

https://github.com/use-the-fork/laravel-elasticsearch/tree/refract/cleanup

I’ll raise a PR when I’m done. I hope you don’t mind. I love the package. 😀

pdphilip commented 1 month ago

Sounds good, two things:

I think you have, but make sure your starting point is from yesterday's commits (v4.0.3), there are several new features (Release coming today)
I have a separate test suite repo (PEST) that is comprehensive and includes yesterday's new features: https://github.com/pdphilip/laravel-elasticsearch-tests - Have a look here for the tests themselves, take, add.

Great!