diff --git a/controllers/admin/task.controller.js b/controllers/admin/task.controller.js index c93c876..920b9c5 100644 --- a/controllers/admin/task.controller.js +++ b/controllers/admin/task.controller.js @@ -62,7 +62,7 @@ async function getTaskListMembers(taskListId) { // ─── Allowed filter/sort fields ─────────────────────────────────────────────── const TASK_LIST_FIELDS = ['name', 'description', 'createdAt', 'updatedAt', 'deletedAt']; -const TASK_FIELDS = ['name', 'description', 'deadline', 'order_index', 'is_required', 'status', 'createdAt', 'updatedAt', 'deletedAt']; +const TASK_FIELDS = ['name', 'description', 'deadline', 'order_index', 'status', 'createdAt', 'updatedAt', 'deletedAt']; const FILTERABLE_MODELS = { TaskList: TaskList, Task: Task, @@ -816,7 +816,7 @@ exports.createTask = async (req, res) => { const t = await sequelize.transaction(); try { const { taskListId } = req.params; - const { name, description, deadline, is_required, requirements = [], prerequisite_task_ids = [] } = req.body; + const { name, description, deadline, requirements = [], prerequisite_task_ids = [] } = req.body; if (!name) return R.error(res, 'Task name is required.', 400); @@ -835,7 +835,6 @@ exports.createTask = async (req, res) => { description, deadline: deadline || null, order_index, - is_required: is_required ?? true, createdBy: req.user.user_id, updatedBy: req.user.user_id, }, @@ -899,7 +898,7 @@ exports.updateTask = async (req, res) => { return R.error(res, 'Task not found.', 404); } - const { name, description, deadline, status, is_required, accepts_submissions, requirements, prerequisite_task_ids } = req.body; + const { name, description, deadline, status, accepts_submissions, requirements, prerequisite_task_ids } = req.body; const wasAccepting = task.accepts_submissions; @@ -908,7 +907,6 @@ exports.updateTask = async (req, res) => { if (description !== undefined) updates.description = description; if (deadline !== undefined) updates.deadline = deadline || null; if (status !== undefined) updates.status = status; - if (is_required !== undefined) updates.is_required = is_required; if (accepts_submissions !== undefined) updates.accepts_submissions = accepts_submissions; await task.update(updates, { transaction: t }); diff --git a/controllers/client/task.controller.js b/controllers/client/task.controller.js index 0678766..9ebed0e 100644 --- a/controllers/client/task.controller.js +++ b/controllers/client/task.controller.js @@ -210,56 +210,31 @@ const isRequirementDone = (r, signals) => { // ─── Helper: resolve which sibling task(s) are blocking a locked task ─────── // Prerequisites are always siblings within the same task list (enforced by // admin's syncTaskPrerequisites), so names can always be resolved from `arr`. -// Mirrors the locked boolean rule above: explicit prereqIds win when present, -// otherwise falls back to earlier is_required tasks in order_index order. +// A task with no explicit task_prerequisites rows is never locked. const resolveLockedBy = (task, i, arr, prereqsByTask, completedById) => { const prereqIds = prereqsByTask.get(task.task_id); - const blockers = (prereqIds && prereqIds.length) - ? arr.filter((t) => prereqIds.includes(t.task_id) && completedById.get(t.task_id) !== true) - : arr.slice(0, i).filter((prev) => prev.is_required && !completedById.get(prev.task_id)); + if (!prereqIds || !prereqIds.length) return []; + const blockers = arr.filter((t) => prereqIds.includes(t.task_id) && completedById.get(t.task_id) !== true); return blockers.map((t) => ({ task_id: t.task_id, name: t.name })); }; // ─── Helper: explicit prerequisite gate ───────────────────────────────────── // Returns true/false when `taskId` has explicit task_prerequisites rows — -// ALL of them must be completed by this user. Returns null when the task has -// no explicit prerequisites configured, signaling the caller to fall back to -// the default order_index/is_required linear sequencing below. +// ALL of them must be completed by this user. Returns true (unlocked) when +// the task has no explicit prerequisites configured. const checkPrerequisitesUnlocked = async (userId, taskId) => { const prereqRows = await TaskPrerequisite.findAll({ where: { task_id: taskId } }); - if (!prereqRows.length) return null; + if (!prereqRows.length) return true; const results = await Promise.all(prereqRows.map((r) => checkTaskCompletion(userId, r.prerequisite_task_id))); return results.every(Boolean); }; // ─── Helper: server-side sequencing gate ─────────────────────────────────── -// Prefers a task's explicit prerequisites (see checkPrerequisitesUnlocked) -// when configured; otherwise falls back to the original rule — rejects a -// completion write if any earlier *required* task in the same list isn't -// complete yet. Shared by this file's submitTask and -// task_progress.controller.js's visitLink/updateProgress. -const assertTaskUnlocked = async (userId, taskListId, orderIndex, taskId) => { - if (taskId) { - const prereqResult = await checkPrerequisitesUnlocked(userId, taskId); - if (prereqResult !== null) return prereqResult; - } - - const earlierRequired = await Task.findAll({ - where: { task_list_id: taskListId, order_index: { [Op.lt]: orderIndex }, is_required: true }, - include: [{ model: TaskRequirement, as: 'requirements' }], - }); - if (!earlierRequired.length) return true; - - const taskIds = earlierRequired.map((t) => t.task_id); - const allRequirements = earlierRequired.flatMap((t) => (t.requirements ?? []).map((r) => r.toJSON())); - const signals = await getTaskCompletionSignals(userId, taskIds, allRequirements); - - return earlierRequired.every((t) => { - const reqs = (t.requirements ?? []); - return reqs.length > 0 && reqs.every((r) => isRequirementDone(r.toJSON ? r.toJSON() : r, signals)); - }); -}; +// A task is locked only by its own explicit task_prerequisites rows — no +// implicit locking based on list position. Shared by this file's submitTask +// and task_progress.controller.js's visitLink/updateProgress. +const assertTaskUnlocked = async (userId, taskId) => checkPrerequisitesUnlocked(userId, taskId); // ─── Helper: is this one task fully done for this user, right now? ───────── const checkTaskCompletion = async (userId, taskId) => { @@ -407,8 +382,8 @@ exports.getGroupTaskList = async (req, res) => { } // ── Bucket + lock each task — a task with explicit prerequisites is - // locked until ALL of them are done; otherwise fall back to the linear - // order_index/is_required rule (same rule assertTaskUnlocked enforces). + // locked until ALL of them are done; a task with none is never locked + // (same rule assertTaskUnlocked enforces). const bucketedTasks = tasks.map((task, i, arr) => { const has_completed = completedById.get(task.task_id); @@ -547,8 +522,8 @@ exports.getGroupTaskLists = async (req, res) => { task.has_completed = computeHasCompleted(task); }); - // has_completed lookup scoped to THIS list's tasks (order_index - // fallback only ever looks at siblings within the same list). + // has_completed lookup scoped to THIS list's tasks (prerequisite + // edges only ever point at siblings within the same list). const completedById = new Map(tasks.map((task) => [task.task_id, task.has_completed])); tasks.forEach((task, i, arr) => { task.lockedBy = resolveLockedBy(task, i, arr, prereqsByTask, completedById); @@ -655,7 +630,7 @@ exports.getTask = async (req, res) => { const data = task.toJSON(); data.latest_completion = data.completions?.[0] ?? null; delete data.completions; - data.locked = !(await assertTaskUnlocked(req.user.user_id, taskListId, task.order_index, taskId)); + data.locked = !(await assertTaskUnlocked(req.user.user_id, taskId)); return R.success(res, 'Task retrieved.', data); } catch (err) { @@ -728,9 +703,9 @@ exports.submitTask = async (req, res) => { return R.error(res, 'This task no longer accepts submissions.', 409); } - if (!(await assertTaskUnlocked(req.user.user_id, taskListId, task.order_index, taskId))) { + if (!(await assertTaskUnlocked(req.user.user_id, taskId))) { await t.rollback(); - return R.error(res, 'Complete the earlier required tasks in this list first.', 409); + return R.error(res, 'Complete this task\'s prerequisite tasks first.', 409); } const wasComplete = await checkTaskCompletion(req.user.user_id, taskId); diff --git a/controllers/client/task_progress.controller.js b/controllers/client/task_progress.controller.js index 392463a..8c7318b 100644 --- a/controllers/client/task_progress.controller.js +++ b/controllers/client/task_progress.controller.js @@ -218,9 +218,9 @@ exports.visitLink = async (req, res) => { const task = await Task.findOne({ where: { task_id: taskId, task_list_id: taskListId }, transaction: t }); if (!task) { await t.rollback(); return R.error(res, 'Task not found.', 404); } - if (!(await assertTaskUnlocked(req.user.user_id, taskListId, task.order_index, taskId))) { + if (!(await assertTaskUnlocked(req.user.user_id, taskId))) { await t.rollback(); - return R.error(res, 'Complete the earlier required tasks in this list first.', 409); + return R.error(res, 'Complete this task\'s prerequisite tasks first.', 409); } const wasComplete = await checkTaskCompletion(req.user.user_id, taskId); @@ -349,9 +349,9 @@ exports.updateProgress = async (req, res) => { const task = await Task.findOne({ where: { task_id: taskId, task_list_id: taskListId }, transaction: t }); if (!task) { await t.rollback(); return R.error(res, 'Task not found.', 404); } - if (!(await assertTaskUnlocked(req.user.user_id, taskListId, task.order_index, taskId))) { + if (!(await assertTaskUnlocked(req.user.user_id, taskId))) { await t.rollback(); - return R.error(res, 'Complete the earlier required tasks in this list first.', 409); + return R.error(res, 'Complete this task\'s prerequisite tasks first.', 409); } const now = new Date(); diff --git a/database/migrations/20260720000001-remove-is-required-from-tasks.js b/database/migrations/20260720000001-remove-is-required-from-tasks.js new file mode 100644 index 0000000..9fb41fa --- /dev/null +++ b/database/migrations/20260720000001-remove-is-required-from-tasks.js @@ -0,0 +1,24 @@ +'use strict'; + +// The order_index/is_required linear sequencing lock (added in +// 20260709000005-add-order-index-and-is-required-to-tasks) is redundant now +// that tasks have an explicit prerequisite graph (task_prerequisites, added +// in 20260717000001-create-task-prerequisites). Locking a task is now purely +// a function of its task_prerequisites rows — a task with none is never +// implicitly locked by its position in the list. order_index is kept as a +// display-order-only column. + +module.exports = { + async up(queryInterface) { + await queryInterface.removeColumn('tasks', 'is_required'); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.addColumn('tasks', 'is_required', { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: true, + after: 'order_index', + }); + }, +}; diff --git a/models/task/task.mdl.js b/models/task/task.mdl.js index 77ea7e0..53460ab 100644 --- a/models/task/task.mdl.js +++ b/models/task/task.mdl.js @@ -41,8 +41,7 @@ const Task = sequelize.define('Task', { name: { type: DataTypes.STRING, allowNull: false, validate: { notEmpty: true }, order: 1, filterable: true }, description: { type: DataTypes.TEXT, allowNull: true, hidden: true, filterable: false }, deadline: { type: DataTypes.DATE, allowNull: true, order: 2, filterable: true }, - order_index: { type: DataTypes.INTEGER, allowNull: false, defaultValue: 0, order: 2.1, filterable: true, comment: 'Position within the task list — drives sequencing lock.' }, - is_required: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true, order: 2.2, filterable: true, comment: 'Optional tasks do not block later tasks in the sequencing lock.' }, + order_index: { type: DataTypes.INTEGER, allowNull: false, defaultValue: 0, order: 2.1, filterable: true, comment: 'Position within the task list — display order only. Locking is driven solely by explicit task_prerequisites.' }, accepts_submissions: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true, order: 2.25, filterable: true, comment: 'When false, no new TaskCompletion submissions are accepted for this task (existing completions are unaffected).' }, status: { type: DataTypes.ENUM('pending', 'in_progress', 'completed', 'overdue'), defaultValue: 'pending', allowNull: false, filterable: true }, auto_marked_at: { type: DataTypes.DATE, allowNull: true, filterable: true, comment: 'Set only by the taskOverdue cron sweep when it auto-flips status; never touched by user-driven completion.' },