Skip to content
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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ramity
Copy link

@ramity ramity commented Jan 7, 2024

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.

Copy link
Collaborator

@OskarStark OskarStark left a 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?

src/Doctrine/EntityRelation.php Outdated Show resolved Hide resolved
@@ -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.',
Copy link
Author

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

Copy link
Member

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.

Copy link
Author

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?',
Copy link
Author

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

@ramity
Copy link
Author

ramity commented Jan 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants