Make model table names configurable

[antonkomarev]

Proof of concept #164 (comment)
Resolves #163

Define any custom table name in config/love.php file

'storage' => [
    'database' => [
        'connection' => env('DB_CONNECTION', 'mysql'),
        'tables' => [
            'love_reacters' => 'custom_reacters',
            'love_reactants' => 'custom_reactants',
            'love_reaction_types' => 'data.reaction_types',
        ],
    ],
],

Table names with null value will use values from Model's $table class property.

Different schema/database could be defined for cross-schema queries by adding its name with dot on the end before the table name, eg. data.reaction_types. More details read in #163

[antonkomarev]

I'll merge it if there will be no other good solutions found next week.

[antonkomarev]

@vesper8 could you confirm that it is solving your issue?

[vesper8]

Hey @antonkomarev ! Yes I'm happy to say your solution does work!

All the modifications to the migrations are not needed however. These are the only two blocks that are needed

    'storage' => [
        'database' => [
            'connection' => 'data',
            'tables' => [
                'love_reacters' => 'ns_api_data.love_reacters',
                'love_reactants' => 'ns_api_data.love_reactants',
                'love_reactions' => 'ns_api_data.love_reactions',
                'love_reaction_types' => 'ns_api_data.love_reaction_types',
                'love_reactant_reaction_counters' => 'ns_api_data.love_reactant_reaction_counters',
                'love_reactant_reaction_totals' => 'ns_api_data.love_reactant_reaction_totals',
            ],
        ],
    ],

    public function getTable(): string
    {
        return Config::get("love.storage.database.tables.{$this->table}")
            ?? $this->table;
    }

Also, noticed I also added love_reactions, did you forget about that table?

Thanks!

[antonkomarev]

All the modifications to the migrations are not needed however.

You don't need migration changes because you are already migrated database, but if somebody want to change default table names? It wouldn't work then. But I haven't tried to perform migrations with database prefix. Need to try it too.

Also, noticed I also added love_reactions, did you forget about that table?

Good catch! Fixed.

[antonkomarev]

Checked database migration with this config:

'storage' => [
    'database' => [
        'connection' => 'data',
        'tables' => [
            'love_reacters' => 'sandbox-laravel-love-data.love_reacters',
            'love_reactants' => 'sandbox-laravel-love-data.love_reactants',
            'love_reaction_types' => 'sandbox-laravel-love-data.love_reaction_types',
            'love_reactions' => 'sandbox-laravel-love-data.love_reactions',
            'love_reactant_reaction_counters' => 'sandbox-laravel-love-data.love_reactant_reaction_counters',
            'love_reactant_reaction_totals' => 'sandbox-laravel-love-data.love_reactant_reaction_totals',
        ],
    ],
],

Working well.

[vesper8]

excellent : ) thanks for the quick support as usual!

[aalyusuf]

Excellent solution, would love for this to be merged

[antonkomarev]

Sorry for the delay. Huge amount of work these days. Will try to find some time to make a release soon.

[antonkomarev]

I've described another one way to implement table names configuration, but not sure it's a good one because this configuration place is not obvious.

#164 (comment)

[aalyusuf]

Can we consider the possibility of extending Models? I think it will be better to be able to define models instead of having table names in a config file.

[antonkomarev]

Extending of models is not so easy thing because relations will resolve base models which would have different methods and behavior. It will require to have some kind of static registry of all the package models like I've implemented many years ago in Laravel Messenger package or like Laravel Passport is doing it.

[antonkomarev]

I see 2 cons here:

1. adding static model registry models makes configuration not obvious;
2. you have to extend each model just to change table names even if you don't want to overwrite model behavior.

[aalyusuf]

I agree, it might not be the most straightforward answer to solve this specific issue but can be an overall solution for other possibilities as well.

[vesper8]

@antonkomarev your proposed, and already implemented solution, is fine, it is backwards compatible and changes nothing for users who do not wish to change their table names. My only comment is that it could be a little clearer that you must prefix your database name to the table names in the config files for those wishing to use the default table names but on a different connection. In an ideal world simply setting that you wish to use a different connection should be enough but we've seen it doesn't work like that with cross-connection relations

As far as extending models as an alternative.. no it isn't so simple at all.. it requires binding multiple classes in a service provider and it's very frail.. 10x more complicated and prone to breaking then your current proposed solution. If your configuration file allowed to specify alternative models like many other packages do then it would be simpler, but still overkill if all one wants to do is change the table name.. or as in my case, simply be able to prefix it with the database name

[antonkomarev]

Finally, I've found half an hour and released Laravel Love v8.4.0.