Skip to content

Commit

Permalink
Fix bugs in task deletion strategies
Browse files Browse the repository at this point in the history
  • Loading branch information
MaddyGuthridge committed Jul 5, 2023
1 parent 8ace71e commit 6774a5c
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
6 changes: 3 additions & 3 deletions src/data/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ export const getTasksInProject = (id: ProjectId): Task[] => {

/**
* Recursively expand a prerequisite to give a list of all prerequisite tasks
* given this task is a prerequisite (includes original task ID)
* given this task is a prerequisite (does not include original task ID)
*/
export const expandTaskPrerequisite = (id: TaskId): TaskId[] => {
const prerequisites = [id];
const prerequisites = [];

const task = getTaskById(id) as Task;

Expand Down Expand Up @@ -61,7 +61,7 @@ export const findAllSuccessorTasks = (id: TaskId): TaskId[] => {
const prereqs = expandTaskPrerequisite(task.id);
if (prereqs.includes(id)) {
// Only include it if it isn't there already
if (successors.includes(task.id)) {
if (!successors.includes(task.id)) {
successors.push(task.id);
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/routes/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ task.put(
`Task with ID ${prereqId} does not belong to same project`
);
}
if (expandTaskPrerequisite(prereqId).includes(taskId)) {
if (
// FIXME: Make ESLint prefer operators at start of lines
expandTaskPrerequisite(prereqId).includes(taskId) || prereqId === taskId
) {
throw HttpError(
400,
`Using task with ID ${prereqId} as a prerequisite would create a circular dependency`
Expand Down
3 changes: 2 additions & 1 deletion tests/task/remove.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ describe('/task remove', () => {

describe('reroute', () => {
it('updates prerequisites for direct dependents', async () => {
jest.setTimeout(99999999);
const { token } = await makeUser();
const { id: projectId } = await makeProject(token);
const { id: taskId1 } = await makeTask(token, projectId);
Expand All @@ -114,7 +115,7 @@ describe('/task remove', () => {

// Delete task 2 with reroute strategy
await expect(
api.task.remove(token, taskId2, TaskDeletionStrategy.Cascade)
api.task.remove(token, taskId2, TaskDeletionStrategy.Reroute)
).resolves.toStrictEqual(
{}
);
Expand Down

0 comments on commit 6774a5c

Please sign in to comment.