search

sarthakm21 / WorkTrack

A nodejs, express and mongodb based productivity tracker application.
https://work-track.herokuapp.com/
6 stars 19 forks source link

Use of MVC Architecture #14

Closed Elemento24 closed 4 years ago

Elemento24 commented 4 years ago

As of now, the app has all the controller functions inside of Routes. Just like any other scalable Node App, I think we should follow the MVC Architecture. So we should make a controllers folder, and move all the controller functions inside that folder, inside separate files, and then link them to the respective routes files. Also, we can use Nodemon, for automatic restarting of the server. Can I please work on this Issue?

sarthakm21 commented 4 years ago

So for Nodemon, you will be adding a script to package.json? I'm slightly confused about that part....

Elemento24 commented 4 years ago

Yes, just install nodemon and change the start script. That's it!

sarthakm21 commented 4 years ago

Sounds good! Go Ahead. And make a separate branch for this please because it's a significant change and may cause some errors

Elemento24 commented 4 years ago

Hey @sarthakm21, I am a bit confused about the branching. Are you saying that I should create a new Branch and test this first, and merge the branch then, and then create a PR, or should I create a new branch, and make a PR, and then you will merge it? Also, can I add the node_modules inside the .gitignore file? I will change the Readme accordingly! Cause otherwise, installing a package is going to create a lot of Commits, which is unreal!

sarthakm21 commented 4 years ago

You create a new branch, then make a PR and I'll merge it. And about the node modules, I forgot to add that into .gitignore, but now, since the folder is already present in the repo, adding it into .gitignore won't change it. I guess i'll have to do that manually :(

Elemento24 commented 4 years ago

I have added that in gitignore, cause otherwise, there will be a huge number of commits, you can remove the node_modules folder from the repo! Also, I can see that there is no logged in check for the delete route. Should I add the middleware, will make sure only the logged in users can delete the posts!

Elemento24 commented 4 years ago

Hey @sarthakm21, I have done all the aforementioned changes, and made a PR. Please review all the changes. I have checked all the routes, except the Inspire Me Routes. I wasn't able to find them anywhere, linked to the UI. You can check them using Postman though!