Review code 13/11

[tuananhl]

File name convention

Current file name convention is very confusing. Consist to file-name.type.js (except special file such as Dockerfile )

Validation

Should separate to a file validation in utils. -> utils/validations/login.validator.js
and then, import to service.

Code Quality

async function loginUser(email, password) {
  if (_.isUndefined(email)) {
    throw new UserInputError('email is required', {
      invalidArgs: 'email',
    });
  }
  if (_.isUndefined(password)) {
    throw new UserInputError('password is required', {
      invalidArgs: 'password',
    });
  }
  const isValidInput = loginValidation({ email, password });
  if (_.isArray(isValidInput)) {
    throw new UserInputError(isValidInput.map((it) => it.message).join(','), {
      invalidArgs: isValidInput.map((it) => it.field).join(','),
    });
  }

  const user = await getUserByEmail(email);
  if (!user) {
    throw new AuthenticationError('Invalid email or password');
  }

  const matchPassword = await comparePassword(password, user.password);
  if (!matchPassword) {
    throw new AuthenticationError('Invalid email or password');
  }

  const token = sign({ id: user.id });

  return { token };
}

if (_.isUndefined(email)) {
    throw new UserInputError('email is required', {
      invalidArgs: 'email',
    });
  }
  if (_.isUndefined(password)) {
    throw new UserInputError('password is required', {
      invalidArgs: 'password',
    });
  }

This code is unnecessary. because in schema is required by exclamation mark or in validator function.

const isValidInput = loginValidation({ email, password });
// isValidInput -> validateResult
if (_.isArray(isValidInput))
// validateResult return true or ValidationError[]. You only check validateResult.length > 0
// true .length = undefined > 0 -> false

if (_.isArray(isValidInput)) {
    throw new UserInputError(isValidInput.map((it) => it.message).join(','), {
      invalidArgs: isValidInput.map((it) => it.field).join(','),
    });
  }
// This is ValidationError. Not UserInputError

const token = sign({ id: user.id });
return { token };

sign is synchours -> js return { token: sign({ id: user.id }) }
should sign profile of user. except id

import pkg from 'apollo-server-express';
const { AuthenticationError, UserInputError } = pkg;

Fix import

js profileUser: (_, args, { user }) => getProfileUserById(user.id),
function name must be shorten. profile
context is contain all information of user. should only return user information. Not need more query on db.

I think authentication is only login or register.
and something like reset password, forgot password is features of user.
getUserLogin should move to context folder.

const users = ['users.name', 'users.email', 'users.id'];
  const userToken = [
    'user_token.token',
    'user_token.type',
    'user_token.id as token_id',
  ];
should export table name into a file

Query builder always return Promise.
-> mark function is async.
-> you can export all function in last line. Don't export in each function

export function removeUserToken(id) {
  return database(TABLE).where({ id }).delete();
}

export function createUser(...arg) {
  return database(TABLE).insert(...arg);
}

Should not spread that. you only put it as data or user