the creator of a meeting is now shown as required#4235
Conversation
staysgt
left a comment
staysgt
left a comment
There was a problem hiding this comment.
Looks good so far! Just a few small comments
| // Returns whether the event creator is part of the list of required members | ||
| const creatorInRequiredMembers = () => { | ||
| for (const member of requiredMemberIds) { | ||
| if (member === submitter.userId) return true; | ||
| } | ||
| return false; | ||
| }; | ||
|
|
There was a problem hiding this comment.
This helper can be reduced to requiredMemberIds.includes(submitter.userId), so you can get rid of the function and replace its call with that line
There was a problem hiding this comment.
Actually, instead of recomputing multiple times whether the creator is in the required members, it would be cleaner to just set a const a the top of the function called allRequiredMembers that has the set of required members + creator, and then use that throughout
| // Returns whether the event creator is part of the list of required members | ||
| const creatorInRequiredMembers = () => { | ||
| for (const member of requiredMemberIds) { | ||
| if (member === submitter.userId) return true; | ||
| } | ||
| return false; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Just like above, can also be replaced with requiredMemberIds.includes(submitter.userId)
There was a problem hiding this comment.
The const allRequiredMembers should also be done in here as well
2 participants
Changes
Notes
Test Cases
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes #4233