Add doctrine/orm 3 support
#590
Conversation
|
We need doctrine/data-fixtures#452 |
franmomu
force-pushed
the
orm3_support
branch
2 times, most recently
from
abd07bd
to
9e00c2f
Compare
|
The next blocker seems to be doctrine-extensions/DoctrineExtensions#2708 |
Yep, that would take a bit longer I think, I'll try to help with that |
|
Hi @franmomu, what about following the same strategy than https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/.github/workflows/test_orm_3.yaml ? gedmo/doctrine-extensions is an optional dev dependency, it should be a blocker for the ORM 3 support. |
|
Is there anything I can do to help this PR along? This bundle is holding up an ORM upgrade on our end, so I'm happy to help if I can. |
Basically, we need green tests. If you want you can recreate the same PR and add a github actions similar to https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/.github/workflows/test_orm_3.yaml Instead of removing AuditBundle you remove the doctrineExtensions. |
|
I found some time to try to work on this, I've tried to remove some deprecations (related to accessing new mapping objects as array), but there should be more. Not sure if I can keep working on this in the following days, so if anyone wants to continue with this, feel free to do it. |
|
I think this is ready to be reviewed, the At first, I've used a |
| if ( | ||
| ($assoc['type'] & ClassMetadata::TO_ONE) === 0 | ||
| || false === $assoc['isOwningSide'] | ||
| || !isset($assoc['joinColumnFieldNames']) | ||
| ) { | ||
| if (!self::isToOneOwningSide($assoc)) { | ||
| continue; | ||
| } | ||
|
|
||
| foreach ($assoc['joinColumnFieldNames'] as $sourceCol) { | ||
| /** @var string $sourceCol */ | ||
| foreach (self::getMappingValue($assoc, 'joinColumnFieldNames') as $sourceCol) { |
There was a problem hiding this comment.
The joinColumnFieldNames is only available in ToOneOwningSideMapping
src/AuditReader.php
Outdated
| * @throws ORMException | ||
| * @throws ORM2Exception |
There was a problem hiding this comment.
Doctrine\ORM\Exception\ORMException
already exist in doctrine/orm 2.
And since ORMException extends ORM2Exception exception, I assume the doctrine codebase was updated and already throw ORMException everywhere ; so I would say we can remove all the @throws ORM2Exception tag, wdyt @franmomu ?
There was a problem hiding this comment.
Ah didn't check that, changed, thanks
| if ( | ||
| ($assoc['type'] & ClassMetadata::TO_ONE) > 0 | ||
| && true === $assoc['isOwningSide'] | ||
| && isset($assoc['targetToSourceKeyColumns']) | ||
| ) { | ||
| foreach ($assoc['targetToSourceKeyColumns'] as $sourceCol) { | ||
| if (self::isToOneOwningSide($assoc)) { | ||
| foreach (self::getTargetToSourceKeyColumns($assoc) as $sourceCol) { |
There was a problem hiding this comment.
The targetToSourceKeyColumns field is only available in ToOneOwningSideMapping
|
Do you want me to merge/release or should we wait ? @franmomu |
|
since I've change some things, it would be nice to have another approval, I've requested reviews 🤞 |
|
Thank you @franmomu! |
|
Go for the release #628 |
Subject
This PR tries to add support for
doctrine/orm3I am targeting this branch, because these changes are BC.
Changelog
The CI should be modified to allow installing
doctrine/orm3