lansilvester / final-project-backend

final-project-backend-khaki.vercel.app
0 stars 0 forks source link

BE code review #1

Open hurricanenara opened 1 year ago

hurricanenara commented 1 year ago
  1. great job generating a random salt number! https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/controllers/auth.js#L26

  2. you could destructure the keys to avoid repeating dot indexing https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/controllers/auth.js#L26 https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/controllers/auth.js#L30-L34

  3. https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/controllers/auth.js#L65 checking user isn't necessary if you use optional chaining to early return undefined like user?.password

  4. there's a couple await keywords missing throughout this file! great work utilizing Joi -- you could also try sending error message from Joi validation itself https://github.com/lansilvester/final-project-backend/blob/main/src/controllers/destinations.js

  5. users could still submit empty strings so we can add .not("") to name, and email https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/helpers/validation_schema.js#L4-L5

  6. mongoose's api docs guide us to use singular naming for model names, would be a good improvement here (file name, model name) https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/models/users.js#L25

  7. since all of the endpoints start with /post we can set this router as middleware like app.user('/post', postRouter) after you import the router into the file https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/routes/destinations.js#L11-L24

  8. we could refactor this area to avoid repeated dot indexing into the fields in the authorization object https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/src/authMiddleware.js#L6-L11

  9. the .env file should never be pushed to GitHub -- please add this file to .gitignore

  10. way to go! this is some great code utilizing the error handling middleware 👍 https://github.com/lansilvester/final-project-backend/blob/552d72dfec846afcafb786fec7d928e87008b7c6/index.js#L32-L40

lansilvester commented 1 year ago

Thank you for the constructive review and positive feedback.