search

cviebrock / eloquent-taggable

Easily add the ability to tag your Eloquent models in Laravel.
MIT License
559 stars 72 forks source link

Support of table prefix #84

Closed tuxfamily closed 5 years ago

tuxfamily commented 5 years ago

Hello.

The migration script does care of the table prefix, so tables are well created using the prefix, but next, in eloquent-taggable the tables names are hardcoded in the queries, so it leads to some "table not found" exceptions.

So I added support of table prefix through the env('DB_TABLE_PREFIX'). There is maybe a better way to do it, but this one "just works".

Hope it can help. Cédric.

cviebrock commented 5 years ago

This is great. A few things:

  1. You shouldn't use env() directly in the code, since this will fail if you use config caching. See https://laravel.com/docs/5.7/configuration#configuration-caching

Offhand, I'm not sure, but I think you can get the prefix via:

$prefix = (new Tag())->getConnection()->getTablePrefix();

The other option would be to re-write those queries in Eloquent or QueryBuilder, I suppose.

  1. I find it weird that you'd need to change the morphedBy/ToMany() calls in src/Models/Tag.php and src/Taggable.php. Doesn't Laravel automagically apply the table prefix here? If not, that sounds like an issue with Laravel.

  2. Any chance you can add some tests for this?

Thanks again!

cviebrock commented 5 years ago

@tuxfamily Feel free to try the 3.5.0 version (for Laravel 5.8). I re-wrote some of your PR but I think it solves your issue now.