search

doctrine-extensions / DoctrineExtensions

Doctrine2 behavioral extensions, Translatable, Sluggable, Tree-NestedSet, Timestampable, Loggable, Sortable
MIT License
4.01k stars 1.26k forks source link

Update SoftDeleteable to work with Doctrine 3.1 #2801

Closed elfantome closed 3 weeks ago

elfantome commented 1 month ago

Update Gedmo\SoftDeleteable\Filter\SoftDeleteableFilter::addFilterConstraint signature to be compatible with Doctrine\ORM\Query\Filter\SQLFilter::addFilterConstraint

mbabker commented 1 month ago

Unfortunately this change breaks compatibility with ORM 2.x.

Fatal error: Declaration of Gedmo\SoftDeleteable\Filter\SoftDeleteableFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, string $targetTableAlias): string must be compatible with Doctrine\ORM\Query\Filter\SQLFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, $targetTableAlias) in /src/SoftDeleteable/Filter/SoftDeleteableFilter.php on line 54
elfantome commented 1 month ago

Unfortunately this change breaks compatibility with ORM 2.x.

Fatal error: Declaration of Gedmo\SoftDeleteable\Filter\SoftDeleteableFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, string $targetTableAlias): string must be compatible with Doctrine\ORM\Query\Filter\SQLFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, $targetTableAlias) in /src/SoftDeleteable/Filter/SoftDeleteableFilter.php on line 54

In this case, maybe bumping the version number with a new release can solve the problem?

mbabker commented 1 month ago

I just looked at the install stats. It is way too early to consider dropping ORM 2.x support, and the volunteer resources don't exist to support two major version branches of this package.

There might be a way to change the signature in a way to support both versions (like only adding the return type without the param type), but based on numbers and resources only supporting ORM 3.x right now is not the right decision.

elfantome commented 1 month ago

Makes sense. I updated my PR to only add a return type to the method to keep compatibility with ORM 2.x

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.40%. Comparing base (0632ab1) to head (18dd98c). Report is 38 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2801 +/- ## ========================================== - Coverage 78.75% 78.40% -0.35% ========================================== Files 163 167 +4 Lines 8593 8627 +34 ========================================== - Hits 6767 6764 -3 - Misses 1826 1863 +37 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

franmomu commented 3 weeks ago

thanks @elfantome!