-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Adding ORM\JoinTable
attribute to the make:entity
command's ManyToMany
field type
#1416
base: main
Are you sure you want to change the base?
Conversation
…ing the creation of a many-to-many relation.
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.
Can you add a test with a join table?
@@ -619,6 +619,20 @@ function ($name) use ($targetClass) { | |||
$relation->setMapInverseRelation($mapInverse); | |||
}; | |||
|
|||
$askJoinTableName = function (EntityRelation $relation) use ($io) { | |||
$joinTableDecision = $io->confirm( | |||
'Do you want to specify a join table? You may want to do this if you plan on having multiple many-to-many relations to the same entity.', |
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.
Open to suggestions on rewording
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.
In the current placement, this question will be asked for all relation types, right? But we only want it for ManyToMany
For the wording, what about just one question:
A join table will be created for the relationship. You can name this, or let Doctrine choose a name for you.
Join table name (default: set automatically):
If they hit enter (leave it blank), then we know to name it automatically.
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 area just defines and stores the function as a variable. The logic within isn't actually ran until the $askJoinTableName($relation);
statement is executed later down in the code here.
As much as I'd prefer the lower complexity of one question, we'll want to give the user the ability to say no to the addition of a join table. Cases like bidirectional many-to-many and self referencing many-to-many require the use of a ManyToMany attribute without a JoinTable attribute on one side.
|
||
if ($joinTableDecision) { | ||
$relation->setJoinTableName($io->ask( | ||
'What should the join table be named?', |
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.
Open to suggestions on rewording
I reverted the previous commit because it was made under the impression I broke something, but it seems like something related to nikic/php-parser. I'll revisit ensuring test coverage for these changes once that's fixed up. |
You can add two ManyToMany relations to the same target entity, but the generated code will be invalid. You'll hit a "Table already exists" error because of how the two relations will share the same join table name under the hood. I whipped up these changes to give the user the power to manually specify the join table name during the prompt.
This PR adds an additional check during the make:entity command's ManyToMany field type steps by asking the user if they'd like to specify a join table, allows them to do so, and adds the ORM\JoinTable attribute automagically.