Skip to content

Add transaction logic for creating order and order item#203

Open
Dmytro234 wants to merge 3 commits into
mate-academy:mainfrom
Dmytro234:develop
Open

Add transaction logic for creating order and order item#203
Dmytro234 wants to merge 3 commits into
mate-academy:mainfrom
Dmytro234:develop

Conversation

@Dmytro234

Copy link
Copy Markdown

Creates a new order outside a transaction (it starts empty), then wraps the order item insert and warehouse amount decrement in a transaction to keep stock counts consistent.

Creates a new order outside a transaction (it starts empty), then
wraps the order item insert and warehouse amount decrement in a
transaction to keep stock counts consistent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@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 a couple of critical issues that can break the required functionality against the provided ShopDB schema and tests. First, your INSERT INTO OrderItems uses a hard‑coded OrderID = 1 (see task.sql where you insert into OrderItems), which will fail as soon as the new order’s ID is not 1; you need to capture the actual newly inserted order ID (for example with LAST_INSERT_ID() into a variable) and use that value in the OrderItems insert so the item is correctly linked to the created order [CHECKLIST ITEM #3, #7]. Second, the OrderItems table is specified to have an ID column, but your insert omits ID (you only list OrderID, ProductID, Count); if ID is not defined as AUTO_INCREMENT in create-database.sql, this will cause an error, so you must either include ID explicitly in the column list or rely on the auto‑increment behavior configured in the schema [CHECKLIST ITEM #4]. Third, you currently create the order outside of the transaction, which means the order row and its item/stock updates are not atomic; to align with the business rule that each order is handled in a single transaction, you should wrap the INSERT INTO Orders, the OrderItems insert, and the Products update in the same START TRANSACTION/COMMIT block [CHECKLIST ITEM #14].

As minor improvements, consider updating WarehouseAmount by subtracting the actual Count inserted for the order item instead of a hard‑coded 1, so the script naturally extends to other quantities in the future. Once you fix the order ID handling, align the OrderItems insert with the real schema, and move order creation inside the transaction, your solution should satisfy the requirements and run reliably on a freshly recreated ShopDB. Keep the overall structure you already have—your general flow matches the business rules well, and these adjustments will make it robust and correct.


✨ 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
-- Adding an order item and decreasing the corresponding product's
-- WarehouseAmount must happen together, or not at all, to keep stock
-- counts consistent with what has actually been ordered.
INSERT INTO OrderItems (OrderID, ProductID, Count)

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 hard-code use of OrderID = 1 for the new order. This will only work if the newly created order’s ID is guaranteed to be 1, which the requirements do not state. To ensure the order item is attached to the order you just inserted, consider capturing the new order’s ID (e.g., via LAST_INSERT_ID() into a variable) and using that value in the OrderItems insert.

Comment thread task.sql
-- Adding an order item and decreasing the corresponding product's
-- WarehouseAmount must happen together, or not at all, to keep stock
-- counts consistent with what has actually been ordered.
INSERT INTO OrderItems (OrderID, ProductID, Count)

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 OrderItems table in the description includes an ID column. Depending on how create-database.sql defines this table, omitting ID here may cause an error if ID is not AUTO_INCREMENT. Check the table schema and adjust the insert to match it (either include ID explicitly or rely on auto-increment if available).

Comment thread task.sql
Comment on lines +22 to +23
UPDATE Products
SET WarehouseAmount = WarehouseAmount - 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 update WarehouseAmount using a hard-coded subtraction of 1 based on the assumption that the order item’s Count is 1. If you later extend this script to handle other counts, consider tying the warehouse update to the actual Count inserted for the item to fully follow the business rule that stock should decrease by the ordered quantity.

Comment thread task.sql
Comment on lines +6 to +8
-- Creating an order is a standalone action: a new order starts out empty,
-- so it does not need to be atomic with anything else.
INSERT INTO Orders (CustomerID, Date)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Placing order creation outside the transaction means the order insert and the subsequent item/stock operations are not atomic together. Given the business rules describe a transaction per order to keep data consistent, consider including the INSERT INTO Orders inside the same transaction so that either all related changes succeed or all are rolled back.

The order insert, its item, and the warehouse stock decrement must all
commit or roll back together, so order creation is moved inside the
transaction. Also capture the new order's ID with LAST_INSERT_ID()
instead of hard-coding it, per review feedback on PR mate-academy#203.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@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 blockers that will prevent your script from running correctly against the expected environment. First, your INSERT INTO OrderItems (OrderID, ProductID, Count) does not match the described OrderItems schema, which only has ID, OrderID, and ProductID; unless create-database.sql really defines a quantity column, this insert will fail at runtime. Please open create-database.sql, confirm the actual columns for OrderItems, and then adjust both the column list and values in your insert so they align with that schema (and represent quantity in a way that the schema supports). Second, the test logs indicate that Orders are being updated inside the transaction, while the task’s tests expect order updates to happen outside the transaction block, so you need to move the write operations on Orders out of the START TRANSACTION ... COMMIT section. A good approach is to keep any reads/validations inside the transaction and then perform the actual Orders modifications after COMMIT, or in a separate transaction that complies with the test’s expectations. Aside from these issues, your use of LAST_INSERT_ID() and the overall transactional flow are solid, so once you fix the schema mismatch and adjust where Orders is updated, your solution should satisfy the task and pass the tests.


✨ 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

SET @OrderID = LAST_INSERT_ID();

INSERT INTO OrderItems (OrderID, ProductID, Count)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the task description, the OrderItems table has columns ID, OrderID, and ProductID only; there is no Count column listed, so INSERT INTO OrderItems (OrderID, ProductID, Count) is likely to fail against the actual create-database.sql schema. Please verify the real table definition and adjust the column list and values to match it (and then update how you represent quantity in the order if needed).

test.sh (the repo's own CI check) requires INSERT INTO Orders to occur
before START TRANSACTION and only OrderItems/Products inside it; the
previous commit moved Orders into the transaction per AI review
feedback, which broke that check. Restore the original structure and
keep capturing the order id via LAST_INSERT_ID() instead of a
hard-coded value.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@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 approving your solution as part of an educational override so you can continue progressing while you refine it further.

You did a solid job with the transaction boundaries: the order is created empty first, and then the item insertion and warehouse stock update happen together inside a transaction, which matches the business rules. The main issue to fix is in task.sql where you insert into OrderItems using a Count column that does not exist in the actual schema—according to create-database.sql, OrderItems only has ID, OrderID, and ProductID, so the current INSERT will fail at runtime. To align with the requirements, you’ll need to adjust the INSERT INTO OrderItems statement to match those columns and represent the quantity using the supported mechanism (for example, inserting multiple rows for the same product when the quantity is greater than one, or using any dedicated quantity column if present in create-database.sql).

Overall, your approach to transactions and business logic is on the right track; once you correct the OrderItems insert to conform to the schema and quantity handling, your solution will be much stronger. Keep going—you’re clearly close to a fully correct implementation, and this is a great opportunity to practice reading and matching database schemas carefully.


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

Footnotes

  1. Rate AI review example

Comment thread task.sql
-- Adding an order item and decreasing the corresponding product's
-- WarehouseAmount must happen together, or not at all, to keep stock
-- counts consistent with what has actually been ordered.
INSERT INTO OrderItems (OrderID, ProductID, Count)

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 OrderItems table in this task is defined with columns ID, OrderID, and ProductID; there is no Count column, so this INSERT will fail. Please open create-database.sql, confirm the actual OrderItems schema, and adjust the column list and values so they match it while still representing the quantity (e.g., using multiple rows or an existing quantity field, depending on the schema).

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