Skip to content

Add transaction for order creation#205

Open
Somewellich wants to merge 2 commits into
mate-academy:mainfrom
Somewellich:develop
Open

Add transaction for order creation#205
Somewellich wants to merge 2 commits into
mate-academy:mainfrom
Somewellich:develop

Conversation

@Somewellich

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the current script does not reliably link the newly created order to its order item, which is a critical functional requirement. After you insert into Orders, you never capture the generated ID, and later you hardcode OrderID = 1 in the OrderItems insert; this will break as soon as the Orders table has more than one row or IDs are auto-incremented and not reset. To fix this, store the result of LAST_INSERT_ID() into a variable right after the INSERT INTO Orders (e.g., SET @order_id = LAST_INSERT_ID();) and then use @order_id instead of 1 when inserting into OrderItems and any related statements. Also, ensure that the order item and the warehouse update remain in the same transaction so they either both succeed or both roll back together. As a minor improvement, you could also tie the warehouse decrement to the same Count value you insert into OrderItems so the stock change always matches the ordered quantity. Once you correctly use the generated order ID in your OrderItems insert, your solution will meet the key requirements and be robust in more realistic database states.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sql
Comment on lines +4 to +5
INSERT INTO Orders (CustomerID, Date)
VALUES (1, '2023-01-01');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you insert the order, but you never capture the generated ID; later you assume it is 1. To meet the requirement of creating a new order reliably, consider storing LAST_INSERT_ID() in a variable so you can reference the actual order you just created.

Comment thread task.sql Outdated
Comment on lines +10 to +11
INSERT INTO OrderItems (OrderID, ProductID, Count)
VALUES (1, 1, 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using OrderID = 1 assumes the new order’s primary key is 1, which may not be true if the table isn’t empty or IDs auto-increment. This can break the linkage between the new order and its item and violates the intent of creating an order item for the order you just inserted.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it correctly implements the required order creation and inventory update logic in task.sql and aligns with the core business rules. You create a new order for CustomerID = 1 with the specified date, then use LAST_INSERT_ID() to capture the generated OrderID, avoiding hardcoded IDs and ensuring the correct linkage between Orders and OrderItems. The insertion of the order item and the corresponding UPDATE of Products.WarehouseAmount are wrapped in a single transaction, so either both succeed or both fail, which fulfills the atomicity requirement and protects data consistency. The script also correctly handles a generic item count via a variable, so the logic scales to other counts (e.g., 5 items decreasing warehouse stock by 5) as required.

There are no critical blockers: the remaining concerns (like environment setup and database creation) are outside the scope of this file and are assumed to be handled by the surrounding infrastructure. As a minor polish, you could remove any stray backticks after COMMIT; to avoid confusion in some editors or markdown renderers, but this does not affect functionality. Overall, you’ve met the functional and transactional requirements for this task—well done.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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.

2 participants