-
Notifications
You must be signed in to change notification settings - Fork 11.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5.8] Fix pluralizing model names ending in words with an irregular plural #26421
Conversation
Part of me thinks we may be putting quite a bit of logic into determining the table name of a very edge-case model name? |
Any model name consisting of more than one word, ending in a word with an irregular plural currently doesn't work correctly. Three lines of tested code seems like a reasonable fix for this problem. I ran into this problem last week when i tried to create a model called |
Is there a reason you didn't want to include the morphToMany fix in this PR as well? Thanks. |
I wanted to get some feedback on the PR before doing the effort of writing a failing test for a morphToMany relationship. I'll make some time tomorrow and write one. |
I've added a test and a fix for morhpToMany relationships. It would be a little bit cleaner with a |
|
||
$lastWord = array_pop($words); | ||
|
||
$table = implode('', $words).Str::plural($lastWord); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you build the table with an empty string implode and not underscores?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PREG_SPLIT_DELIM_CAPTURE
makes it also capture the delimiter (in this case the underscore), so it can be imploded without a delimiter to rebuild the original string. I copied this code from the Str::pluralStudly
method, that's why its like this. Doing it this way is necessary when exploding studly case strings, it isn't really necessary to do it this way for snake case strings.
It could be changed to more familiar php like this:
$words = explode('_', $name);
$words[count($words) - 1] = Str::plural($words[count($words) - 1]);
$table = implode('_', $words);
Currently, running
artisan make:model RealHuman -m
gives you a migration called2018_11_07_133204_create_real_humen_table
and a table calledreal_humen
. This happens because it tries to pluralize the word RealHuman, and since that word isn't in the list of irregular plurals the Doctrine Inflector applies the normal (in this case incorrect) rules to pluralize the word.This PR adds a
Str::pluralStudly
helper that pluralizes only the last word of a studly caps case string. This helper is used throughout the framework to correctly generate plurals of model names.The oldStr::plural
is still being used in themorphToMany
method. If this PR gets merged that should also be fixed and tested.Edit:
turns out someone had this issue before: doctrine/inflector#35