2.6 add 'atomic' option to "save()" API - delivery #3604
Conversation
| } | ||
| return $success; | ||
| } | ||
| catch (Exception $e) { |
|
some weird error appears in travis. |
|
That error is unrelated to your changes. It will be taken care of. Can you please squash your commits. |
This commit adds a transaction context to 'save()' API in order to rollback possible modifications done in some 'Model.beforeSave' listener callback. This will allow cakephp 2.x to behave like 3.0 . It uses try/catch to better handle transaction. Previous save() API is renamed to protected _doSave() method. A new save() method is created for transaction handling. 'atomic' option is disabled for internal 'save()' call.
|
I didn't know what 'squash your commits' means, so I hope I didn't do something bad lol. |
Yup that's what i meant, sorry for not being clear enough :) |
| $transactionBegun = $db->begin(); | ||
| try { | ||
| $success = $this->_doSave($data, $options); | ||
| if ($transactionBegun) { |
There was a problem hiding this comment.
This if is not actually needed, I think. If atomic, it will always evaluate to true
|
Apart from my nitpick, this looks great. Awesome job! |
2.6 add 'atomic' option to "save()" API - delivery
|
I'll update the book. |
|
first contribution to an opensource project ever. |
|
@Haititi 👏 Nice job! |
|
👏 🍸 |
New PR created for delivery of #3516 .
Due to some branch problem, I had to create another branch for delivery, but the previous PR could not be updated with the new branch.
Moreover, conflicts due to merged commits from #3590 were resolved by me (the author of these commits) to make this new PR easier to merge.