-
Notifications
You must be signed in to change notification settings - Fork 288
feat: ability to customize pivot table columns (belongsToMany) #221
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
feat: ability to customize pivot table columns (belongsToMany) #221
Conversation
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.
This is a good start. But incomplete.
As noted in a previous comment, let's limit this to the custom table name only.
In addition, the MigrationGenerator
will also need to be updated to honor any custom table name when generating code for the table name.
src/Generators/ModelGenerator.php
Outdated
$relationship = ""; | ||
if (!is_null($class) && $type === 'belongsToMany') { | ||
if (preg_match("/^[A-Z]/", $name)) { | ||
$relationship = sprintf("\$this->%s(%s::class)->using(%s::class)", $type, '\\' . $model->fullyQualifiedNamespace() . '\\' . $relationShipClass, '\\' . $model->fullyQualifiedNamespace() . '\\' . $name); |
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.
Please use single quote for all strings.
src/Generators/ModelGenerator.php
Outdated
|
||
$relationship = ""; | ||
if (!is_null($class) && $type === 'belongsToMany') { | ||
if (preg_match("/^[A-Z]/", $name)) { |
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.
It's not clear that this preg_match
determines if the developer passed a model reference.
I am open to supporting this. But I feel there are more implications (e.g. generating the pivot model). For now, I think we should focus on the original issue, which was specifying the custom table name.
Said another way, let's assume, for now, that whatever they pass is a custom table name.
Ok, tomorrow I will update the pull request with the requested changes. |
@Pr3d4dor, no rush. Thanks. |
…om:Pr3d4dor/blueprint into feature/ability-to-customize-pivot-columns
This pull request adds the ability to customize pivot table columns (belongsToMany).
Closes #175