icebob / vue-express-mongo-boilerplate

:star: MEVN Full stack JS web app boilerplate with NodeJS, Express, Mongo and VueJS
http://vemapp.moleculer.services/
2.85k stars 756 forks source link

Initial Suggestions and Questions from a potential contributor #42

Closed swyxio closed 7 years ago

swyxio commented 7 years ago

hey @icebob, big fan. spend a couple hours yesterday playing around with this and I think the default functionality here is really impressive!

  1. default frontend is not mobile friendly. this is a problem in hackathons. this requires a larger fix and im not a good designer but i just wanted to point out how important it is.
  2. you ship with email verification enabled by default. but i have had a lot of trouble trying to deploy this on heroku with smtp mailtrap. this problem is probably due to my lack of knowledge of nodemailer, but meanwhile i would actually suggest shipping with email verification off since its not a critical feature for anyone.
  3. i have never contributed to opensource before but i would like to start. where is the best place to start? i see that notifications are not implemented yet according to your todo list and the live app. is that a good place to start?
icebob commented 7 years ago

Hi @sw-yx, I'm glad you like my work.

  1. good point. And yes, mobile friendly layout is important. In the future I would like to fix it but I think it has lower priority than some other tasks. (But I add it to todo list)
  2. Good idea. The default value can be false for verification.
  3. I'm glad you would like to contribute on my project but unfortunately the time is not too good. Currently I'm rewriting the whole backend side to my new microservices framework. You can check it in ice-services branch.

As first-time contributor maybe create a PR with the new verification defaults (point 2). After it, check the ice-services branch. If you like & understand the new service-based logic, talk again about next step. OK?

swyxio commented 7 years ago

ok! I'm not "advanced" enough yet to understand the need for a microservices framework (i can grok basic event bus systems) but I definitely understand the rest of the boilerplate so I can still help with those. it looks like you are VERY sophisticated in your requirements for moleculer so I hope to be in a position to understand it in future.

I switched the verification off but the app still fails on new user creation because it still wants to send a welcome email. Would you be ok if I just disabled all emailing by default? So basically how I would do it is, I would implement a check on the config.js file to see if mail is configured, and if yes, turn on email verification and turn on email sending, else turn it off. which is basically exactly how you do the social auth.

one more thing to check. i am a bit confused around how you prefer to store confidential information. your mongo URI/username/password is stored in environment variables. I am very familiar with doing this for deployment to heroku as it means i dont have to maintain separate branches for deploy and for github commit. however, all your other confidential info like social keys and mail server settings is in config.js, mixed in with other less confidential config options. Is this the "industry standard" way of doing this? (i am still learning but it seems confusing to have mixed sources of confidential info.) Have you looked into using the dotenv module to locally store environment variables?

sorry for the long questions, i hope this will help lay the ground for me to contribute smoothly.

swyxio commented 7 years ago

regarding "disabling all emailing", the goal would be "progressive deployment" so when the hackathoner first deploys the app they would not have to configure the mail option and the app "progresses" gracefully from no mail configuration to valid mail configuration.

icebob commented 7 years ago

OK. In this case my suggestion is we need to add an enabled: false to mailer in config/base.js. And every place where the app want to send email check this property. This value is false defaultly, so if you install the app, it won't try to send email.

Mongo credential comes from env or config.js. I use this form because Heroku, Openshift..etc sets the mongo credentials to environment. The social keys are different because these will be configured by only developer. And there is a config.js file in the root of repo, which is ignored in gitignore. In development you can use your dev configurations. In prod, you need to create a new config.js and add only this file to your container. I think this solution similar as dotenv (except it won't load every settings from environment, but you can set process.env.XY values to all properites in config.js if you want.

P.S.: I'm working hard on Moleculer and I think it will be a very good framework for NodeJS if it will be ready for production. Currently I also collect experience while rewrite this boilerplate to Moleculer framework.

swyxio commented 7 years ago

yay. I will do this this weekend.