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 PersistentCollection::get() returning entity not in the collection #9035

Closed
wants to merge 3 commits into from

Conversation

sztyup
Copy link
Contributor

@sztyup sztyup commented Sep 23, 2021

Fixes #8607

@sztyup sztyup marked this pull request as ready for review September 23, 2021 10:14
]);
} catch (Exception $e) {
// skip errors
}
Copy link
Member

Choose a reason for hiding this comment

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

Please apply a change similar to what I did in #8962 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@beberlei
Copy link
Member

beberlei commented Oct 3, 2021

Does this really require a new testcase with new entities? There is already a bunch of tests related to persistent collections and indexBy feature in general, from my pov it would make sense to just put it there.

@derrabus derrabus changed the base branch from 2.9.x to 2.10.x October 3, 2021 18:36
@derrabus derrabus added this to the 2.10.1 milestone Oct 3, 2021
@derrabus
Copy link
Member

derrabus commented Oct 3, 2021

Can you please rebase your PR onto 2.10.x? The 2.9.x branch has been closed for bugfixes.

@derrabus derrabus modified the milestones: 2.10.1, 2.10.2 Oct 5, 2021
@greg0ire greg0ire modified the milestones: 2.10.2, 2.10.3 Oct 21, 2021
@derrabus derrabus modified the milestones: 2.10.3, 2.10.4 Dec 3, 2021
@derrabus derrabus removed this from the 2.10.4 milestone Dec 20, 2021
@Spilky
Copy link

Spilky commented Jan 20, 2023

@greg0ire would it be possible to merge this bugfix? It would be really helpful. I spent some time with debbuging because of this and fix seems easy.

@greg0ire
Copy link
Member

@Spilky it targets 2.10.x, which is not maintained, so no. Feel free to open a new PR based on it though. If you do, target 2.14.x

@Spilky
Copy link

Spilky commented Jan 20, 2023

@greg0ire new PR opened here: #10439

@derrabus
Copy link
Member

Closing in favor of #10439.

@derrabus derrabus closed this Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra lazy PersistentCollection::get() with indexBy doesn't check mappedBy value
5 participants