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

Allow custom object types as foreign keys #9927

Merged
merged 3 commits into from
Jul 24, 2022
Merged

Conversation

louisezetterlund
Copy link

Continues #9793 which updates IdentifierFlattener to work with foreign keys with custom id object types. This PR adds a test for #9793.
Fixes #9335

Hello, I would like to make a small change.
The need arose when using \Symfony\Bridge\Doctrine\IdGenerator\UuidGenerator in "symfony/doctrine-bridge" composite foreign keys
I'm sure these changes will not hurt performance
and allow other objects to be used as identifiers
Fix for coding-standards / Coding Standards (8.1)
@derrabus derrabus added the Bug label Jul 22, 2022
@derrabus
Copy link
Member

Your test is failing, can you have a look?

@louisezetterlund
Copy link
Author

I'm on it! 😊

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Thanks, that looks promising. A little nit-picking from my side. 🙂

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9335Test.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/ORM/Functional/Ticket/GH9335Test.php Outdated Show resolved Hide resolved
@greg0ire greg0ire enabled auto-merge July 22, 2022 17:38
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

The PR looks good.

One more thing, @louisezetterlund: Your commit is signed with some kind of local e-mail-address. You can see that when you run git log on your branch.

I don't know if that was your intention or if your git client is just not set up properly. I can merge this PR as is, but then your GitHub account won't be listed as a contributor to this repository.

You can find out more about setting up you commit email address here. Here you can also learn what email address to use if you want to keep your real email address private.

@greg0ire greg0ire merged commit f5246bd into doctrine:2.12.x Jul 24, 2022
@derrabus
Copy link
Member

All right, @greg0ire has already merged it. 🤷🏻 Anyway, thanks for your PR. And please have a look at the article I've linked, so future contributions can be attributed to you properly.

@derrabus derrabus added this to the 2.12.4 milestone Jul 25, 2022
@derrabus derrabus changed the title GH9335: Fix for bug with objects as foreign keys Allow custom object types as foreign keys Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doctrine orm Identity through foreign Entities Bug (when identifiers are of type RamseyUuid)
5 participants