Code Improvements -02

[Nehal409]

Issue Description

1. In the ER diagram, the orders table contains dishId but lacks a relationship with the dishes table.
2. Several relationships between tables are missing:
   • One chef can create many dishes (using chefId as a foreign key in the dishes table).
   • One foodie can place multiple orders (using foodieId in the orders table).
   • A chef can process multiple orders (using chefId in the orders table).
3. Global error middleware is absent from the codebase.
4. Try-catch blocks are unnecessary in repository files; knex.js will handle errors.
5. The services folder is unused. Controllers should only handle incoming requests and responses. Services should contain business logic, such as calling repository functions, implementing conditions, and throwing errors using hapi/boom.
6. Remove try-catch blocks from repositories, services, and controllers. Let the error middleware handle errors. If specific errors need handling, throw them using @hapi/boom. Ensure express-async-errors middleware is added to server.js for asynchronous error handling.
7. Save userId instead of username in the request middleware.

   req.userId = userId;

8. Foodie ID or username retrieval can be done in the authenticationUser function instead of creating another function (setFoodieUsername).
9. Move payload validation to a utils function. Services should focus on business logic.
10. Include the Postman collection file within the codebase.
11. Create a README.md file providing an overview of the project and instructions for project setup.
12. Provide a .env.sample file containing demo environment variables, as they will be necessary if another developer needs to run your code.

[aitchkhan]

We need one login-service for both foodie and chefs. It should live in auth controller/service.

https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/foodie.js#L14C35-L14C42

[aitchkhan]

The service name should not end with service

https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/foodie.js#L10 It should just say createFoodie

[aitchkhan]

We don't need to fetch all the orders that are in our database.

https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/order.js#L4

[aitchkhan]

The order table should have a status field. which can contain 'PENDING', 'FULFILLED' etc

https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/migrations/20240402194720_foodie_table.js#L32

[aitchkhan]

We can simply return, no need to await https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/repositories/foodie.js#L10C5-L10C10

[aitchkhan]

The same can be used everywhere just return instead of await https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/repositories/foodie.js#L5-L6

[aitchkhan]

we don't need this line https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/chef.js#L22C1-L22C47

[mhammadalam112]

We don't need to fetch all the orders that are in our database.

https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/order.js#L4

Please confirm how should we display the orders to the chef? As per my understanding in the task sheet it was written to list Orders, so I have showed all the orders. Should i just filter it by 'pending' or any more condition?

[mhammadalam112]

We can simply return, no need to await https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/repositories/foodie.js#L10C5-L10C10

Is this because we are already using await in the service function? Because as per my understanding we should use await with database functions as it may take time to return the results sometimes. Please enlighten

[mhammadalam112]

we don't need this line https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/chef.js#L22C1-L22C47

then how should we extract the authorization token?

[Nehal409]

We don't need to fetch all the orders that are in our database. https://github.com/mhammadalam112/EhsaanTechMernMonth02/blob/d12d2a79eb332a4ad648b0753fb2d2df118ace58/src/services/order.js#L4

Please confirm how should we display the orders to the chef? As per my understanding in the task sheet it was written to list Orders, so I have showed all the orders. Should i just filter it by 'pending' or any more condition?

@mhammadalam112 To display chef orders, you must apply a join query between chef, dishes, and orders table because there is no direct relation between orders and chef's table. There is no need to display all the orders in the databse

[Nehal409]

Is this because we are already using await in the ser

@mhammadalam112 Yes, since the service function already handles waiting for asynchronous operations, there's no need to use "await" again in the repository.

[Nehal409]

then how should we extract the authorization token?

@mhammadalam112 On login, you would return the token as part of the response object. On the frontend, you can then retrieve the token from the response and add it to the request headers for later requests. In postman, you'll manually include the Authorization header for secured requests.