Fix subtasks copy (#546)

This commit is contained in:
Chris 2025-11-15 22:00:18 +02:00 committed by GitHub
parent 6fb87ac80a
commit b6748aa0d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 126 additions and 161 deletions

View file

@ -1,29 +1,21 @@
const { Task, sequelize } = require('../models');
const { Op } = require('sequelize');
const { logError } = require('./logService');
const taskRepository = require('../repositories/TaskRepository');
// Simple in-memory lock to prevent concurrent generation per user
const generationLocks = new Map();
// Helper function for pure calculations
const addDays = (date, days) => {
const result = new Date(date);
result.setDate(result.getDate() + days);
return result;
};
/**
* Generate recurring tasks with locking to prevent concurrent execution
* @param {number} userId - User ID to generate tasks for
* @param {number} lookAheadDays - Number of days to look ahead (default: 7)
* @returns {Promise<Array>} Array of newly created tasks
*/
const generateRecurringTasksWithLock = async (userId, lookAheadDays = 7) => {
const lockKey = `user_${userId}`;
// Check if generation is already in progress for this user
if (generationLocks.get(lockKey)) {
return []; // Already generating, skip this request
return [];
}
try {
@ -37,12 +29,6 @@ const generateRecurringTasksWithLock = async (userId, lookAheadDays = 7) => {
}
};
/**
* Generate new tasks from recurring task templates
* @param {number} userId - Optional user ID to limit processing
* @param {number} lookAheadDays - Number of days to look ahead for generating tasks (default: 7)
* @returns {Promise<Array>} Array of newly created tasks
*/
const generateRecurringTasks = async (userId = null, lookAheadDays = 7) => {
try {
const whereClause = {
@ -54,7 +40,6 @@ const generateRecurringTasks = async (userId = null, lookAheadDays = 7) => {
whereClause.user_id = userId;
}
// Find all recurring tasks that need processing
const recurringTasks = await Task.findAll({
where: whereClause,
order: [['last_generated_date', 'ASC']],
@ -80,31 +65,18 @@ const generateRecurringTasks = async (userId = null, lookAheadDays = 7) => {
}
};
/**
* Process a single recurring task and generate new instances if needed
* @param {Object} task - The recurring task template
* @param {Date} now - Current timestamp
* @param {Date} lookAheadDate - Date to generate tasks up to (default: now)
* @returns {Promise<Array>} Array of newly created task instances
*/
const processRecurringTask = async (task, now, lookAheadDate = null) => {
const newTasks = [];
const generateUpTo = lookAheadDate || now;
// Skip if recurrence has ended
if (task.recurrence_end_date && now > task.recurrence_end_date) {
return newTasks;
}
// Special handling for the initial due date - if the task has a due_date
// and no instances have been generated yet, check if the original due_date
// falls within our generation window
if (!task.last_generated_date && task.due_date) {
const originalDueDate = new Date(task.due_date.getTime());
// If the original due date is within our generation window, create it
if (originalDueDate <= generateUpTo) {
// Use date range to handle potential precision issues
const startOfDay = new Date(originalDueDate);
startOfDay.setUTCHours(0, 0, 0, 0);
const endOfDay = new Date(originalDueDate);
@ -118,7 +90,6 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
},
};
// Only add project_id to where clause if it's not null/undefined
if (task.project_id !== null && task.project_id !== undefined) {
whereClause.project_id = task.project_id;
} else {
@ -134,7 +105,6 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
newTasks.push(newTask);
}
// Update last generated date if the original due date has passed
if (originalDueDate <= now) {
task.last_generated_date = originalDueDate;
await task.save();
@ -142,16 +112,12 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
}
}
// Now generate subsequent recurring instances
let nextDueDate = calculateNextDueDate(
task,
task.last_generated_date || task.due_date || now
);
// Generate tasks up to the look-ahead date
while (nextDueDate && nextDueDate <= generateUpTo) {
// Check if this due date already has a task instance
// Use date range to handle potential precision issues
const startOfDay = new Date(nextDueDate);
startOfDay.setUTCHours(0, 0, 0, 0);
const endOfDay = new Date(nextDueDate);
@ -165,26 +131,22 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
},
};
// Only add project_id to where clause if it's not null/undefined
if (task.project_id !== null && task.project_id !== undefined) {
whereClause.project_id = task.project_id;
} else {
whereClause.project_id = null;
}
// Use a transaction to ensure atomic check-and-create
const result = await sequelize.transaction(async (transaction) => {
// Check if task exists within transaction
const existingTask = await Task.findOne({
where: whereClause,
transaction,
});
if (existingTask) {
return null; // Task already exists
return null;
}
// Create the task within the same transaction
return await createTaskInstance(task, nextDueDate, transaction);
});
@ -192,16 +154,13 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
newTasks.push(result);
}
// Update last generated date only for tasks that are due today or in the past
if (nextDueDate <= now) {
task.last_generated_date = nextDueDate;
await task.save();
}
// Calculate next due date from this one
nextDueDate = calculateNextDueDate(task, nextDueDate);
// Safety check to prevent infinite loops
if (newTasks.length > 100) {
console.warn(
`Generated 100+ tasks for recurring task ${task.id}, stopping to prevent overflow`
@ -213,12 +172,6 @@ const processRecurringTask = async (task, now, lookAheadDate = null) => {
return newTasks;
};
/**
* Create a new task instance from a recurring task template
* @param {Object} template - The recurring task template
* @param {Date} dueDate - Due date for the new task instance
* @returns {Promise<Object>} The newly created task
*/
const createTaskInstance = async (template, dueDate, transaction = null) => {
const taskData = {
name: template.name,
@ -230,8 +183,8 @@ const createTaskInstance = async (template, dueDate, transaction = null) => {
note: template.note,
user_id: template.user_id,
project_id: template.project_id,
recurrence_type: 'none', // Instances are not recurring themselves
recurring_parent_id: template.id, // Link to the original recurring task
recurrence_type: 'none',
recurring_parent_id: template.id,
};
const options = {};
@ -239,17 +192,42 @@ const createTaskInstance = async (template, dueDate, transaction = null) => {
options.transaction = transaction;
}
return await Task.create(taskData, options);
const newTask = await Task.create(taskData, options);
const subtasks = await taskRepository.findChildren(
template.id,
template.user_id
);
if (subtasks && subtasks.length > 0) {
const subtasksData = subtasks.map((subtask) => ({
name: subtask.name,
description: subtask.description,
parent_task_id: newTask.id,
user_id: template.user_id,
priority: subtask.priority,
status: Task.STATUS.NOT_STARTED,
note: subtask.note,
today: false,
recurrence_type: 'none',
completion_based: false,
}));
if (transaction) {
await Promise.all(
subtasksData.map((subtaskData) =>
Task.create(subtaskData, { transaction })
)
);
} else {
await taskRepository.createMany(subtasksData);
}
}
return newTask;
};
/**
* Calculate the next due date for a recurring task
* @param {Object} task - The recurring task
* @param {Date} fromDate - Date to calculate from
* @returns {Date|null} Next due date or null if no more recurrences
*/
const calculateNextDueDate = (task, fromDate) => {
// Handle invalid inputs
if (
!task ||
!task.recurrence_type ||
@ -259,8 +237,6 @@ const calculateNextDueDate = (task, fromDate) => {
return null;
}
// Use the fromDate parameter directly for calculation instead of baseDate logic
// This ensures we always move forward from the provided date
const startDate = new Date(fromDate.getTime());
switch (task.recurrence_type) {
@ -303,30 +279,16 @@ const calculateNextDueDate = (task, fromDate) => {
}
};
/**
* Calculate next daily recurrence
* @param {Date} fromDate - Starting date
* @param {number} interval - Days between recurrences
* @returns {Date} Next due date
*/
const calculateDailyRecurrence = (fromDate, interval) => {
const nextDate = new Date(fromDate);
nextDate.setDate(nextDate.getDate() + interval);
return nextDate;
};
/**
* Calculate next weekly recurrence
* @param {Date} fromDate - Starting date
* @param {number} interval - Weeks between recurrences
* @param {number} weekday - Target day of week (0=Sunday, 6=Saturday)
* @returns {Date} Next due date
*/
const calculateWeeklyRecurrence = (fromDate, interval, weekday) => {
const nextDate = new Date(fromDate);
if (weekday !== null && weekday !== undefined) {
// Find next occurrence of the specified weekday
const currentWeekday = nextDate.getDay();
const daysUntilTarget = (weekday - currentWeekday + 7) % 7;
@ -334,7 +296,6 @@ const calculateWeeklyRecurrence = (fromDate, interval, weekday) => {
daysUntilTarget === 0 &&
nextDate.getTime() === fromDate.getTime()
) {
// If today is the target weekday and we're calculating from today, add interval weeks
nextDate.setDate(nextDate.getDate() + interval * 7);
} else {
nextDate.setDate(nextDate.getDate() + daysUntilTarget);
@ -343,36 +304,25 @@ const calculateWeeklyRecurrence = (fromDate, interval, weekday) => {
}
}
} else {
// No specific weekday, just add interval weeks
nextDate.setDate(nextDate.getDate() + interval * 7);
}
return nextDate;
};
/**
* Calculate next monthly recurrence on specific day
* @param {Date} fromDate - Starting date
* @param {number} interval - Months between recurrences
* @param {number} dayOfMonth - Target day of month (1-31)
* @returns {Date} Next due date
*/
const calculateMonthlyRecurrence = (fromDate, interval, dayOfMonth) => {
const nextDate = new Date(fromDate);
const targetDay = dayOfMonth || fromDate.getUTCDate();
// Move to target month
const targetMonth = nextDate.getUTCMonth() + interval;
const targetYear = nextDate.getUTCFullYear() + Math.floor(targetMonth / 12);
const finalMonth = targetMonth % 12;
// Get the max day for the target month
const maxDay = new Date(
Date.UTC(targetYear, finalMonth + 1, 0)
).getUTCDate();
const finalDay = Math.min(targetDay, maxDay);
// Create the new date
const result = new Date(
Date.UTC(
targetYear,
@ -388,14 +338,6 @@ const calculateMonthlyRecurrence = (fromDate, interval, dayOfMonth) => {
return result;
};
/**
* Calculate next monthly recurrence on specific weekday of month
* @param {Date} fromDate - Starting date
* @param {number} interval - Months between recurrences
* @param {number} weekday - Target weekday (0=Sunday, 6=Saturday)
* @param {number} weekOfMonth - Which occurrence in month (1-5)
* @returns {Date} Next due date
*/
const calculateMonthlyWeekdayRecurrence = (
fromDate,
interval,
@ -405,28 +347,22 @@ const calculateMonthlyWeekdayRecurrence = (
const nextDate = new Date(fromDate);
nextDate.setUTCMonth(nextDate.getUTCMonth() + interval);
// Find the first day of the month
const firstOfMonth = new Date(
Date.UTC(nextDate.getUTCFullYear(), nextDate.getUTCMonth(), 1)
);
const firstWeekday = firstOfMonth.getUTCDay();
// Calculate the first occurrence of the target weekday
const daysToAdd = (weekday - firstWeekday + 7) % 7;
const firstOccurrence = new Date(firstOfMonth);
firstOccurrence.setUTCDate(1 + daysToAdd);
// Add weeks to get to the target week of month
const targetDate = new Date(firstOccurrence);
targetDate.setUTCDate(firstOccurrence.getUTCDate() + (weekOfMonth - 1) * 7);
// Make sure we're still in the same month
if (targetDate.getUTCMonth() !== nextDate.getUTCMonth()) {
// Week doesn't exist in this month, use last occurrence
targetDate.setUTCDate(targetDate.getUTCDate() - 7);
}
// Preserve the original time
targetDate.setUTCHours(
fromDate.getUTCHours(),
fromDate.getUTCMinutes(),
@ -437,29 +373,15 @@ const calculateMonthlyWeekdayRecurrence = (
return targetDate;
};
/**
* Calculate next monthly recurrence on last day of month
* @param {Date} fromDate - Starting date
* @param {number} interval - Months between recurrences
* @returns {Date} Next due date
*/
const calculateMonthlyLastDayRecurrence = (fromDate, interval) => {
const nextDate = new Date(fromDate);
nextDate.setUTCMonth(nextDate.getUTCMonth() + interval);
// Set to last day of month
nextDate.setUTCMonth(nextDate.getUTCMonth() + 1, 0);
return nextDate;
};
/**
* Helper function to get first weekday of month
* @param {number} year - Year
* @param {number} month - Month (0-11)
* @param {number} weekday - Weekday (0=Sunday, 6=Saturday)
* @returns {Date} First occurrence of weekday in month
*/
const getFirstWeekdayOfMonth = (year, month, weekday) => {
const firstOfMonth = new Date(year, month, 1);
const firstWeekday = firstOfMonth.getDay();
@ -467,13 +389,6 @@ const getFirstWeekdayOfMonth = (year, month, weekday) => {
return new Date(year, month, 1 + daysToAdd);
};
/**
* Helper function to get last weekday of month
* @param {number} year - Year
* @param {number} month - Month (0-11)
* @param {number} weekday - Weekday (0=Sunday, 6=Saturday)
* @returns {Date} Last occurrence of weekday in month
*/
const getLastWeekdayOfMonth = (year, month, weekday) => {
const lastOfMonth = new Date(year, month + 1, 0);
const lastWeekday = lastOfMonth.getDay();
@ -481,20 +396,11 @@ const getLastWeekdayOfMonth = (year, month, weekday) => {
return new Date(year, month, lastOfMonth.getDate() - daysToSubtract);
};
/**
* Helper function to get nth weekday of month
* @param {number} year - Year
* @param {number} month - Month (0-11)
* @param {number} weekday - Weekday (0=Saturday)
* @param {number} n - Which occurrence (1-5)
* @returns {Date} Nth occurrence of weekday in month
*/
const getNthWeekdayOfMonth = (year, month, weekday, n) => {
const firstOccurrence = getFirstWeekdayOfMonth(year, month, weekday);
const targetDate = new Date(firstOccurrence);
targetDate.setDate(firstOccurrence.getDate() + (n - 1) * 7);
// If target date is in next month, return null
if (targetDate.getMonth() !== month) {
return null;
}
@ -502,12 +408,6 @@ const getNthWeekdayOfMonth = (year, month, weekday, n) => {
return targetDate;
};
/**
* Helper function to check if next task should be generated
* @param {Object} task - The recurring task
* @param {Date} nextDate - Next due date
* @returns {boolean} Whether to generate next task
*/
const shouldGenerateNextTask = (task, nextDate) => {
if (!task.recurrence_end_date) {
return true;
@ -515,35 +415,24 @@ const shouldGenerateNextTask = (task, nextDate) => {
return nextDate < task.recurrence_end_date;
};
/**
* Handle task completion for recurring tasks
* @param {Object} task - The completed task
* @returns {Promise<Object|null>} Next task instance if applicable
*/
const handleTaskCompletion = async (task) => {
// Check if the completed task itself is a recurring task
if (!task.recurrence_type || task.recurrence_type === 'none') {
return null;
}
// Only generate next task if completion_based is true
if (!task.completion_based) {
return null;
}
// Update the task's last generated date to completion date
task.last_generated_date = new Date();
await task.save();
// For completion-based tasks, create the next instance immediately
const nextDueDate = calculateNextDueDate(task, new Date());
if (!nextDueDate) {
return null;
}
// Check if this due date already has a task instance
// Use date range to handle potential precision issues
const startOfDay = new Date(nextDueDate);
startOfDay.setUTCHours(0, 0, 0, 0);
const endOfDay = new Date(nextDueDate);
@ -557,7 +446,6 @@ const handleTaskCompletion = async (task) => {
},
};
// Only add project_id to where clause if it's not null/undefined
if (task.project_id !== null && task.project_id !== undefined) {
whereClause.project_id = task.project_id;
} else {
@ -569,10 +457,9 @@ const handleTaskCompletion = async (task) => {
});
if (existingTask) {
return null; // Task already exists for this date
return null;
}
// Create the next task instance
const nextTask = await createTaskInstance(task, nextDueDate);
return nextTask;
};
@ -593,7 +480,6 @@ module.exports = {
getFirstWeekdayOfMonth,
getLastWeekdayOfMonth,
getNthWeekdayOfMonth,
// For testing compatibility - underscore versions
_getFirstWeekdayOfMonth: getFirstWeekdayOfMonth,
_getLastWeekdayOfMonth: getLastWeekdayOfMonth,
_getNthWeekdayOfMonth: getNthWeekdayOfMonth,

View file

@ -693,4 +693,74 @@ describe('Recurring Tasks API', () => {
expect(recurringTask.original_name).toBe('Take vitamins');
});
});
describe('Recurring tasks with subtasks', () => {
it('should copy subtasks when generating recurring task instances', async () => {
const recurringTaskService = require('../../services/recurringTaskService');
// Create a recurring task with subtasks
const taskData = {
name: 'Weekly grocery shopping',
recurrence_type: 'daily',
recurrence_interval: 1,
priority: 1,
completion_based: false,
subtasks: [
{ name: 'Buy milk', priority: 0 },
{ name: 'Buy bread', priority: 1 },
{ name: 'Buy eggs', priority: 0 },
],
};
const createResponse = await agent.post('/api/task').send(taskData);
expect(createResponse.status).toBe(201);
const recurringTaskId = createResponse.body.id;
// Verify subtasks were created
const subtasksResponse = await agent.get(
`/api/task/${recurringTaskId}/subtasks`
);
expect(subtasksResponse.status).toBe(200);
expect(subtasksResponse.body.length).toBe(3);
// Generate recurring task instances
const tomorrow = new Date();
tomorrow.setDate(tomorrow.getDate() + 1);
await recurringTaskService.generateRecurringTasks(user.id, 2);
// Find the generated instance
const instances = await Task.findAll({
where: {
user_id: user.id,
recurring_parent_id: recurringTaskId,
},
});
expect(instances.length).toBeGreaterThan(0);
const firstInstance = instances[0];
// Check if subtasks were copied to the instance
const instanceSubtasksResponse = await agent.get(
`/api/task/${firstInstance.id}/subtasks`
);
expect(instanceSubtasksResponse.status).toBe(200);
expect(instanceSubtasksResponse.body.length).toBe(3);
// Verify subtask names match
const subtaskNames = instanceSubtasksResponse.body.map(
(s) => s.name
);
expect(subtaskNames).toContain('Buy milk');
expect(subtaskNames).toContain('Buy bread');
expect(subtaskNames).toContain('Buy eggs');
// Verify all subtasks are in NOT_STARTED status
instanceSubtasksResponse.body.forEach((subtask) => {
expect(subtask.status).toBe(Task.STATUS.NOT_STARTED);
});
});
});
});

View file

@ -43,7 +43,7 @@ test.describe('Notes - Basic Functionality', () => {
// Verify note appears in the list
await expect(
page.locator('h3', { hasText: noteTitle })
page.locator('h3', { hasText: noteTitle }).first()
).toBeVisible();
});
@ -66,10 +66,10 @@ test.describe('Notes - Basic Functionality', () => {
await page.keyboard.press('Escape');
// Verify note was created
await expect(page.locator('h3', { hasText: originalTitle })).toBeVisible();
await expect(page.locator('h3', { hasText: originalTitle }).first()).toBeVisible();
// Click on the note in the list to select it
await page.locator('h3', { hasText: originalTitle }).click();
await page.locator('h3', { hasText: originalTitle }).first().click();
await page.waitForTimeout(300);
// Verify we're in preview mode
@ -98,9 +98,9 @@ test.describe('Notes - Basic Functionality', () => {
).toBeVisible();
// Verify updated note appears in the list
await expect(page.locator('h3', { hasText: updatedTitle })).toBeVisible();
await expect(page.locator('h3', { hasText: updatedTitle }).first()).toBeVisible();
// Verify old title is not shown
await expect(page.locator('h3', { hasText: originalTitle })).not.toBeVisible();
await expect(page.locator('h3', { hasText: originalTitle })).toHaveCount(0);
});
});

View file

@ -70,7 +70,9 @@ test('user can set task priority to high', async ({ page, baseURL }) => {
// Wait for dropdown to close
await expect(page.locator('[data-testid="priority-dropdown"][data-state="closed"]')).toBeVisible();
// Save the task
// Wait for the save button to be stable after priority change
await page.waitForTimeout(200);
await expect(page.locator('[data-testid="task-save-button"]')).toBeVisible();
await page.locator('[data-testid="task-save-button"]').click();
// Wait for saving state then idle state
@ -113,7 +115,9 @@ test('user can set task priority to medium and low', async ({ page, baseURL }) =
await mediumPriorityOption.click();
await expect(page.locator('[data-testid="priority-dropdown"][data-state="closed"]')).toBeVisible();
// Save the task
// Wait for the save button to be stable after priority change
await page.waitForTimeout(200);
await expect(page.locator('[data-testid="task-save-button"]')).toBeVisible();
await page.locator('[data-testid="task-save-button"]').click();
await expect(page.locator('[data-testid="task-modal"][data-state="saving"]')).toBeVisible();
await expect(page.locator('[data-testid="task-modal"]')).not.toBeVisible({ timeout: 10000 });
@ -145,6 +149,9 @@ test('user can set task priority to medium and low', async ({ page, baseURL }) =
await lowPriorityOption.click();
await expect(page.locator('[data-testid="priority-dropdown"][data-state="closed"]')).toBeVisible();
// Wait for the save button to be stable after priority change
await page.waitForTimeout(200);
await expect(page.locator('[data-testid="task-save-button"]')).toBeVisible();
await page.locator('[data-testid="task-save-button"]').click();
await expect(page.locator('[data-testid="task-modal"][data-state="saving"]')).toBeVisible();
await expect(page.locator('[data-testid="task-modal"]')).not.toBeVisible({ timeout: 10000 });
@ -188,7 +195,9 @@ test('user can set a due date for a task', async ({ page, baseURL }) => {
await dayButton.click();
await expect(page.locator('[data-testid="datepicker"][data-state="closed"]')).toBeVisible();
// Save the task
// Wait for the save button to be stable after date change
await page.waitForTimeout(200);
await expect(page.locator('[data-testid="task-save-button"]')).toBeVisible();
await page.locator('[data-testid="task-save-button"]').click();
await expect(page.locator('[data-testid="task-modal"][data-state="saving"]')).toBeVisible();
await expect(page.locator('[data-testid="task-modal"]')).not.toBeVisible({ timeout: 10000 });