Skip to content
This repository was archived by the owner on Aug 17, 2024. It is now read-only.

Conversation

khmdagal
Copy link

master

Copy link

@Ara225 Ara225 left a comment

Choose a reason for hiding this comment

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

Good work, I can see you're starting to develop your own code style which is nice :) I've made a few comments, nothing too important.

@@ -35,16 +36,305 @@ Open the file `cyf_ecommerce.sql` in VSCode and examine the SQL code. Take a pie
Once you understand the database that you are going to work with, solve the following challenge by writing SQL queries using everything you learned about SQL:

1. Retrieve all the customers' names and addresses who live in the United States

Copy link

Choose a reason for hiding this comment

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

Your work here is really good, well done. It being SQL, there's normally only one way to do things and you seem to have done everything in a way that makes sense. The only thing I would comment that you could improve is that some results have more columns in them than the question asked for. You can use a column in the where clause without including it in the final result, which might be where you tripped up.


pool
.query(
`SELECT c.name, c.address, c.city, c.country FROM customers c WHERE id=${customerId}`
Copy link

Choose a reason for hiding this comment

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

As we metioned earlier, puting the aurguements directly into the sql query when they come from an untrusted source, like the API user in this case is never recommended cos of the SQL injection risk :)


app.get("/cyf-ecommerce-api/productsBySuppliers", (req, res) => {
pool
.query(`${productsBySuppliers}`)
Copy link

Choose a reason for hiding this comment

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

You can just put productsBySuppliers without the the backticks etc. I'm fine with this cos the data doesn't come from an untrusted source

app.post("/customers/:customerId/AddNewOrders", (req, res) => {
const custID = +req.params.customerId;
const query = `INSERT INTO orders (order_date,order_reference,customer_id)
VALUES ('2023-01-10','0RD012',${Number(custID)})`;
Copy link

Choose a reason for hiding this comment

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

See comment above re untrusted inputs

`
UPDATE customers
SET name='Ahmed',address='10 AliClose M82UY',city='Manchester',country='Canada'
WHERE id ='${custID}'
Copy link

Choose a reason for hiding this comment

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

See comment above re untrusted inputs

app.delete("/deleteOrder/:orderId", (req, res) => {
const orderId = +req.params.orderId;
pool
.query(`DELETE FROM orders WHERE orders.id='${Number(orderId)}'`)
Copy link

Choose a reason for hiding this comment

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

Another one

8. Retrieve all orders, including order items, from customer ID `1`. Include order id, reference, date and total cost (calculated as quantity \* unit price).


SELECT orders.customer_id, order_items.order_id, orders.order_reference, orders.order_date, order_items.quantity*product_availability.unit_price as "Total Cost" FROM order_items
Copy link

Choose a reason for hiding this comment

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

Lovely, better to stick to lowercase no spaces for the column names cos the way Postgres behaves with capitals is not too nice - it makes them lowercase if you forget the quotes, so it's better to avoid the source of confusion. Noticed it a couple times but this will be the only one I'm calling out I would be inclined to put brackets around this custom column definition - I intially read it as order_items.quantity,*,product_availability.unit_price which is completely diffrent


SELECT customers.name, orders.order_reference, orders.order_date, products.product_name,
order_items.quantity, product_availability.unit_price,
(order_items.quantity * product_availability.unit_price) AS "Total Amount"
Copy link

Choose a reason for hiding this comment

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

Very nice, you've done the brackets I suggested here are well

host: "localhost",
database: "cyf_ecommerce",
password: "Kdagaal123",
port: 5432,
Copy link

Choose a reason for hiding this comment

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

Good you're using a pool rather than a single connection. Best to get into the habit of putting creds outside the app as soon as you can though cos it may not matter now, but if you do it when it does it's a big deal. I know the course doesn't cover it but I still think it's a good habit

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants