Skip to content

Solution#169

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

Solution#169
just-oleh wants to merge 3 commits into
mate-academy:mainfrom
just-oleh:develop

Conversation

@just-oleh
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

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

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 few critical issues that can break the required functionality and violate the checklist. In task.sql, your INSERT INTO Orders ... currently runs before START TRANSACTION;, which means the order can be created even if adding items or updating stock fails; move START TRANSACTION; so it appears before this INSERT to keep everything atomic [CHECKLIST ITEM #16]. In the INSERT INTO OrderItems statement you hardcode OrderID = 1; instead, after inserting into Orders, capture the new ID with something like SET @order_id = LAST_INSERT_ID(); and then use @order_id in the OrderItems insert so the item is always linked to the correct order [CHECKLIST ITEMS #1, #2, #3, #7]. Your UPDATE Products currently decreases WarehouseAmount by a fixed 1; update this to subtract the Count value from the corresponding OrderItems row so stock stays consistent with the ordered quantity and keep this inside the same transaction [CHECKLIST ITEM #8]. Another blocking issue is that the required create-database.sql script is missing; you need to add it to the repository, ensuring it drops ShopDB if it exists and recreates it from the provided schema before running task.sql [CHECKLIST ITEMS #14, #15]. As a minor improvement, consider checking that there is enough stock before decrementing WarehouseAmount and adding a short comment around the transaction block explaining which operations are grouped together. Once you address these points, your solution will be robust and fully aligned 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sql
@@ -1,11 +1,15 @@
-- Use our database
USE ShopDB;
USE ShopDB;
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 script issues USE ShopDB; but the task requires you to create the ShopDB database using the provided create-database.sql (and drop/recreate it if it exists). Make sure ShopDB exists and was created from the repository script before running this task.sql.

Comment thread task.sql
Comment on lines +3 to +4
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 INSERT INTO Orders is executed outside any transaction. To ensure atomicity (so the order, its items, and the stock update all succeed or all roll back) start the transaction before inserting the order per checklist item #16.

Comment thread task.sql

-- Start the transaction
START TRANSACTION;
START TRANSACTION;
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 start the transaction after creating the order. Move START TRANSACTION; before the INSERT INTO Orders so the order insert is part of the same transaction as adding items and updating stock.

Comment thread task.sql Outdated
Comment on lines +8 to +9
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.

You're inserting the OrderItem with a hardcoded OrderID = 1. Instead capture the actual ID of the inserted order (e.g. use LAST_INSERT_ID() or SET @order_id = LAST_INSERT_ID();) and use that variable in the OrderItems INSERT so the item links to the correct order (checklist items #1, #2, #3, #7).

Comment thread task.sql Outdated
Comment on lines +11 to +12
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.

The UPDATE Products decrements WarehouseAmount by a hardcoded 1. Ensure this update uses the quantity from the inserted OrderItem (Count) and that it runs inside the same transaction so warehouse amounts stay consistent (checklist item #8).

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

Changes are needed before this solution can be considered complete, mainly because required repository-level and structural requirements are not yet met.

  1. Critical blockers
  • The repository is missing the mandatory create-database.sql script. You must add this file so that it: (a) runs DROP DATABASE IF EXISTS ShopDB;, (b) recreates ShopDB with the expected schema from the provided specification, and only then is task.sql run against that database. This is required for automated setup and is explicitly checked by the tests.
  • In task.sql, at least one statement that modifies the Orders table (such as INSERT INTO Orders ... or UPDATE Orders ...) is currently placed inside a transaction block between START TRANSACTION (or BEGIN) and COMMIT. The bash-based tests expect all Orders modifications to occur outside of any explicit transaction block, so you need to move those Orders statements before the START TRANSACTION or after the COMMIT while keeping the rest of the order-item and stock-update logic transactional.
  1. How to fix the blockers
  • Create create-database.sql in the repo root (or expected location) with commands to drop ShopDB if it exists, create it again, and then define all tables/columns/constraints exactly as in the provided schema. This ensures anyone (and the CI) can recreate the database from scratch.
  • In task.sql, locate the block that looks like:
    START TRANSACTION;
      -- some statements
      INSERT INTO Orders ...;  -- or UPDATE/DELETE on Orders
      -- other statements
    COMMIT;
    and refactor it so that the Orders-modifying statements are before START TRANSACTION; or after COMMIT;, while only the OrderItems insertion and Products.WarehouseAmount update remain inside the transaction.
  1. Minor (non-blocking) suggestion
  • Where you select or reference the Count column, consider quoting it as `Count` to avoid ambiguity with the COUNT() aggregate function and to improve clarity; this is stylistic and not a test blocker.

Your transactional logic for creating an order, inserting into OrderItems, and decrementing Products.WarehouseAmount is otherwise on the right track, so once you add create-database.sql and adjust the placement of the Orders statements, your solution should satisfy the task and pass the remaining 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
@@ -1,11 +1,21 @@
-- Use our database
USE ShopDB;
USE ShopDB;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make sure the repository contains and you run the required create-database.sql (which must DROP DATABASE ShopDB if it exists and recreate ShopDB from the provided schema) before executing this script. task.sql assumes ShopDB already exists and selected here.

Comment thread task.sql Outdated

UPDATE Products
SET WarehouseAmount = WarehouseAmount - (
SELECT 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.

Using the identifier Count without quotes can be ambiguous because COUNT() is an SQL aggregate. Consider quoting the column name (for example, Count) in the SELECT to avoid any parser ambiguity and improve clarity.

Copy link
Copy Markdown

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

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 and it correctly creates the ShopDB schema and implements the required order creation and transactional behavior (including an order for customer ID = 1 on 2023-01-01 with one AwersomeProduct item), satisfying the core checklist items such as [CHECKLIST ITEM #1#17] and [CHECKLIST ITEM #20#32]. This approval is an educational override to help you progress while learning, because there are a few minor robustness and style issues you can still improve. In task.sql, in the UPDATE Products SET WarehouseAmount = ... statement right after the INSERT INTO OrderItems for the new order, the subquery may subtract multiple rows if there are ever multiple OrderItems entries for the same product; consider using an aggregate like SUM(Count) in that subquery so stock is decremented by the total count for that product [related to CHECKLIST ITEM #11]. In create-database.sql, where you define the OrderItems table, and in task.sql where you insert into OrderItems, it would be clearer and safer to consistently quote the Count column as `Count` to avoid confusion with the COUNT() function. Also in create-database.sql, in the INSERT INTO Products that adds AwersomeProduct, you can fix the small typo 'Desctiption' to 'Description' to keep your data clean. As a further robustness improvement, you could add a check before inserting into OrderItems (or a constraint) to ensure WarehouseAmount never becomes negative when fulfilling an order. Overall, you met the functional and transactional requirements very well, and these small refinements will help you write even more reliable and readable SQL going forward.


✨ 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