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.

Sign up for GitHub

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
Open

Adding ORM\JoinTable attribute to the make:entity command's ManyToMany field type #1416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d10e50c
Added a prompt if the user would like to specify a joinTable name dur…
ramity Jan 7, 2024
309bf05
fixed the two php-cs-fixer errors
ramity Jan 7, 2024
17b939d
fixed 3 places where ManyToMany relations were being tested
ramity Jan 7, 2024
5abc8c2
updated tests comments to match others, added testcase for custom joi…
ramity Jan 7, 2024
f4eda5a
fixed test name + added suggested change to add type to EntityRelatio…
ramity Jan 7, 2024
b5b7ccf
initialized value to null to avoid errors from uninit access
ramity Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Doctrine/BaseRelation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public function __construct(
private bool $isOwning = false,
private bool $orphanRemoval = false,
private bool $isNullable = false,
private ?string $joinTableName = null,
) {
}

Expand Down Expand Up @@ -85,4 +86,9 @@ public function isNullable(): bool
{
return $this->isNullable;
}

public function getJoinTableName(): ?string
{
return $this->joinTableName;
}
}
12 changes: 12 additions & 0 deletions src/Doctrine/EntityRelation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ final class EntityRelation
private bool $isSelfReferencing = false;
private bool $orphanRemoval = false;
private bool $mapInverseRelation = true;
private ?string $joinTableName = null;

public function __construct(
private string $type,
Expand Down Expand Up @@ -97,6 +98,7 @@ public function getOwningRelation(): RelationManyToMany|RelationOneToOne|Relatio
isSelfReferencing: $this->isSelfReferencing,
mapInverseRelation: $this->mapInverseRelation,
isOwning: true,
joinTableName: $this->joinTableName,
)),
self::ONE_TO_ONE => (new RelationOneToOne(
propertyName: $this->owningProperty,
Expand Down Expand Up @@ -186,4 +188,14 @@ public function setMapInverseRelation(bool $mapInverseRelation): void

$this->mapInverseRelation = $mapInverseRelation;
}

public function getJoinTableName(): ?string
{
return $this->joinTableName;
}

public function setJoinTableName(?string $joinTableName)
{
$this->joinTableName = $joinTableName;
}
}
16 changes: 16 additions & 0 deletions src/Maker/MakeEntity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

false
);

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

lcfirst(Str::getShortClassName($relation->getOwningClass())).ucfirst($relation->getOwningProperty())
));
}
};

switch ($type) {
case EntityRelation::MANY_TO_ONE:
$relation = new EntityRelation(
Expand Down Expand Up @@ -708,6 +722,8 @@ function ($name) use ($targetClass) {
));
}

$askJoinTableName($relation);

break;
case EntityRelation::ONE_TO_ONE:
$relation = new EntityRelation(
Expand Down
6 changes: 6 additions & 0 deletions src/Util/ClassSourceManipulator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Embedded;
use Doctrine\ORM\Mapping\JoinColumn;
use Doctrine\ORM\Mapping\JoinTable;
use Doctrine\ORM\Mapping\ManyToMany;
use Doctrine\ORM\Mapping\ManyToOne;
use Doctrine\ORM\Mapping\OneToMany;
Expand Down Expand Up @@ -558,6 +559,11 @@ private function addCollectionRelation(BaseCollectionRelation $relation): void
),
];

// Conditionally add joinTable attribute
if ($relation->getJoinTableName()) {
$attributes[] = $this->buildAttributeNode(JoinTable::class, ['name' => $relation->getJoinTableName()], 'ORM');
}

$this->addProperty(
name: $relation->getPropertyName(),
attributes: $attributes,
Expand Down
37 changes: 37 additions & 0 deletions tests/Maker/MakeEntityTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ public function getTestDetails(): \Generator
'y',
// field name on opposite side - use default 'courses'
'',
// join table? - use default 'n'
'',
// finish adding fields
'',
]);
Expand Down Expand Up @@ -272,6 +274,8 @@ public function getTestDetails(): \Generator
'y',
// field name on opposite side - use default 'courses'
'',
// join table? - use default 'n'
'',
// finish adding fields
'',
]);
Expand All @@ -280,6 +284,37 @@ public function getTestDetails(): \Generator
}),
];

yield 'it_adds_many_to_many_with_custom_join_table' => [$this->createMakeEntityTest()
->run(function (MakerTestRunner $runner) {
$this->copyEntity($runner, 'User-basic.php');

$runner->runMaker([
// entity class name
'Course',
// field name
'students',
// add a relationship field
'relation',
// the target entity
'User',
// relation type
'ManyToMany',
// inverse side?
'y',
// field name on opposite side - use default 'courses'
'',
// join table?
'y',
// join table name - use default 'courseStudents'
'',
// finish adding fields
'',
]);

$this->runCustomTest($runner, 'it_adds_many_to_many_with_custom_join_table.php');
}),
];

yield 'it_adds_one_to_one_simple' => [$this->createMakeEntityTest()
->run(function (MakerTestRunner $runner) {
$this->copyEntity($runner, 'User-basic.php');
Expand Down Expand Up @@ -363,6 +398,8 @@ public function getTestDetails(): \Generator
* normally, we ask for the field on the *other* side, but we
* do not here, since the other side won't be mapped.
*/
// join table? - use default 'n'
'',
// finish adding fields
'',
]);
Expand Down
Loading