-
Notifications
You must be signed in to change notification settings - Fork 49
Project 3 - Ecommerce App (Marina and Ian) #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5e26ebe
759432d
7b35d57
14bf474
48cf220
66d2011
1d3f637
10b34fa
a068eae
0ae3389
f8090d7
e2aca4c
d6b81ef
52aadcc
32f6532
1e27414
ad67fec
77afefd
84f4f6f
706452b
1a29f97
9feda20
3df5eb4
70032d2
c849447
829db05
82cfbeb
72aab8b
6e8fb55
b0d7a8c
da1c7aa
5148272
62c8bd2
8d49fcf
0e106f7
64ae8da
0365378
ff1eca5
003e267
36e78b5
12c4cb0
0dcaed9
e99ec4f
e08d950
ace4d57
783eda0
5af380d
bfb2464
28863a0
141e1a2
8159684
b3b6060
99c6649
a5850a7
0dd8a16
b3c3a31
4b118e1
49562c8
0cad869
ceb69b1
18e5d17
77b6eba
0dd4f8c
4c5becc
7f39c99
9197d3d
e8b0437
31a801f
948dc9b
5d2efe7
bdf03ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,3 @@ | ||
| node_modules/ | ||
| node_modules/ | ||
|
|
||
| .env | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| const path = require('path'); | ||
|
|
||
| module.exports = { | ||
| config: path.resolve('config', 'database.js'), | ||
| 'models-path': path.resolve('db', 'models'), | ||
| 'seeders-path': path.resolve('db', 'seeders'), | ||
| 'migrations-path': path.resolve('db', 'migrations'), | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| require('dotenv').config(); | ||
|
|
||
| module.exports = { | ||
| development: { | ||
| username: process.env.DB_USERNAME, | ||
| password: process.env.DB_PASSWORD, | ||
| database: process.env.DB_NAME, | ||
| host: process.env.DB_HOST, | ||
| dialect: process.env.DB_DIALECT, | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| const BaseController = require('./baseController'); | ||
| const { QueryTypes } = require('sequelize'); // Import DataTypes and QueryTypes | ||
|
|
||
| class AddressesController extends BaseController { | ||
| constructor(model, userModel) { | ||
| super(model); | ||
| this.userModel = userModel; | ||
| } | ||
|
|
||
| async getAllAddresses(req, res) { | ||
| try { | ||
| const address = await this.model.sequelize.query( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you use a raw query here? |
||
| `SELECT "id", "buyer_id", "address", "created_at" AS "createdAt", "updated_at" AS "updatedAt", "buyer_id" AS "user_id" FROM "addresses"`, | ||
| { | ||
| type: this.model.sequelize.QueryTypes.SELECT, | ||
| }, | ||
| ); | ||
|
|
||
| return res.json(address); // Assuming you expect only one address | ||
| } catch (err) { | ||
| console.error(err); | ||
| return res.status(400).send(err); | ||
| } | ||
| } | ||
|
|
||
| // Get a single address | ||
| async getAddress(req, res) { | ||
| const { addressId } = req.params; | ||
| try { | ||
| const address = await this.model.sequelize.query( | ||
| `SELECT "id", "buyer_id", "address", "created_at" AS "createdAt", "updated_at" AS "updatedAt", "buyer_id" AS "user_id" FROM "addresses" WHERE "id" = :addressId`, | ||
| { | ||
| replacements: { addressId }, | ||
| type: this.model.sequelize.QueryTypes.SELECT, | ||
| }, | ||
| ); | ||
| return res.json(address[0]); // Assuming you expect only one address | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use |
||
| } catch (err) { | ||
| console.error(err); | ||
| return res.status(400).send(err); | ||
| } | ||
| } | ||
|
|
||
| async getAddressIdBasedOnActualAddress(req, res) { | ||
| const { delivery_address } = req.query; | ||
|
|
||
| try { | ||
| const address = await this.model.findOne({ | ||
| where: { | ||
| address: delivery_address, | ||
| }, | ||
| }); | ||
|
|
||
| return res.status(200).json(address.id); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if no address is found? You would probably try to access id on undefined, which causes a server error. You might catch the error, but then respond with 400 (bad request), which is not the appropriate response for the issue at hand. |
||
| } catch (err) { | ||
| console.error(err); | ||
| return res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
|
|
||
| async postNewAddress(req, res) { | ||
| const { email, address } = req.body; | ||
|
|
||
| const allAddresses = await this.model.sequelize.query( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use findOne above, but then go back to raw queries? Why use sequelize, if we execute raw queries only? :) |
||
| `SELECT "id", "buyer_id", "address", "created_at" AS "createdAt", "updated_at" AS "updatedAt", "buyer_id" AS "user_id" FROM "addresses"`, | ||
| { | ||
| type: this.model.sequelize.QueryTypes.SELECT, | ||
| }, | ||
| ); | ||
|
|
||
| const arrayOfAddresses = allAddresses.map( | ||
| (singleAddress) => singleAddress.address, | ||
| ); | ||
|
|
||
| if (!arrayOfAddresses.includes(address)) { | ||
| try { | ||
| const [buyer] = await this.userModel.findOrCreate({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't create a user on the addresses controller. You should reconsider the scope of your function |
||
| where: { email: email }, | ||
| }); | ||
|
|
||
| const queryResult = await this.model.sequelize.query( | ||
| 'INSERT INTO "addresses" ("buyer_id", "address", "created_at", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id", "buyer_id", "address", "created_at", "updated_at"', | ||
| { | ||
| bind: [buyer.id, address, new Date(), new Date()], | ||
| type: QueryTypes.INSERT, | ||
| }, | ||
| ); | ||
|
|
||
| const newAddress = queryResult[0]; | ||
|
|
||
| return res.json(newAddress); | ||
| } catch (err) { | ||
| console.error(err); | ||
| return res.status(400).json({ error: true, msg: err.message }); | ||
| } | ||
| } else { | ||
| return res.send('Address already exists!'); | ||
| } | ||
| } | ||
|
|
||
| async deleteOne(req, res) { | ||
| const { addressId } = req.params; | ||
|
|
||
| try { | ||
| await this.model.destroy({ | ||
| where: { | ||
| id: addressId, | ||
| }, | ||
| }); | ||
|
|
||
| res.status(200).send(`Successfully deleted address at id ${addressId}`); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where is the confirmation that it happened successfully? All I see is you trying to destroy it, but there is no confirmation anywhere. |
||
| } catch (error) { | ||
| console.error(error); | ||
| res.status(400).send({ error: true, msg: error }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = AddressesController; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| class BaseController { | ||
| constructor(model) { | ||
| this.model = model; | ||
| } | ||
|
|
||
| async getAll(req, res) { | ||
| try { | ||
| const output = await this.model.findAll(); | ||
| return res.json(output); | ||
| } catch (err) { | ||
| return res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = BaseController; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| const BaseController = require('./baseController'); | ||
|
|
||
| class CategoriesController extends BaseController { | ||
| constructor(model, productModel) { | ||
| super(model); | ||
| this.productModel = productModel; | ||
| } | ||
|
|
||
| async getOne(req, res) { | ||
| const { categoryId } = req.params; | ||
|
|
||
| try { | ||
| const category = await this.model.findByPk(categoryId); | ||
|
|
||
| res.send(category); | ||
| } catch (err) { | ||
| res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
|
|
||
| async postOne(req, res) { | ||
| const { name } = req.body; | ||
|
|
||
| try { | ||
| const newCategory = await this.model.create({ | ||
| name: name, | ||
| }); | ||
|
|
||
| res.send(newCategory); | ||
| } catch (err) { | ||
| res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
|
|
||
| async updateOne(req, res) { | ||
| const { categoryId } = req.params; | ||
|
|
||
| const { name } = req.body; | ||
|
Comment on lines
+36
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. generally I do not see you validate any parameters. We should check if these parameters and body values even exist. And then also if they are of the correct data type. |
||
|
|
||
| try { | ||
| const categoryToBeUpdated = await this.model.findByPk(categoryId); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. basically can check here if category exists. If no, respond to the client |
||
| const updatedCategory = await categoryToBeUpdated.update({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you couldn't find a category? Then this would cause your server to crash as you try to call |
||
| name: name, | ||
| }); | ||
|
|
||
| res.send(updatedCategory); | ||
| } catch (err) { | ||
| return res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
|
|
||
| async deleteOne(req, res) { | ||
| const { categoryId } = req.params; | ||
|
|
||
| try { | ||
| await this.model.destroy({ | ||
| where: { | ||
| id: categoryId, | ||
| }, | ||
| }); | ||
|
|
||
| res.send('Category has been deleted!'); | ||
| } catch (err) { | ||
| res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = CategoriesController; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| const BaseController = require('./baseController'); | ||
| const Sequelize = require('sequelize'); | ||
| const sequelize = new Sequelize( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you initialize a DB in your controller? This should not happen here |
||
| `postgres://${process.env.DB_USERNAME}:@${process.env.DB_HOST}:5432/${process.env.DB_NAME}`, | ||
| ); | ||
|
|
||
| class OrdersController extends BaseController { | ||
| constructor(model, userModel, productModel, orderProductModel, addressModel) { | ||
| super(model); | ||
| this.userModel = userModel; | ||
| this.productModel = productModel; | ||
| this.orderProductModel = orderProductModel; | ||
| this.addressModel = addressModel; | ||
| } | ||
|
|
||
| async getAllOrdersOfCurrUser(req, res) { | ||
| const { email } = req.query; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We definitely should validate this. Also, we should use authentication properly. I could probably query your endpoint here with any email I can find, and see if that user has any orders, what he ordered, get his address and his user id. With the user id I could then freely make requests against your user routes. |
||
| try { | ||
| const user = await this.userModel.findOrCreate({ | ||
| where: { email: email }, | ||
| }); | ||
| const user_id = user[0].dataValues.id; | ||
| const orders = await this.model.findAll({ | ||
| where: { user_id: user_id }, | ||
| include: [ | ||
| { | ||
| model: this.productModel, | ||
| attributes: ['id', 'title', 'price', 'img'], | ||
| through: { | ||
| model: this.orderProductModel, | ||
| attributes: ['quantity'], | ||
| }, | ||
| }, | ||
| { model: this.addressModel, attributes: ['address'] }, | ||
| ], | ||
| }); | ||
| return res.json(orders); | ||
| } catch (err) { | ||
| return res.status(400).json({ error: true, msg: err.message }); | ||
| } | ||
| } | ||
|
|
||
| async postOne(req, res) { | ||
| const { address_id, user_id, total_price, products } = req.body; | ||
|
|
||
| if (!products || products.length === 0) { | ||
| return res | ||
| .status(400) | ||
| .json({ error: true, msg: 'No products in the order' }); | ||
| } | ||
|
|
||
| try { | ||
| await sequelize.transaction(async (t) => { | ||
| const newOrder = await this.model.create( | ||
| { | ||
| address_id: address_id, | ||
| user_id: user_id, | ||
| total_price: total_price, | ||
| }, | ||
| { transaction: t }, | ||
| ); | ||
|
|
||
| const arrayOfJunctionTableEntries = []; | ||
| for (let i = 0; i < products.length; i++) { | ||
| const productObj = products[i]; | ||
| const [id, quantity] = Object.entries(productObj)[0]; | ||
| const newEntryInOrderProducts = await this.orderProductModel.create( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this you are creating n amount of entries in your DB in a loop. By awaiting each item, this can take a long time potentially. You should rather try to do such operations asynchronously via Promise.all or use a bulk insert even better. Operations like here will highly increase your latency on big orders |
||
| { | ||
| order_id: newOrder.dataValues.id, | ||
| product_id: id, | ||
| quantity: quantity, | ||
| }, | ||
| { transaction: t }, | ||
| ); | ||
| arrayOfJunctionTableEntries.push(newEntryInOrderProducts); | ||
| } | ||
|
|
||
| const purchasedProducts = []; | ||
|
|
||
| for (let i = 0; i < products.length; i++) { | ||
| const productObj = products[i]; | ||
| const [id, quantity] = Object.entries(productObj)[0]; | ||
|
|
||
| const product = await this.productModel.findByPk(id); | ||
|
|
||
| const stock = product.dataValues.stock_left; | ||
|
|
||
| const stock_left = stock - quantity; | ||
|
|
||
| const updatedProduct = await product.update( | ||
| { stock_left: stock_left }, | ||
| { transaction: t }, | ||
| ); | ||
|
Comment on lines
+84
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you loop again and make 2 separate database requests. That means you will make 2*n requests here, which you both await. This is really inefficient and you should think of an alternative way to handle this. Most likely your database design could be improved in order to facilitate this. |
||
|
|
||
| purchasedProducts.push(updatedProduct); | ||
| } | ||
|
|
||
| return res.send([ | ||
| newOrder, | ||
| arrayOfJunctionTableEntries, | ||
| purchasedProducts, | ||
| ]); | ||
| }); | ||
| } catch (err) { | ||
| console.error(err.message); | ||
| return res.status(400).json({ error: true, msg: err }); | ||
| } | ||
| } | ||
|
|
||
| async getOne(req, res) { | ||
| const { orderId } = req.params; | ||
|
|
||
| const order = await this.model.findByPk(orderId, { | ||
| include: [ | ||
| { | ||
| association: 'products', | ||
| }, | ||
| { model: this.addressModel, attributes: ['address'] }, | ||
| ], | ||
| }); | ||
|
|
||
| return res.send(order); | ||
| } | ||
|
|
||
| async deleteOne(req, res) { | ||
| const { orderId } = req.params; | ||
|
|
||
| try { | ||
| await this.model.destroy({ | ||
| where: { | ||
| id: orderId, | ||
| }, | ||
| }); | ||
|
|
||
| res.status(200).send('Success:'); | ||
| } catch (error) { | ||
| console.error(error); | ||
| res.status(400).send({ error: true, msg: error }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = OrdersController; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no readme