Skip to content

Invitation email sending (only in development and only when RESEND_API_KEY is set now) #1513

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hoshinotsuyoshi
Copy link
Member

@hoshinotsuyoshi hoshinotsuyoshi commented Apr 25, 2025

Issue

Since I haven't set up a custom domain for Resend, this feature is enabled only in development and only when RESEND_API_KEY is set. 🙏
In all other cases, the behavior remains unchanged from before.

Why is this change needed?

To integrate Resend's API SDK and enable email invitations during local development.

What would you like reviewers to focus on?

  • Does the fallback behavior look safe in non-development environments?
  • Any concerns about the token handling or email template logic?

Testing Verification


verification:

  • Email sent ✅
スクリーンショット 2025-04-25 18 59 51
  • invitation url is correct ✅

What was done

🤖 Generated by PR Agent at c3d13bd

  • Implement organization invitation email sending via Resend API
    • Add sendInvitationEmail utility and integrate into invite flow
    • Only enabled in development with RESEND_API_KEY set
    • Email includes invitation link with token
  • Refactor and extend database function for invitations
    • invite_organization_member now returns invitation token in response
    • SQL logic updated to always generate and return a new token
  • Expand and improve database tests for invitation logic
    • Tests now verify presence and correctness of invitation token
    • Additional assertions for error handling and token behavior
  • Update dependencies and configuration
    • Add resend package and related dependencies
    • Add RESEND_API_KEY to .env.template

Detailed Changes

Relevant files
Enhancement
4 files
inviteMember.ts
Integrate invitation email sending after invite action     
+21/-1   
sendInvitationEmail.ts
Add utility to send invitation emails via Resend                 
+98/-0   
schema.sql
Update invite_organization_member to return invitation token
+21/-5   
20250425090250_add_token_to_invite_organization_member.sql
Migration: add token return to invite_organization_member
+112/-0 
Miscellaneous
2 files
20250423123348_invite_organization_member.sql
Add placeholder migration for production compatibility     
+2/-0     
20250423123350_refine_invite_organization_member.sql
Add placeholder migration for production compatibility     
+2/-96   
Tests
1 files
01-invite_organization_member.test.sql
Expand tests for invitation token and error handling         
+54/-18 
Configuration changes
1 files
.env.template
Add RESEND_API_KEY to environment template                             
+1/-0     
Dependencies
2 files
package.json
Add resend package to dependencies                                             
+1/-0     
pnpm-lock.yaml
Update lockfile for resend and related packages                   
+222/-2 

