feat: add reminders to log mentoring sessions #955
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant C as ReminderEmailsController
participant S as ReminderEmailsService
participant M as ConMentoringSessionsService
C->>S: sendMentoringSessionsLoggingReminder()
S->>M: getMentorsAndMenteesWithoutMentoringSessionsLogged({ mentorshipMatchAgeInDays: 14 })
M-->>S: Return mentorship matches
S->>C: Check mentorship matches
alt Matches found
S->>S: sendLogMentoringSessionsReminder(details)
S-->>C: Reminders sent
else No matches
S-->>C: No reminders needed
end
Assessment against linked issues
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 Configuration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (1 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.module.ts (2 hunks)
- apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (4 hunks)
Additional comments not posted (5)
apps/nestjs-api/src/reminder-emails/reminder-emails.module.ts (1)
Line range hint
3-15: LGTM! The module import is correctly added.The addition of
ConMentoringSessionsModuleto the imports array is appropriate and enhances the functionality of theReminderEmailsModule.apps/nestjs-api/src/reminder-emails/reminder-emails.controller.ts (2)
178-203: LGTM! The reminder logic is well-implemented.The
sendMentoringSessionsLoggingRemindermethod effectively retrieves and processes mentorship matches for sending reminders. The use ofObject.keysis an efficient way to check for entries.
185-218: Verify the commented-out code for future implementation.The commented-out code for the 28-day reminder is a planned feature. Ensure that it is correctly implemented and tested when activated.
Verification successful
Commented-out code for 28-day reminder is correctly structured and ready for future implementation.
The code for the 28-day reminder is present and follows the same structure as the 14-day reminder. It should be uncommented and merged according to the TODO comments when ready.
- Location:
apps/nestjs-api/src/reminder-emails/reminder-emails.controller.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the commented-out 28-day reminder code for future implementation. # Test: Check for the presence of the commented-out code. Expect: Code should be present and correctly structured. rg --type js --context 5 'mentorshipMatchesWithMentorAndMenteeDetails_4weeks'Length of output: 77
Script:
#!/bin/bash # Searching for the commented-out 28-day reminder code with a broader context rg --type ts --context 10 'mentorshipMatchesWithMentorAndMenteeDetails_4weeks'Length of output: 4663
apps/nestjs-api/src/reminder-emails/reminder-emails.service.ts (2)
223-323: LGTM! The method for retrieving mentorship matches is well-implemented.The
getMentorsAndMenteesWithoutMentoringSessionsLoggedmethod efficiently retrieves and processes mentorship matches and profiles. The use of lodash for filtering and transforming data is appropriate.
600-701: LGTM! The email sending logic is well-refactored.The
sendLogMentoringSessionsRemindermethod effectively uses email templates and personalizes the content. The refactoring improves clarity and functionality.
There was a problem hiding this comment.
@helloanil, I left one comment. Otherwise, it looks good to me.
|
@coderabbitai review |
Actions performedReview triggered.
|
| Object.keys(mentorshipMatchesWithMentorAndMenteeDetails_2weeks).length > 0 | ||
| ) { | ||
| // send reminder emails | ||
| Object.keys(mentorshipMatchesWithMentorAndMenteeDetails_2weeks).forEach( |
There was a problem hiding this comment.
Doesn't Object.keys(mentorshipMatchesWithMentorAndMenteeDetails_2weeks) return an array of strings?
There was a problem hiding this comment.
Yes, then if you check two lines later, I do something like this:
mentorshipMatchesWithMentorAndMenteeDetails_2weeks[key]
This is because for some reason, the lodash function I use to get mentorshipMatchesWithMentorAndMenteeDetails (transform) doesn't return an array but kind of an object. I didn't have time to dig deeper in that, but I noticed I cannot just do mentorshipMatchesWithMentorAndMenteeDetails_2weeks.forEach so I have to first iterate the keys, then use the key to obtain the value.
| // { mentorshipMatchAgeInDays: 28 } | ||
| // ) | ||
|
|
||
| if ( |
There was a problem hiding this comment.
Simplify? ChatGPT offered me this:
const matches = Object.values(mentorshipMatchesWithMentorAndMenteeDetails_2weeks);
if (matches.length > 0) {
matches.forEach(async (match) => {
await this.reminderEmailsService.sendLogMentoringSessionsReminder(match, 14);
});
}
| @@ -217,6 +220,106 @@ export class ReminderEmailsService { | |||
| return transformedReducedMatches | |||
| } | |||
|
|
|||
| async getMentorsAndMenteesWithoutMentoringSessionsLogged({ | |||
There was a problem hiding this comment.
This looks quite familiar to another PR where I offered some pseudocode to simplify. What happened to that? Is it reusable here?
What Github issue does this PR relate to? Insert link.
Closes #712
What should the reviewer know?
This PR implements the last two reminders of the issue #712. These are reminding mentees and mentors to log mentoring sessions, after 14 and 28 days.
Important Note: The reminders that will send after 28 days are not enabled in this PR. They are commented out. This is because this email contains a sentence like this:
Since this would be confusing to receive as the first email, I suggest merging and deploying this PR with the 14 days reminder only, and in two weeks, merging and deploying the commented part so we make sure only the mentees and mentors who received the first email will receive the second email.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Chores