Implement Coffee Chat Dao #598
Conversation
|
[diff-counting] Significant lines: 115. |
|
|
||
| if (prevChats.some((c) => c.members.includes(member1) && c.members.includes(member2))) { | ||
| throw new Error( | ||
| `Cannot create coffee chat with member. Previous coffee chats from previous semesters exist.` |
There was a problem hiding this comment.
There should probably also be a check to make sure a member cannot coffee chat themselves. There doesn't seem to be anything preventing this right now.
There was a problem hiding this comment.
Good catch, let's put this and the other validation logic in the API layer, not Dao.
…into set-up-coffee-chat-dao
| } | ||
|
|
||
| /** | ||
| * Deletes all coffee chat for all users |
| const [member1, member2] = coffeeChat.members; | ||
|
|
||
| if (member1.netid === member2.netid) { | ||
| throw new Error(`Cannot create coffee chat with yourself.`); |
There was a problem hiding this comment.
nit: no need for backticks if the string is not a template string
There was a problem hiding this comment.
interesting that eslint doesn't complain about this... 🤔
| const prevChats2 = await this.getCoffeeChatsByUser(member2); | ||
| const prevChats = [...prevChats1, ...prevChats2]; | ||
|
|
||
| if (prevChats.some((c) => c.members.includes(member1) && c.members.includes(member2))) { |
There was a problem hiding this comment.
I would be very careful about this check. includes(...) uses object equality and we've had a bug introduced in candidate decider in the past from this. Consider instead checking against netIDs or emails.
There was a problem hiding this comment.
Alternatively, we use lodash's isequal which checks for deep equality for the profile update notification job. That can be used here.
Summary
This PR implements the creates the functions that are used to set up coffee chat DAO.
Some things to keep in mind:
Notion/Figma Link
https://www.notion.so/Dev-Roadmap-741d645c627a466da392fc8038ef0143?p=9719280ab9634834bba9d51a94ddeab8&pm=s