Fix/1497 #1498
Fix/1497 #1498
Conversation
|
Thanks for the PR! Since this bug is already present in 1.1.x, it needs to target the 1.1.x branch, not master. I could change the target branch of the PR, but due to the different branch ancestor the PR wouldn't come out clean. You can either create a new PR based on the 1.0.x branch or manually reset your branch to 1.1.x before re-applying the commits and then force-pushing to the branch, after which we can change the target branch of the PR. If this is too complicated, you can also enable this checkbox in the PR sidebar and I'll update the branch for you: Edit: this needs a separate PR for 1.0.x due to the migration from |
| @@ -540,6 +540,7 @@ public function clear() | |||
| } | |||
|
|
|||
| if ($this->isOrphanRemovalEnabled()) { | |||
| $this->initialize(); | |||
There was a problem hiding this comment.
It still won't execute due to the $this->initialized you pointed out earlier. At first glance I would just remove $this->initialized from if, but I did not read further down the code to be sure.
I believe that if you're calling clear, then you want the collection to be cleared regardless if it was already initialized or not though.
There was a problem hiding this comment.
If collection is not initialized then if statements will evaluate to false thus reaching this point
There was a problem hiding this comment.
I'v made two commits so you can see the test failing and then not failing any more with the fix commit. In the bug-case $this->initialized is false, so the code after the if IS executed :). Removing the $this->initialized check & not inserting the $this->initialize() line cause the test to fail again. It looks like $this->isEmpty() is not (implicitly) triggering initialization either.
There was a problem hiding this comment.
It looks like
$this->isEmpty()is not (implicitly) triggering initialization either.
You are correct. isEmpty uses the count method defined in PersistentCollectionTrait if the collection is not initialized, which again does not initialize the collection (because it doesn't need to).
|
I'v reset & reapplied it from 1.1.x. I'v also made a PR from 1.0.x #1500 |
Failing test (1. commit) and fix (2. commit)