diff --git a/backend/modules/projects/dueProjectService.js b/backend/modules/projects/dueProjectService.js index 250b407..51bb310 100644 --- a/backend/modules/projects/dueProjectService.js +++ b/backend/modules/projects/dueProjectService.js @@ -70,8 +70,7 @@ async function checkDueProjects() { continue; } - // Check for existing notifications (including dismissed ones) - // If a notification was dismissed, don't create it again + // Check for existing notifications const recentNotifications = await Notification.findAll({ where: { user_id: project.user_id, @@ -91,9 +90,19 @@ async function checkDueProjects() { ); if (existingNotification) { - // Skip if notification exists, even if it was dismissed - // This prevents re-notifying users about tasks they've already dismissed - continue; + // If notification was dismissed, don't create it again + if (existingNotification.dismissed_at) { + continue; + } + + // If notification is unread, delete it before creating the new one + // This prevents duplicate notifications from piling up + if (!existingNotification.read_at) { + await existingNotification.destroy(); + } else { + // If it was already read, skip creating a new one + continue; + } } const { title, message } = generateNotificationContent( diff --git a/backend/modules/tasks/deferredTaskService.js b/backend/modules/tasks/deferredTaskService.js index 03d9231..240ab3b 100644 --- a/backend/modules/tasks/deferredTaskService.js +++ b/backend/modules/tasks/deferredTaskService.js @@ -68,7 +68,19 @@ async function checkDeferredTasks() { ); if (existingNotification) { - continue; + // If notification was dismissed, don't create it again + if (existingNotification.dismissed_at) { + continue; + } + + // If notification is unread, delete it before creating the new one + // This prevents duplicate notifications from piling up + if (!existingNotification.read_at) { + await existingNotification.destroy(); + } else { + // If it was already read, skip creating a new one + continue; + } } const sources = []; diff --git a/backend/modules/tasks/dueTaskService.js b/backend/modules/tasks/dueTaskService.js index 04b5f62..d2815cc 100644 --- a/backend/modules/tasks/dueTaskService.js +++ b/backend/modules/tasks/dueTaskService.js @@ -68,8 +68,7 @@ async function checkDueTasks() { continue; } - // Check for existing notifications (including dismissed ones) - // If a notification was dismissed, don't create it again + // Check for existing notifications const recentNotifications = await Notification.findAll({ where: { user_id: task.user_id, @@ -89,9 +88,19 @@ async function checkDueTasks() { ); if (existingNotification) { - // Skip if notification exists, even if it was dismissed - // This prevents re-notifying users about tasks they've already dismissed - continue; + // If notification was dismissed, don't create it again + if (existingNotification.dismissed_at) { + continue; + } + + // If notification is unread, delete it before creating the new one + // This prevents duplicate notifications from piling up + if (!existingNotification.read_at) { + await existingNotification.destroy(); + } else { + // If it was already read, skip creating a new one + continue; + } } const { title, message } = generateNotificationContent( diff --git a/backend/tests/unit/modules/tasks/dueTaskService.test.js b/backend/tests/unit/modules/tasks/dueTaskService.test.js new file mode 100644 index 0000000..905805f --- /dev/null +++ b/backend/tests/unit/modules/tasks/dueTaskService.test.js @@ -0,0 +1,305 @@ +const { Task, Notification, User } = require('../../../../models'); +const { checkDueTasks } = require('../../../../modules/tasks/dueTaskService'); +const bcrypt = require('bcrypt'); + +describe('dueTaskService', () => { + let user; + + beforeEach(async () => { + user = await User.create({ + email: 'test@example.com', + password_digest: await bcrypt.hash('password123', 10), + notification_preferences: { + inApp: { + task_due_soon: true, + task_overdue: true, + }, + }, + }); + }); + + describe('checkDueTasks', () => { + describe('notification deduplication', () => { + it('should delete existing unread notification before creating a new one', async () => { + const tomorrow = new Date(); + tomorrow.setDate(tomorrow.getDate() + 1); + + const task = await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: tomorrow, + status: Task.STATUS.NOT_STARTED, + }); + + // Create initial notification + const firstNotification = await Notification.createNotification( + { + userId: user.id, + type: 'task_due_soon', + title: 'Task due soon', + message: 'Your task "Test Task" is due tomorrow', + data: { + taskUid: task.uid, + taskName: task.name, + dueDate: task.due_date, + isOverdue: false, + }, + } + ); + + expect(firstNotification.read_at).toBeFalsy(); + + // Run the check again (simulating the next day's cron job) + await checkDueTasks(); + + // First notification should be deleted + const deletedNotification = await Notification.findByPk( + firstNotification.id + ); + expect(deletedNotification).toBeNull(); + + // New notification should exist + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_due_soon', + }, + }); + + expect(notifications.length).toBe(1); + expect(notifications[0].id).not.toBe(firstNotification.id); + expect(notifications[0].data.taskUid).toBe(task.uid); + }); + + it('should not create duplicate if previous notification was read', async () => { + const tomorrow = new Date(); + tomorrow.setDate(tomorrow.getDate() + 1); + + const task = await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: tomorrow, + status: Task.STATUS.NOT_STARTED, + }); + + // Create and mark as read + const firstNotification = await Notification.createNotification( + { + userId: user.id, + type: 'task_due_soon', + title: 'Task due soon', + message: 'Your task "Test Task" is due tomorrow', + data: { + taskUid: task.uid, + taskName: task.name, + dueDate: task.due_date, + isOverdue: false, + }, + } + ); + + await firstNotification.markAsRead(); + + // Run the check again + const result = await checkDueTasks(); + + // Should not create a new notification + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_due_soon', + }, + }); + + expect(notifications.length).toBe(1); + expect(notifications[0].id).toBe(firstNotification.id); + expect(result.notificationsCreated).toBe(0); + }); + + it('should not create duplicate if previous notification was dismissed', async () => { + const tomorrow = new Date(); + tomorrow.setDate(tomorrow.getDate() + 1); + + const task = await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: tomorrow, + status: Task.STATUS.NOT_STARTED, + }); + + // Create and dismiss + const firstNotification = await Notification.createNotification( + { + userId: user.id, + type: 'task_due_soon', + title: 'Task due soon', + message: 'Your task "Test Task" is due tomorrow', + data: { + taskUid: task.uid, + taskName: task.name, + dueDate: task.due_date, + isOverdue: false, + }, + } + ); + + await firstNotification.dismiss(); + + // Run the check again + const result = await checkDueTasks(); + + // Should not create a new notification + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_due_soon', + }, + }); + + expect(notifications.length).toBe(1); + expect(notifications[0].id).toBe(firstNotification.id); + expect(result.notificationsCreated).toBe(0); + }); + + it('should handle transition from task_due_soon to task_overdue', async () => { + const yesterday = new Date(); + yesterday.setDate(yesterday.getDate() - 1); + + const task = await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: yesterday, + status: Task.STATUS.NOT_STARTED, + }); + + // Create task_due_soon notification (from when it was due tomorrow) + await Notification.createNotification({ + userId: user.id, + type: 'task_due_soon', + title: 'Task due soon', + message: 'Your task "Test Task" is due tomorrow', + data: { + taskUid: task.uid, + taskName: task.name, + dueDate: task.due_date, + isOverdue: false, + }, + }); + + // Run the check (task is now overdue) + await checkDueTasks(); + + // Should create task_overdue notification + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + }, + order: [['created_at', 'DESC']], + }); + + // Both notifications should exist (different types) + expect(notifications.length).toBe(2); + expect(notifications[0].type).toBe('task_overdue'); + expect(notifications[1].type).toBe('task_due_soon'); + }); + + it('should only keep one notification for the same overdue task over multiple days', async () => { + const threeDaysAgo = new Date(); + threeDaysAgo.setDate(threeDaysAgo.getDate() - 3); + + const task = await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: threeDaysAgo, + status: Task.STATUS.NOT_STARTED, + }); + + // Simulate 3 days of cron jobs + for (let i = 0; i < 3; i++) { + await checkDueTasks(); + } + + // Should only have 1 notification + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_overdue', + }, + }); + + expect(notifications.length).toBe(1); + expect(notifications[0].data.taskUid).toBe(task.uid); + }); + }); + + describe('basic functionality', () => { + it('should create notification for task due tomorrow', async () => { + const tomorrow = new Date(); + tomorrow.setDate(tomorrow.getDate() + 1); + + await Task.create({ + name: 'Test Task', + user_id: user.id, + due_date: tomorrow, + status: Task.STATUS.NOT_STARTED, + }); + + const result = await checkDueTasks(); + + expect(result.success).toBe(true); + expect(result.notificationsCreated).toBe(1); + + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_due_soon', + }, + }); + + expect(notifications.length).toBe(1); + }); + + it('should create notification for overdue task', async () => { + const yesterday = new Date(); + yesterday.setDate(yesterday.getDate() - 1); + + await Task.create({ + name: 'Overdue Task', + user_id: user.id, + due_date: yesterday, + status: Task.STATUS.NOT_STARTED, + }); + + const result = await checkDueTasks(); + + expect(result.success).toBe(true); + expect(result.notificationsCreated).toBe(1); + + const notifications = await Notification.findAll({ + where: { + user_id: user.id, + type: 'task_overdue', + }, + }); + + expect(notifications.length).toBe(1); + }); + + it('should not create notification for completed tasks', async () => { + const tomorrow = new Date(); + tomorrow.setDate(tomorrow.getDate() + 1); + + await Task.create({ + name: 'Completed Task', + user_id: user.id, + due_date: tomorrow, + status: Task.STATUS.DONE, + }); + + const result = await checkDueTasks(); + + expect(result.notificationsCreated).toBe(0); + }); + }); + }); +}); diff --git a/docs/MEMORY.md b/docs/MEMORY.md index 8ef47d2..1abb938 100644 --- a/docs/MEMORY.md +++ b/docs/MEMORY.md @@ -61,6 +61,15 @@ This document contains preferences, patterns, and memory items specific to worki --- +## GitHub Issue Preferences + +### Bug Reports +- **Always follow** the GitHub bug template (`.github/ISSUE_TEMPLATE/bug_report.yml`) +- Include all required fields: Description, Steps to Reproduce, Expected Behavior, Actual Behavior, OS, Browser +- Add technical context in Additional Context section (root cause, solution, files affected) + +--- + ## Common Issues & Solutions ### Date Formatting