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

Fix OneToManyPersister::deleteEntityCollection missing discriminator column/value. (GH-11500) #11501

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

gitbugr
Copy link
Contributor

@gitbugr gitbugr commented Jun 14, 2024

Fixes #11500

This issue may exist in 3.x and 4.x, I haven't yet had a chance to test. If so I can create seperate pull requests for those branches.

@greg0ire
Copy link
Member

It seems there are CI jobs failing. Please take a look at this guide for more on how to handle those.

@gitbugr
Copy link
Contributor Author

gitbugr commented Jun 15, 2024

It seems there are CI jobs failing. Please take a look at this guide for more on how to handle those.

Thanks! Ran the code-beautifier, checked with codesniffer and static analysis tools and pushed amendments.

greg0ire
greg0ire previously approved these changes Jun 15, 2024
Comment on lines 171 to 175
/**
* @throws EntityNotFoundException
* @throws MappingException
*/
Copy link
Member

Choose a reason for hiding this comment

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

This replaces DBALException with ORMExceptions. Is this an additional fix because none of them extends the Exception of DBAL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DBALException should not have been removed - my IDE autofix appears to have removed that line by mistake. I have pushed an amendment to fix that.

The ORM exceptions are potentially thrown by UnitOfWork::getEntityIdentifier() (EntityNotFoundException) and ClassMetadataInfo::getFieldForColumn() (MappingException).

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is a breaking change as this is going to be merged up to 3 and 4 of ORM. But it looks like the DBALException isn't handled either by ORM despite being an Exception of a different library. Now ORMExceptions were added too.

@greg0ire greg0ire merged commit cc2ad19 into doctrine:2.19.x Jun 17, 2024
58 checks passed
@greg0ire greg0ire added this to the 2.19.6 milestone Jun 17, 2024
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.

None yet

3 participants