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

Persistance of position relocations moved to other events to fix issue #1209 #1214

Merged
merged 4 commits into from Feb 24, 2015

Conversation

leonex-cs1
Copy link
Contributor

This PR fixes issue #1209. Please refer this issue to understand the background of this patch.

  • I have added an unit test for issue [Sortable] Positions are updated on entity remove, even if removing failed #1209
  • Moved the persistance of position relocations from onFlush event, which is called before database transaction started, to the first call of postPersist, preUpdate or preRemove. So that the update query is executed within the transaction and if errors happen during queries the position changes will be rolled back.
  • The synchronization of the objects has been separated from the database persistance, so that it will happen only if all queries were successfully executed (in postFlush).

The only thing I don't like with this solution is, that it will only work with entities that have identifiers. I am not sure if this is a problem.
I had to use these because the INSERTs are executed before the UPDATE query for the relocation of the positions. And that will manipulate the positions of inserted records. But I cannot use another event in this case.

Doctrine lacks an event which is fired between the beginning of a transaction and the inserts :-( This would have made the patch much simpler.

…doctrine-extensions#1209

Added an unit test for issue doctrine-extensions#1209 and moved the persistance of position relocations from onFlush event, which is called before database transaction started, to the first call of postPersist, preUpdate or preRemove. So that the update query is executed within the transaction and if errors happen during queries the position changes will be rolled back. The synchronization of the objects has been separated from the database persistance, so that it will happen only if all queries were successfully executed.
…s the persistence of positions after removal
@leonex-cs1
Copy link
Contributor Author

Testing on a live system showed that the shouldSyncPositionAfterDelete did not test if the positions have been persisted correctly. So I have extended the test in 79544ff.

Further I had to move the call to relocation persistance from preRemove to the postRemove event, because preRemove is executed right after calling $em->remove($entity) and not in flush.

I hope it's okay, that I have done that in multiple commits.

l3pp4rd added a commit that referenced this pull request Feb 24, 2015
Persistance of position relocations moved to other events to fix issue #1209
@l3pp4rd l3pp4rd merged commit c044532 into doctrine-extensions:master Feb 24, 2015
@l3pp4rd
Copy link
Contributor

l3pp4rd commented Feb 24, 2015

thanks, that was very useful

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.

None yet

2 participants