Skip to content

Type error fix for optimistic lock with embeddable entity#24

Merged
butschster merged 1 commit intocycle:1.xfrom
wpdmitry:fix/optimistic-lock
Feb 18, 2022
Merged

Type error fix for optimistic lock with embeddable entity#24
butschster merged 1 commit intocycle:1.xfrom
wpdmitry:fix/optimistic-lock

Conversation

@wpdmitry
Copy link
Contributor

When using Embeddable Entities for OptimisticLock Behavior, an occurs error:

TypeError: Cycle\ORM\Relation\Embedded::queue(): Argument #3 ($command) must be of type ?Cycle\ORM\Command\StoreCommandInterface, Cycle\ORM\Command\Special\WrappedCommand given, called in /app/vendor/cycle/orm/src/Transaction/UnitOfWork.php on line 349

In this PR, Author (Embeddable entity) support for the Comment was added and after that tests (OptimisticLock/ListenerTest.php) was crashed. As it was found out, this happens when repeated calls persist method of EntityManager. It turned out to fix it by replacing the WrappedCommand with the WrappedStorageCommand for the OptimisticLock Listener.

@wpdmitry
Copy link
Contributor Author

@msmakouz Hello Maxim! Could you look at this PR?

@msmakouz msmakouz self-requested a review February 16, 2022 07:23
@msmakouz msmakouz requested a review from butschster February 16, 2022 07:23
@wpdmitry
Copy link
Contributor Author

@butschster Hello :)
Will you have time to look at this PR?

@butschster butschster merged commit 60cc56f into cycle:1.x Feb 18, 2022
@butschster
Copy link
Contributor

Hi @wpdmitry ! Sorry for long response!. Thank you for contributing!

$command->setScope($this->field, $nodeValue);

return WrappedCommand::wrapCommand($command)
return WrappedStoreCommand::wrapCommand($command)
Copy link
Member

Choose a reason for hiding this comment

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

Should be tested when an entity deletion (with and without SoftDelete behavior)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants