-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated email invitation logic to include sender details in the From … #8858
Updated email invitation logic to include sender details in the From … #8858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR updates the email invitation system to include sender details in both the email content and "From" field, enhancing personalization and clarity of workspace invitations.
- Fixed sender name format in
workspace-invitation.service.ts
to show as "FirstName LastName (via Twenty)" in email "From" field - Added
mailto:
link for sender's email insend-invite-link.email.tsx
for better interactivity - Removed unnecessary email parameter from invitation link URL in
workspace-invitation.service.ts
- Contains syntax error in
send-invite-link.email.tsx
with extra>
character that needs fixing - Old email sending code remains commented out instead of being removed in
workspace-invitation.service.ts
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
2 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
{capitalize(sender.firstName)} ( | ||
<Link | ||
href={sender.email} | ||
<b>{senderFullName}</b>> ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Extra '>' character after senderFullName appears to be a typo
export const SendInviteLinkEmail = ({ | ||
link, | ||
workspace, | ||
sender, | ||
serverUrl, | ||
}: SendInviteLinkEmailProps) => { | ||
const workspaceLogo = getImageAbsoluteURI(workspace.logo, serverUrl); | ||
const senderFullName = `${capitalize(sender.firstName)} (via Twenty)`; //restructuring and adding via Twenty (per requirement) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Inline comment about requirement is not needed in production code
@@ -22,20 +22,23 @@ type SendInviteLinkEmailProps = { | |||
serverUrl?: string; | |||
}; | |||
|
|||
|
|||
//work from here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 'work from here' comment should be removed
// await this.emailService.send({ | ||
// from: `${this.environmentService.get( | ||
// 'EMAIL_FROM_NAME', | ||
// )} <${this.environmentService.get('EMAIL_FROM_ADDRESS')}>`, | ||
// to: invitation.value.email, | ||
// subject: 'Join your team on Twenty', | ||
// text, | ||
// html, | ||
// }); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: remove commented code instead of leaving it in place
from: `${this.environmentService.get( | ||
'EMAIL_FROM_NAME', | ||
)} <${this.environmentService.get('EMAIL_FROM_ADDRESS')}>`, | ||
from: `${sender.firstName} ${sender.lastName} (via Twenty) <${this.environmentService.get('EMAIL_FROM_ADDRESS')}>`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: potential email client issues if sender name contains special characters - consider sanitizing firstName/lastName
2020a68
to
2dd8852
Compare
return ( | ||
<BaseEmail width={333}> | ||
<Title value="Join your team on Twenty" /> | ||
<MainText> | ||
{capitalize(sender.firstName)} ( | ||
<Link | ||
href={sender.email} | ||
href={`mailto:${sender.email}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a few changes (don't leave unnecessary comments in code). Thanks!
Updated email invitation logic to include sender details in the From field.
Please feel free to provide comments so that we can make adjustments as early as possible if needed
Fixes #7001