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

Jump to bottom

Check if association already contains object #9580

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Check if association already contains object #9580

merged 1 commit into from
Mar 18, 2022
+163 −2

Conversation

klammbueddel
Copy link

This PR adds a test to reproduce #9579

@klammbueddel klammbueddel force-pushed the bug/duplicate-object-in-nested-collections branch from f43ae5e to 2702b89 Compare March 10, 2022 14:02
@klammbueddel klammbueddel mentioned this pull request Mar 10, 2022
Closed
@derrabus derrabus added the Bug label Mar 10, 2022
@derrabus
Copy link
Member

Please run ./vendor/bin/phpcbf and commit the result. That should make the code style check happy.

@klammbueddel klammbueddel force-pushed the bug/duplicate-object-in-nested-collections branch from d1515b3 to 53bfb55 Compare March 11, 2022 08:18
@klammbueddel
Copy link
Author

@derrabus: Thank you for the hint, I did that.

@klammbueddel klammbueddel force-pushed the bug/duplicate-object-in-nested-collections branch from 53bfb55 to 04ea736 Compare March 11, 2022 09:21
greg0ire
greg0ire reviewed Mar 18, 2022
View reviewed changes
tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
{
parent::setUp();

$this->_schemaTool->createSchema(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use $this->createSchemaForModels instead, and remove tearDown (see #9526)

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Show resolved Hide resolved
tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
{
$this->createData();

$dql = <<<DQL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$dql = <<<DQL
$dql = <<<'DQL'

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
self::assertCount(1, $containers[0]->items);
self::assertCount(1, $containers[0]->items[0]->parts);

$dql = <<<DQL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$dql = <<<DQL
$dql = <<<'DQL'

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
public $id;

/**
* @var GH9579Item[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @var GH9579Item[]
* @var Collection<int, GH9579Item>

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
public $id;

/**
* @var GH9579Part
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @var GH9579Part
* @var Collection<int, GH9579Part>

@klammbueddel klammbueddel force-pushed the bug/duplicate-object-in-nested-collections branch 2 times, most recently from 64a0f85 to b558cb2 Compare March 18, 2022 08:18
@klammbueddel
Copy link
Author

@greg0ire Thank you for the review. I updated the PR.

@klammbueddel klammbueddel changed the base branch from 2.11.x to 2.12.x March 18, 2022 08:21
greg0ire
greg0ire reviewed Mar 18, 2022
View reviewed changes
tests/Doctrine/Tests/ORM/Functional/Ticket/GH9579Test.php Outdated
$this->createSchemaForModels(
GH9579Container::class,
GH9579Item::class,
GH9579Part::class,
Copy link
Member

@greg0ire greg0ire Mar 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing commas are not allowed on old versions of PHP

Suggested change
GH9579Part::class,
GH9579Part::class

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just saw this, too.. already fixed ;)

@klammbueddel klammbueddel force-pushed the bug/duplicate-object-in-nested-collections branch from b558cb2 to 61cb557 Compare March 18, 2022 10:00
@greg0ire greg0ire merged commit 08de12e into doctrine:2.12.x Mar 18, 2022
@greg0ire greg0ire added this to the 2.12.0 milestone Mar 18, 2022
@greg0ire
Copy link
Member

Thanks @klammbueddel !

@derrabus
Copy link
Member

Can we please change the PR title to something we want to read in the changelog? 🙂

@greg0ire greg0ire changed the title Add test to reproduce #9579 Check if association already contains object Mar 19, 2022
@github-actions github-actions bot mentioned this pull request Apr 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SenseException SenseException SenseException approved these changes

@greg0ire greg0ire greg0ire approved these changes

@beberlei beberlei Awaiting requested review from beberlei

@derrabus derrabus Awaiting requested review from derrabus

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
2.12.0
Development

Successfully merging this pull request may close these issues.
4 participants
@klammbueddel @derrabus @greg0ire @SenseException