Use db transactions in create and update operations #5403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @promatik this is ready for review we will launch it as in a minor version 6.6 in a few weeks or so, can you give it a go ? |
promatik
reviewed
Jan 8, 2024
|
Did the changes @promatik thanks for the review 🙏 It's still Cheers |
promatik
approved these changes
Jan 27, 2024
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY
BEFORE - What was wrong? What was happening before this PR?
The queries to create/update an entry and it's relationships are not wrapped in a DB transaction, that means that any error during the operation would not invalidate the queries that were already commited, so you could have an entry in the database with only one of it's two relationships for example, or no relationships at all depending on when the error occurred.
AFTER - What is happening after this PR?
We discussed this during our meeting and got to the conclusion that wrapping those processes in a transaction is a key factor when data integrity must be assured. So if any query during those processes fail, the whole process is reverted and no queries are commited to the database.
HOW
How did you achieve that, in technical terms?
Using the DB::transaction provided by Laravel and making it configurable.
Is it a breaking change?
It can be seen as a BC yes, so it needed to be behind a feature flag that need to be enabled. We plan to make this the default in next version.