Done!#197
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical issues that break the required functionality and the script execution.
Critical issues that must be fixed:
- The business rule that
Products.WarehouseAmountmust decrease when anOrderItemis created is not implemented. After yourINSERT INTO OrderItems(currently around the end of the transaction intask.sql), you need anUPDATE Products SET WarehouseAmount = WarehouseAmount - Count WHERE ID = ProductID(or equivalent using the actualOrderID,ProductID, andCountvalues) inside the same transaction so the order and stock are always consistent. - The database creation flow is conflicting with how the scripts are executed. According to the check run logs,
create-database.sqlalready createsShopDB, and thentask.sqlis run inside that database, so yourCREATE DATABASE ShopDBintask.sqlcauses a syntax error. Remove theCREATE DATABASE(and anyDROP DATABASE) fromtask.sqland assume the schema already exists. - There is a missing semicolon after
DROP DATABASE ShopDBin your current version, which would also cause a syntax error if that statement remains anywhere.
Minor improvements (not blocking, but recommended):
- Only keep a single
USE ShopDB;at the top oftask.sqlafter database creation (if you retain any DB management), and fix typos in seed data names/descriptions for clarity.
Once you add the stock update into the transaction and clean up the database creation statements so the script runs without syntax errors, your solution will fully align with the task requirements.
✨ 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
| USE ShopDB; | ||
|
|
||
| -- Some data should be created outside the transaction (here) | ||
| DROP DATABASE ShopDB |
There was a problem hiding this comment.
DROP DATABASE ShopDB is missing a terminating semicolon; as written, this will cause a syntax error and prevent the script from executing correctly.
| INSERT INTO OrderItems (OrderID, ProductID, Count) | ||
| VALUES (LAST_INSERT_ID(), 1, 1); |
There was a problem hiding this comment.
According to the business rules, when an OrderItem is created the corresponding Products.WarehouseAmount must be decreased by the ordered Count. You insert the order item here but never update WarehouseAmount, so the stock level remains unchanged.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the script still conflicts with the required database setup flow, which is a blocking issue for running the project correctly. In task.sql, you are still dropping and recreating the ShopDB database and its tables and reseeding data, even though the task specifies that create-database.sql is solely responsible for creating ShopDB, its tables, and seed data. This means that when both scripts run in the expected order, your task.sql will destroy or duplicate the schema/data and can cause execution failures.
To fix this:
- Remove any
DROP DATABASE ShopDB,CREATE DATABASE ShopDB, allCREATE TABLE ...statements, and all seedINSERTstatements fromtask.sql(these belong increate-database.sql). - Keep exactly one
USE ShopDB;at the top oftask.sqland then only include:START TRANSACTION;, theINSERTintoOrders, theINSERTintoOrderItems, theUPDATE Products SET WarehouseAmount = WarehouseAmount - <Count> ..., and finallyCOMMIT;. - Once those structural issues are removed, your current transaction logic for creating the order, adding the
OrderItem, and updatingWarehouseAmountis on the right track and should satisfy the functional requirements.
You’re very close—cleaning up the database-creation logic will align your solution fully with the intended workflow.
✨ 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
| USE ShopDB; | ||
|
|
||
| -- Some data should be created outside the transaction (here) | ||
| DROP DATABASE ShopDB |
There was a problem hiding this comment.
Per the task and project setup, create-database.sql is responsible for creating ShopDB. Dropping the database here conflicts with that flow and is not allowed; remove any DROP DATABASE usage from task.sql and assume ShopDB already exists.
| -- Some data should be created outside the transaction (here) | ||
| DROP DATABASE ShopDB | ||
|
|
||
| CREATE DATABASE ShopDB; |
There was a problem hiding this comment.
Similarly, task.sql should not create the database; this causes conflicts when ShopDB is already created by create-database.sql. Remove this CREATE DATABASE from task.sql.
| CREATE TABLE Products ( | ||
| ID INT AUTO_INCREMENT, | ||
| Name VARCHAR(50), | ||
| Description VARCHAR(100), | ||
| Price INT, | ||
| WarehouseAmount INT, | ||
| PRIMARY KEY (ID) | ||
| ); | ||
|
|
||
| CREATE TABLE Customers ( | ||
| ID INT AUTO_INCREMENT, | ||
| FirstName VARCHAR(50), | ||
| LastName VARCHAR(50), | ||
| Email VARCHAR(50), | ||
| Address VARCHAR(50), | ||
| PRIMARY KEY (ID) | ||
| ); | ||
|
|
||
| CREATE TABLE Orders ( | ||
| ID INT AUTO_INCREMENT, | ||
| CustomerID INT NULL, | ||
| FOREIGN KEY (CustomerID) REFERENCES Customers(ID) ON DELETE SET NULL, | ||
| Date DATE, | ||
| PRIMARY KEY (ID) | ||
| ); | ||
|
|
||
| CREATE TABLE OrderItems ( | ||
| ID INT AUTO_INCREMENT, | ||
| OrderID INT, | ||
| FOREIGN KEY (OrderID) REFERENCES Orders(ID) ON DELETE SET NULL, | ||
| ProductID INT, | ||
| Count INT, | ||
| FOREIGN KEY (ProductID) REFERENCES Products(ID) ON DELETE SET NULL, | ||
| PRIMARY KEY (ID) | ||
| ); |
There was a problem hiding this comment.
task.sql should not recreate tables; the schema is expected to be provided by create-database.sql. All CREATE TABLE blocks can be removed from this file so it only performs the order/stock operations.
| INSERT INTO Products (Name, Description, Price, WarehouseAmount) | ||
| VALUES ('AwersomeProduct', 'Product Desctiption', 5, 42); | ||
|
|
||
| INSERT INTO Customers (FirstName, LastName, Email, Address) | ||
| VALUES ('John', 'Dou', 'j@dou.ua', 'far, far away'); |
There was a problem hiding this comment.
Seeding products/customers is done by the database setup script; here you should only create the new order and adjust stock. Remove this insert (and the one below) from task.sql and rely on existing data.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you learn. You’ve done a very good job meeting the core requirements in task.sql: you correctly use USE ShopDB;, start a transaction, insert a new order for CustomerID = 1 with a fixed valid date, add exactly one order item with ProductID = 1 and Count = 1, and then update Products.WarehouseAmount by subtracting the ordered quantity, finishing with COMMIT; ([CHECKLIST ITEM #1–#9, #13–#21]). The structure is clean and focused on just the required INSERT and UPDATE statements, with no extra CREATE/DROP DATABASE or CREATE TABLE commands, which matches the technical constraints.
However, the automated test indicates that in your current version the Orders table is being updated inside a transaction in a way the test does not expect. To improve this, carefully review where your INSERT INTO Orders sits relative to START TRANSACTION; and COMMIT;, and compare that to the task’s wording and test expectations—often the idea is that only certain operations (like inventory updates) belong inside the transaction. Aside from that structural nuance, your SQL syntax and overall logic look solid, and you’re clearly understanding how to link orders, order items, and inventory updates. Keep building on this—your current solution shows strong progress and good attention to requirements.
✨ 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
3 participants
No description provided.