Allow Inviting Users to Course by Email #7655
Conversation
bivanalhar
force-pushed
the
bivan/course-invitation-email-copy
branch
from
5d70495
to
eff71f6
Compare
bivanalhar
force-pushed
the
bivan/course-invitation-email-copy
branch
from
eff71f6
to
494dc57
Compare
| export const splitEntries = (input: string): string[] => { | ||
| const entries: string[] = []; | ||
| let currentEntry = ''; | ||
| let inQuotes = false; | ||
| for (let i = 0; i < input.length; i++) { | ||
| const char = input[i]; | ||
| if (char === '"') { | ||
| inQuotes = !inQuotes; | ||
| currentEntry += char; | ||
| } else if ((char === ',' || char === ';') && !inQuotes) { | ||
| if (currentEntry.trim()) { | ||
| entries.push(currentEntry.trim()); | ||
| } | ||
| currentEntry = ''; | ||
| } else { | ||
| currentEntry += char; | ||
| } | ||
| } | ||
| if (currentEntry.trim()) { | ||
| entries.push(currentEntry.trim()); | ||
| } | ||
| return entries; | ||
| }; |
There was a problem hiding this comment.
Split using regex instead of implementing an algorithm here instead.
difference is 2 lines vs 22 lines.
| export const parseEmails = ( | ||
| input: string, | ||
| ): { results: InvitationEntry[]; errors: string[] } => { | ||
| const regex = /^(?:"([^"]+)"|([^"<]+))?\s*<([^>]+)>$|^([^"<\s]+@[^\s,;<>]+)$/; |
There was a problem hiding this comment.
Update variable name to formattedEmailRegex? regex is not meaningful
| const match = regex.exec(entry); | ||
| if (match) { | ||
| if (match[3]) { | ||
| const name = (match[1] || match[2] || '').trim(); |
There was a problem hiding this comment.
Your regex should be written to exclude the surrounding whitespaces, so you won't need to use .trim() here.
| if (match[3]) { | ||
| const name = (match[1] || match[2] || '').trim(); |
There was a problem hiding this comment.
Can we simplify the regex? Why so many cases?
| }); | ||
| }; | ||
|
|
||
| const parsedEmail = parseEmails(nameEmailInput); |
| name: entry.name, | ||
| email: entry.email, | ||
| role: lastRow.role, | ||
| phantom: lastRow.phantom, | ||
| ...(permissions.canManagePersonalTimes && { | ||
| timelineAlgorithm: lastRow.timelineAlgorithm, |
There was a problem hiding this comment.
Code duplication here. You should make use of appendNewRow and extend it by allowing name and email arguments (default '').
| variant="filled" | ||
| /> | ||
| <Button | ||
| className="w-1/8 h-[3.5rem]" |
There was a problem hiding this comment.
Nit: avoid hardcoding [3.5rem] if possible, can we use a default value?
| parsedEmail.errors.length > 0 | ||
| ? parsedEmail.errors.join('; ') | ||
| : '', |
There was a problem hiding this comment.
If the length is 0 then parsedEmail.errors.join('; ') will still be an empty string, what's the point of this ternary if-else then?
Feature
Inside Manage Users > Invite Users, we have an additional feature in which user can copy-paste list of names and emails, and then the invitation lists will be updated automatically to include all the users enlisted within the list.
Should the format has an error in some of the users, then we updated the list to include those correctly-formatted users and inform user about the errors, so that the inviter can modify the name / email accordingly