Skip to content

1#193

Open
ll221 wants to merge 7 commits into
mate-academy:mainfrom
ll221:main
Open

1#193
ll221 wants to merge 7 commits into
mate-academy:mainfrom
ll221:main

Conversation

@ll221

@ll221 ll221 commented Jun 15, 2026

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 some core functional requirements are still missing, so the script would not satisfy the task as described. You correctly use USE ShopDB;, start a transaction, insert a new Orders row for CustomerID = 1 with date 2023-01-01, and create an OrderItems row linked via LAST_INSERT_ID() — that part is good.

However, in task.sql you currently insert into OrderItems with only OrderID and ProductID (around the INSERT INTO OrderItems statement), and you do not set the required Count = 1; this breaks the requirement that OrderItems must store ProductID, Count, and OrderID and that the AwersomeProduct is ordered with quantity 1 [CHECKLIST ITEM #3, #4, #14]. Also, after inserting the order item, there is no UPDATE Products that decreases WarehouseAmount for that product by the ordered quantity, so the business rule that stock is reduced when an order item is created is not implemented [CHECKLIST ITEM #7]. Both of these are critical, because they directly violate the described schema and business rules and will cause the tests to fail.

To fix this, update your INSERT INTO OrderItems to explicitly include the Count column with value 1, for example by adding Count to the column list and 1 to the values list. Then add an UPDATE Products statement inside the same transaction that subtracts that count (1) from WarehouseAmount for ProductID = 1. You may also want to adjust your comments around the transaction to clearly show that all order-related changes (insert into Orders, OrderItems, and the Products update) are atomic together. Once these changes are in place, your solution should meet the core functional requirements, and the rest of your structure looks solid — you are very close.


✨ 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 +9
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.

The Orders insert correctly creates a new order for customer 1 with a valid date, but the overall transaction is incomplete until you also update Products.WarehouseAmount in the same transaction as required by the business rules (checklist #7 and #11).

Comment thread task.sql Outdated
Comment on lines +11 to +12
INSERT INTO OrderItems (OrderID, ProductID)
VALUES (LAST_INSERT_ID(), 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.

According to the description, OrderItems must store ProductID, Count, and OrderID, and the order item must be AwersomeProduct with count 1. Here you only insert OrderID and ProductID, so the required Count = 1 is missing (checklist #3, #4, and #14).

@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 a couple of core functional requirements missing, which will block the task from passing. You correctly use ShopDB, seed the product and customer, and create an order for customer 1 with the right date inside a transaction, and you link the order item using LAST_INSERT_ID(), which is all good.

Critical issues that must be fixed:

  1. In task.sql, your INSERT INTO OrderItems (currently specifying only (OrderID, ProductID)) does not include the Count column, even though the requirement says each order item must store ProductID, Count, and OrderID, and this order must contain AwersomeProduct (ID: 1, count: 1). Update that statement to include the Count column in the column list and pass the value 1, for example: (OrderID, ProductID, Count) VALUES (LAST_INSERT_ID(), 1, 1), and keep it inside the transaction.
  2. The business rule also requires that when an OrderItem is created, the product’s WarehouseAmount is decreased by the ordered quantity within the same transaction. After inserting into OrderItems, add an UPDATE Products that subtracts 1 from WarehouseAmount for ProductID = 1, and ensure this UPDATE remains between START TRANSACTION; and COMMIT; so all order-related changes are atomic.

A smaller, environment-related note: if create-database.sql already inserts the product with ID = 1, make sure your task.sql doesn’t re-insert it with the same primary key (or use INSERT IGNORE/ON DUPLICATE KEY UPDATE) to avoid duplicate key errors during tests. Once you address the missing Count column and the stock update, your script should fully match the described business rules and pass the functional checks.


✨ 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
INSERT INTO Orders (CustomerID, Date)
VALUES (1, '2023-01-01');

INSERT INTO OrderItems (OrderID, ProductID)

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, each order item must store ProductID, Count, and OrderID, and this order must contain AwersomeProduct (ID: 1, count: 1). Here you only insert OrderID and ProductID, so the Count column is missing and the quantity is never recorded. Update this INSERT to include the Count column in the column list and set its value to 1.

Comment thread task.sql Outdated

INSERT INTO OrderItems (OrderID, ProductID)
VALUES (LAST_INSERT_ID(), 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.

The business rule requires that when a new OrderItem is created, the corresponding product’s WarehouseAmount in Products is decreased by the ordered quantity, and this should happen inside the same transaction as the order and order item creation. There is currently no UPDATE Products here, so stock levels are not adjusted. Add an UPDATE Products after inserting into OrderItems that subtracts the ordered count (1) from WarehouseAmount for ProductID = 1.

@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 an educational override so you can continue progressing while you learn, even though there are still a couple of important points to improve. You correctly create the product, customer, order, and order item, and you update Products.WarehouseAmount after inserting into OrderItems, which aligns well with the business rules and covers items like [CHECKLIST ITEM #1], [#5], and [#6]. However, your order-related operations (INSERT INTO Orders, INSERT INTO OrderItems, and UPDATE Products) are not currently wrapped in an explicit transaction: there is a COMMIT; at the end of the script, but no START TRANSACTION;/BEGIN; before the first order INSERT, so they still run in autocommit mode and are not atomic as required by [CHECKLIST ITEM #15]. To fix this, add START TRANSACTION; right before the INSERT INTO Orders statement and keep all order-related DML up to the final COMMIT; inside that block. Additionally, the failed check log shows a Duplicate entry '1' for key 'Products.PRIMARY' error, meaning your INSERT INTO Products with ID = 1 conflicts with existing seeded data—consider using INSERT IGNORE, REPLACE, ON DUPLICATE KEY UPDATE, or omitting the explicit ID if the product is already created. Overall, you’re very close: your data relationships and update logic look good, and with a proper transaction wrapper and a small tweak to avoid the primary key conflict, your solution would be fully robust and production-ready.


✨ 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 +11 to +12
-- Start the transaction
START TRANSACTION;
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.

You have a COMMIT; at the end of the script, but there is no corresponding START TRANSACTION;/BEGIN; before the order-related statements. Without explicitly starting a transaction, these INSERT and UPDATE operations run in autocommit mode and are not grouped atomically as required. Add START TRANSACTION; before the INSERT INTO Orders so that creating the order, inserting the order item, and updating WarehouseAmount happen within a single transaction, matching the business rules and checklist item #15.

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.

4 participants