search

kodeine / laravel-meta

Fluent Meta Data for Eloquent Models, as if it is a property on your model
MIT License
398 stars 89 forks source link

Check model relation on __set method #76

Closed ericp-mrel closed 3 years ago

ericp-mrel commented 4 years ago

What this PR fixes. When I was trying to use this package and save a BelongsTo relation it was being saved to the meta table instead of the model's table. This is probably because I am using OctoberCMS which is based on Laravel, but defines relations slightly differently (as attributes instead of methods but are still accessed and written to the same way).

So, this commit will check to see if that method returns a relation and if so, will save it to the model like normal.

I haven't actually tested this change in Laravel, but I assume this should still work. Also, if you don't want to merge this change I'd also understand I would appreciate it though so I wouldn't need to maintain a separate fork.

ericp-mrel commented 4 years ago

Actually, after doing some further testing with this change Looks like there's some issues I've come across. I'll leave this open for now and let you know when I think this would be ready if you'd want to merge this change.

kodeine commented 4 years ago

@ericp-mrel can you please tell me what other issues you faced?

mrelevance commented 4 years ago

The issue I was facing after making the initial PR is when you went to set some attribute it would run the __set() method and in turn try and call a method that potentially didn't exist and of course would throw an exception. So, now what I've done is extracted that into its own block surrounded by a try catch when testing it, and now if calling that method fails it is simply ignored and just moves on like normal. But, if a relation method is detected it will then call setAttribute and set the value to the relationship instead of the meta table.

I can do some testing in actual Laravel sometime later today just to make sure this will still work with both (OctoberCMS and Laravel).

ericp-mrel commented 4 years ago

Woops. The response above was accidentally posted from my other GitHub account. (Currently transitioning stuff on GitHub so logged into different accounts in different browsers.)