Skip to content

feat(api): added support for email preferences #978

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

Merged

Conversation

Allan2000-Git
Copy link
Contributor

@Allan2000-Git Allan2000-Git commented May 21, 2025

User description

Description

This PR adds support for user-configurable email preferences. Users can now choose which types of emails they want to receive: promotional, operational, and critical. This includes:

  • Adding an EmailPreference model linked to User in Prisma
  • Updating services to allow users to modify their preferences
  • Ensuring the mail service respects user preferences before sending emails. This includes:
    • invitedToWorkspace
    • removedFromWorkspace

Pending:
Integrating email preferences to other mail services

Fixes #122

Dependencies

N/A

Future Improvements

N/A

Mentions

@rajdip-b

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

PR Type

Enhancement, Tests


Description

  • Add EmailPreference model and schema to Prisma

    • Create migration and update user schema for preferences
    • Add one-to-one relation between User and EmailPreference
  • Integrate email preferences into user creation and update flows

    • Default preferences set on user creation
    • Update preferences via user update endpoints
  • Enforce email preferences in notification logic

    • Respect user opt-out for activity and critical emails
    • Update workspace membership service to check preferences
  • Update API schemas and e2e tests for email preferences

    • Add email preference fields to user types and test users
    • Update Zod schemas for API requests/responses

Changes walkthrough 📝

Relevant files
Enhancement
8 files
schema.prisma
Add EmailPreference model and user relation in Prisma schema
+16/-0   
migration.sql
Migration for EmailPreference table and indexes                   
+21/-0   
user.ts
Create user with default email preferences; include in fetch
+14/-1   
user.service.ts
Update user service to handle email preferences on update
+20/-4   
update.user.ts
Add DTO for updating email preferences in user update       
+22/-1   
user.types.ts
Extend user types with EmailPreference                                     
+2/-1     
workspace-membership.service.ts
Enforce email preferences for workspace notifications       
+14/-0   
index.ts
Add emailPreference to UserSchema and update request schema
+14/-2   
Tests
10 files
api-key.e2e.spec.ts
Add emailPreference to test user objects                                 
+13/-1   
environment.e2e.spec.ts
Add emailPreference to test user objects                                 
+26/-2   
event.e2e.spec.ts
Add emailPreference to test user objects                                 
+13/-1   
integration.e2e.spec.ts
Add emailPreference to test user objects                                 
+26/-2   
project.e2e.spec.ts
Add emailPreference to test user objects                                 
+39/-3   
secret.e2e.spec.ts
Add emailPreference to test user objects                                 
+26/-2   
variable.e2e.spec.ts
Add emailPreference to test user objects                                 
+26/-2   
workspace-membership.e2e.spec.ts
Add emailPreference to test user objects                                 
+39/-3   
workspace-role.e2e.spec.ts
Add emailPreference to test user objects                                 
+39/-3   
workspace.e2e.spec.ts
Add emailPreference to test user objects                                 
+26/-2   

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    codiumai-pr-agent-free bot commented May 21, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit d7bdb9e)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    122 - Partially compliant

    Compliant requirements:

    • Add a model in prisma schema named EmailPreference and tie it with User in a one-to-one relation
    • Add relevant field names to store the preferences for promotional, operational, and critical emails

    Non-compliant requirements:

    • Make the mail service check the preference of the receiver before sending emails

    Requires further human verification:

    • The PR is marked as WIP and explicitly states that integrating email preferences to other mail services is pending
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Incomplete Implementation

    The update user function is missing the data parameter in the prisma.user.update call, which could cause the update to fail or not apply changes properly.

    const updatedUser = await this.prisma.user.update({
      where: {
        id: user.id
      },
      data: {
        name: dto?.name,
        profilePictureUrl: dto?.profilePictureUrl,
        isOnboardingFinished: dto.isOnboardingFinished,
        emailPreference: {
          update: {
            marketing: dto.emailPreferences?.marketing,
            activity: dto.emailPreferences?.activity,
            critical: dto.emailPreferences?.critical
          }
        }
      },
      include: {
        emailPreference: true
      }
    })
    Incorrect User Reference

    User2 is incorrectly using createUser1's data instead of createUser2's data, which could lead to test failures or incorrect test behavior.

    user2 = {
      ...createUser1,
      ipAddress: USER_IP_ADDRESS,
      emailPreference: {
        id: expect.any(String),
        userId: createUser1.id,
        marketing: true,
        activity: true,
        critical: true,
        createdAt: expect.any(Date),
        updatedAt: expect.any(Date)
      }
    }

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented May 21, 2025

    PR Code Suggestions ✨

    Latest suggestions up to d7bdb9e
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix incorrect user reference

    The test is incorrectly using createUser1 data for user2 object. This will cause
    the test to have incorrect user ID references. Replace createUser1 with
    createUser2 to ensure the test uses the correct user data.

    apps/api/src/integration/integration.e2e.spec.ts [114-126]

     user2 = {
    -  ...createUser1,
    +  ...createUser2,
       ipAddress: USER_IP_ADDRESS,
       emailPreference: {
         id: expect.any(String),
    -    userId: createUser1.id,
    +    userId: createUser2.id,
         marketing: true,
         activity: true,
         critical: true,
         createdAt: expect.any(Date),
         updatedAt: expect.any(Date)
       }
     }
    • Apply / Chat
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies a critical bug where user2 is constructed using createUser1's data, leading to incorrect user identity and potential test failures or misleading results. Fixing this is essential for test correctness.

    High
    Add missing optional chaining

    The code is trying to access dto.isOnboardingFinished directly but uses optional
    chaining for other properties. This could cause runtime errors if dto is
    undefined. Add optional chaining to this property access as well.

    apps/api/src/user/user.service.ts [101-112]

     data: {
       name: dto?.name,
       profilePictureUrl: dto?.profilePictureUrl,
    -  isOnboardingFinished: dto.isOnboardingFinished,
    +  isOnboardingFinished: dto?.isOnboardingFinished,
       emailPreference: {
         update: {
           marketing: dto.emailPreferences?.marketing,
           activity: dto.emailPreferences?.activity,
           critical: dto.emailPreferences?.critical
         }
       }
     },
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: Adding optional chaining to dto.isOnboardingFinished is a minor but reasonable defensive improvement for consistency and safety, though in practice dto should always be defined in this context. The impact is moderate.

    Low
    • Update

    Previous suggestions

    ✅ Suggestions up to commit 15c5233
    CategorySuggestion                                                                                                                                    Impact
    General
    Remove redundant database query
    Suggestion Impact:The commit removed the redundant database query for email preferences as suggested and replaced the direct assignment of emailPreference with user.emailPreference in two places

    code diff:

    -
    -  logger.log(`Retrieving email preference for user ${user.id}`)
    -  const emailPreference = await prisma.emailPreference.findUnique({
    -    where: {
    -      userId: user.id
    -    }
    -  })
       logger.log(`Email preference for user ${user.id} has been retrieved.`)
     
       if (user.isAdmin) {
    @@ -165,7 +158,7 @@
         return {
           ...user,
           defaultWorkspace: null,
    -      emailPreference
    +      emailPreference: user.emailPreference
         }
       } else {
         logger.log(
    @@ -204,7 +197,7 @@
         return {
           ...user,
           defaultWorkspace,
    -      emailPreference
    +      emailPreference: user.emailPreference
         }

    This code redundantly fetches email preferences that were already included in
    the user query above. Since user.emailPreference is already available from the
    previous query, this additional database call is unnecessary.

    apps/api/src/common/user.ts [155-161]

    -logger.log(`Retrieving email preference for user ${user.id}`)
    -const emailPreference = await prisma.emailPreference.findUnique({
    -  where: {
    -    userId: user.id
    -  }
    -})
    -logger.log(`Email preference for user ${user.id} has been retrieved.`)
    +logger.log(`Email preference for user ${user.id} is already available.`)
    +const emailPreference = user.emailPreference

    [Suggestion processed]

    Suggestion importance[1-10]: 8

    __

    Why: The suggestion accurately points out a redundant database call, as the email preference is already loaded with the user. Removing this unnecessary query improves performance and reduces database load, making this a high-impact optimization.

    Medium
    Fix misleading error message
    Suggestion Impact:The commit implemented the exact changes suggested, replacing 'invitations' with 'critical notifications' in both the log message and the error message to make them more accurate

    code diff:

    -      this.log.log(`User ${member.id} has opted out of receiving invitations`)
    +      this.log.log(
    +        `User ${member.id} has opted out of receiving critical notifications`
    +      )
           throw new BadRequestException(
             constructErrorBody(
               'User has opted out',
    -          'The user has opted out of receiving invitations'
    +          'The user has opted out of receiving critical notifications'
             )

    The error message is misleading. Since you're checking the critical preference,
    the error should indicate that the user has opted out of critical notifications,
    not invitations in general.

    apps/api/src/workspace-membership/workspace-membership.service.ts [911-922]

     if (
       userWithWorkspace.emailPreference &&
       !userWithWorkspace.emailPreference.critical
     ) {
    -  this.log.log(`User ${member.id} has opted out of receiving invitations`)
    +  this.log.log(`User ${member.id} has opted out of receiving critical notifications`)
       throw new BadRequestException(
         constructErrorBody(
           'User has opted out',
    -      'The user has opted out of receiving invitations'
    +      'The user has opted out of receiving critical notifications'
         )
       )
     }

    [Suggestion processed]

    Suggestion importance[1-10]: 6

    __

    Why: The suggestion improves the clarity of the error message to accurately reflect the user's notification preferences. While this enhances user experience and debugging, it does not affect core functionality, so the impact is moderate.

    Low
    Possible issue
    Cache updated data
    Suggestion Impact:The commit implemented exactly what was suggested - replacing user.emailPreference with updatedEmailPreferences || user.emailPreference in the cache.setUser call to ensure the cache contains the most up-to-date email preferences

    code diff:

         await this.cache.setUser({
           ...updatedUser,
           defaultWorkspace: user.defaultWorkspace,
    -      emailPreference: user.emailPreference
    +      emailPreference: updatedEmailPreferences || user.emailPreference
         })

    Use the updated email preferences in the cache instead of the old ones. The
    current code is caching the old email preferences (user.emailPreference) instead
    of the newly updated ones (updatedEmailPreferences).

    apps/api/src/user/user.service.ts [124-128]

     await this.cache.setUser({
       ...updatedUser,
       defaultWorkspace: user.defaultWorkspace,
    -  emailPreference: user.emailPreference
    +  emailPreference: updatedEmailPreferences || user.emailPreference
     })
    Suggestion importance[1-10]: 7

    __

    Why: This suggestion correctly identifies that the cache should use the updated email preferences if available, rather than the potentially stale ones from the user object. This improves data consistency but is not a critical bug, so a score of 7 is appropriate.

    Medium

    @Allan2000-Git Allan2000-Git marked this pull request as draft May 21, 2025 18:27
    Copy link

    @Allan2000-Git, please resolve all open reviews!

    @Allan2000-Git Allan2000-Git marked this pull request as ready for review May 24, 2025 13:04
    @Allan2000-Git Allan2000-Git changed the title feat(api): added support for email preferences [WIP] feat(api): added support for email preferences May 24, 2025
    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The tests are failing

    Copy link

    @Allan2000-Git, please resolve all open reviews; otherwise this PR will be closed after Wed May 28 2025 18:24:54 GMT+0000 (Coordinated Universal Time)!

    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Approving this for now. We need a dedicated notification service to take care of these.

    @rajdip-b rajdip-b merged commit f7e5028 into keyshade-xyz:develop Jun 4, 2025
    5 checks passed
    rajdip-b pushed a commit that referenced this pull request Jun 4, 2025
    ## [2.23.0-stage.1](v2.22.3-stage.3...v2.23.0-stage.1) (2025-06-04)
    
    ### 🚀 Features
    
    * **api:** added support for email preferences ([#978](#978)) ([f7e5028](f7e5028))
    @rajdip-b
    Copy link
    Member

    rajdip-b commented Jun 4, 2025

    🎉 This PR is included in version 2.23.0-stage.1 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    rajdip-b pushed a commit that referenced this pull request Jun 5, 2025
    ## [2.23.0](v2.22.2...v2.23.0) (2025-06-05)
    
    ### 🚀 Features
    
    * **api:** add cross-platform sleep utility for e2e preparation ([#985](#985)) ([befb695](befb695))
    * **api:** added support for email preferences ([#978](#978)) ([f7e5028](f7e5028))
    * **platform:** Integration setup ([#976](#976)) ([ac98f27](ac98f27))
    
    ### 🐛 Bug Fixes
    
    * **cli:** keyshade run wouldn't decrypt encrypted [secure]s ([13a9c11](13a9c11))
    * **platform:** Suspense boundary fix ([#983](#983)) ([dedba9b](dedba9b))
    * **web:** Broken blog link on the footer > web page ([#980](#980)) ([8dac31b](8dac31b))
    
    ### 📚 Documentation
    
    * **cli:** add download badge ([9999ec8](9999ec8))
    
    ### 🔧 Miscellaneous Chores
    
    * **release:** 2.22.3-stage.1 [skip ci] ([73cc605](73cc605))
    * **release:** 2.22.3-stage.2 [skip ci] ([b2a605e](b2a605e))
    * **release:** 2.22.3-stage.3 [skip ci] ([3acae33](3acae33))
    * **release:** 2.23.0-stage.1 [skip ci] ([b7ff3f7](b7ff3f7))
    * **release:** 2.23.0-stage.2 [skip ci] ([291b22d](291b22d))
    * **release:** 2.23.0-stage.3 [skip ci] ([81fc3b2](81fc3b2))
    
    ### 🔨 Code Refactoring
    
    * **platform:** Improve readability and maintainability ([#977](#977)) ([be4bad0](be4bad0))
    @rajdip-b
    Copy link
    Member

    rajdip-b commented Jun 5, 2025

    🎉 This PR is included in version 2.23.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    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.

    Ability to opt out of email updates
    2 participants