feat(email): add all email templates #352
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request updates the hackathon email template and its related components. It enhances the email content by adding multiple events with an array of buttons, updates function signatures to include explicit TypeScript typings, and refactors component layouts. The schema for emails in the database has been expanded with new optional fields (second button details and events) and the text field type changed to a textarea. Additionally, the Firefox package is removed from the development shell configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client
participant P as Preview
participant G as Generic Renderer
participant C as Content Renderer
participant F as Footer Renderer
User->>P: Request email preview
P->>G: Generate email content
G->>C: Render events with button arrays
C->>C: Iterate over event buttons
G->>F: Append footer layout
F-->>G: Return structured footer
G-->>P: Assemble complete email
P-->>User: Deliver email preview
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Progress: resolved 0, reused 0, downloaded 1, added 0 This error happened while installing a direct dependency of /tmp/eslint 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (9)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
HasithDeAlwis
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
31a4096 to
5d5013d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/email/templates/cuhacking/registered-for-hackathon.tsx(4 hunks)infra/shell.nix(1 hunks)libs/db/collections/models/Emails.ts(5 hunks)libs/db/payload-types.ts(1 hunks)libs/email/components/banner.tsx(1 hunks)libs/email/components/content.tsx(2 hunks)libs/email/components/footer.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`infra/**`: "- Validate infrastructure security best practic...
infra/**: "- Validate infrastructure security best practices
- Ensure proper error handling in non-generated code
- Verify deployment configuration parameters"
infra/shell.nix
🪛 Gitleaks (8.21.2)
libs/email/components/footer.tsx
30-30: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
45-45: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
60-60: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
75-75: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
96-96: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
145-145: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
158-158: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
171-171: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
184-184: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
195-195: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (8)
infra/shell.nix (1)
25-25: Commented out Firefox from development environmentThe Firefox package has been removed from the development environment. This reduces the environment size without impacting functionality since Firefox isn't required for building or serving the email templates.
libs/db/payload-types.ts (1)
368-375: Email interface extension looks goodThe Email interface has been expanded to include optional properties for a second button and events array. These changes are backward compatible since all new fields are optional.
libs/email/components/content.tsx (2)
5-5: Initialization of events array is correctThe defensive coding with fallback to an empty array prevents null reference errors when
body.eventsis undefined.
17-19: Consider accessibility implications of using pre tagReplacing the Text component with a
<pre>tag changes how content is rendered, preserving whitespace and using monospace font. This might affect readability in some email clients.Can you verify this renders as expected across different email clients? Also, please ensure content is formatted appropriately in the source text since
<pre>preserves all whitespace.libs/email/components/footer.tsx (1)
9-127: Implementation of table-based layout enhances email client compatibilityThe switch from div-based to table-based layout is a good practice for email templates, as it provides better cross-client compatibility.
🧰 Tools
🪛 Gitleaks (8.21.2)
30-30: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
45-45: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
60-60: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
75-75: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
96-96: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
apps/email/templates/cuhacking/registered-for-hackathon.tsx (1)
101-101: Improved type safety with parameter typesGood addition of TypeScript type annotations for the function parameters.
libs/db/collections/models/Emails.ts (2)
41-41: Improved field type for multiline contentChanging from 'text' to 'textarea' is appropriate for the expanded email content.
108-111: Handle new fields in seed functionGood defensive programming by providing fallbacks for optional fields.
| @@ -9,8 +9,7 @@ export function Banner() { | |||
| <Tailwind> | |||
| <Body className="m-0 p-0 bg-black"> | |||
| <Container | |||
| // className="w-full max-w-[600px] h-[192px] bg-[url('/static/bg-keycap-2.webp')] bg-cover bg-center text-white font-mono text-center" | |||
| className="w-full max-w-[600px] h-[192px] bg-[url('https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/cuhacking/media/bg-keycap-2.webp?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJjdWhhY2tpbmcvbWVkaWEvYmcta2V5Y2FwLTIud2VicCIsImlhdCI6MTc0MDkxMjI5MCwiZXhwIjoxNzcyNDQ4MjkwfQ.S1oQiR--_NtIsXl5NPF_6oFY2VK-SIMO36LT7N5WwmM')] bg-cover bg-center text-white font-mono text-center" | |||
| className="w-full max-w-[600px] h-[192px] bg-[url('https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_email_header_bg.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfZW1haWxfaGVhZGVyX2JnLnBuZyIsImlhdCI6MTc0MTYzODczMCwiZXhwIjoxNzQ0MjMwNzMwfQ.ejQMJXMlUkiRgd5lJRYipzeEiKCWh63_r1BPNwTgZIQ')] bg-cover bg-center text-white font-mono text-center" | |||
There was a problem hiding this comment.
Security concern with hardcoded token in background image URL
The background image URL contains a hardcoded JWT token with an expiration date (February 2025). This presents two issues:
- The image will stop working when the token expires
- Having authentication tokens in client-side code is a security risk
Consider using environment variables and a server-side approach to generate signed URLs dynamically at runtime. For example:
- className="w-full max-w-[600px] h-[192px] bg-[url('https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_email_header_bg.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfZW1haWxfaGVhZGVyX2JnLnBuZyIsImlhdCI6MTc0MTYzODczMCwiZXhwIjoxNzQ0MjMwNzMwfQ.ejQMJXMlUkiRgd5lJRYipzeEiKCWh63_r1BPNwTgZIQ')] bg-cover bg-center text-white font-mono text-center"
+ className={`w-full max-w-[600px] h-[192px] bg-[url('${process.env.EMAIL_HEADER_BG_URL}')] bg-cover bg-center text-white font-mono text-center`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className="w-full max-w-[600px] h-[192px] bg-[url('https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_email_header_bg.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfZW1haWxfaGVhZGVyX2JnLnBuZyIsImlhdCI6MTc0MTYzODczMCwiZXhwIjoxNzQ0MjMwNzMwfQ.ejQMJXMlUkiRgd5lJRYipzeEiKCWh63_r1BPNwTgZIQ')] bg-cover bg-center text-white font-mono text-center" | |
| className={`w-full max-w-[600px] h-[192px] bg-[url('${process.env.EMAIL_HEADER_BG_URL}')] bg-cover bg-center text-white font-mono text-center`} |
| <> | ||
| <a href={body.buttonLink} className="block"> | ||
| <Button> | ||
| <Text className="m-0 p-0"> | ||
| {body.buttonText} | ||
| </Text> | ||
| </Button> | ||
| </a> | ||
| <br /> | ||
| <a | ||
| href={body.secondButtonLink || body.buttonText} | ||
| className="block" | ||
| > | ||
| <Button> | ||
| <Text className="m-0 p-0"> | ||
| {body.secondButtonText || body.buttonText} | ||
| </Text> | ||
| </Button> | ||
| </a> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check the second button fallback logic
The second button uses the first button's text as fallback, which might be confusing to users. Also, there's inconsistency in the fallback for links vs text.
Consider more appropriate fallbacks or don't render the second button if secondButtonText is missing:
- <a
- href={body.secondButtonLink || body.buttonText}
- className="block"
- >
+ {body.secondButtonText && (
+ <a
+ href={body.secondButtonLink || body.buttonLink}
+ className="block"
+ >
<Button>
<Text className="m-0 p-0">
- {body.secondButtonText || body.buttonText}
+ {body.secondButtonText}
</Text>
</Button>
</a>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <> | |
| <a href={body.buttonLink} className="block"> | |
| <Button> | |
| <Text className="m-0 p-0"> | |
| {body.buttonText} | |
| </Text> | |
| </Button> | |
| </a> | |
| <br /> | |
| <a | |
| href={body.secondButtonLink || body.buttonText} | |
| className="block" | |
| > | |
| <Button> | |
| <Text className="m-0 p-0"> | |
| {body.secondButtonText || body.buttonText} | |
| </Text> | |
| </Button> | |
| </a> | |
| </> | |
| )} | |
| <> | |
| <a href={body.buttonLink} className="block"> | |
| <Button> | |
| <Text className="m-0 p-0"> | |
| {body.buttonText} | |
| </Text> | |
| </Button> | |
| </a> | |
| <br /> | |
| {body.secondButtonText && ( | |
| <a | |
| href={body.secondButtonLink || body.buttonLink} | |
| className="block" | |
| > | |
| <Button> | |
| <Text className="m-0 p-0"> | |
| {body.secondButtonText} | |
| </Text> | |
| </Button> | |
| </a> | |
| )} | |
| </> | |
| )} |
| <Img | ||
| // src={`${baseUrl}/static/instagram-white.png`} | ||
| src="https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/cuhacking/media/instagram-white.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJjdWhhY2tpbmcvbWVkaWEvaW5zdGFncmFtLXdoaXRlLnBuZyIsImlhdCI6MTc0MDkxMDUyOSwiZXhwIjoxNzcyNDQ2NTI5fQ.OMmwhkvoT3U0yuqTdpqXodLZCXzB4XksWN_EEU5sAXg" | ||
| alt="Instagram" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern with hardcoded JWT tokens in image URLs
The image URLs contain JWT tokens with very long expiration times (approximately 32 years). These tokens grant access to your Supabase storage and could potentially lead to unauthorized access.
Also applies to: 43-46, 58-61, 73-76, 94-97
🏁 Script executed:
#!/bin/bash
# Check if these tokens are publicly accessible or tied to specific environments
# Extract one of the tokens for testing
TOKEN=$(echo "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJjdWhhY2tpbmcvbWVkaWEvaW5zdGFncmFtLXdoaXRlLnBuZyIsImlhdCI6MTc0MDkxMDUyOSwiZXhwIjoxNzcyNDQ2NTI5fQ.OMmwhkvoT3U0yuqTdpqXodLZCXzB4XksWN_EEU5sAXg" | base64 -d 2>/dev/null || echo "Not valid base64")
# Display token expiration details if possible
echo "Token details:"
echo $TOKEN | jq 2>/dev/null || echo "Unable to parse token"Length of output: 383
Security Concern: Hardcoded Long-Lived JWT in Image URLs
The image URL in libs/email/components/footer.tsx (lines 28–31) currently includes a hardcoded JWT token. Our verification attempt (using a simple base64 decode) was inconclusive due to the specifics of JWT base64 URL encoding, but the token's embedded expiration time still indicates an excessively long lifetime (approximately 32 years). This practice risks unauthorized access to your Supabase storage if the URL becomes public.
Recommendations:
- Replace the hardcoded token with a dynamic solution that generates short-lived, environment-specific tokens.
- Review and update similar token usages in the same file (lines 43–46, 58–61, 73–76, and 94–97) to ensure consistency across the codebase.
🧰 Tools
🪛 Gitleaks (8.21.2)
30-30: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
| events: [ | ||
| { | ||
| title: 'Upcoming events...', | ||
| }, | ||
| } | ||
| { | ||
| title: 'Intro to QNX', | ||
| text: 'Learn about QNX with 3 awesome speakers + meet some interns. Make sure you bring your laptop. Happening March 11th, 6:30 to 8:00 pm. Register now!', | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSfni0BpF_2vf9xM02Ux4t979C_jVVXLvxVDv0u1hrrobVan-A/viewform" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Learn about Gadget", | ||
| text: "Gadget is hosting a Q&A webinar to teach students the basics of their platform. It's happening Wednesday, March 12th at 2:30pm. You must create an account using the link below. If you already have an account let Gadget know during the webinar.", | ||
| buttons: [ | ||
| { | ||
| text: "CREATE ACCOUNT", | ||
| link: "https://app.gadget.dev/auth/login?returnTo=/auth/hackathon" | ||
| }, | ||
| { | ||
| text: "WEBINAR", | ||
| link: "https://meet.google.com/ybk-wumk-hig" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Resume Roast", | ||
| text: "Get your resume roasted at cuHacking! Sign up below to be eligible. Brought to you by uO SESA", | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSdljqp8Z6F6ADINhJergvcqJZ6aTYSW2SSSVrngsY6ltx1eHQ/viewform" | ||
| } | ||
| ] | ||
| } | ||
| ], |
There was a problem hiding this comment.
Events structure implementation
The events array implementation allows for dynamic event content in emails, which is a good enhancement. However, the structure in the template differs from what's defined in the Emails collection model - here you have buttons array with text/link, while in the model you only have buttonText.
|
|
||
| 🥗 Food + Merch will be provided for the first 300 hackers on site!<br> | ||
| What to bring? | ||
| 🪪 Government ID< |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix typo in HTML tag
There's a missing closing angle bracket in the Government ID line.
-🪪 Government ID<
+🪪 Government ID📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 🪪 Government ID< | |
| 🪪 Government ID |
| // { | ||
| // title: 'Welcome to the Community!', | ||
| // body: { | ||
| // text: `We're excited to have you on board. Stay tuned for updates and events!\n\n🎉 Join our Discord server to connect with other hackers\n💻 Check out our workshops\n🏆 Get ready for the upcoming hackathon`, | ||
| // buttonText: 'JOIN DISCORD', | ||
| // buttonLink: 'https://discord.gg/VnbWdAe8kA', | ||
| // secondButtonText: 'EXPLORE WORKSHOPS', | ||
| // secondButtonLink: 'https://cuhacking.ca/workshops', | ||
| // events: [ | ||
| // { | ||
| // title: 'Upcoming events...', | ||
| // }, | ||
| // { | ||
| // title: 'Networking Mixer', | ||
| // text: 'Meet industry professionals and fellow hackers', | ||
| // buttonText: 'RSVP', | ||
| // } | ||
| // ], | ||
| // footer: 'Connect with the community!', | ||
| // }, | ||
| // }, | ||
| // { | ||
| // title: 'Your Weekly Update', | ||
| // body: { | ||
| // text: `Here's what's happening this week at cuHacking.\n\n🚀 New workshops announced\n🎙️ Guest speakers from QNX and Fullscript\n🧩 Practice hackathon challenges`, | ||
| // buttonText: 'READ MORE', | ||
| // buttonLink: 'https://cuhacking.ca/blog', | ||
| // secondButtonText: 'VIEW SCHEDULE', | ||
| // secondButtonLink: 'https://cuhacking.ca/schedule', | ||
| // events: [ | ||
| // { | ||
| // title: 'Upcoming events...', | ||
| // }, | ||
| // { | ||
| // title: 'Workshop: Intro to React', | ||
| // text: 'Learn the basics of React with our experienced mentors', | ||
| // buttonText: 'REGISTER', | ||
| // }, | ||
| // { | ||
| // title: 'Tech Talk: Cybersecurity Basics', | ||
| // text: 'Join CSE for an introduction to cybersecurity principles', | ||
| // buttonText: 'REGISTER', | ||
| // } | ||
| // ], | ||
| // footer: 'Check out the latest news!', | ||
| // }, | ||
| // }, | ||
| // { | ||
| // title: 'Reminder: Hackathon Starts Soon!', | ||
| // body: { | ||
| // text: `The hackathon kicks off in just 3 days. Make sure you're prepared!\n\nWhat to bring?\n🪪 Government ID\n💻 Your laptop + charger\n🛏️ Sleeping bags\n🪥 Personal hygiene (plz bring deodorant, plz ...)\n🥤 Reusable water bottle\n🤪 A positive attitude!`, | ||
| // buttonText: 'EVENT DETAILS', | ||
| // buttonLink: 'https://cuhacking.ca/hackathon', | ||
| // secondButtonText: 'JOIN DISCORD', | ||
| // secondButtonLink: 'https://discord.gg/VnbWdAe8kA', | ||
| // events: [ | ||
| // { | ||
| // title: 'Pre-Hackathon Workshop', | ||
| // text: 'Get a head start with our pre-hackathon workshop', | ||
| // buttonText: 'JOIN NOW', | ||
| // } | ||
| // ], | ||
| // footer: 'See the schedule here.', | ||
| // }, | ||
| // }, | ||
| // { | ||
| // title: 'Thank You for Attending!', | ||
| // body: { | ||
| // text: 'We hope you had a great time at our event. Your feedback is valuable to us.\n\n🏆 Congratulations to all winners!\n📸 Check out event photos\n🗓️ Stay tuned for our next hackathon', | ||
| // buttonText: 'GIVE FEEDBACK', | ||
| // buttonLink: 'https://forms.example.com/feedback', | ||
| // secondButtonText: 'VIEW PHOTOS', | ||
| // secondButtonLink: 'https://cuhacking.ca/gallery', | ||
| // events: [ | ||
| // { | ||
| // title: 'Upcoming events...', | ||
| // }, | ||
| // { | ||
| // title: 'Project Showcase', | ||
| // text: 'Present your hackathon project to industry professionals', | ||
| // buttonText: 'REGISTER', | ||
| // } | ||
| // ], | ||
| // footer: 'Help us improve!', | ||
| // }, | ||
| // }, | ||
| // { | ||
| // title: 'Exclusive Offer for cuHacking Members!', | ||
| // body: { | ||
| // text: `As a valued member, we're giving you early access to our upcoming event!\n\n🎁 Special perks for cuHacking members\n🔥 Limited spots available\n🚀 Exclusive networking opportunities`, | ||
| // buttonText: 'CLAIM YOUR SPOT', | ||
| // buttonLink: 'https://cuhacking.ca/exclusive', | ||
| // secondButtonText: 'LEARN MORE', | ||
| // secondButtonLink: 'https://cuhacking.ca/membership-benefits', | ||
| // events: [ | ||
| // ], | ||
| // footer: 'Limited spots available!', | ||
| // }, | ||
| // }, | ||
| // ] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Commented WIP code should be in a separate branch
Large blocks of commented code marked as "WIP" should ideally be in a separate development branch rather than committed to the main codebase. This makes the code cleaner and easier to maintain.
| { | ||
| name: 'secondButtonText', | ||
| type: 'text', | ||
| required: false, | ||
| }, | ||
| { | ||
| name: 'secondButtonLink', | ||
| type: 'text', | ||
| required: false, | ||
| }, | ||
| { | ||
| name: 'events', | ||
| type: 'array', | ||
| fields: [ | ||
| { | ||
| name: 'title', | ||
| type: 'text', | ||
| required: false, | ||
| }, | ||
| { | ||
| name: 'text', | ||
| type: 'textarea', | ||
| required: false, | ||
| }, | ||
| { | ||
| name: 'buttonText', | ||
| type: 'text', | ||
| required: false, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Model doesn't match template implementation
The events array structure in the model only has buttonText field, but the template implementation uses an array of buttons with text/link pairs. This mismatch will cause issues when fetching and rendering events.
Update the model to match the template implementation:
{
name: 'events',
type: 'array',
fields: [
{
name: 'title',
type: 'text',
required: false,
},
{
name: 'text',
type: 'textarea',
required: false,
},
- {
- name: 'buttonText',
- type: 'text',
- required: false,
- },
+ {
+ name: 'buttons',
+ type: 'array',
+ required: false,
+ fields: [
+ {
+ name: 'text',
+ type: 'text',
+ required: true,
+ },
+ {
+ name: 'link',
+ type: 'text',
+ required: true,
+ }
+ ]
+ },
],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| name: 'secondButtonText', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'secondButtonLink', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'events', | |
| type: 'array', | |
| fields: [ | |
| { | |
| name: 'title', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'text', | |
| type: 'textarea', | |
| required: false, | |
| }, | |
| { | |
| name: 'buttonText', | |
| type: 'text', | |
| required: false, | |
| }, | |
| ], | |
| }, | |
| { | |
| name: 'secondButtonText', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'secondButtonLink', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'events', | |
| type: 'array', | |
| fields: [ | |
| { | |
| name: 'title', | |
| type: 'text', | |
| required: false, | |
| }, | |
| { | |
| name: 'text', | |
| type: 'textarea', | |
| required: false, | |
| }, | |
| { | |
| name: 'buttons', | |
| type: 'array', | |
| required: false, | |
| fields: [ | |
| { | |
| name: 'text', | |
| type: 'text', | |
| required: true, | |
| }, | |
| { | |
| name: 'link', | |
| type: 'text', | |
| required: true, | |
| } | |
| ] | |
| }, | |
| ], | |
| }, |
HasithDeAlwis
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
5d5013d to
aab8e9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/email/templates/cuhacking/registered-for-hackathon.tsx (1)
111-131:⚠️ Potential issueType mismatch in function signature.
The
bodyparameter here is an object with multiple fields, but it's typed asstring. Update the signature accordingly:-function Generic({ title, body } : { title: string, body: string }) { +interface EmailBody { + text: string + buttonText?: string + buttonLink?: string + secondButtonText?: string + secondButtonLink?: string + events?: Array<{ + title: string + text?: string + buttons?: Array<{ text: string; link: string }> + }> +} +function Generic({ title, body }: { title: string; body: EmailBody }) { // ... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/email/templates/cuhacking/registered-for-hackathon.tsx(4 hunks)infra/shell.nix(1 hunks)libs/email/components/banner.tsx(1 hunks)libs/email/components/content.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`infra/**`: "- Validate infrastructure security best practic...
infra/**: "- Validate infrastructure security best practices
- Ensure proper error handling in non-generated code
- Verify deployment configuration parameters"
infra/shell.nix
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build (22.13.1)
🔇 Additional comments (8)
libs/email/components/banner.tsx (1)
12-12: Security concern with the hardcoded token in the background image URL.libs/email/components/content.tsx (4)
5-5: Good use of defaulting to an empty array.
17-19: Preserving whitespace with<pre>is a nice touch.
21-41: Fallback logic for the second button might be confusing to users.
49-106: Verify the interface to ensureevent.buttonsis defined.apps/email/templates/cuhacking/registered-for-hackathon.tsx (3)
3-3: Remove unused import.
29-48: Fix the stray angle bracket in line 37.
50-101: Enhanced event details look good.
| @@ -22,7 +22,7 @@ in pkgs.mkShell { | |||
| pkgs.yazi | |||
| pkgs.bat | |||
| pkgs.browsh | |||
| pkgs.firefox | |||
| #pkgs.firefox | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove commented line if it's no longer needed.
If there's no plan to reintroduce Firefox support soon, consider removing the line altogether to keep the shell environment tidy.
MFarabi619
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
aab8e9e to
d7613eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/email/templates/cuhacking/registered-for-hackathon.tsx(4 hunks)libs/email/components/banner.tsx(1 hunks)libs/email/components/content.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build (22.13.1)
🔇 Additional comments (7)
libs/email/components/banner.tsx (1)
12-12: Avoid storing tokens client-side
This approach repeats a previous security concern. Hardcoding a JWT in client code can expose secrets and break functionality when it expires.libs/email/components/content.tsx (4)
5-6: Good fallback for events
Usingbody.events || []prevents runtime errors if no events are provided.
21-41: Revisit second button fallback logic
This replicates the previously noted fallback pattern, wheresecondButtonTextdefaults tobuttonText. This may confuse users if the second link serves a different purpose.
49-106: Potential mismatch with event interface
Accessingevent.buttons.map(...)repeats a previous concern. Confirm that your event interface indeed supports an array namedbuttons.
17-19:✅ Verification successful
Consider sanitizing user text
Placing rawbody.textinside<pre>could allow unintended HTML/script injection ifbody.textis not sanitized.
🏁 Script executed:
#!/bin/bash # Description: Check for any sanitizing or escaping logic in the codebase rg -A 3 -i "sanitize" rg -A 3 -i "escape"Length of output: 20691
Action Required: Confirm
body.textRemains Plain Text
Rendering{body.text}inside the<pre>element is safe as long as it remains a plain string—React automatically escapes any text to prevent XSS. However, if there's any chance thatbody.textmight include HTML (or if you plan to support rich text rendering viadangerouslySetInnerHTMLin the future), you should explicitly sanitize the content before rendering. Please verify that:
body.textis guaranteed to be plain text so that React’s automatic escaping is sufficient.- If raw HTML is expected at any point, a trusted sanitization library should be applied prior to rendering, and that behavior should be clearly documented.
apps/email/templates/cuhacking/registered-for-hackathon.tsx (2)
116-116: Basic footer usage
No issues found with rendering<Footer />without props.
124-127: Preview component looks good
Rendering<Generic {...genericEmailConstants} />to preview email content is straightforward and maintainable.
| return { | ||
| title: 'Heeyy :333', | ||
| body: { | ||
| text: `You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥 | ||
|
|
||
| Details ... | ||
|
|
||
| 📍 Richcraft Hall (1125 Colonel By Dr, Carleton University) | ||
| ⌛ March 14-16 (Check-in from 5pm - 9pm) | ||
|
|
||
| 🥗 Food + Merch will be provided for the first 300 hackers on site!<br> | ||
| What to bring? | ||
| 🪪 Government ID | ||
| 💻 Your laptop + charger | ||
| 🛏 Sleeping bags | ||
| 🪥 Personal hygiene (plz bring deodorant, plz ...) | ||
| 🥤 Reusable water bottle | ||
| 🤪 A positive attitude!`, | ||
|
|
||
| buttonText: 'JOIN DISCORD', | ||
| buttonLink: 'https://discord.gg/VnbWdAe8kA', | ||
|
|
||
| secondButtonText: 'ADD TO CALENDAR', | ||
| secondButtonLink: 'https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MDY1bnFyb3B2c2Z1NjM4dDRxODcybmhtYWwgZmNkYmEzZjM1NGQ0ZTAxNTUyZTI0OTVkNzQzMTA1YmQ5ZWZjZTRlMTA3NmNkYTNjMWVjMTQ1NGQwZjVmYWE3M0Bn&tmsrc=fcdba3f354d4e01552e2495d743105bd9efce4e1076cda3c1ec1454d0f5faa73%40group.calendar.google.com', | ||
|
|
||
| events: [ | ||
| { | ||
| title: 'Upcoming events...', | ||
| }, | ||
| } | ||
| /* } */ | ||
| { | ||
| title: 'Intro to QNX', | ||
| text: 'Learn about QNX with 3 awesome speakers + meet some interns. Make sure you bring your laptop. Happening March 11th, 6:30 to 8:00 pm. Register now!', | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSfni0BpF_2vf9xM02Ux4t979C_jVVXLvxVDv0u1hrrobVan-A/viewform" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Learn about Gadget", | ||
| text: "Gadget is hosting a Q&A webinar to teach students the basics of their platform. It's happening Wednesday, March 12th at 2:30pm. You must create an account using the link below. If you already have an account let Gadget know during the webinar.", | ||
| buttons: [ | ||
| { | ||
| text: "CREATE ACCOUNT", | ||
| link: "https://app.gadget.dev/auth/login?returnTo=/auth/hackathon" | ||
| }, | ||
| { | ||
| text: "WEBINAR", | ||
| link: "https://meet.google.com/ybk-wumk-hig" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Resume Roast", | ||
| text: "Get your resume roasted at cuHacking! Sign up below to be eligible. Brought to you by uO SESA", | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSdljqp8Z6F6ADINhJergvcqJZ6aTYSW2SSSVrngsY6ltx1eHQ/viewform" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Sign up for our portal!", | ||
| text: "Create your account on our portal! You must do this to be eligible for our hackathon", | ||
| buttons: [ | ||
| { | ||
| text: "CREATE ACCOUNT", | ||
| link: "https://portal.cuhacking.ca" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use environment variables for dynamic links
Hardcoding these external URLs (Discord, Calendar, QNX, etc.) can cause maintenance hurdles. Consider storing them in environment variables or a config file for easier updates.
MFarabi619
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
d7613eb to
ac82907
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
libs/email/components/banner.tsx (1)
12-12:⚠️ Potential issueSecurity concern with hardcoded token in background image URL
The background image URL contains a hardcoded JWT token with an expiration date (February 2025), creating both a reliability issue (image will stop working) and a security risk (authentication tokens in client-side code).
Consider using environment variables for dynamic URL generation.
libs/email/components/content.tsx (1)
21-41:⚠️ Potential issueIncorrect fallback for second button link
The second button uses
body.buttonTextas a fallback forbody.secondButtonLink, which is likely a mistake.- href={body.secondButtonLink || body.buttonText} + href={body.secondButtonLink || body.buttonLink}Additionally, consider conditionally rendering the second button only when
secondButtonTextis present:- <a - href={body.secondButtonLink || body.buttonText} - className="block" - > - <Button> - <Text className="m-0 p-0"> - {body.secondButtonText || body.buttonText} - </Text> - </Button> - </a> + {body.secondButtonText && ( + <a + href={body.secondButtonLink || body.buttonLink} + className="block" + > + <Button> + <Text className="m-0 p-0"> + {body.secondButtonText} + </Text> + </Button> + </a> + )}apps/email/templates/cuhacking/welcome.tsx (1)
27-103: 🛠️ Refactor suggestionHardcoded email content should be configurable
The email content is hardcoded, making it difficult to update without code changes. Consider using the commented-out code as a foundation to fetch this content dynamically from your database.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/email/templates/cuhacking/registered-for-hackathon.tsx(1 hunks)apps/email/templates/cuhacking/welcome.tsx(1 hunks)libs/email/components/banner.tsx(1 hunks)libs/email/components/content.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build (22.13.1)
🔇 Additional comments (3)
apps/email/templates/cuhacking/registered-for-hackathon.tsx (1)
56-56: Function name change improves semantic clarityRenaming from
PreviewtoRegisteredForHackathonbetter reflects the function's purpose. Consider adding a proper TypeScript return type for completeness.libs/email/components/content.tsx (2)
17-19: Improved text formatting with pre tagUsing a
pretag preserves the original text formatting, which is a good improvement for email content display.
49-106:❓ Verification inconclusive
Type mismatch between component and interface definition
The code accesses
event.buttons.map()on line 78, but according to previous review comments, thepayload-types.tsonly has abuttonTextproperty, not abuttonsarray.Consider updating either the interface or the component implementation to match:
- event.buttons.map(button => - ( - <div key={button.text} style={{ marginBottom: 8 }}> - <a href={button.link} className="block no-underline text-black"> - <Button> - <Text className="m-0 p-0"> - {button.text} - </Text> - </Button> - </a> - </div> - ), - ) + {event.buttonText && ( + <div style={{ marginBottom: 8 }}> + <Button> + <Text className="m-0 p-0"> + {event.buttonText} + </Text> + </Button> + </div> + )}
🏁 Script executed:
#!/bin/bash # Check the Email interface definition to confirm what properties are available fd "Email.*interface" --type f | xargs catLength of output: 44
Action Required: Verify Email Interface Consistency
The review comment highlights a potential type mismatch: the component uses
event.buttons.map()(line 78) while previous notes indicate that thepayload-types.ts) defines only abuttonTextproperty. Our initial verification didn't yield any output for the Email interface definition, so we must manually confirm the intended structure.Next Steps:
- Manually verify the
libs/email/payload-types.tsto determine if it defines abuttonsarray or just a singlebuttonTextproperty.- If only
buttonTextis defined: Update the component code to remove the array access in favor of a singular property (as shown in the diff snippet below).- If a
buttonsproperty is intended: Ensure the interface is updated accordingly, and update the component if necessary.- event.buttons.map(button => - ( - <div key={button.text} style={{ marginBottom: 8 }}> - <a href={button.link} className="block no-underline text-black"> - <Button> - <Text className="m-0 p-0"> - {button.text} - </Text> - </Button> - </a> - </div> - ), - ) + {event.buttonText && ( + <div style={{ marginBottom: 8 }}> + <Button> + <Text className="m-0 p-0"> + {event.buttonText} + </Text> + </Button> + </div> + )}Please verify this discrepancy manually to ensure the component and the interface definitions are in sync.
| @@ -2,6 +2,8 @@ import { Button, Card } from '@/email' | |||
| import { Text } from '@react-email/components' | |||
|
|
|||
| export function Content({ title, body }) { | |||
| const events = body.events || [] | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Missing type definition for events
The array fallback is good, but consider adding appropriate type definitions for the events variable.
- const events = body.events || []
+ const events: Array<{
+ title?: string;
+ text?: string;
+ buttons?: Array<{ text: string; link: string }>;
+ buttonText?: string;
+ }> = body.events || []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const events = body.events || [] | |
| const events: Array<{ | |
| title?: string; | |
| text?: string; | |
| buttons?: Array<{ text: string; link: string }>; | |
| buttonText?: string; | |
| }> = body.events || [] |
| export default function Preview() { | ||
| return ( | ||
| <Generic {...genericEmailConstants} /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider consistent function naming across templates
For consistency with the RegisteredForHackathon function in the other template, consider renaming this function to something more descriptive.
-export default function Preview() {
+export default function WelcomeEmail() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function Preview() { | |
| return ( | |
| <Generic {...genericEmailConstants} /> | |
| ) | |
| } | |
| export default function WelcomeEmail() { | |
| return ( | |
| <Generic {...genericEmailConstants} /> | |
| ) | |
| } |
| <Banner /> | ||
| <Content title={title} body={body} /> | ||
| <Keyboard /> | ||
| <Footer /> |
There was a problem hiding this comment.
Missing footer content
The Footer component is used without any children, unlike in the registered-for-hackathon.tsx file where body.footer is passed as children.
- <Footer />
+ <Footer>{body.footer}</Footer>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Footer /> | |
| <Footer>{body.footer}</Footer> |
|
|
||
| const genericEmailConstants = getGenericEmailConstants() | ||
|
|
||
| function Generic({ title, body } : { title: string, body: string }) { |
There was a problem hiding this comment.
Fix type mismatch on function signature
The type { title: string, body: string } doesn't reflect the actual structure of body. The body parameter is a complex object, not a string.
- function Generic({ title, body } : { title: string, body: string }) {
+ function Generic({
+ title,
+ body,
+ }: {
+ title: string,
+ body: {
+ text: string
+ buttonText?: string
+ buttonLink?: string
+ secondButtonText?: string
+ secondButtonLink?: string
+ events?: {
+ title?: string
+ text?: string
+ buttons?: { text: string; link: string }[]
+ }[]
+ }
+ }) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Generic({ title, body } : { title: string, body: string }) { | |
| function Generic({ | |
| title, | |
| body, | |
| }: { | |
| title: string, | |
| body: { | |
| text: string | |
| buttonText?: string | |
| buttonLink?: string | |
| secondButtonText?: string | |
| secondButtonLink?: string | |
| events?: { | |
| title?: string | |
| text?: string | |
| buttons?: { text: string; link: string }[] | |
| }[] | |
| } | |
| }) { |
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (5)
libs/email/components/content.tsx (3)
5-5: 🧹 Nitpick (assertive)Missing type definition for events
The array fallback is good, but consider adding appropriate type definitions for the events variable.
- const events = body.events || [] + const events: Array<{ + title?: string; + text?: string; + buttons?: Array<{ text: string; link: string }>; + buttonText?: string; + }> = body.events || []
46-56: 🛠️ Refactor suggestionCheck the second button fallback logic
The second button uses the first button's text as fallback, which might be confusing. There's also inconsistency in the fallback for links vs text.
- <a - href={body.secondButtonLink || body.buttonText} - target="_blank" - className="block" - > + {body.secondButtonText && ( + <a + href={body.secondButtonLink || body.buttonLink} + target="_blank" + rel="noreferrer" + className="block" + > <Button> <Text className="m-0 p-0 font-mono uppercase w-40"> - {body.secondButtonText || body.buttonText} + {body.secondButtonText} </Text> </Button> </a> + )}🧰 Tools
🪛 Biome (1.9.4)
[error] 48-49: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.(lint/a11y/noBlankTarget)
66-123:⚠️ Potential issueType mismatch between component and interface definition
The code attempts to access
event.buttons.map(), but this might not match the Email interface definition.- { - event.buttons.map(button => - ( - <div key={button.text} style={{ marginBottom: 8 }}> - <a href={button.link} target="_blank" className="block no-underline text-black"> - <Button> - <Text className="m-0 p-0 font-mono uppercase w-40"> - {button.text} - </Text> - </Button> - </a> - </div> - ), - ) - } + {event.buttons?.map(button => ( + <div key={button.text} style={{ marginBottom: 8 }}> + <a href={button.link} target="_blank" rel="noreferrer" className="block no-underline text-black"> + <Button> + <Text className="m-0 p-0 font-mono uppercase w-40"> + {button.text} + </Text> + </Button> + </a> + </div> + ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 98-98: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.(lint/a11y/noBlankTarget)
apps/email/templates/cuhacking/welcome.tsx (2)
109-109:⚠️ Potential issueFix type mismatch on function signature
The type
{ title: string, body: string }doesn't reflect the actual structure ofbody. Thebodyparameter is a complex object with multiple properties.- function Generic({ title, body } : { title: string, body: string }) { + function Generic({ + title, + body, + }: { + title: string, + body: { + text: string + buttonText?: string + buttonLink?: string + secondButtonText?: string + secondButtonLink?: string + events?: { + title?: string + text?: string + buttons?: { text: string; link: string }[] + }[] + } + }) {
126-130: 🧹 Nitpick (assertive)Consider consistent function naming across templates
For consistency with other templates, rename this function to something more descriptive.
-export default function Preview() { +export default function WelcomeEmail() {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/email/templates/cuhacking/welcome.tsx(1 hunks)libs/email/components/banner.tsx(2 hunks)libs/email/components/content.tsx(1 hunks)libs/email/components/footer.tsx(1 hunks)libs/email/components/keyboard.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/email/components/content.tsx
[error] 19-20: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 22-22: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 38-38: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 48-49: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 98-98: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
apps/email/templates/cuhacking/welcome.tsx
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (9)
libs/email/components/keyboard.tsx (2)
7-7: LGTM: Function signature now accepts dynamic image sourceThe updated function signature makes the component more reusable by accepting a
keyboardSrcparameter instead of using a hardcoded value.
13-13: LGTM: Dynamic image source implementationThe image source is now correctly set using the provided parameter.
libs/email/components/banner.tsx (1)
27-27: LGTM: Dynamic logo source implementationThe image source is now correctly set using the provided parameter.
libs/email/components/footer.tsx (3)
7-24: LGTM: Well-structured helper function for social linksThe
socialLinkshelper function provides a clean abstraction for generating social media links with consistent styling.
26-26: LGTM: Enhanced component signature with appropriate typesThe function signature now accepts parameters for dynamic content, making the component more reusable.
28-90: LGTM: Improved email layout using tablesThe refactored layout using tables is more compatible with email clients, which is a good practice for email templates.
apps/email/templates/cuhacking/welcome.tsx (3)
1-2: The component imports look goodAppropriate components are imported from both local paths and the React Email library.
110-123: Email structure looks solidThe email template structure with Banner, Content, Keyboard, and Footer components follows good design patterns and provides a visually consistent layout.
118-118:❓ Verification inconclusive
Ensure Footer component usage aligns with other templates
The
Footercomponent is receivingsocialsandlogoSrcprops, but past review comments indicate that other templates passbody.footeras children. Verify this implementation is consistent with the updated Footer component.
🏁 Script executed:
#!/bin/bash # Check how Footer component is used in other template files echo "Checking Footer implementation:" find apps/email/templates -type f -not -path "*/welcome.tsx" -name "*.tsx" | xargs grep -A 3 "<Footer" # Check Footer component definition to verify expected props echo "Checking Footer component definition:" find apps/email/components -name "Footer.tsx" -exec cat {} \;Length of output: 723
🏁 Script executed:
#!/bin/bash # Find the Footer component definition anywhere in the repository echo "Searching for Footer component definition:" find . -type f -name "Footer.tsx" -printLength of output: 135
Footer Component Usage Consistency: Verify Alignment
It appears that in the cuhacking templates the Footer component is used in two different ways:
- In
apps/email/templates/cuhacking/registered-for-hackathon.tsx, Footer is rendered with children ({body.footer}).- In
apps/email/templates/cuhacking/welcome.tsx(line 118), Footer is invoked with explicit props (socialsandlogoSrc).Without a visible Footer component definition in the repository, it’s unclear which API is correct. Please verify the Footer component’s expected API and ensure consistent usage across all templates. If the updated Footer component now accepts props like
socials/logoSrc, consider updating other templates accordingly; otherwise, adjustwelcome.tsxto passbody.footeras children.
libs/email/components/content.tsx
Outdated
| <pre className="text-sm text-[#e6e6e6] font-mono text-left text-wrap"> | ||
| You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥 | ||
| <br></br> | ||
| We're excited to have you join us for a weekend of hacking, learning, and fun!!! | ||
| </pre> | ||
| <a href="https://portal.cuhacking.ca" target="_blank" className="block w-full"> | ||
| <Button className="w-full bg-[#f9a826]"> | ||
| <Text className="m-0 p-0 font-mono uppercase"> | ||
| plz make an account, we want to see you there pookie ↗ | ||
| </Text> | ||
| </Button> | ||
| </a> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded content should be parameterized
The welcome message and button text are hardcoded, which reduces reusability. Consider parameterizing this content to make the component more flexible.
<pre className="text-sm text-[#e6e6e6] font-mono text-left text-wrap">
- You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥
+ {body.welcomeMessage || "You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥"}
<br></br>
- We're excited to have you join us for a weekend of hacking, learning, and fun!!!
+ {body.welcomeSubtext || "We're excited to have you join us for a weekend of hacking, learning, and fun!!!"}
</pre>
-<a href="https://portal.cuhacking.ca" target="_blank" className="block w-full">
+<a href={body.portalLink || "https://portal.cuhacking.ca"} target="_blank" rel="noreferrer" className="block w-full">
<Button className="w-full bg-[#f9a826]">
<Text className="m-0 p-0 font-mono uppercase">
- plz make an account, we want to see you there pookie ↗
+ {body.portalButtonText || "plz make an account, we want to see you there pookie ↗"}
</Text>
</Button>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <pre className="text-sm text-[#e6e6e6] font-mono text-left text-wrap"> | |
| You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥 | |
| <br></br> | |
| We're excited to have you join us for a weekend of hacking, learning, and fun!!! | |
| </pre> | |
| <a href="https://portal.cuhacking.ca" target="_blank" className="block w-full"> | |
| <Button className="w-full bg-[#f9a826]"> | |
| <Text className="m-0 p-0 font-mono uppercase"> | |
| plz make an account, we want to see you there pookie ↗ | |
| </Text> | |
| </Button> | |
| </a> | |
| <pre className="text-sm text-[#e6e6e6] font-mono text-left text-wrap"> | |
| {body.welcomeMessage || "You're registered for cuHacking 2025 at Carleton University! 🔥🔥🔥"} | |
| <br></br> | |
| {body.welcomeSubtext || "We're excited to have you join us for a weekend of hacking, learning, and fun!!!"} | |
| </pre> | |
| <a href={body.portalLink || "https://portal.cuhacking.ca"} target="_blank" rel="noreferrer" className="block w-full"> | |
| <Button className="w-full bg-[#f9a826]"> | |
| <Text className="m-0 p-0 font-mono uppercase"> | |
| {body.portalButtonText || "plz make an account, we want to see you there pookie ↗"} | |
| </Text> | |
| </Button> | |
| </a> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 19-20: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 22-22: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
libs/email/components/footer.tsx
Outdated
| href="https://www.youtube.com/watch?v=xvFZjo5PgG0" | ||
| className="text-[#888] underline hover:text-white" | ||
| target="_blank" | ||
| rel="noopener noreferrer" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the unsubscribe link
The current unsubscribe link appears to be a joke (YouTube Rick Roll). For production emails, ensure there's a valid unsubscribe mechanism to comply with anti-spam regulations.
🌐 Web query:
What are the legal requirements for unsubscribe links in marketing emails?
💡 Result:
Marketing emails must comply with legal requirements to protect recipients' rights and avoid penalties. Below is a detailed breakdown of key regulations, primarily under the U.S. CAN-SPAM Act, along with recent updates and best practices:
Core Legal Requirements
-
Unsubscribe Link Mandate
Every commercial email must include a clear, conspicuous, and functional unsubscribe link that allows recipients to opt out of future messages. This link must remain valid for at least 30 days after the email is sent[1][5][9]. -
Simple Opt-Out Process
- Recipients cannot be required to log in, pay fees, or provide personal information beyond their email address to unsubscribe[1][5][14].
- A single click or webpage visit must suffice (e.g., "Click here to unsubscribe")[1][9].
-
Prompt Processing
Opt-out requests must be honored within 10 business days. After unsubscribing, sending further commercial emails is prohibited[5][13][14]. -
Sender Identification
- Emails must include a valid physical postal address (e.g., P.O. Box or street address)[4][13][17].
- Headers (From, To, Reply-To) must be accurate and non-misleading[14][17].
-
Transparent Content
- Subject lines must accurately reflect the email’s content[1][14].
- Commercial emails must be clearly labeled as advertisements[14][17].
2024 FTC "Click-to-Cancel" Rule Update
The FTC now requires businesses to make unsubscribing as easy as subscribing. Key provisions include:
- No hidden cancellation steps or deceptive interfaces[1][8].
- Immediate opt-out mechanisms for recurring subscriptions (e.g., no forced account logins)[1][8].
Penalties for Non-Compliance
Violations of CAN-SPAM can result in fines of up to $53,088 per email[14]. Businesses are liable even if third-party vendors send non-compliant emails on their behalf[8][14].
Best Practices Beyond Legal Minimums
- Preference Centers: Allow users to adjust email frequency or content types while still offering a full opt-out option[4][12].
- Avoid Confirmation Emails: Do not send follow-up emails after unsubscribing; redirect to a confirmation page instead[4][12].
- Audit Compliance: Regularly check email content, headers, and unsubscribe functionality[10][16].
Examples of Compliant Unsubscribe Messages
- "To unsubscribe from future emails, click here: [Link]."
- "Manage preferences or unsubscribe: [Link]."[1][4]
International Considerations
While CAN-SPAM applies to U.S.-focused emails, other regions have stricter rules:
- Canada (CASL): Requires opt-out processing within 10 days.
- EU (GDPR): Mandates explicit consent and easy withdrawal[10][16].
By adhering to these rules, businesses maintain trust, avoid penalties, and foster ethical email marketing practices.
Citations:
- 1: https://campaignrefinery.com/email-unsubscribe-law/
- 2: https://www.iubenda.com/en/help/22723-what-is-can-spam-opt-out
- 3: https://mailchimp.com/help/why-we-require-an-unsubscribe-link/
- 4: https://www.uschamber.com/co/grow/marketing/email-marketing-unsubscribe-laws
- 5: https://termly.io/resources/articles/can-spam-act/
- 6: https://learn.customer.io/deliverability/unsubscribe-link
- 7: https://transcend.io/blog/can-spam-act
- 8: https://www.ftc.gov/business-guidance/blog/2023/08/when-sending-commercial-email-businesses-cant-unsubscribe-can-spam-compliance
- 9: https://en.wikipedia.org/wiki/CAN-SPAM_Act_of_2003
- 10: https://www.dyspatch.io/blog/what-is-really-required-for-unsubscribes-and-other-legal-requirements-demystified/
- 11: https://www.cloudflare.com/learning/privacy/what-is-the-can-spam-act/
- 12: https://www.campaignmonitor.com/resources/knowledge-base/is-it-illegal-to-not-have-an-unsubscribe-link/
- 13: https://brandguide.asu.edu/execution-guidelines/email/can-spam
- 14: https://www.ftc.gov/business-guidance/resources/can-spam-act-compliance-guide-business
- 15: https://securiti.ai/can-spam-opt-out/
- 16: https://mailtrap.io/blog/email-marketing-laws/
- 17: https://verticalresponse.com/blog/its-the-law-7-email-marketing-rules-you-should-know/
- 18: https://termly.io/resources/articles/legal-requirements-for-email-marketing/
Action: Replace the Unsubscribe Link
The current unsubscribe URL in libs/email/components/footer.tsx (lines 70–73) is a placeholder (a Rick Roll) and does not provide a clear, functional opt-out mechanism. Per legal requirements (e.g., CAN-SPAM) and best practices, production emails must include a valid unsubscribe link that:
- Clearly allows recipients to opt out through a simple, one-click process (no extra authentication or additional steps required).
- Remains functional for at least 30 days after the email is sent.
Please update the link accordingly.
| const EMAIL_SUPABASE_URLS = { | ||
| headerBg: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_email_header_bg.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfZW1haWxfaGVhZGVyX2JnLnBuZyIsImlhdCI6MTc0MTY2MDQ1MCwiZXhwIjoxNzczMTk2NDUwfQ.uwIZdlao58676NBpt8LWSTdg4UEjRjCoArhQFbzF9p8', | ||
| footerBg: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_email_footer_bg.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfZW1haWxfZm9vdGVyX2JnLnBuZyIsImlhdCI6MTc0MTY2MDQwNywiZXhwIjoxNzczMTk2NDA3fQ.is1-j7yWwHq8qs6jryPaI2Qv5ANhZjd-oJk2P1kB2Mw', | ||
| logo: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/keycap-logo/cuhacking_keycap_logo_grayscale.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9rZXljYXAtbG9nby9jdWhhY2tpbmdfa2V5Y2FwX2xvZ29fZ3JheXNjYWxlLnN2ZyIsImlhdCI6MTc0MTY2MDQ4OSwiZXhwIjoxNzczMTk2NDg5fQ.Aq2n6299l_Rqweb73r8JdLInvFgTr1wKjUFQatG7NN8', | ||
| socials: [ | ||
| { | ||
| alt: 'Instagram', | ||
| img: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_icon_instagram.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfaWNvbl9pbnN0YWdyYW0uc3ZnIiwiaWF0IjoxNzQxNjYwNjQ1LCJleHAiOjE3NzMxOTY2NDV9.bdquMbCSiGSUD8DU-lGiSMN6nBhi_yTq-wdCBoKrEjU', | ||
| link: 'https://instagram.com/cuhacking' | ||
| }, | ||
| { | ||
| alt: 'discord', | ||
| img: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_icon_discord.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfaWNvbl9kaXNjb3JkLnN2ZyIsImlhdCI6MTc0MTY2MDYyNiwiZXhwIjoxNzczMTk2NjI2fQ.RXwKHPbLSh9YSAlRfivfaqCyoyuOVzIFrZDJlg-wCDY', | ||
| link: 'https://discord.gg/VnbWdAe8kA' | ||
| }, | ||
| { | ||
| alt: 'Website', | ||
| img: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_icon_website.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfaWNvbl93ZWJzaXRlLnN2ZyIsImlhdCI6MTc0MTY2MDY4OCwiZXhwIjoxNzczMTk2Njg4fQ.XS4UbAagQHg6APz2r88tfn-psIkoKN3gubf6TeSCyOI', | ||
| link: 'https://cuhacking.ca/' | ||
| }, | ||
| { | ||
| alt: 'LinkedIn', | ||
| img: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_icon_linkedin.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfaWNvbl9saW5rZWRpbi5zdmciLCJpYXQiOjE3NDE2NjA2NjMsImV4cCI6MTc3MzE5NjY2M30.h7E9pCwlw4APrDreAYpcBKRaOHRMLzQC_RTcvzNEaX0', | ||
| link: 'https://www.linkedin.com/company/cuhacking/posts/?feedView=all' | ||
| }, | ||
| { | ||
| alt: 'Linktree', | ||
| img: 'https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/development/email/cuhacking_icon_linktree.svg?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJkZXZlbG9wbWVudC9lbWFpbC9jdWhhY2tpbmdfaWNvbl9saW5rdHJlZS5zdmciLCJpYXQiOjE3NDE2NjA1NTEsImV4cCI6MTc3MzE5NjU1MX0.gSojk40HY3yecdC7tytDi5zrH3ciPNS9YNl7Tdb-Y-Y', | ||
| link: 'https://linktr.ee/cuhacking_' | ||
| }, | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider extracting hardcoded URLs to environment variables or configuration files
These Supabase URLs contain authentication tokens that will expire. For better maintainability and security, consider:
- Moving these URLs to environment variables
- Using URL generation functions that fetch fresh tokens when needed
- Documenting the expiration date of these tokens
| return { | ||
| title: 'Heeyy :333', | ||
| body: { | ||
| text: `Details ... | ||
|
|
||
| 📍 Richcraft Hall (1125 Colonel By Dr, Carleton University) | ||
| ⌛ March 14-16 (Check-in from 5pm - 9pm) | ||
|
|
||
| 🥗 Food + Merch will be provided for the first 300 hackers on site!<br> | ||
| What to bring? | ||
|
|
||
| 🪪 Government ID | ||
| 💻 Your laptop + charger | ||
| 🛏 Sleeping bags | ||
| 🪥 Personal hygiene (plz bring deodorant, plz ...) | ||
| 🥤 Reusable water bottle | ||
| 🤪 A positive attitude!`, | ||
|
|
||
| buttonText: 'JOIN DISCORD ↗', | ||
| buttonLink: 'https://discord.gg/VnbWdAe8kA', | ||
|
|
||
| secondButtonText: '+ ADD TO CALENDAR', | ||
| secondButtonLink: 'https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MDY1bnFyb3B2c2Z1NjM4dDRxODcybmhtYWwgZmNkYmEzZjM1NGQ0ZTAxNTUyZTI0OTVkNzQzMTA1YmQ5ZWZjZTRlMTA3NmNkYTNjMWVjMTQ1NGQwZjVmYWE3M0Bn&tmsrc=fcdba3f354d4e01552e2495d743105bd9efce4e1076cda3c1ec1454d0f5faa73%40group.calendar.google.com', | ||
|
|
||
| events: [ | ||
| { | ||
| title: 'Upcoming events...', | ||
| }, | ||
| { | ||
| title: 'Intro to QNX', | ||
| text: 'Learn about QNX with 3 awesome speakers + meet some interns. Make sure you bring your laptop. Happening March 11th, 6:30 to 8:00 pm. Register now!', | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER ↗", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSfni0BpF_2vf9xM02Ux4t979C_jVVXLvxVDv0u1hrrobVan-A/viewform" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Learn about Gadget", | ||
| text: "Gadget is hosting a Q&A webinar to teach students the basics of their platform. It's happening Wednesday, March 12th at 2:30pm. You must create an account using the link below. If you already have an account let Gadget know during the webinar.", | ||
| buttons: [ | ||
| { | ||
| text: "CREATE ACCOUNT ↗", | ||
| link: "https://app.gadget.dev/auth/login?returnTo=/auth/hackathon" | ||
| }, | ||
| { | ||
| text: "WEBINAR ↗", | ||
| link: "https://meet.google.com/ybk-wumk-hig" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| title: "Resume Roast", | ||
| text: "Get your resume roasted at cuHacking! Sign up below to be eligible. Brought to you by uO SESA", | ||
| buttons: [ | ||
| { | ||
| text: "REGISTER ↗", | ||
| link: "https://docs.google.com/forms/d/e/1FAIpQLSdljqp8Z6F6ADINhJergvcqJZ6aTYSW2SSSVrngsY6ltx1eHQ/viewform" | ||
| } | ||
| ] | ||
| }, | ||
| ], | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider externalizing email content for easier maintenance
The email content is hardcoded in the getGenericEmailConstants function. For better maintainability and to enable non-developers to update content:
- Consider moving this content to a database or CMS
- Alternatively, extract it to a separate configuration file
- Structure content to support i18n if internationalization might be needed
| @@ -0,0 +1,130 @@ | |||
| import { Banner, Content, Footer, Keyboard } from '@/email/components' | |||
| import { Container, Html, Section, Tailwind } from '@react-email/components' | |||
| import { link } from 'fs' | |||
There was a problem hiding this comment.
Remove unused import
The link import from fs is not used in this file and should be removed. Additionally, as flagged by static analysis, Node.js built-in modules should use the node: protocol.
-import { link } from 'fs'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { link } from 'fs' |
🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
Aashna-Verma
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
4d0ba24 to
33fbb45
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apps/email/templates/cuhacking/welcome.tsx (3)
1-3: 🧹 Nitpick (assertive)Remove unused import and use node: protocol if required
Currently,
import { link } from 'fs':
- is not used anywhere in this file
- should use the
node:prefix when importing a Node.js builtin moduleIf you don’t need it, remove it:
-import { link } from 'fs'Otherwise, update:
-import { link } from 'fs' +import { link } from 'node:fs'🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
107-107:⚠️ Potential issueUpdate body’s type definition
The
bodyprop is treated as an object (with keys liketext,buttonText) but typed asstring. This leads to type mismatches.Fix with:
-function Generic({ title, body } : { title: string, body: string }) { +function Generic({ + title, + body, +}: { + title: string, + body: { + text: string + buttonText?: string + buttonLink?: string + secondButtonText?: string + secondButtonLink?: string + events?: { + title?: string + text?: string + buttons?: { text: string; link: string }[] + }[] + } +}) {
5-36: 🛠️ Refactor suggestionStore tokens in environment variables
Hardcoding long-lived tokens can pose security risks, especially if the repository is public. Recommend storing these tokens in environment variables or using short-lived tokens.
Here’s an example of refactoring to environment variables (showing only relevant changes):
-const EMAIL_SUPABASE_URLS = { - headerBg: 'https://…token=eyJhbGciOiJI...', - footerBg: 'https://…token=eyJhbGciOiJI...', - ... -} +const EMAIL_SUPABASE_URLS = { + headerBg: process.env.NX_SUPABASE_HEADER_BG || '', + footerBg: process.env.NX_SUPABASE_FOOTER_BG || '', + ... +}libs/email/components/footer.tsx (1)
70-73: 🧹 Nitpick (assertive)
⚠️ Potential issueReplace placeholder unsubscribe link
This Rick Roll link is not a valid unsubscribe endpoint, which can violate email compliance. Replace it with a functioning unsubscribe link or a temporary placeholder that clearly indicates it is a WIP.
Here’s a minimal suggestion:
- href="https://www.youtube.com/watch?v=xvFZjo5PgG0" + href="https://unsubscribe-link.com"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/email/templates/cuhacking/welcome.tsx(1 hunks)libs/email/components/banner.tsx(2 hunks)libs/email/components/content.tsx(1 hunks)libs/email/components/footer.tsx(1 hunks)libs/email/components/keyboard.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
libs/email/components/footer.tsx (1)
Learnt from: HasithDeAlwis
PR: cuhacking/2025#308
File: tools/email-templates/components/email-footer.tsx:73-73
Timestamp: 2025-02-10T23:34:58.359Z
Learning: The unsubscribe link in tools/email-templates/components/email-footer.tsx is intentionally set to a placeholder URL ("https://unsubscribe-link.com") until the actual unsubscribe email functionality is implemented.
🪛 Biome (1.9.4)
apps/email/templates/cuhacking/welcome.tsx
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
libs/email/components/content.tsx
[error] 20-20: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 36-36: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 46-47: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
[error] 96-96: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.
(lint/a11y/noBlankTarget)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build (22.13.1)
🔇 Additional comments (5)
libs/email/components/banner.tsx (1)
7-7: Fix the parameter name to avoid confusion.
The repeated misspelling “bacgroundSrc” can be easily corrected to “backgroundSrc” for clarity.libs/email/components/content.tsx (2)
45-46: Verify fallback logic for the second button.
Usingbody.buttonTextas a fallback forbody.secondButtonLinkcan be confusing if the main button text is not a valid link.Do you want me to check the usage of these props throughout the codebase or open an issue to clarify this fallback?
93-96:❓ Verification inconclusive
Confirm interface alignment with
event.buttons.
Ensure thebuttonsas an array to avoid runtime errors when callingevent.buttons.map().
🏁 Script executed:
#!/bin/bash # Searching for any interface definitions referencing 'buttons' ast-grep --pattern $'interface $_ {\n $$$\n buttons?: $_\n $$$\n}'Length of output: 66
Verify Email Interface for
buttonsProperty Alignment
The current automated search didn’t return any interface definitions containing abuttonsproperty, so please manually verify that the Email interface (or the corresponding model) definesbuttonsas an array. This is necessary to avoid potential runtime errors when callingevent.buttons.map().🧰 Tools
🪛 Biome (1.9.4)
[error] 96-96: Avoid using target="_blank" without rel="noreferrer".
Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noreferrer" attribute.(lint/a11y/noBlankTarget)
apps/email/templates/cuhacking/welcome.tsx (1)
124-128: Use more descriptive component name [Duplicate of past suggestion]For consistency with other email templates (e.g.
RegisteredForHackathon), consider renamingPreviewto something likeWelcomeEmail.-export default function Preview() { +export default function WelcomeEmail() {libs/email/components/footer.tsx (1)
41-43: Add a unique key when mapping over socials [Duplicate of past suggestion]Each JSX element created in
socials.mapneeds akey:{socials.map((social, index) => ( - socialLinks(social) + <React.Fragment key={social.alt || index}> + {socialLinks(social)} + </React.Fragment> ))}
libs/email/components/keyboard.tsx
Outdated
| export function Keyboard({ keyboardSrc }: { keyboardSrc: string }) { | ||
| return ( | ||
| <div> | ||
| {/* Image Section */} | ||
| <div className="flex justify-center"> | ||
| <Img | ||
| src="https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/cuhacking/media/keyboard.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJjdWhhY2tpbmcvbWVkaWEva2V5Ym9hcmQucG5nIiwiaWF0IjoxNzQwOTEwMzM5LCJleHAiOjE3NzI0NDYzMzl9.I7WASWsZtIGfac67CbSRrMrIPrx0kYY8BgzOYl_A4Ok" | ||
| src={keyboardSrc} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding a fallback.
If keyboardSrc is undefined or empty, the <Img> component might break the layout. Adding a safe default source or a conditional check could help maintain resilience.
export function Keyboard({ keyboardSrc }: { keyboardSrc: string }) {
return (
<div>
{/* Image Section */}
<div className="flex justify-center">
<Img
- src={keyboardSrc}
+ src={keyboardSrc || "/images/keyboard-default.png"}
alt="Keyboard Image"
className="w-full max-w-xl h-auto mx-auto"
/>
</div>
</div>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function Keyboard({ keyboardSrc }: { keyboardSrc: string }) { | |
| return ( | |
| <div> | |
| {/* Image Section */} | |
| <div className="flex justify-center"> | |
| <Img | |
| src="https://rtmsxedfqcyusebbrull.supabase.co/storage/v1/object/sign/cuhacking/media/keyboard.png?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1cmwiOiJjdWhhY2tpbmcvbWVkaWEva2V5Ym9hcmQucG5nIiwiaWF0IjoxNzQwOTEwMzM5LCJleHAiOjE3NzI0NDYzMzl9.I7WASWsZtIGfac67CbSRrMrIPrx0kYY8BgzOYl_A4Ok" | |
| src={keyboardSrc} | |
| export function Keyboard({ keyboardSrc }: { keyboardSrc: string }) { | |
| return ( | |
| <div> | |
| {/* Image Section */} | |
| <div className="flex justify-center"> | |
| <Img | |
| src={keyboardSrc || "/images/keyboard-default.png"} | |
| alt="Keyboard Image" | |
| className="w-full max-w-xl h-auto mx-auto" | |
| /> | |
| </div> | |
| </div> | |
| ) | |
| } |
HasithDeAlwis
force-pushed
the
zrahim/feat/335-add-all-email-templates
branch
from
33fbb45 to
30c9de7
Compare
|
🎉 This PR is included in version 3.11.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Checklist
mainbefore creating this PR.Summary by CodeRabbit
New Features
Bannercomponent with a new background image for improved visual appeal.Footercomponent layout using a table structure for better organization.Bug Fixes
Style