schmiedeone / vacabot Goto Github PK

Even though DEFAULT_VACATION_BALANCE has been created in consts.js, still in models/users.js we have hardcoded values for default vacation balance. Replace it with the defined const imported from consts.js.

Currently, we have so many instance and class methods, which makes new changes difficult.
If we can move out a few responsibilities as utility methods in the helper file, we can improve readability and adaptability.

Example:
user model have this method:

userSchema.methods.generateChannelId = function () {
  let self = this;
  return new Promise((resolve, reject) => {
    triggerSlack(C.CHANNEL_ID_URL, { users: self.userId }).then((res) => {
      if (res.channel) {
        self.channelId = res.channel.id;
        self.save();
        resolve(res.channel.id);
      } else {
        reject();
      }
    });
  });
};

An instance method shouldn't be responsible for making an API call.
This can be an external function which receives userId and then we can just call update user method with a new channelId.

You can join the slack community given in README.
Take a look at general channel.
Use the app yourself, take out screenshots for each feature.
Add those screenshots in docs, also add a new markdown file which shows all these screenshots.
Show a link to this markdown in README as "Demo" or something.

Expected behavior:
Throws error when committing code which has eslint errors.

You can do something like:
npx prettier --write "vacabot/**/*.js"

Right now every change in code needs a restart of the server.
For hot reloading, you can use any of the solutions available for NodeJS. Webpack suggested.

README is very cluttered right now. Only because of slack documentation with images.

Suggestion:

Create another documentation file in docs folder and move out Slack integration steps from README. And only add the link for the new document.

Right now the way we import files and modules in our code follows ES5 syntax.
We have the opportunity to refactor it and move to ES6.
You can look it up here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

reduceVacationBalance function in models/vacation.js allows vacation to go even in negative.
First step could be making 0 minimum.
Second step could be to reject vacation request when vacation reached below zero.

It doesn't have to be endless.
This repo is to promote new developers mostly. So we need some setup that enforces these rules and reduces the time in PR approvals.
We can have rules like tab size, auto indentations, formatting on save.

For reference: https://choosealicense.com/licenses/mit/

Please follow the contribution guidelines.