Additional Notes


Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @hoshinotsuyoshi hoshinotsuyoshi self-assigned this Apr 25, 2025
    Copy link

    changeset-bot bot commented Apr 25, 2025

    ⚠️ No Changeset found

    Latest commit: c3d13bd

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    Copy link

    vercel bot commented Apr 25, 2025

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    liam-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 10:37am
    liam-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 10:37am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 10:37am

    Copy link

    supabase bot commented Apr 25, 2025

    Updates to Preview Branch (invitation-email-sending) ↗︎

    Deployments Status Updated
    Database Fri, 25 Apr 2025 10:34:25 UTC
    Services Fri, 25 Apr 2025 10:34:25 UTC
    APIs Fri, 25 Apr 2025 10:34:25 UTC

    Tasks are run on every commit but only new migration files are pushed.
    Close and reopen this PR if you want to apply changes from existing seed or migration files.

    Tasks Status Updated
    Configurations Fri, 25 Apr 2025 10:34:28 UTC
    Migrations Fri, 25 Apr 2025 10:34:29 UTC
    Seeding Fri, 25 Apr 2025 10:34:29 UTC
    Edge Functions Fri, 25 Apr 2025 10:34:29 UTC

    View logs for this Workflow Run ↗︎.
    Learn more about Supabase for Git ↗︎.

    Copy link
    Contributor

    liam-migration bot commented Apr 25, 2025

    The migration updates the invitation workflow by adding a new token in the invite_organization_member function and integrating email sending via Resend. The most important concerns are the atomicity of the migration processes, data integrity during token updates, and potential security risks of exposing raw tokens. Overall, the changes follow project conventions and improve functionality, though clarifications on legacy migration file handling, token refresh behavior, and potential token hashing are recommended.

    Migration URL: https://liam-erd-web.vercel.app/app/projects/284f1bca-2633-4a93-9567-7bc98ab298f7/ref/invitation-email-sending/migrations/14c4a7ba-cf91-4c92-ae85-d57f65df06ed

    ER Diagram:

    - Added RESEND_API_KEY to .env.template
    - Implemented sendInvitationEmail utility to send invitation emails
    - Updated inviteMember action to trigger email sending after successful invitation
    @hoshinotsuyoshi hoshinotsuyoshi force-pushed the invitation-email-sending branch from f6e7e2f to c3d13bd Compare April 25, 2025 10:33
    Copy link

    This migration update adds a new invitation token to the database and integrates email sending using Resend while ensuring transactions and error handling. A key concern is the synchronous email sending which may impact performance and data integrity when re-inviting users, and additional clarifications on security practices would be valuable. Overall, the changes align well with project naming conventions and migration patterns, demonstrating a thoughtful approach to schema updates.

    Migration URL: https://liam-app-git-staging-route-06-core.vercel.app/app/projects/9d777f64-400a-42f3-a60e-98a59fc97279/ref/invitation-email-sending/migrations/95feb215-b6a1-4151-93c5-2c5e8c0c242c

    ER Diagram:

    @hoshinotsuyoshi hoshinotsuyoshi changed the title Invitation email sending (when development && RESEND_API_KEY set only ) Invitation email sending (only in development and only when RESEND_API_KEY is set now) Apr 25, 2025
    @hoshinotsuyoshi hoshinotsuyoshi marked this pull request as ready for review April 25, 2025 10:47
    @hoshinotsuyoshi hoshinotsuyoshi requested a review from a team as a code owner April 25, 2025 10:47
    @hoshinotsuyoshi hoshinotsuyoshi requested review from FunamaYukina, junkisai, MH4GF and NoritakaIkeda and removed request for a team April 25, 2025 10:47
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 Security concerns

    Sensitive information exposure:
    The code includes email sending functionality that handles invitation tokens. While the implementation generally follows good practices, there's a potential issue with how the invitation link is constructed. If the baseUrl variable is undefined and the fallback empty string is used, it could create an invalid URL that might lead users to unexpected domains if clicked from email clients that try to "fix" malformed URLs.

    ⚡ Recommended focus areas for review

    Email Security

    The email template uses direct string interpolation for organization name which could potentially lead to XSS if organization names contain malicious HTML.

    return `
      <!DOCTYPE html>
      <html>
        <head>
          <meta charset="utf-8">
          <title>Organization Invitation</title>
        </head>
        <body>
          <h1>You've been invited to join ${organizationName}</h1>
          <p>You have been invited to join ${organizationName} on Liam.</p>
          <p>Click the link below to accept the invitation:</p>
          <a href="${invitationLink}">Accept Invitation</a>
          <p>If you did not expect this invitation, you can safely ignore this email.</p>
        </body>
      </html>
    `
    Environment Handling

    The function returns success even when emails aren't sent in non-development environments, which might lead to confusion about whether invitations were actually delivered.

    if (process.env.NODE_ENV !== 'development' || !process.env.RESEND_API_KEY) {
      return { success: true, error: null }
    }
    URL Construction

    The baseUrl fallback logic could result in an empty string being used, potentially creating invalid invitation links.

    const invitationLink = `${baseUrl || ''}/app/invitations/tokens/${invitationToken}`

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate environment variables

    There's no validation that environment variables like NEXT_PUBLIC_BASE_URL and
    VERCEL_BRANCH_URL are defined before using them. If these variables are
    undefined, it could lead to invalid URLs.

    frontend/apps/app/features/organizations/actions/sendInvitationEmail.ts [61-72]

    -let baseUrl: string | undefined = undefined
    +let baseUrl: string = 'http://localhost:3001' // Default fallback
     switch (process.env.NEXT_PUBLIC_ENV_NAME) {
       case 'production':
    -    baseUrl = process.env.NEXT_PUBLIC_BASE_URL // NEXT_PUBLIC_BASE_URL includes "https://"
    +    baseUrl = process.env.NEXT_PUBLIC_BASE_URL || baseUrl // NEXT_PUBLIC_BASE_URL includes "https://"
         break
       case 'preview':
    -    baseUrl = `https://${process.env.VERCEL_BRANCH_URL}` // VERCEL_BRANCH_URL does not include "https://"
    -    break
    -  default:
    -    baseUrl = 'http://localhost:3001'
    +    baseUrl = process.env.VERCEL_BRANCH_URL ? `https://${process.env.VERCEL_BRANCH_URL}` : baseUrl // VERCEL_BRANCH_URL does not include "https://"
         break
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion addresses a significant issue by adding proper validation for environment variables that are critical for URL construction. This prevents potential runtime errors and ensures the application works correctly across different environments.

    Medium
    Fix potential invalid URL

    The baseUrl variable could be empty in some cases, which would result in an
    invalid URL. You should ensure baseUrl is always defined before constructing the
    invitation link.

    frontend/apps/app/features/organizations/actions/sendInvitationEmail.ts [74-75]

     // Construct invitation link
    -const invitationLink = `${baseUrl || ''}/app/invitations/tokens/${invitationToken}`
    +const invitationLink = `${baseUrl}/app/invitations/tokens/${invitationToken}`
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a potential issue where baseUrl could be undefined, resulting in an invalid URL. The fix ensures a valid URL by removing the fallback to an empty string, which is important for email functionality.

    Medium
    Handle missing organization name

    The email is being sent without validating that orgData.name exists. If the
    organization data retrieval was successful but the name is undefined, this could
    cause issues in the email template.

    frontend/apps/app/features/organizations/actions/sendInvitationEmail.ts [79-87]

     const { error: emailError } = await resend.emails.send({
       from: process.env.EMAIL_FROM_ADDRESS || '[email protected]',
       to: email,
    -  subject: `Invitation to join ${orgData.name} on Liam`,
    +  subject: `Invitation to join ${orgData.name || 'an organization'} on Liam`,
       html: InvitationEmail({
    -    organizationName: orgData.name,
    +    organizationName: orgData.name || 'an organization',
         invitationLink,
       }),
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    __

    Why: This suggestion adds a fallback value for organization name in case it's undefined, which improves error handling and user experience by ensuring the email still makes sense even if organization data is incomplete.

    Low
    Learned
    best practice
    Validate required parameters first

    Add validation for required parameters at the beginning of the function to
    prevent potential runtime errors. Check that email, organizationId, and
    invitationToken are all provided and valid before proceeding.

    frontend/apps/app/features/organizations/actions/sendInvitationEmail.ts [33-42]

     export const sendInvitationEmail = async ({
       email,
       organizationId,
       invitationToken,
     }: SendInvitationEmailParams) => {
    +  // Validate required parameters
    +  if (!email || !organizationId || !invitationToken) {
    +    return { success: false, error: 'Missing required parameters' }
    +  }
    +  
       // TODO: Enable email sending in preview/production
       // For now, we don't want to send emails in preview/production
       if (process.env.NODE_ENV !== 'development' || !process.env.RESEND_API_KEY) {
         return { success: true, error: null }
       }
    • Apply this suggestion
    Suggestion importance[1-10]: 6
    Low
    • More

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant