Con Reminder Emails (Continued) #924
Conversation
|
Warning Rate limit exceeded@helloanil has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 51 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes primarily enhance the ReminderEmailsController and ReminderEmailsService by adding new functionalities for sending reminders to unmatched mentees and pending mentorships. Variable names in ConMentorshipMatchesService have been updated for clarity, and additional utility functions have been introduced for better data handling in ReminderEmailsService. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant ReminderEmailsController
participant ReminderEmailsService
participant ConMentorshipMatchesService
Admin ->> ReminderEmailsController: sendUnmatchedMenteesReminder()
ReminderEmailsController ->> ReminderEmailsService: getUnmatchedMenteesFor45Days()
ReminderEmailsService ->> ConMentorshipMatchesService: Retrieve Mentees
ConMentorshipMatchesService -->> ReminderEmailsService: Return Mentees List
ReminderEmailsService ->> ReminderEmailsController: Send Emails and Return Count
ReminderEmailsController -->> Admin: Platform Update Reminder Emails Sent
Admin ->> ReminderEmailsController: sendPendingMentorshipsReminder()
ReminderEmailsController ->> ReminderEmailsService: getPendingMenteeApplications()
ReminderEmailsService ->> ConMentorshipMatchesService: Retrieve Pending Applications
ConMentorshipMatchesService -->> ReminderEmailsService: Return Applications List
ReminderEmailsService ->> ReminderEmailsController: Send Emails and Return Count
ReminderEmailsController -->> Admin: Pending Mentorship Reminder Emails Sent
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (4 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (8 hunks)
Additional comments not posted (13)
apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (6)
30-30: Ensure consistent message formatting.The message string should consistently use either 'sent' or 'Sent' across all endpoints for uniformity.
53-53: Ensure consistent message formatting.The message string should consistently use either 'sent' or 'Sent' across all endpoints for uniformity.
97-97: Ensure consistent message formatting.The message string should consistently use either 'sent' or 'Sent' across all endpoints for uniformity.
129-131: Ensure consistent message formatting.The message string should consistently use either 'sent' or 'Sent' across all endpoints for uniformity. Additionally, consider using template literals for better readability and maintainability when constructing response messages.
157-174: Review logic for sending emails to mentors with pending applications.Ensure that the method
sendMentorPendingApplicationReminderis correctly implemented and that it handles all edge cases, such as no pending applications. Additionally, verify that the method includes all necessary error handling and data validation.Verification successful
Review logic for sending emails to mentors with pending applications.
The method
sendMentorPendingApplicationReminderis implemented in theReminderEmailsServiceclass. It handles the process of sending reminder emails to mentors with pending applications. The method includes fetching an email template, sanitizing the subject and HTML content, and sending the email using AWS SES. The implementation appears to handle potential errors, such as missing email templates.
- Location:
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts(lines 600-649)The implementation seems robust and includes necessary error handling and data validation.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `sendMentorPendingApplicationReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsController { $$$ sendMentorPendingApplicationReminder($_) { $$$ } $$$ }'Length of output: 146
Script:
#!/bin/bash # Description: Locate the implementation of `sendMentorPendingApplicationReminder` method in the codebase. ast-grep --lang typescript --pattern $'sendMentorPendingApplicationReminder($_) { $$$ }'Length of output: 564
Script:
#!/bin/bash # Description: Locate the implementation of `sendMentorPendingApplicationReminder` method in the codebase. ast-grep --lang typescript --pattern 'function sendMentorPendingApplicationReminder'Length of output: 431
Script:
#!/bin/bash # Description: Locate the implementation of `sendMentorPendingApplicationReminder` method as a class method in the codebase. ast-grep --lang typescript --pattern $'class $_ { $$$ sendMentorPendingApplicationReminder($_) { $$$ } $$$ }'Length of output: 61647
135-154: Review logic for sending emails to unmatched mentees.Ensure the logic correctly handles scenarios where no mentees are unmatched for 45 days to avoid unnecessary processing. Additionally, confirm that the method
sendMenteesPlatformAndNewMentorsReminderis correctly implemented and invoked with the appropriate parameters.Verification successful
Review logic for sending emails to unmatched mentees.
The
sendMenteesPlatformAndNewMentorsRemindermethod is correctly implemented in theReminderEmailsServiceclass and appropriately invoked with the required parameters. The logic handles scenarios where no mentees are unmatched for 45 days, thus avoiding unnecessary processing.
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts, lines 502-543: Implementation ofsendMenteesPlatformAndNewMentorsReminder.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `sendMenteesPlatformAndNewMentorsReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsController { $$$ sendMenteesPlatformAndNewMentorsReminder($_, $_) { $$$ } $$$ }'Length of output: 154
Script:
#!/bin/bash # Description: Broad search for the `sendMenteesPlatformAndNewMentorsReminder` method across the entire codebase. ast-grep --lang typescript --pattern $'function sendMenteesPlatformAndNewMentorsReminder($_, $_) { $$$ }'Length of output: 108
Script:
#!/bin/bash # Description: Search for method definitions named `sendMenteesPlatformAndNewMentorsReminder` across the entire codebase. ast-grep --lang typescript --pattern $'class $_ { $$$ sendMenteesPlatformAndNewMentorsReminder($_, $_) { $$$ } $$$ }'Length of output: 130
Script:
#!/bin/bash # Description: Search for the `sendMenteesPlatformAndNewMentorsReminder` method name across the entire codebase. rg 'sendMenteesPlatformAndNewMentorsReminder'Length of output: 330
Script:
#!/bin/bash # Description: Extract the implementation of `sendMenteesPlatformAndNewMentorsReminder` from `reminder-emails.service.ts`. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ async sendMenteesPlatformAndNewMentorsReminder({ email, firstName }) { $$$ } $$$ }' apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 60509
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (7)
600-649: Review email sending logic for pending mentorship application reminders.Ensure that the
sendMentorPendingApplicationRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.Verification successful
BCC and Source Formatting Verified
The BCC address is set to
SENDER_EMAILand the source is formatted as${SENDER_NAME} <${SENDER_EMAIL}>, which are correctly defined as:
SENDER_EMAIL = 'career@redi-school.org'SENDER_NAME = 'ReDI Talent Success Team'These values are appropriate for the intended use.
apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the BCC and source formatting in `sendMentorPendingApplicationReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendMentorPendingApplicationReminder($_) { $$$ } $$$ }'Length of output: 61668
Script:
#!/bin/bash # Search for the definition of SENDER_EMAIL and SENDER_NAME in the codebase rg 'SENDER_EMAIL' --type=ts rg 'SENDER_NAME' --type=tsLength of output: 1908
545-598: Review email sending logic for logging mentoring sessions reminders.Ensure that the
sendLogMentoringSessionsRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.Verification successful
Verification of
sendLogMentoringSessionsReminderMethod:
- The
sendLogMentoringSessionsRemindermethod does not include BCC addresses in its email parameters.- The source address is correctly formatted as
career@redi-school.org.No issues were found regarding the BCC addresses (as they are not present) and the source formatting.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the BCC and source formatting in `sendLogMentoringSessionsReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendLogMentoringSessionsReminder($_, $_, $_, $_) { $$$ } $$$ }'Length of output: 151
Script:
#!/bin/bash # Description: Extract the `sendLogMentoringSessionsReminder` method and inspect email parameters for BCC and source formatting. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendLogMentoringSessionsReminder({ email, firstName, userType, mentorshipMatchAgeInDays, }) { $$$ } $$$ }' --jsonLength of output: 38904
483-483: Review email sending logic for mentorship follow-up reminders.Ensure that the
sendMentorshipFollowUpRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.Also applies to: 491-491
Verification successful
BCC and Source Formatting Verified
The
sendMentorshipFollowUpRemindermethod correctly constructs the BCC address and source formatting for the email.
- BCC Address:
[SENDER_EMAIL]- Source:
${SENDER_NAME} <${SENDER_EMAIL}>Both the BCC address and source formatting are appropriate for the intended use.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the BCC and source formatting in `sendMentorshipFollowUpReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendMentorshipFollowUpReminder($_, $_, $_, $_) { $$$ } $$$ }'Length of output: 149
Script:
#!/bin/bash # Description: Locate the `sendMentorshipFollowUpReminder` method and verify the BCC and source formatting. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendMentorshipFollowUpReminder($$$) { $$$ } $$$ }'Length of output: 61663
431-431: Review email sending logic for apply to mentor reminders.Ensure that the
sendApplyToMentorRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.Also applies to: 439-439
Verification successful
To proceed with the verification, we need to gather information about the definitions of
SENDER_EMAILandSENDER_NAMEto ensure they are correctly set up.
Verification successful for email sending logic in
sendApplyToMentorReminder.The
sendApplyToMentorRemindermethod correctly constructs and sends emails with appropriate BCC addresses and source formatting.
SENDER_EMAILis defined as 'career@redi-school.org'.SENDER_NAMEis defined as 'ReDI Talent Success Team'.- The BCC address and source are correctly set in the email parameters.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the BCC and source formatting in `sendApplyToMentorReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendApplyToMentorReminder($_) { $$$ } $$$ }'Length of output: 61657
Script:
#!/bin/bash # Searching for the definitions of SENDER_EMAIL and SENDER_NAME in the codebase rg 'SENDER_EMAIL' --type ts rg 'SENDER_NAME' --type tsLength of output: 1908
502-543: Review email sending logic for mentees platform and new mentors reminders.Ensure that the
sendMenteesPlatformAndNewMentorsRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.
373-373: Review email sending logic for complete profile reminders.Ensure that the
sendCompleteProfileRemindermethod correctly constructs and sends emails. Verify that the BCC addresses and source formatting are appropriate for the intended use.Also applies to: 381-381
Verification successful
Verification successful: BCC and source formatting in
sendCompleteProfileRemindermethod.The
sendCompleteProfileRemindermethod correctly sets theBccAddressesto[SENDER_EMAIL]and theSourceto${SENDER_NAME} <${SENDER_EMAIL}>. These settings are appropriate for the intended use.
- Location:
apps/nestjs-api/src/reminder-emails/reminder-emails.service.tslines 373 and 381.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the BCC and source formatting in `sendCompleteProfileReminder` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ sendCompleteProfileReminder($_, $_, $_) { $$$ } $$$ }'Length of output: 142
Script:
#!/bin/bash # Searching for the sendCompleteProfileReminder method in reminder-emails.service.ts file to verify BCC and source formatting. fd reminder-emails.service.ts | xargs rg 'sendCompleteProfileReminder'Length of output: 108
Script:
#!/bin/bash # Extracting the full implementation of the sendCompleteProfileReminder method from reminder-emails.service.ts for review. fd reminder-emails.service.ts | xargs rg -A 20 'async sendCompleteProfileReminder'Length of output: 627
Script:
#!/bin/bash # Extracting the full implementation of the sendCompleteProfileReminder method from reminder-emails.service.ts for review by increasing the number of lines. fd reminder-emails.service.ts | xargs rg -A 50 'async sendCompleteProfileReminder'Length of output: 1397
270-335: Review logic for fetching pending mentorship matches.Ensure the method
getPendingMentorshipMatchescorrectly fetches matches based on the specified criteria. Verify that the method handles scenarios where no matches meet the criteria and that the transformation logic is correct and efficient.Verification successful
Review logic for fetching pending mentorship matches.
The method
getPendingMentorshipMatchesinreminder-emails.service.tscorrectly implements the logic for fetching mentorship matches, reducing them, fetching profiles, and transforming the reduced matches. Each part of the method has been verified:
- Fetching mentorship matches: The method fetches matches with the specified criteria.
- Reducing matches: The matches are correctly reduced to a simpler structure.
- Fetching profiles: Profiles for the mentees and mentors are fetched based on the reduced matches.
- Transforming reduced matches: The reduced matches are transformed to include additional details from the profiles.
All parts of the method have been confirmed to work as intended.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the transformation logic in `getPendingMentorshipMatches` method. ast-grep --lang typescript --pattern $'class ReminderEmailsService { $$$ getPendingMentorshipMatches() { $$$ } $$$ }'Length of output: 61657
Script:
#!/bin/bash # Description: Verify the logic within the `getPendingMentorshipMatches` method in `reminder-emails.service.ts`. ast-grep --lang typescript --pattern $'async getPendingMentorshipMatches() { const threeMonthsOldMentorshipMatches = await this.conMentorshipMatchesService.findAll({ Mentorship_Match_Age_In_Days__c: 13, Status__c: MentorshipMatchStatus.APPLIED, }) const reducedMatches = threeMonthsOldMentorshipMatches.reduce( (acc, match) => { acc[match.props.id] = { matchDate: match.props.createdAt, menteeId: match.props.menteeId, mentorId: match.props.mentorId, } return acc }, {} ) const menteeIds = threeMonthsOldMentorshipMatches.map( (match) => match.props.menteeId ) const mentorIds = threeMonthsOldMentorshipMatches.map( (match) => match.props.mentorId ) let matchConProfiles: ConProfileEntity[] = [] if ([...menteeIds, ...mentorIds].length > 0) { matchConProfiles = await this.conProfilesService.findAll({ \'Contact__r.Id\': { $in: [...menteeIds, ...mentorIds], }, }) } const transformedReducedMatches = transform( reducedMatches, (result, value, key) => { const mentee = matchConProfiles.find( (profile) => profile?.props.userId === value[\'menteeId\'] ) const mentor = matchConProfiles.find( (profile) => profile?.props.userId === value[\'mentorId\'] ) result[key] = { mentorEmail: mentor?.props.email, mentorFirstName: mentor?.props.firstName, menteeFirstName: mentee?.props.firstName, menteeFullName: mentee?.props.fullName, matchDate: formatDate(new Date(value[\'matchDate\']), \'PPP\'), } }, [] ) return transformedReducedMatches }' ./apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 3813
Script:
#!/bin/bash # Description: Verify the logic within the `getPendingMentorshipMatches` method in `reminder-emails.service.ts`. # Check fetching mentorship matches ast-grep --lang typescript --pattern $'async getPendingMentorshipMatches() { const threeMonthsOldMentorshipMatches = await this.conMentorshipMatchesService.findAll({ Mentorship_Match_Age_In_Days__c: 13, Status__c: MentorshipMatchStatus.APPLIED, })' ./apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts # Check reducing matches ast-grep --lang typescript --pattern $'const reducedMatches = threeMonthsOldMentorshipMatches.reduce( (acc, match) => { acc[match.props.id] = { matchDate: match.props.createdAt, menteeId: match.props.menteeId, mentorId: match.props.mentorId, } return acc }, {} )' ./apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts # Check fetching profiles ast-grep --lang typescript --pattern $'if ([...menteeIds, ...mentorIds].length > 0) { matchConProfiles = await this.conProfilesService.findAll({ \'Contact__r.Id\': { $in: [...menteeIds, ...mentorIds], }, }) }' ./apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts # Check transforming reduced matches ast-grep --lang typescript --pattern $'const transformedReducedMatches = transform( reducedMatches, (result, value, key) => { const mentee = matchConProfiles.find( (profile) => profile?.props.userId === value[\'menteeId\'] ) const mentor = matchConProfiles.find( (profile) => profile?.props.userId === value[\'mentorId\'] ) result[key] = { mentorEmail: mentor?.props.email, mentorFirstName: mentor?.props.firstName, menteeFirstName: mentee?.props.firstName, menteeFullName: mentee?.props.fullName, matchDate: formatDate(new Date(value[\'matchDate\']), \'PPP\'), } }, [] )' ./apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 6717
|
Hi @helloanil , thank you! |
Hi @astkhikatredi, it's because of Gmail. When the same email is received many times, Gmail stacks them and show some of them in purple. It won't happen to users. |
apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts
Outdated
Show resolved
Hide resolved
| const menteeIdsWithoutMentorshipMatchOrApplied = union( | ||
| difference(approvedMenteeIds, mentorshipMatchMenteeIds), | ||
| appliedMentorshipMatchMenteeIds | ||
| ) |
There was a problem hiding this comment.
We want to send a Platform And New Mentors Reminder to mentees approved 45 days ago (or is it actually - approved >= 45 days ago?) AND who have no match AND no pending applications sent.
Do we check also mentees' pending applications here or am I confused with the naming?
There was a problem hiding this comment.
By here I meant the getUnmatchedMenteesFor45Days function.
There was a problem hiding this comment.
Hey Kate, let me explain how this works:
First we find all approved mentees exactly 45 days ago. But of course some of them have already applied for mentorships during that 45 days period, so the "difference" function takes care of that. It finds the mentees that are approved BUT don't have any mentorship match yet.
The problem is, having a mentorship match doesn't mean they have a running mentorship, because fetching mentorship matches from Salesforce also returns the applications. So the "difference" unfortunately removes the mentees with active applications too. To fix that, I'm using "union" to merge the first set of mentees with the mentees that have "Applied" status mentorship matches.
Hope it's clear.
There was a problem hiding this comment.
@helloanil, the variables' names are confusing for me. It makes it hard to understand the logic. I don't understand which of them return matches and which return applications.
Status__c: MentorshipMatchStatus.APPLIED - is a pending application for me
Status__c: MentorshipMatchStatus.ACCEPTED - is a match for me
For example, this
is pending applications for me, not mentorship matches.
Can we improve the naming of the variables in this function (including comments)?
There was a problem hiding this comment.
The problem is, either it's "Status: Applied" or "Status: Accepted", we create a "MentorshipMatch" object for a Mentee/Mentor tuple, hence even in the Applied status, it's a "Mentee with Mentorship Match"
But fair, let me try an attempt to improve them 👍
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/nestjs-api/src/con-mentorship-matches/con-mentorship-matches.service.ts (1)
Line range hint
45-47: Remove unnecessary else clause.The else clause can be omitted because the previous branches break early.
- } else { - throw new NotFoundException('ConMentorshipMatch not found') - } + } + throw new NotFoundException('ConMentorshipMatch not found')
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/nestjs-api/src/con-mentorship-matches/con-mentorship-matches.service.ts (1 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (4 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (8 hunks)
Additional context used
Biome
apps/nestjs-api/src/con-mentorship-matches/con-mentorship-matches.service.ts
[error] 45-47: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
| @@ -105,12 +105,12 @@ export class ConMentorshipMatchesService { | |||
| rediLocation: menteeProfile.props.rediLocation, | |||
| }) | |||
|
|
|||
| const menteePendingMentorshipMatches = await this.findAll({ | |||
| const menteependingMenteeApplications = await this.findAll({ | |||
There was a problem hiding this comment.
The camel case got broken here. Please also check my comment below regarding the naming of this variable.
| Mentee__c: menteeProfile.props.userId, | ||
| Status__c: MentorshipMatchStatus.APPLIED, | ||
| }) | ||
|
|
||
| menteePendingMentorshipMatches.forEach(async (match) => { | ||
| menteependingMenteeApplications.forEach(async (match) => { | ||
| match.props.status = | ||
| MentorshipMatchStatus.INVALIDATED_AS_OTHER_MENTOR_ACCEPTED |
There was a problem hiding this comment.
I am super confused by the naming in the acceptMentorship function. I had to checkout the branch and read the code line by line.
Does mentorshipMatch = application here?
Does match = application here?
Aren't we breaking the sendNotificationToMentorThatPendingApplicationExpiredSinceOtherMentorAccepted by using pendingMenteeApplications array?
To send this email, we want to filter out the applications with the Invalidated status here. But this
gives us an array of applications with Applied status only. Shouldn't we iterate on the array of all applications instead?
| await this.reminderEmailsService.getpendingMenteeApplications() | ||
|
|
||
| if (Object.keys(pendingMenteeApplications).length > 0) { | ||
| Object.keys(pendingMenteeApplications).forEach(async (match) => { |
There was a problem hiding this comment.
Again the confusing match word here. Shouldn't it be an application?
| @@ -201,6 +267,73 @@ export class ReminderEmailsService { | |||
| // return [] | |||
| // } | |||
|
|
|||
| async getpendingMenteeApplications() { | |||
There was a problem hiding this comment.
I understand that this one is related to Long-Term (3+ months) Mentorships. So the name of this function should be different.
| const menteeIdsWithoutMentorshipMatchOrApplied = union( | ||
| difference(approvedMenteeIds, mentorshipMatchMenteeIds), | ||
| appliedMentorshipMatchMenteeIds | ||
| ) |
There was a problem hiding this comment.
@helloanil, the variables' names are confusing for me. It makes it hard to understand the logic. I don't understand which of them return matches and which return applications.
Status__c: MentorshipMatchStatus.APPLIED - is a pending application for me
Status__c: MentorshipMatchStatus.ACCEPTED - is a match for me
For example, this
is pending applications for me, not mentorship matches.
Can we improve the naming of the variables in this function (including comments)?
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (4 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (8 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts
Additional comments not posted (6)
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (6)
138-142: Confirm the correctness of list operations.Ensure that the difference operation correctly identifies mentees without mentorship matches.
Verification successful
The list operations are correctly implemented.
The
differencefunction from lodash is used to identify mentees without mentorship matches, and its usage is consistent with the expected behavior of subtracting elements of the second array from the first array.
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts:import difference from 'lodash/difference'Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the list operations in getUnmatchedMenteesFor45Days. # Test: Ensure that the difference operation correctly identifies mentees without mentorship matches. rg --type python $'difference(approvedMenteeIds, mentorshipMatchMenteeIds)'Length of output: 107
Script:
#!/bin/bash # Description: Verify the list operations in getUnmatchedMenteesFor45Days in the TypeScript file. # Test: Ensure that the difference operation correctly identifies mentees without mentorship matches. rg 'difference(approvedMenteeIds, mentorshipMatchMenteeIds)' --type ts # Check where the difference function is defined or imported rg 'difference' --type tsLength of output: 583
122-123: Ensure proper handling of edge cases.Verify that the method handles cases where no mentees meet the criteria.
Also applies to: 145-151
Verification successful
Edge case properly handled.
The method correctly handles cases where no mentees meet the criteria by returning an empty array.
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts: Lines 122-123Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge case handling in getUnmatchedMenteesFor45Days. # Test: Ensure that the method handles cases where no mentees meet the criteria. rg --type python $'if (approvedMenteeIds.length === 0) return []'Length of output: 217
Script:
#!/bin/bash # Description: Verify edge case handling in getUnmatchedMenteesFor45Days. # Test: Ensure that the method handles cases where no mentees meet the criteria. rg --type ts 'if \(approvedMenteeIds\.length === 0\) return \[\]'Length of output: 293
265-276: Confirm the correctness of list operations.Ensure that the reduce operation correctly processes pending mentee applications.
Verification successful
The reduce operation correctly processes pending mentee applications.
The
pendingMentorshipApplicationsarray is correctly reduced to accumulate objects with the required properties (id,createdAt,menteeId, andmentorId). The properties are correctly accessed from thematch.propsobject.
- File:
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts- Lines: 265-276
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the list operations in getPendingMenteeApplications. # Test: Ensure that the reduce operation correctly processes pending mentee applications. rg --type python $'pendingMentorshipApplications.reduce'Length of output: 86
Script:
#!/bin/bash # Description: Verify the list operations in getPendingMenteeApplications. # Test: Ensure that the reduce operation correctly processes pending mentee applications. rg --type typescript 'pendingMentorshipApplications.reduce'Length of output: 94
Script:
#!/bin/bash # Description: Verify the list operations in getPendingMenteeApplications. # Test: Ensure that the reduce operation correctly processes pending mentee applications. rg --type ts 'pendingMentorshipApplications.reduce'Length of output: 179
Script:
#!/bin/bash # Description: Verify the structure of pendingMentorshipApplications and its elements. # Search for the definition of pendingMentorshipApplications rg 'pendingMentorshipApplications' --type ts # Search for the properties used in the reduce operation rg 'props\.id' --type ts rg 'props\.createdAt' --type ts rg 'props\.menteeId' --type ts rg 'props\.mentorId' --type tsLength of output: 15900
105-108: Verify the date calculation logic.Ensure that the date calculation correctly identifies mentees approved 45 days ago.
258-263: Verify the database query logic.Ensure that the database query correctly retrieves pending mentee applications.
285-293: Ensure proper handling of edge cases.Verify that the method handles cases where no pending applications are found.
Verification successful
Edge case handling verified.
The method
getPendingMenteeApplicationscorrectly handles the case where no pending applications are found by ensuringmatchConProfilesremains an empty array whenmenteeIdsandmentorIdsare empty.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge case handling in getPendingMenteeApplications. # Test: Ensure that the method handles cases where no pending applications are found. rg --type python $'if ([...menteeIds, ...mentorIds].length > 0) {'Length of output: 223
Script:
#!/bin/bash # Description: Verify edge case handling in reminder-emails.service.ts. # Test: Ensure that the method handles cases where no pending applications are found. rg --type js --type ts 'if \(\[\.\.\.menteeIds, \.\.\.mentorIds\]\.length > 0\) {' apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 325
Script:
#!/bin/bash # Description: Verify edge case handling in reminder-emails.service.ts. # Test: Ensure that the method handles cases where no pending applications are found. rg --type js --type ts 'if \(\[\.\.\.menteeIds, \.\.\.mentorIds\]\.length > 0\)' apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 301
Script:
#!/bin/bash # Description: Extract context around the edge case handling in reminder-emails.service.ts. # Test: Ensure that the method handles cases where no pending applications are found. rg --type js --type ts -A 5 -B 5 'if \(\[\.\.\.menteeIds, \.\.\.mentorIds\]\.length > 0\)' apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 1115
Script:
#!/bin/bash # Description: Extract the entire method to understand edge case handling in reminder-emails.service.ts. # Test: Ensure that the method handles cases where no pending applications are found. rg --type js --type ts -A 20 -B 20 'if \(\[\.\.\.menteeIds, \.\.\.mentorIds\]\.length > 0\)' apps/nestjs-api/src/reminder-emails/reminder-emails.service.tsLength of output: 3895
| async getUnmatchedMenteesFor45Days() { | ||
| const approvedDate = new Date() | ||
| approvedDate.setDate(approvedDate.getDate() - 45) | ||
|
|
||
| // 1st Step: Get all mentees that have been approved 45 days ago | ||
| const approvedMenteesFrom45DaysAgo = await this.conProfilesService.findAll({ | ||
| 'RecordType.DeveloperName': UserType.MENTEE, | ||
| Profile_Status__c: ConnectProfileStatus.APPROVED, | ||
| Profile_First_Approved_At__c: { | ||
| $eq: jsforce.SfDate.toDateLiteral(approvedDate), | ||
| }, | ||
| }) | ||
|
|
||
| const approvedMenteeIds = approvedMenteesFrom45DaysAgo.map( | ||
| (mentee) => mentee.props.userId | ||
| ) | ||
|
|
||
| if (approvedMenteeIds.length === 0) return [] | ||
|
|
||
| // 2nd Step: Get all mentorship matches where the mentee is one of the approved mentees above | ||
| const mentorshipMatches = await this.conMentorshipMatchesService.findAll({ | ||
| Status__c: { | ||
| $ne: MentorshipMatchStatus.APPLIED, | ||
| }, | ||
| 'Mentee__r.id': { | ||
| $in: approvedMenteeIds, | ||
| }, | ||
| }) | ||
|
|
||
| const mentorshipMatchMenteeIds = mentorshipMatches.map( | ||
| (match) => match.props.menteeId | ||
| ) | ||
|
|
||
| // 4th Step: Find approved mentees that do not have a mentorship match or waiting applied | ||
| const menteeIdsWithoutMentorshipMatch = difference( | ||
| approvedMenteeIds, | ||
| mentorshipMatchMenteeIds | ||
| ) | ||
|
|
||
| // 5th Step: Return approved mentees that do not have a mentorship match or waiting applied | ||
| if (menteeIdsWithoutMentorshipMatch.length > 0) { | ||
| return approvedMenteesFrom45DaysAgo.filter((mentee) => | ||
| menteeIdsWithoutMentorshipMatch.includes(mentee.props.userId) | ||
| ) | ||
| } | ||
|
|
||
| return [] | ||
| } |
There was a problem hiding this comment.
Improve variable names for clarity.
Consider renaming approvedMenteesFrom45DaysAgo to menteesApproved45DaysAgo for better readability. Similarly, menteeIdsWithoutMentorshipMatch can be renamed to unmatchedMenteeIds.
Optimize filtering logic.
The filtering logic can be optimized by using a single filter operation instead of mapping and then filtering.
async getUnmatchedMenteesFor45Days() {
const approvedDate = new Date();
approvedDate.setDate(approvedDate.getDate() - 45);
const menteesApproved45DaysAgo = await this.conProfilesService.findAll({
'RecordType.DeveloperName': UserType.MENTEE,
Profile_Status__c: ConnectProfileStatus.APPROVED,
Profile_First_Approved_At__c: {
$eq: jsforce.SfDate.toDateLiteral(approvedDate),
},
});
const approvedMenteeIds = new Set(menteesApproved45DaysAgo.map(
(mentee) => mentee.props.userId
));
if (approvedMenteeIds.size === 0) return [];
const mentorshipMatches = await this.conMentorshipMatchesService.findAll({
Status__c: {
$ne: MentorshipMatchStatus.APPLIED,
},
'Mentee__r.id': {
$in: Array.from(approvedMenteeIds),
},
});
const mentorshipMatchMenteeIds = new Set(mentorshipMatches.map(
(match) => match.props.menteeId
));
const unmatchedMenteeIds = [...approvedMenteeIds].filter(
(id) => !mentorshipMatchMenteeIds.has(id)
);
return menteesApproved45DaysAgo.filter((mentee) =>
unmatchedMenteeIds.includes(mentee.props.userId)
);
}| async getPendingMenteeApplications() { | ||
| const pendingMentorshipApplications = | ||
| await this.conMentorshipMatchesService.findAll({ | ||
| Mentorship_Match_Age_In_Days__c: 14, | ||
| Status__c: MentorshipMatchStatus.APPLIED, | ||
| }) | ||
|
|
||
| const reducedMatches = pendingMentorshipApplications.reduce( | ||
| (acc, match) => { | ||
| acc[match.props.id] = { | ||
| matchDate: match.props.createdAt, | ||
| menteeId: match.props.menteeId, | ||
| mentorId: match.props.mentorId, | ||
| } | ||
|
|
||
| return acc | ||
| }, | ||
| {} | ||
| ) | ||
|
|
||
| const menteeIds = pendingMentorshipApplications.map( | ||
| (match) => match.props.menteeId | ||
| ) | ||
| const mentorIds = pendingMentorshipApplications.map( | ||
| (match) => match.props.mentorId | ||
| ) | ||
|
|
||
| let matchConProfiles: ConProfileEntity[] = [] | ||
|
|
||
| if ([...menteeIds, ...mentorIds].length > 0) { | ||
| matchConProfiles = await this.conProfilesService.findAll({ | ||
| 'Contact__r.Id': { | ||
| $in: [...menteeIds, ...mentorIds], | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * Some holy moly is happening here. The idea is, transforming the map of mentorship matches | ||
| * to a map of mentorship matches with the first names and emails of the mentee and mentor. | ||
| * See here for lodash/transform: https://lodash.com/docs/4.17.15#transform | ||
| */ | ||
| const transformedReducedMatches = transform( | ||
| reducedMatches, | ||
| (result, value, key) => { | ||
| const mentee = matchConProfiles.find( | ||
| (profile) => profile?.props.userId === value['menteeId'] | ||
| ) | ||
|
|
||
| const mentor = matchConProfiles.find( | ||
| (profile) => profile?.props.userId === value['mentorId'] | ||
| ) | ||
|
|
||
| result[key] = { | ||
| mentorEmail: mentor?.props.email, | ||
| mentorFirstName: mentor?.props.firstName, | ||
| menteeFirstName: mentee?.props.firstName, | ||
| menteeFullName: mentee?.props.fullName, | ||
| matchDate: formatDate(new Date(value['matchDate']), 'PPP'), | ||
| } | ||
| }, | ||
| [] | ||
| ) | ||
|
|
||
| return transformedReducedMatches | ||
| } |
There was a problem hiding this comment.
Improve variable names for clarity.
Consider renaming pendingMentorshipApplications to pendingApplications for better readability. Similarly, reducedMatches can be renamed to applicationsMap.
Optimize transformation logic.
The transformation logic can be optimized by using a single reduce operation instead of multiple map operations.
async getPendingMenteeApplications() {
const pendingApplications = await this.conMentorshipMatchesService.findAll({
Mentorship_Match_Age_In_Days__c: 14,
Status__c: MentorshipMatchStatus.APPLIED,
});
const applicationsMap = pendingApplications.reduce((acc, match) => {
acc[match.props.id] = {
matchDate: match.props.createdAt,
menteeId: match.props.menteeId,
mentorId: match.props.mentorId,
};
return acc;
}, {});
const menteeIds = new Set(pendingApplications.map((match) => match.props.menteeId));
const mentorIds = new Set(pendingApplications.map((match) => match.props.mentorId));
let matchConProfiles: ConProfileEntity[] = [];
if (menteeIds.size + mentorIds.size > 0) {
matchConProfiles = await this.conProfilesService.findAll({
'Contact__r.Id': {
$in: [...menteeIds, ...mentorIds],
},
});
}
const transformedApplicationsMap = transform(
applicationsMap,
(result, value, key) => {
const mentee = matchConProfiles.find(
(profile) => profile?.props.userId === value.menteeId
);
const mentor = matchConProfiles.find(
(profile) => profile?.props.userId === value.mentorId
);
result[key] = {
mentorEmail: mentor?.props.email,
mentorFirstName: mentor?.props.firstName,
menteeFirstName: mentee?.props.firstName,
menteeFullName: mentee?.props.fullName,
matchDate: formatDate(new Date(value.matchDate), 'PPP'),
};
},
[]
);
return transformedApplicationsMap;
}| async sendMenteesPlatformAndNewMentorsReminder({ email, firstName }) { | ||
| const sfEmailTemplateDeveloperName = | ||
| 'Mentee_Platform_And_New_Mentors_Reminder_1711367982313' | ||
|
|
||
| // const template = await this.emailTemplatesService.getEmailTemplate( | ||
| // sfEmailTemplateDeveloperName | ||
| // ) | ||
| const template = await this.emailTemplatesService.getEmailTemplate( | ||
| sfEmailTemplateDeveloperName | ||
| ) | ||
|
|
||
| // if (!template) { | ||
| // throw new Error( | ||
| // `Email template not found: ${sfEmailTemplateDeveloperName}` | ||
| // ) | ||
| // } | ||
| if (!template) { | ||
| throw new Error( | ||
| `Email template not found: ${sfEmailTemplateDeveloperName}` | ||
| ) | ||
| } | ||
|
|
||
| // const sanitizedHtml = template.HtmlValue.replace( | ||
| // /{{{Recipient\.FirstName}}}/g, | ||
| // `${firstName} ${email}` | ||
| // ) | ||
| const sanitizedHtml = template.HtmlValue.replace( | ||
| /{{{Recipient.FirstName}}}/g, | ||
| `${firstName}` | ||
| ) | ||
|
|
||
| // const params = { | ||
| // Destination: { | ||
| // ToAddresses: isProductionOrDemonstration() | ||
| // ? ['anilakarsu93@gmail.com'] | ||
| // : [this.config.get('NX_DEV_MODE_EMAIL_RECIPIENT')], | ||
| // }, | ||
| // Message: { | ||
| // Body: { | ||
| // Html: { Data: sanitizedHtml }, | ||
| // }, | ||
| // Subject: { Data: template.Subject }, | ||
| // }, | ||
| // Source: 'career@redi-school.org', | ||
| // } | ||
| const params = { | ||
| Destination: { | ||
| ToAddresses: isProductionOrDemonstration() | ||
| ? [email] | ||
| : [this.config.get('NX_DEV_MODE_EMAIL_RECIPIENT')], | ||
| BccAddresses: [SENDER_EMAIL], | ||
| }, | ||
| Message: { | ||
| Body: { | ||
| Html: { Data: sanitizedHtml }, | ||
| }, | ||
| Subject: { Data: template.Subject }, | ||
| }, | ||
| Source: `${SENDER_NAME} <${SENDER_EMAIL}>`, | ||
| } | ||
|
|
||
| // this.ses.sendEmail(params, (err, data) => { | ||
| // if (err) console.log(err, err.stack) | ||
| // else console.log(data) | ||
| // }) | ||
| this.ses.sendEmail(params, (err, data) => { | ||
| if (err) console.log(err, err.stack) | ||
| else console.log(data) | ||
| }) | ||
|
|
||
| // return { message: 'Email sent' } | ||
| // } | ||
| return { message: 'Email sent' } | ||
| } |
There was a problem hiding this comment.
Improve error handling and logging.
Consider adding more detailed error handling and logging to capture and log specific errors.
Optimize template fetching and sanitizing logic.
The template fetching and sanitizing logic can be optimized by using a single replace operation.
async sendMenteesPlatformAndNewMentorsReminder({ email, firstName }) {
const sfEmailTemplateDeveloperName =
'Mentee_Platform_And_New_Mentors_Reminder_1711367982313';
const template = await this.emailTemplatesService.getEmailTemplate(
sfEmailTemplateDeveloperName
);
if (!template) {
throw new Error(
`Email template not found: ${sfEmailTemplateDeveloperName}`
);
}
const sanitizedHtml = template.HtmlValue.replace(
/{{{Recipient.FirstName}}}/g,
firstName
);
const params = {
Destination: {
ToAddresses: isProductionOrDemonstration()
? [email]
: [this.config.get('NX_DEV_MODE_EMAIL_RECIPIENT')],
BccAddresses: [SENDER_EMAIL],
},
Message: {
Body: {
Html: { Data: sanitizedHtml },
},
Subject: { Data: template.Subject },
},
Source: `${SENDER_NAME} <${SENDER_EMAIL}>`,
};
try {
const data = await this.ses.sendEmail(params).promise();
console.log('Email sent successfully:', data);
} catch (err) {
console.error('Error sending email:', err, err.stack);
}
return { message: 'Email sent' };
}
What Github issue does this PR relate to? Insert link.
#712
What should the reviewer know?
Adding a new endpoint
sendUnmatchedMenteesReminderto theReminderEmailsControllerinreminder-emails.controller.ts. It retrieves a list of unmatched mentees for 45 days and sends reminder emails to each mentee. The email includes the mentee's email and first name.Also includeing a return message indicating the number of reminder emails sent.
And adding a new endpoint in the
ReminderEmailsControllerto send reminder emails to mentors with pending mentorship applications. ThesendPendingMentorshipsReminderfunction retrieves the pending mentorship matches and sends reminder emails to the corresponding mentors. The number of reminder emails sent is included in the response message.Screenshots
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor