Conversation
AdamFipke
left a comment
AdamFipke
left a comment
There was a problem hiding this comment.
Good start! I haven't yet ran the code just yet. Some initial feedback in comments
| <hr style="border: 1px solid #ecf0f1; margin: 30px 0;"> | ||
| <p style="color: #95a5a6; font-size: 12px; text-align: center;"> | ||
| Weekly summary from HelpMe.<br> | ||
| Manage your email preferences in settings. |
There was a problem hiding this comment.
Add a link to the page here. Also, did you test that you can actually disable the emails under course preferences?
| if (queueStats.totalQuestions > 0 && queueStats.totalQuestions < 5) { | ||
| recommendations.push({ | ||
| type: 'info', | ||
| message: 'Queue usage is low. Consider reminding students about office hours availability.', |
There was a problem hiding this comment.
Most queues are not office hours and there isn't really a good way to tell. I would probably just remove this suggestion
| <ul style="line-height: 1.8; color: #34495e;"> | ||
| <li><strong>${chatbotStats.totalQuestions}</strong> questions asked by <strong>${chatbotStats.uniqueStudents}</strong> unique student${chatbotStats.uniqueStudents !== 1 ? 's' : ''}</li> | ||
| <li>Average: <strong>${chatbotStats.avgQuestionsPerStudent.toFixed(1)}</strong> questions per student</li> | ||
| <li>Most active day: <strong>${chatbotStats.mostActiveDay}</strong></li> |
There was a problem hiding this comment.
most active day won't be needed if there's a chart that already showcases that
| <li><strong>${asyncStats.total}</strong> total questions</li> | ||
| <li><strong style="color: #27ae60;">${asyncStats.aiResolved}</strong> resolved by AI</li> | ||
| <li><strong style="color: #3498db;">${asyncStats.humanAnswered}</strong> answered by staff</li> | ||
| <li><strong style="color: #e74c3c;">${asyncStats.stillNeedHelp}</strong> still need help</li> |
There was a problem hiding this comment.
Does the async questions that still need help include those from previous weeks? Since it should.
Even better, if there are any async questions that are older than 1 week, it should probably highlight those. Saying something like:
<a href="/course/34/async_centre">Anytime Questions</a> older than 1 week that still need help
- "Need help on Lab 3 question 2..." - 10 days ago
- "I don't understand abstraction I n...." - 68 days ago
And I would probably make it bold and red. You could also include it in the Recommendations section, but that might just result in a lot of duplicate text (which is bad, 'cause people don't read)
There was a problem hiding this comment.
Also, this goes for async questions that still need help that are younger than 1 week; I would display a list of the async questions with their abstract with how many days ago it was posted. Or maybe even just have 1 consolidated list of all async questions that still need help, their abstracts, and how many days ago it was posted.
| if (asyncStats.stillNeedHelp > 0) { | ||
| recommendations.push({ | ||
| type: 'warning', | ||
| message: `${asyncStats.stillNeedHelp} async question${asyncStats.stillNeedHelp !== 1 ? 's' : ''} still need${asyncStats.stillNeedHelp === 1 ? 's' : ''} attention from staff.`, |
There was a problem hiding this comment.
Personally I would just remove this one. It's a good idea but another part of the email already mentions how many anytime questions still need help.
| constructor(private mailService: MailService) {} | ||
|
|
||
| // Run every week | ||
| @Cron(CronExpression.EVERY_WEEK) |
There was a problem hiding this comment.
When is every week; what day does it start on? I've given it some more thought, and probably sending it out on Sunday morning might be better (in the original issue I said Monday), though in truth it probably doesn't matter all that much
| // Run every week | ||
| @Cron(CronExpression.EVERY_WEEK) | ||
| async sendWeeklySummaries() { | ||
| console.log('Starting weekly summary email job...'); |
There was a problem hiding this comment.
- "I have performed a code review of my own code (under the "Files Changed" tab on github) to ensure nothing is committed that shouldn't be (e.g. leftover
console.logs, leftover unused logic, or anything else that was accidentally committed)"
There was a problem hiding this comment.
oopsies my bad :(
There was a problem hiding this comment.
haha it's all good i'm just buggin ya
There was a problem hiding this comment.
haha it's all good i'm just buggin ya
2 participants
Description
This PR implements automated weekly email summaries sent to professors every week with detailed usage statistics for all their HelpMe courses. Each professor receives a single consolidated email covering all courses they teach.
Features Included:
Closes: #309
Type of change
yarn installHow Has This Been Tested?
Testing instructions:
Checklist:
console.logs, leftover unused logic, or anything else that was accidentally committed)Sample screenshots:
