From f5b40b70b58edf1fb7cfc88dc0e5f8a9e339c1a5 Mon Sep 17 00:00:00 2001 From: Chris Veleris Date: Sat, 13 Sep 2025 23:04:57 +0300 Subject: [PATCH] Revert "Fix search for recurring" This reverts commit d86b3401280c1be7913e5963e2cb19f86d3fae0a. --- backend/routes/tasks.js | 108 +++++------------- .../global-recurring-filter.test.js | 15 +-- backend/tests/integration/tasks.test.js | 87 +++----------- 3 files changed, 47 insertions(+), 163 deletions(-) diff --git a/backend/routes/tasks.js b/backend/routes/tasks.js index af4b760..f39bfc3 100644 --- a/backend/routes/tasks.js +++ b/backend/routes/tasks.js @@ -452,67 +452,6 @@ async function undoneAllSubtasks(parentTaskId, userId) { } } -// Helper function to filter recurring instances to show only the next one per template -function filterToNextRecurringInstance(tasks) { - const now = new Date(); - const recurringGroups = {}; - const filteredTasks = []; - - // Group tasks by their recurring parent or themselves if they're templates - for (const task of tasks) { - if (task.recurring_parent_id) { - // This is a recurring instance - const parentId = task.recurring_parent_id; - if (!recurringGroups[parentId]) { - recurringGroups[parentId] = { instances: [] }; - } - recurringGroups[parentId].instances.push(task); - } else if (task.recurrence_type && task.recurrence_type !== 'none') { - // This is a recurring template - const templateId = task.id; - if (!recurringGroups[templateId]) { - recurringGroups[templateId] = { template: task, instances: [] }; - } else { - recurringGroups[templateId].template = task; - } - } else { - // Regular non-recurring task - include it - filteredTasks.push(task); - } - } - - // For each recurring group, include only the next instance or template if no future instances - for (const [groupId, group] of Object.entries(recurringGroups)) { - if (group.instances.length > 0) { - // Sort instances by due_date (earliest first), null dates go to end - const sortedInstances = group.instances.sort((a, b) => { - if (!a.due_date && !b.due_date) return 0; - if (!a.due_date) return 1; - if (!b.due_date) return -1; - return new Date(a.due_date) - new Date(b.due_date); - }); - - // Find the next future instance or the earliest one if no future instances - let nextInstance = sortedInstances.find( - (instance) => - !instance.due_date || new Date(instance.due_date) >= now - ); - - if (!nextInstance) { - // No future instances, take the most recent past one - nextInstance = sortedInstances[sortedInstances.length - 1]; - } - - filteredTasks.push(nextInstance); - } else if (group.template) { - // No instances, include the template - filteredTasks.push(group.template); - } - } - - return filteredTasks; -} - // Filter tasks by parameters async function filterTasksByParams(params, userId, userTimezone) { let whereClause = { @@ -617,15 +556,36 @@ async function filterTasksByParams(params, userId, userTimezone) { const safeTimezone = getSafeTimezone(userTimezone); const upcomingRange = getUpcomingRangeInUTC(safeTimezone, 7); - // Override the default whereClause - exclude recurring instances for upcoming view + // For upcoming view, we want to show recurring instances (children) with due dates + // Override the default whereClause to include recurring instances whereClause = { user_id: userId, parent_task_id: null, // Exclude subtasks from main task lists - recurring_parent_id: null, // Exclude recurring task instances due_date: { [Op.between]: [upcomingRange.start, upcomingRange.end], }, status: { [Op.notIn]: [Task.STATUS.DONE, 'done'] }, + [Op.or]: [ + // Include non-recurring tasks + { + [Op.and]: [ + { recurring_parent_id: null }, + { + [Op.or]: [ + { recurrence_type: 'none' }, + { recurrence_type: null }, + ], + }, + ], + }, + // Include recurring task instances (children) - this is the key change! + { + [Op.and]: [ + { recurring_parent_id: { [Op.ne]: null } }, // Has a parent (is an instance) + { recurrence_type: 'none' }, // Instances have recurrence_type: 'none' + ], + }, + ], }; break; } @@ -648,10 +608,8 @@ async function filterTasksByParams(params, userId, userTimezone) { whereClause.status = Task.STATUS.WAITING; break; case 'all': - // For 'all' view, include recurring instances when client_side_filtering is enabled (e.g., for search) - if (!params.client_side_filtering && !params.include_instances) { - whereClause.recurring_parent_id = null; - } + // Exclude recurring task instances from all view + whereClause.recurring_parent_id = null; if (params.status === 'done') { whereClause.status = { [Op.in]: [Task.STATUS.DONE, 'done'] }; } else if (!params.client_side_filtering) { @@ -660,8 +618,8 @@ async function filterTasksByParams(params, userId, userTimezone) { } break; default: - // For 'default' view, include recurring instances when client_side_filtering is enabled (e.g., for search) - if (!params.client_side_filtering && !params.include_instances) { + // Exclude recurring task instances from default view unless include_instances is specified + if (!params.include_instances) { whereClause.recurring_parent_id = null; } if (params.status === 'done') { @@ -723,22 +681,12 @@ async function filterTasksByParams(params, userId, userTimezone) { } } - const tasks = await Task.findAll({ + return await Task.findAll({ where: whereClause, include: includeClause, order: orderClause, distinct: true, }); - - // If client_side_filtering or include_instances is enabled (for search), filter recurring instances to show only the next one per template - if ( - (params.client_side_filtering || params.include_instances) && - (params.type === 'all' || !params.type) - ) { - return filterToNextRecurringInstance(tasks); - } - - return tasks; } // Compute task metrics diff --git a/backend/tests/integration/global-recurring-filter.test.js b/backend/tests/integration/global-recurring-filter.test.js index 6059579..625d1da 100644 --- a/backend/tests/integration/global-recurring-filter.test.js +++ b/backend/tests/integration/global-recurring-filter.test.js @@ -130,9 +130,9 @@ describe('Global Recurring Task Instance Filtering', () => { expect(taskNames).not.toContain('Daily Workout - Aug 24'); }); - it('should exclude recurring instances from upcoming tasks view', async () => { + it('should include recurring instances (not templates) in upcoming tasks view', async () => { // Create recurring instances with future due dates - const instance1 = await Task.create({ + await Task.create({ name: 'Daily Workout - Tomorrow', user_id: user.id, project_id: project.id, @@ -149,15 +149,12 @@ describe('Global Recurring Task Instance Filtering', () => { expect(response.body.tasks).toBeDefined(); const taskNames = response.body.tasks.map((t) => t.name); - const taskIds = response.body.tasks.map((t) => t.id); - // Should include only the recurring template (not instances) - expect(taskNames).toContain('Daily'); - expect(taskIds).toContain(recurringTemplate.id); + // Should include recurring instances that are in the upcoming range + expect(taskNames).toContain('Daily Workout Template'); // This is the generated instance name - // Should NOT include the recurring instance - expect(taskNames).not.toContain('Daily Workout - Tomorrow'); - expect(taskIds).not.toContain(instance1.id); + // Should NOT include the template (it stays in other views) + // Templates don't have specific due dates in upcoming range }); it('should exclude recurring instances from someday tasks view', async () => { diff --git a/backend/tests/integration/tasks.test.js b/backend/tests/integration/tasks.test.js index 36fc484..22ed677 100644 --- a/backend/tests/integration/tasks.test.js +++ b/backend/tests/integration/tasks.test.js @@ -382,15 +382,17 @@ describe('Tasks Routes', () => { const taskIds = response.body.tasks.map((t) => t.id); const taskNames = response.body.tasks.map((t) => t.name); - // Should include only the next recurring instance (tomorrow) not the template (today) + // Should include the recurring template + expect(taskIds).toContain(recurringTemplate.id); + + // Should include the recurring instance (this is the key fix - instances should be searchable) expect(taskIds).toContain(recurringInstance.id); - expect(taskIds).not.toContain(recurringTemplate.id); // Should include the regular task expect(taskIds).toContain(regularTask.id); expect(taskNames).toContain('Review Pull Request'); - // Verify we only have the instance (next occurrence) in search results + // Verify we have both the template and instance - this proves search will work on both const allTasks = response.body.tasks; const templateTask = allTasks.find( (t) => t.id === recurringTemplate.id @@ -399,7 +401,14 @@ describe('Tasks Routes', () => { (t) => t.id === recurringInstance.id ); - expect(templateTask).toBeUndefined(); + expect(templateTask).toBeDefined(); + // The template name gets transformed to show the recurrence type in the API response + expect(templateTask.name).toBe('Daily'); + expect(templateTask.recurrence_type).toBe('daily'); + expect(templateTask.recurring_parent_id).toBeNull(); + // The original name is preserved in original_name field + expect(templateTask.original_name).toBe('RecurringTask'); + expect(instanceTask).toBeDefined(); // Instances keep their original name expect(instanceTask.name).toBe('RecurringTask'); @@ -462,75 +471,5 @@ describe('Tasks Routes', () => { // Template should not be included because it's in the past expect(taskIds).not.toContain(recurringTemplate.id); }); - - it('should include recurring task instances with priority ordering', async () => { - const today = new Date(); - today.setHours(0, 0, 0, 0); - - // Create a recurring task template with high priority - const recurringTemplate = await Task.create({ - name: 'Important Daily Task', - user_id: user.id, - recurrence_type: 'daily', - recurrence_interval: 1, - due_date: today, - status: 0, - priority: 2, // High priority - }); - - // Create a recurring task instance with medium priority - const tomorrow = new Date(); - tomorrow.setDate(tomorrow.getDate() + 1); - - const recurringInstance = await Task.create({ - name: 'Important Daily Task', - user_id: user.id, - recurring_parent_id: recurringTemplate.id, - due_date: tomorrow, - status: 0, - priority: 1, // Medium priority - }); - - // Create a regular non-recurring task with low priority - const regularTask = await Task.create({ - name: 'Regular Low Priority Task', - user_id: user.id, - status: 0, - priority: 0, // Low priority - }); - - // Test with priority ordering and client_side_filtering (simulates search) - const response = await agent.get( - '/api/tasks?client_side_filtering=true&order_by=priority:desc' - ); - - expect(response.status).toBe(200); - expect(response.body.tasks).toBeDefined(); - - const taskIds = response.body.tasks.map((t) => t.id); - const taskNames = response.body.tasks.map((t) => t.name); - - // Should include only the next recurring instance (not both template and instance) - // The future instance should be preferred over the template - expect(taskIds).toContain(recurringInstance.id); - expect(taskIds).not.toContain(recurringTemplate.id); - - // Should include the regular task - expect(taskIds).toContain(regularTask.id); - expect(taskNames).toContain('Regular Low Priority Task'); - - // Verify we only have the instance (next occurrence) in search results - const allTasks = response.body.tasks; - const templateTask = allTasks.find( - (t) => t.id === recurringTemplate.id - ); - const instanceTask = allTasks.find( - (t) => t.id === recurringInstance.id - ); - - expect(templateTask).toBeUndefined(); - expect(instanceTask).toBeDefined(); - expect(instanceTask.recurring_parent_id).toBe(recurringTemplate.id); - }); }); });