From 45d405e0985c1d29970ec341bbbfbf3b9600f3fd Mon Sep 17 00:00:00 2001 From: devjaes Date: Sun, 30 Nov 2025 21:17:10 -0500 Subject: [PATCH] refactor: update budget summary job to use CronJob and prevent duplicate notifications - Replaced setTimeout with CronJob for the budget summary notifications, scheduling it to run on the 28th of each month at 6 PM. - Implemented caching and database checks to prevent sending duplicate budget summary notifications within a month. - Updated notification repository to include a method for finding recent notifications by user, type, and title pattern. - Enhanced email template to include a dynamic frontend URL for application links. --- src/app.ts | 2 +- .../cron/budget-notifications.cron.ts | 123 +++++++++++++----- .../cron/debt-notifications.cron.ts | 5 +- .../cron/goal-notifications.cron.ts | 5 +- src/core/infrastructure/env/env.ts | 2 + .../ports/notification-repository.port.ts | 8 +- .../adapters/notification.repository.ts | 101 +++++++++----- .../infrastructure/templates/base.template.ts | 5 +- 8 files changed, 170 insertions(+), 81 deletions(-) diff --git a/src/app.ts b/src/app.ts index 25f8c90..e83c189 100644 --- a/src/app.ts +++ b/src/app.ts @@ -47,7 +47,7 @@ startNotificationsCleanupJob(); recalculateContributionAmountCron.start(); startDebtNotificationsJob(); -startBudgetSummaryJob(); +startBudgetSummaryJob(); // Fixed: Now using proper CronJob instead of setTimeout startGoalNotificationsJob(); startFinancialSuggestionsJob(); startGoalSuggestionsJob(); diff --git a/src/core/infrastructure/cron/budget-notifications.cron.ts b/src/core/infrastructure/cron/budget-notifications.cron.ts index 1f98367..6b762a8 100644 --- a/src/core/infrastructure/cron/budget-notifications.cron.ts +++ b/src/core/infrastructure/cron/budget-notifications.cron.ts @@ -1,10 +1,49 @@ +import { CronJob } from 'cron'; import { PgBudgetRepository } from '@/budgets/infrastructure/adapters/budget.repository'; import { PgCategoryRepository } from '@/categories/infrastructure/adapters/category.repository'; import { PgNotificationRepository } from '@/notifications/infrastructure/adapters/notification.repository'; import { NotificationUtilsService } from '@/notifications/application/services/notification-utils.service'; +import { NotificationType } from '@/notifications/domain/entities/INotification'; let isRunning = false; +// Cache to track sent notifications and prevent duplicates +const sentNotificationsCache = new Map(); + +/** + * Check if a notification was sent recently to prevent duplicates + * @param cacheKey Unique key for the notification + * @returns True if notification was sent recently + */ +function hasRecentNotification(cacheKey: string): boolean { + const lastSent = sentNotificationsCache.get(cacheKey); + if (!lastSent) return false; + + const hoursSinceLastNotification = (Date.now() - lastSent.getTime()) / (1000 * 60 * 60); + return hoursSinceLastNotification < 24; // Don't send more than once per day +} + +/** + * Record that a notification was sent + * @param cacheKey Unique key for the notification + */ +function recordNotificationSent(cacheKey: string): void { + sentNotificationsCache.set(cacheKey, new Date()); +} + +/** + * Get the name of a month in Spanish + * @param monthIndex Month index (0-11) + * @returns Month name in Spanish + */ +function getMonthName(monthIndex: number): string { + const months = [ + 'enero', 'febrero', 'marzo', 'abril', 'mayo', 'junio', + 'julio', 'agosto', 'septiembre', 'octubre', 'noviembre', 'diciembre' + ]; + return months[monthIndex]; +} + /** * Check budgets for monthly summary and generate notifications */ @@ -40,6 +79,31 @@ const generateBudgetSummaries = async () => { // Generate summary notifications for each user for (const [userId, userBudgetList] of userBudgets.entries()) { + // Check if notification was already sent for this user and month + const monthKey = `${userId}_${now.getFullYear()}_${now.getMonth()}`; + const cacheKey = `BUDGET_SUMMARY_${monthKey}`; + + // Check in-memory cache first + if (hasRecentNotification(cacheKey)) { + console.log(`Skipping budget summary for user ${userId} - already sent this month (cache)`); + continue; + } + + // Check in database for existing notifications in the last 25 days + const startOfMonth = new Date(now.getFullYear(), now.getMonth(), 1); + const existingNotifications = await notificationRepository.findRecentByUserTypeAndTitle( + userId, + NotificationType.SUGGESTION, + `Resumen mensual de presupuestos: ${getMonthName(now.getMonth())}`, + startOfMonth + ); + + if (existingNotifications.length > 0) { + console.log(`Skipping budget summary for user ${userId} - already sent this month (database)`); + recordNotificationSent(cacheKey); + continue; + } + const overBudgetItems = []; const nearLimitItems = []; const underBudgetItems = []; @@ -111,6 +175,8 @@ const generateBudgetSummaries = async () => { true // Send email ); + // Record that notification was sent + recordNotificationSent(cacheKey); notificationsCreated++; } @@ -122,46 +188,33 @@ const generateBudgetSummaries = async () => { } }; +/** + * Cron job for monthly budget summaries + * Runs on the 28th of each month at 6 PM + * (Using 28th instead of last day to avoid date calculation issues) + */ +const budgetSummaryJob = new CronJob( + '0 18 28 * *', // At 6:00 PM on day 28 of every month + async () => { + await generateBudgetSummaries(); + }, + null, + false, + 'America/New_York' +); + /** * Start the budget summary notifications job */ export const startBudgetSummaryJob = () => { - // Run on the last day of each month at 6 PM - const scheduleCheck = () => { - const now = new Date(); - - // Last day of current month - const lastDayOfMonth = new Date(now.getFullYear(), now.getMonth() + 1, 0); - lastDayOfMonth.setHours(18, 0, 0, 0); // 6 PM - - // If it's already past 6 PM on the last day, schedule for next month - if (now > lastDayOfMonth) { - lastDayOfMonth.setMonth(lastDayOfMonth.getMonth() + 1); - } - - const delay = lastDayOfMonth.getTime() - now.getTime(); - console.log(`Scheduled budget summary notifications for ${lastDayOfMonth.toISOString()}`); - - setTimeout(() => { - generateBudgetSummaries().then(() => { - scheduleCheck(); // Reschedule for the next month - }); - }, delay); - }; - - // Schedule recurring checks - scheduleCheck(); + budgetSummaryJob.start(); + console.log('Budget summary job started (runs at 6 PM on the 28th of each month)'); }; /** - * Get the name of a month in Spanish - * @param monthIndex Month index (0-11) - * @returns Month name in Spanish + * Stop the budget summary notifications job */ -function getMonthName(monthIndex: number): string { - const months = [ - 'enero', 'febrero', 'marzo', 'abril', 'mayo', 'junio', - 'julio', 'agosto', 'septiembre', 'octubre', 'noviembre', 'diciembre' - ]; - return months[monthIndex]; -} +export const stopBudgetSummaryJob = () => { + budgetSummaryJob.stop(); + console.log('Budget summary job stopped'); +}; diff --git a/src/core/infrastructure/cron/debt-notifications.cron.ts b/src/core/infrastructure/cron/debt-notifications.cron.ts index aaa7680..0da05f8 100644 --- a/src/core/infrastructure/cron/debt-notifications.cron.ts +++ b/src/core/infrastructure/cron/debt-notifications.cron.ts @@ -95,9 +95,6 @@ export const startDebtNotificationsJob = () => { }, delay); }; - // Run once immediately at startup - checkDebtsForNotifications(); - - // Schedule recurring checks + // Schedule recurring checks (don't run immediately to avoid duplicate notifications on server restart) scheduleCheck(); }; diff --git a/src/core/infrastructure/cron/goal-notifications.cron.ts b/src/core/infrastructure/cron/goal-notifications.cron.ts index b1e54c1..0d19bc3 100644 --- a/src/core/infrastructure/cron/goal-notifications.cron.ts +++ b/src/core/infrastructure/cron/goal-notifications.cron.ts @@ -151,10 +151,7 @@ export const startGoalNotificationsJob = () => { }, delay); }; - // Run once immediately at startup - checkGoalsForNotifications(); - - // Schedule recurring checks + // Schedule recurring checks (don't run immediately to avoid duplicate notifications on server restart) scheduleCheck(); }; diff --git a/src/core/infrastructure/env/env.ts b/src/core/infrastructure/env/env.ts index 5091ec4..0d7e570 100644 --- a/src/core/infrastructure/env/env.ts +++ b/src/core/infrastructure/env/env.ts @@ -44,6 +44,8 @@ const EnvSchema = z BREVO_API_KEY: z.string().optional(), EMAIL_FROM_ADDRESS: z.string().email().optional(), EMAIL_FROM_NAME: z.string().optional(), + // Frontend URL for email links + FRONTEND_URL: z.string().url().default("http://localhost:3001"), }) .superRefine((input, ctx) => { if (input.NODE_ENV === "production" && !input.DATABASE_AUTH_TOKEN) { diff --git a/src/features/notifications/domain/ports/notification-repository.port.ts b/src/features/notifications/domain/ports/notification-repository.port.ts index 9ede5e1..e786155 100644 --- a/src/features/notifications/domain/ports/notification-repository.port.ts +++ b/src/features/notifications/domain/ports/notification-repository.port.ts @@ -1,4 +1,4 @@ -import { INotification } from "../entities/INotification"; +import { INotification, NotificationType } from "../entities/INotification"; export interface INotificationRepository { findAll(): Promise; @@ -11,4 +11,10 @@ export interface INotificationRepository { markAsRead(id: number): Promise; markAllAsRead(userId: number): Promise; deleteExpired(): Promise; + findRecentByUserTypeAndTitle( + userId: number, + type: NotificationType, + titlePattern: string, + afterDate: Date + ): Promise; } diff --git a/src/features/notifications/infrastructure/adapters/notification.repository.ts b/src/features/notifications/infrastructure/adapters/notification.repository.ts index e949c6f..25c1389 100644 --- a/src/features/notifications/infrastructure/adapters/notification.repository.ts +++ b/src/features/notifications/infrastructure/adapters/notification.repository.ts @@ -1,7 +1,10 @@ -import { eq, and, lt } from "drizzle-orm"; +import { eq, and, lt, desc } from "drizzle-orm"; import DatabaseConnection from "@/core/infrastructure/database"; import { notifications } from "@/schema"; -import { INotification, NotificationType } from "../../domain/entities/INotification"; +import { + INotification, + NotificationType, +} from "../../domain/entities/INotification"; import { INotificationRepository } from "../../domain/ports/notification-repository.port"; export class PgNotificationRepository implements INotificationRepository { @@ -18,7 +21,11 @@ export class PgNotificationRepository implements INotificationRepository { } async findAll(): Promise { - const result = await this.db.select().from(notifications); + const result = await this.db + .select() + .from(notifications) + .orderBy(desc(notifications.created_at)) + .limit(500); // Limit to last 500 notifications for admin purposes return result.map(this.mapToEntity); } @@ -35,7 +42,9 @@ export class PgNotificationRepository implements INotificationRepository { const result = await this.db .select() .from(notifications) - .where(eq(notifications.user_id, userId)); + .where(eq(notifications.user_id, userId)) + .orderBy(desc(notifications.created_at)) + .limit(100); // Limit to last 100 notifications return result.map(this.mapToEntity); } @@ -45,16 +54,17 @@ export class PgNotificationRepository implements INotificationRepository { .select() .from(notifications) .where( - and( - eq(notifications.user_id, userId), - eq(notifications.read, false) - ) - ); + and(eq(notifications.user_id, userId), eq(notifications.read, false)) + ) + .orderBy(desc(notifications.created_at)) + .limit(50); // Limit to last 50 unread notifications return result.map(this.mapToEntity); } - async create(notificationData: Omit): Promise { + async create( + notificationData: Omit + ): Promise { const result = await this.db .insert(notifications) .values({ @@ -71,15 +81,24 @@ export class PgNotificationRepository implements INotificationRepository { return this.mapToEntity(result[0]); } - async update(id: number, notificationData: Partial): Promise { + async update( + id: number, + notificationData: Partial + ): Promise { const updateData: Record = {}; - if (notificationData.title !== undefined) updateData.title = notificationData.title; - if (notificationData.subtitle !== undefined) updateData.subtitle = notificationData.subtitle; - if (notificationData.message !== undefined) updateData.message = notificationData.message; - if (notificationData.read !== undefined) updateData.read = notificationData.read; - if (notificationData.type !== undefined) updateData.type = notificationData.type; - if (notificationData.expiresAt !== undefined) updateData.expires_at = notificationData.expiresAt; + if (notificationData.title !== undefined) + updateData.title = notificationData.title; + if (notificationData.subtitle !== undefined) + updateData.subtitle = notificationData.subtitle; + if (notificationData.message !== undefined) + updateData.message = notificationData.message; + if (notificationData.read !== undefined) + updateData.read = notificationData.read; + if (notificationData.type !== undefined) + updateData.type = notificationData.type; + if (notificationData.expiresAt !== undefined) + updateData.expires_at = notificationData.expiresAt; const result = await this.db .update(notifications) @@ -114,10 +133,7 @@ export class PgNotificationRepository implements INotificationRepository { .update(notifications) .set({ read: true }) .where( - and( - eq(notifications.user_id, userId), - eq(notifications.read, false) - ) + and(eq(notifications.user_id, userId), eq(notifications.read, false)) ) .returning(); @@ -128,16 +144,12 @@ export class PgNotificationRepository implements INotificationRepository { const now = new Date(); const result = await this.db .delete(notifications) - .where( - and( - lt(notifications.expires_at, now) - ) - ) + .where(and(lt(notifications.expires_at, now))) .returning(); return result.length; } - + async findByUserIdAndType( userId: number, type: NotificationType, @@ -147,20 +159,39 @@ export class PgNotificationRepository implements INotificationRepository { eq(notifications.user_id, userId), eq(notifications.type, type) ); - + if (afterDate) { - conditions = and( - conditions, - lt(notifications.created_at, afterDate) - ); + conditions = and(conditions, lt(notifications.created_at, afterDate)); } - + + const result = await this.db.select().from(notifications).where(conditions); + + return result.map(this.mapToEntity); + } + + /** + * Find notifications by user, type and title pattern created after a specific date + * Used to prevent duplicate notifications + */ + async findRecentByUserTypeAndTitle( + userId: number, + type: NotificationType, + titlePattern: string, + afterDate: Date + ): Promise { const result = await this.db .select() .from(notifications) - .where(conditions); + .where( + and(eq(notifications.user_id, userId), eq(notifications.type, type)) + ); - return result.map(this.mapToEntity); + // Filter by date and title pattern in memory + return result + .map(this.mapToEntity) + .filter( + (n) => n.createdAt >= afterDate && n.title.includes(titlePattern) + ); } private mapToEntity(raw: any): INotification { diff --git a/src/features/notifications/infrastructure/templates/base.template.ts b/src/features/notifications/infrastructure/templates/base.template.ts index e250d34..f822429 100644 --- a/src/features/notifications/infrastructure/templates/base.template.ts +++ b/src/features/notifications/infrastructure/templates/base.template.ts @@ -1,3 +1,5 @@ +import env from '@/env'; + export const baseTemplate = (title: string, subtitle: string | null, message: string, type: string) => { const typeColor = { GOAL: '#4CAF50', @@ -8,6 +10,7 @@ export const baseTemplate = (title: string, subtitle: string | null, message: st }; const color = typeColor[type as keyof typeof typeColor] || '#757575'; + const frontendUrl = env.FRONTEND_URL || 'http://localhost:3001'; return ` @@ -74,7 +77,7 @@ export const baseTemplate = (title: string, subtitle: string | null, message: st

${message.replace(/\n/g, '
')}

- Ver en la aplicación + Ver en la aplicación