HackYourFuture-CPH / simply-name.it

Final Project for Class17
MIT License
3 stars 1 forks source link

Backend: fix addMember function in users.controller #247

Closed shpomp closed 3 years ago

shpomp commented 3 years ago

Currently, the function adds only members with role 'basic', which is hardcoded.

need to refactor it and introduce the role parameter, so it could be passed to the endpoint.

shpomp commented 3 years ago

@senner007

I was about to do this, as we talked in #243 :

const addMember = async (userId, boardId, memberId, role) => {
  if (
    !Number.isInteger(Number(userId)) ||
    !Number.isInteger(Number(boardId)) ||
    !Number.isInteger(Number(memberId))
  ) {
    throw new InvalidIdError(‘Id should be an integer’);
  }
  if (role !== ‘basic’ || role !== ‘owner’) {
    throw new IncorrectEntryError(Role should be either 'basic' or 'owner');
  }
  await userIsOwner(userId, boardId);
  const addNewMember = await knex(‘members’).insert({
    boardId,
    userId: memberId,
    role: role,
  });
  return addNewMember;
};

But then realized I am struggling to change the router and swagger - how do I use the role? I cannot have it as path param, as it is string, and having the ids as path params + role as body did not work either.

image

I am looking at Diny's suggestion again and liking it more:

if(userId===memberId){
    const addNewMember = await knex('members').insert({
      boardId,
      userId: memberId,
      role: 'owner',
    });
    return addNewMember;
  }
  else{
    const addNewMember = await knex('members').insert({
    boardId,
    userId: memberId,
    role: 'basic',
  });
  return addNewMember;
}

or maybe I am utterly confused 😵

senner007 commented 3 years ago

I am looking at Diny's suggestion again and liking it more:

if(userId===memberId){
    const addNewMember = await knex('members').insert({
      boardId,
      userId: memberId,
      role: 'owner',
    });
    return addNewMember;
  }
  else{
    const addNewMember = await knex('members').insert({
    boardId,
    userId: memberId,
    role: 'basic',
  });
  return addNewMember;
}

or maybe I am utterly confused 😵

The question as stated earlier is to decide of the owner has to be added automatically when the board is created, as I suggested, or not. Secondly, the above indicates the later but you should get rid of the duplication here. If you want to send the role from the frontend in the body, then it would go to the req.body

shpomp commented 3 years ago

Somehow swagger did not like to have path params and body at all.... 😩 Let's say the owner should be added automatically, I will make the above, removing duplication. Thank you!