NW5-Manchester-Khadar-SQL-W2 - #188
Conversation
Ara225
left a comment
There was a problem hiding this comment.
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.
| 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 | ||
|
|
There was a problem hiding this comment.
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}` |
There was a problem hiding this comment.
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}`) |
There was a problem hiding this comment.
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)})`; |
| ` | ||
| UPDATE customers | ||
| SET name='Ahmed',address='10 AliClose M82UY',city='Manchester',country='Canada' | ||
| WHERE id ='${custID}' |
| app.delete("/deleteOrder/:orderId", (req, res) => { | ||
| const orderId = +req.params.orderId; | ||
| pool | ||
| .query(`DELETE FROM orders WHERE orders.id='${Number(orderId)}'`) |
| 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 |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
Very nice, you've done the brackets I suggested here are well
| host: "localhost", | ||
| database: "cyf_ecommerce", | ||
| password: "Kdagaal123", | ||
| port: 5432, |
There was a problem hiding this comment.
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
master