add new option in relation_name_strategy key to name has many Relations with plural(Field_name)

[khattab17]

add new option in relation_name_strategy key to name has many Relations with plural(Field_name) like : authors , editiors Not (posts_where_author,posts_where_editor ).
also in the RelationHelper we should replace _$primaryKey in the studly case to match the [if (Str::snake($relationName) === Str::snake($this->parent->getClassName()))] condtion in the HasMany.php Relation Class

[coatesap]

Hi @khattab17 - can you add some real world examples of where this is useful? It sounds a bit like it might be better just to rename the foreign key...

[khattab17]

Hi @khattab17 - can you add some real world examples of where this is useful? It sounds a bit like it might be better just to rename the foreign key...

i have a table called posts has two foreign keys ( author_id, editor_id) and both fields linked to users table id field the 'relation_name_strategy' => 'foreign_key' option gives me relations in User models with names posts_where_author,posts_where_editor and i think it should only be authors,editors cause this is the right naming convention in eloquent relations

[coatesap]

For the schema you described, using the 'foreign_key' naming strategy should give you:

Post::author()
Post::editor()
User::posts_where_author()
User::posts_where_editor()

Having User::authors() or User::editors() wouldn't make any sense as a relation name because the User model would effectively be referring to itself...

[khattab17]

For the schema you described, using the 'foreign_key' naming strategy should give you:

Post::author()
Post::editor()
User::posts_where_author()
User::posts_where_editor()

Having User::authors() or User::editors() wouldn't make any sense as a relation name because the User model would effectively be referring to itself...

sorry it was my fault i fixed it to be

User::editorPosts()
User::authorPosts()
i think now it make sence
also i was having another problem with the 'relation_name_strategy' => 'foreign_key' option , i have another table called comments has foreign key user_id linked with users table its relation was (i set the 'snake_attributes' => false in the config)
User::commentsWhereUser()
and i think it should be
User::comments()
cause there is no other foreign key linked with users table in comments table

[CristianLlanos]

This is a breaking change

[khattab17]

as i mentioned before this change fixes the User::comments() relation example cause in the HasMany Relation class we have this condition "if (Str::snake($relationName) === Str::snake($this->parent->getClassName()))" to check the foreign key name
before my change it returns "user_ === user" because the old code does not replace the "_" from the foreign key , we need to replace _id or Id from the foreign key to generate the relation name with plural of the other table (comments not userComments)

[CristianLlanos]

Thanks for clarifying, @khattab17. It does break some of our tests though. Which means it is introducing some unexpected behaviour as well. If you run phpunit, you might be able to see what's going on.

[khattab17]

i fixed this bug , i think now it passes the phpunit test

[CristianLlanos]

I'm not sure, @coatesap. It think it might be interesting to support this new style. However, I believe it would be better if it came with tests. Moreover, since it has a breaking change, according to semver it cannot go into master nor v1.x; a v2.x branch should be a good candidate. I fully support what @coatesap decides is best for the package.

[coatesap]

I'm conflicted on whether this style of relation naming is a good thing or not. I can see that by removing the word "where", it makes the method/property a bit shorter. However, it does also make the resulting code harder to read.
For instance, it would be easy to assume that the following property gave you a list of all staff who were managers, instead of a list of staff managed by the current staff member:

/** @var Employee $joe */
$employees = $joe->managerEmployees;

Whereas the current approach does seem to be a little bit clearer:

$employees = $joe->employeesWhereManager;