Skip to content

Solved task transactions#200

Open
Loki-sudo007 wants to merge 3 commits into
mate-academy:mainfrom
Loki-sudo007:main
Open

Solved task transactions#200
Loki-sudo007 wants to merge 3 commits into
mate-academy:mainfrom
Loki-sudo007:main

Conversation

@Loki-sudo007

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 there are still critical issues that block the task from being considered complete. First, your final COMMIT in task.sql is missing a terminating semicolon; this can cause the transaction not to commit properly or make the script fail to execute in common MySQL clients, so you must add COMMIT; at the end of the transaction. Second, the automated tests report that the Orders table is being updated inside the transaction, while the task’s test script expects Orders modifications to happen outside the transaction; this violates the required behavior and leads to failing tests. To fix this, move any INSERT/UPDATE on Orders so that they occur either before the explicit transaction starts or after the COMMIT, leaving the transaction to handle only the intended tables (like OrderItems and Products). Once these two issues are resolved, your script should both satisfy the functional requirements and pass the tests. Other points like using numeric literals instead of string literals for INT fields are minor style issues and do not block acceptance, but improving them will make your SQL more idiomatic. Overall, your core logic is on the right track; addressing the transaction boundary and the missing semicolon will complete the solution.


✨ 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 Outdated

COMMIT; No newline at end of file
UPDATE Products SET WarehouseAmount = WarehouseAmount - 1 WHERE ID = '1';
COMMIT No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The COMMIT statement is missing a terminating semicolon; without it, the transaction may not be properly committed and the script can fail to execute as intended.

@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 there is still a critical issue with how the transaction boundaries are expressed relative to the task’s requirements. The task explicitly requires that only OrderItems and Products updates be inside the START TRANSACTION/COMMIT block, with Orders modifications clearly outside of it, and your current solution relies on LAST_INSERT_ID() inside the transaction without a stable, clearly defined OrderID. This can lead to incorrect behavior under concurrent inserts and makes it harder to verify that the tests are truly checking only transactional behavior for OrderItems and Products. To fix this, you should capture the new order’s ID before starting the transaction (for example, by assigning LAST_INSERT_ID() to a variable) and then use that fixed OrderID value inside the transaction, while keeping all Orders statements outside the transactional block. Once you clearly separate the non-transactional Orders insert from the transactional OrderItems and Products updates and use a stable OrderID within the transaction, your solution will match the business rules and test expectations. As a minor improvement, you can also switch to unquoted numeric literals for IDs (e.g., 1 instead of '1') for clearer intent, though this is not blocking. Overall, your functional logic is very close, and with this adjustment to transaction handling, your script should fully satisfy the task.


✨ 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 Outdated
Comment on lines +8 to +14
INSERT INTO Orders (CustomerID, Date)
VALUES ('1', '2023-01-01');

START TRANSACTION;

-- And some data should be created inside the transaction
INSERT INTO OrderItems (OrderID, ProductID , Count)
VALUES (LAST_INSERT_ID(), '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.

You currently insert into Orders before starting the transaction, which is correct according to the requirement that Orders modifications happen outside the explicit transaction, but you rely on LAST_INSERT_ID() inside the transaction without clearly documenting or controlling concurrency. The task and tests want the transaction to encapsulate only OrderItems and Products changes; consider making the separation explicit (e.g., by ensuring the order creation is clearly outside and using a stable OrderID value inside the transaction) so the transactional responsibilities are unambiguous and match the expected behavior.

@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. The task.sql script correctly creates a new order for CustomerID = 1 on 2023-01-01, then uses LAST_INSERT_ID() to store the order ID in @order_id before starting the transaction, which is exactly what the requirements call for [CHECKLIST ITEM #1, #2, #3, #10, #11, #28, #29]. Inside the transaction, you properly insert into OrderItems using OrderID, ProductID, and Count, and then update Products.WarehouseAmount to reflect the ordered quantity, satisfying the core business rules [CHECKLIST ITEM #4, #5, #6, #7, #8, #9, #20, #21, #22, #27].

One small improvement for learning: instead of hardcoding the decrement as WarehouseAmount = WarehouseAmount - 1, consider tying it directly to the Count value (for example, WarehouseAmount = WarehouseAmount - <Count>), so the logic is more obviously generalizable if you later change the quantity [CHECKLIST ITEM #7]. Overall, you’ve done a solid job handling transaction boundaries and using USE ShopDB; correctly, and the script should run cleanly assuming the schema matches the described tables [CHECKLIST ITEM #15, #19]. Great work—this is a strong, correct implementation that shows a good understanding of transactional updates and order handling.


✨ 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