-
Notifications
You must be signed in to change notification settings - Fork 1
feat: email reminders for follow-up dates via Brevo #26
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ALTER TABLE "JobApplication" ADD COLUMN "reminderSentAt" DATETIME; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { SendFollowUpRemindersUseCase } from '@/use-cases/reminders/SendFollowUpRemindersUseCase.js'; | ||
| import { makeApplicationRepository, makeUserRepository, makeApplication, makeUser } from '@/__tests__/helpers/mocks.js'; | ||
| import type { IEmailService } from '@/use-cases/ports/IEmailService.js'; | ||
|
|
||
| const makeEmailService = (overrides?: Partial<IEmailService>): IEmailService => ({ | ||
| sendFollowUpReminder: vi.fn().mockResolvedValue(undefined), | ||
| ...overrides, | ||
| }); | ||
|
|
||
| describe('SendFollowUpRemindersUseCase', () => { | ||
| it('sends email and updates reminderSentAt for due applications', async () => { | ||
| const followUpAt = new Date(Date.now() + 12 * 60 * 60 * 1000); | ||
| const app = makeApplication({ followUpAt }); | ||
| const user = makeUser(); | ||
| const applicationRepository = makeApplicationRepository({ | ||
| findDueForReminder: vi.fn().mockResolvedValue([app]), | ||
| updateReminderSentAt: vi.fn().mockResolvedValue(undefined), | ||
| }); | ||
| const userRepository = makeUserRepository({ | ||
| findById: vi.fn().mockResolvedValue(user), | ||
| }); | ||
| const emailService = makeEmailService(); | ||
|
|
||
| const useCase = new SendFollowUpRemindersUseCase({ | ||
| applicationRepository, | ||
| userRepository, | ||
| emailService, | ||
| }); | ||
| await useCase.execute(); | ||
|
|
||
| expect(emailService.sendFollowUpReminder).toHaveBeenCalledWith( | ||
| user.email, | ||
| app.company, | ||
| app.role, | ||
| followUpAt, | ||
| ); | ||
| expect(applicationRepository.updateReminderSentAt).toHaveBeenCalledWith( | ||
| app.id, | ||
| expect.any(Date), | ||
| ); | ||
| }); | ||
|
|
||
| it('skips applications when user is not found', async () => { | ||
| const app = makeApplication({ followUpAt: new Date() }); | ||
| const applicationRepository = makeApplicationRepository({ | ||
| findDueForReminder: vi.fn().mockResolvedValue([app]), | ||
| updateReminderSentAt: vi.fn(), | ||
| }); | ||
| const userRepository = makeUserRepository({ | ||
| findById: vi.fn().mockResolvedValue(null), | ||
| }); | ||
| const emailService = makeEmailService(); | ||
|
|
||
| await new SendFollowUpRemindersUseCase({ applicationRepository, userRepository, emailService }).execute(); | ||
|
|
||
| expect(emailService.sendFollowUpReminder).not.toHaveBeenCalled(); | ||
| expect(applicationRepository.updateReminderSentAt).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('continues past individual email failures without throwing', async () => { | ||
| const apps = [ | ||
| makeApplication({ id: 'app-1', followUpAt: new Date() }), | ||
| makeApplication({ id: 'app-2', followUpAt: new Date() }), | ||
| ]; | ||
| const user = makeUser(); | ||
| const applicationRepository = makeApplicationRepository({ | ||
| findDueForReminder: vi.fn().mockResolvedValue(apps), | ||
| updateReminderSentAt: vi.fn().mockResolvedValue(undefined), | ||
| }); | ||
| const userRepository = makeUserRepository({ | ||
| findById: vi.fn().mockResolvedValue(user), | ||
| }); | ||
| const emailService = makeEmailService({ | ||
| sendFollowUpReminder: vi.fn() | ||
| .mockRejectedValueOnce(new Error('Brevo timeout')) | ||
| .mockResolvedValueOnce(undefined), | ||
| }); | ||
|
|
||
| await expect( | ||
| new SendFollowUpRemindersUseCase({ applicationRepository, userRepository, emailService }).execute(), | ||
| ).resolves.not.toThrow(); | ||
|
|
||
| expect(emailService.sendFollowUpReminder).toHaveBeenCalledTimes(2); | ||
| // Only the second app (which succeeded) should have reminderSentAt updated | ||
| expect(applicationRepository.updateReminderSentAt).toHaveBeenCalledTimes(1); | ||
| expect(applicationRepository.updateReminderSentAt).toHaveBeenCalledWith('app-2', expect.any(Date)); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||||||||||||||||||||||
| import fp from 'fastify-plugin'; | ||||||||||||||||||||||||||||||||||
| import type { FastifyInstance } from 'fastify'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export default fp(async (fastify: FastifyInstance) => { | ||||||||||||||||||||||||||||||||||
| const INTERVAL_MS = 60 * 60 * 1000; // 1 hour | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const run = async () => { | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| const { sendFollowUpRemindersUseCase } = fastify.diContainer.cradle; | ||||||||||||||||||||||||||||||||||
| await sendFollowUpRemindersUseCase.execute(); | ||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||
| // swallow — don't crash the server | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Log errors instead of silently swallowing them. Silently catching and ignoring errors will make it very difficult to debug if the background reminder process starts failing (e.g., due to database connectivity issues or email service misconfigurations). Consider logging the error using Fastify's built-in logger. 💻 Proposed fix const run = async () => {
try {
const { sendFollowUpRemindersUseCase } = fastify.diContainer.cradle;
await sendFollowUpRemindersUseCase.execute();
- } catch {
- // swallow — don't crash the server
+ } catch (error) {
+ fastify.log.error(error, 'Failed to execute follow-up reminders process');
}
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const timer = setInterval(run, INTERVAL_MS); | ||||||||||||||||||||||||||||||||||
| fastify.addHook('onClose', () => { clearInterval(timer); }); | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -70,4 +70,13 @@ export class CachedApplicationRepository implements IApplicationRepository { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| this.userIdByAppId.delete(id); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (userId) this.cache.deleteByPrefix(`apps:list:${userId}:`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async findDueForReminder(): Promise<Application[]> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return this.inner.findDueForReminder(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async updateReminderSentAt(id: string, sentAt: Date): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await this.inner.updateReminderSentAt(id, sentAt); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.cache.delete(`apps:byId:${id}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+73
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Invalidate the list cache to prevent stale application data. When a reminder is recorded, the To enable ♻️ Proposed fix to keep the cache consistent- async findDueForReminder(): Promise<Application[]> {
- return this.inner.findDueForReminder();
- }
-
- async updateReminderSentAt(id: string, sentAt: Date): Promise<void> {
- await this.inner.updateReminderSentAt(id, sentAt);
- this.cache.delete(`apps:byId:${id}`);
- }
+ async findDueForReminder(): Promise<Application[]> {
+ const result = await this.inner.findDueForReminder();
+ for (const app of result) {
+ this.userIdByAppId.set(app.id, app.userId);
+ }
+ return result;
+ }
+
+ async updateReminderSentAt(id: string, sentAt: Date): Promise<void> {
+ await this.inner.updateReminderSentAt(id, sentAt);
+ this.cache.delete(`apps:byId:${id}`);
+ const userId = this.userIdByAppId.get(id);
+ if (userId) {
+ this.cache.deleteByPrefix(`apps:list:${userId}:`);
+ }
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import type { IEmailService } from '@/use-cases/ports/IEmailService.js'; | ||
|
|
||
| export class BrevoEmailService implements IEmailService { | ||
| private readonly apiKey: string; | ||
| private readonly fromEmail: string; | ||
| private readonly fromName: string; | ||
|
|
||
| constructor() { | ||
| this.apiKey = process.env.BREVO_API_KEY ?? ''; | ||
| this.fromEmail = process.env.FROM_EMAIL ?? 'noreply@jobfinder.app'; | ||
| this.fromName = process.env.FROM_NAME ?? 'Job Finder'; | ||
| } | ||
|
|
||
| async sendFollowUpReminder( | ||
| to: string, | ||
| company: string, | ||
| role: string, | ||
| followUpAt: Date, | ||
| ): Promise<void> { | ||
| const date = followUpAt.toLocaleDateString('en-US', { | ||
| weekday: 'long', | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric', | ||
| }); | ||
| const response = await fetch('https://api.brevo.com/v3/smtp/email', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'api-key': this.apiKey, | ||
| }, | ||
| body: JSON.stringify({ | ||
| sender: { name: this.fromName, email: this.fromEmail }, | ||
| to: [{ email: to }], | ||
| subject: `Reminder: Follow up on ${role} at ${company}`, | ||
| htmlContent: `<p>This is a reminder to follow up on your <strong>${role}</strong> application at <strong>${company}</strong>.</p><p>Your scheduled follow-up date is <strong>${date}</strong>.</p>`, | ||
| }), | ||
| }); | ||
| if (!response.ok && response.status !== 201) { | ||
| const body = await response.text(); | ||
| throw new Error(`Brevo API error ${response.status}: ${body}`); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| export interface IEmailService { | ||
| sendFollowUpReminder( | ||
| to: string, | ||
| company: string, | ||
| role: string, | ||
| followUpAt: Date, | ||
| ): Promise<void>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import type { IApplicationRepository } from '@/use-cases/ports/IApplicationRepository.js'; | ||
| import type { IUserRepository } from '@/use-cases/ports/IUserRepository.js'; | ||
| import type { IEmailService } from '@/use-cases/ports/IEmailService.js'; | ||
|
|
||
| interface Deps { | ||
| applicationRepository: IApplicationRepository; | ||
| userRepository: IUserRepository; | ||
| emailService: IEmailService; | ||
| } | ||
|
|
||
| export class SendFollowUpRemindersUseCase { | ||
| constructor(private readonly deps: Deps) {} | ||
|
|
||
| async execute(): Promise<void> { | ||
| const apps = await this.deps.applicationRepository.findDueForReminder(); | ||
| for (const app of apps) { | ||
| const user = await this.deps.userRepository.findById(app.userId); | ||
| if (!user) continue; | ||
| try { | ||
| await this.deps.emailService.sendFollowUpReminder( | ||
| user.email, | ||
| app.company, | ||
| app.role, | ||
| app.followUpAt!, | ||
| ); | ||
| await this.deps.applicationRepository.updateReminderSentAt(app.id, new Date()); | ||
| } catch { | ||
| // continue — one failure shouldn't block the rest | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Quote environment variables that contain spaces.
As flagged by the static analysis tools, variables containing spaces should be enclosed in quotes to ensure compatibility across different environment variable parsers and deployment platforms.
🛠 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 27-27: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
Source: Linters/SAST tools