Skip to content

Commit

Permalink
Feat: add uniqueness constraint to task table (#454)
Browse files Browse the repository at this point in the history
* Feat: add uniqueness constraint to task table

Add uniqueness constraint on tasks to prevent assigning the same
benchmark (subgoal) to the same para (assignee).

* Katrina feedback: more informative error message

* Remove comment, code is self-explanatory

* Add custom error to assignTaskToParas as well, plus spec

* Tweak error message

* Add error message to frontend modal

* cause createPara to return para or throw rather than undefined

* handle type of error in frontend
  • Loading branch information
canjalal authored Oct 24, 2024
1 parent 3fe2344 commit aea652d
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- Each task should have a unique subgoal_id - assignee_id combination
-- which corresponds to a unique benchmark / para combo
ALTER TABLE task
ADD CONSTRAINT subgoal_assignee_unique UNIQUE (subgoal_id, assignee_id);

-- Add index to allow easy queries of tasks by assignee
CREATE INDEX idx_task_assignee ON task(assignee_id);
2 changes: 1 addition & 1 deletion src/backend/db/zapatos/schema.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/backend/lib/db_helpers/case_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function createPara(
from_email: string,
to_email: string,
env: Env
): Promise<user.Selectable | undefined> {
): Promise<user.Selectable> {
const { first_name, last_name, email } = para;

let paraData = await db
Expand All @@ -40,7 +40,7 @@ export async function createPara(
role: "staff",
})
.returningAll()
.executeTakeFirst();
.executeTakeFirstOrThrow();

// promise, will not interfere with returning paraData
void sendInviteEmail(
Expand Down
2 changes: 1 addition & 1 deletion src/backend/routers/case_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export const case_manager = router({
);

return await assignParaToCaseManager(
para?.user_id || "",
para.user_id,
req.ctx.auth.userId,
req.ctx.db
);
Expand Down
137 changes: 135 additions & 2 deletions src/backend/routers/iep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => {
number_of_trials: 15,
});

const subgoal1 = await trpc.iep.addSubgoal.mutate({
goal_id: goal1!.goal_id,
status: "Complete",
description: "subgoal 1",
setup: "",
instructions: "",
materials: "materials",
target_level: 100,
baseline_level: 20,
metric_name: "words",
attempts_per_trial: 10,
number_of_trials: 30,
});
const subgoal1Id = subgoal1!.subgoal_id;

const subgoal2 = await trpc.iep.addSubgoal.mutate({
goal_id: goal1!.goal_id,
status: "Complete",
Expand All @@ -51,7 +66,7 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => {
const subgoal2Id = subgoal2!.subgoal_id;

await trpc.iep.addTask.mutate({
subgoal_id: subgoal2Id,
subgoal_id: subgoal1Id,
assignee_id: para_id,
due_date: new Date("2023-12-31"),
trial_count: 5,
Expand All @@ -70,7 +85,7 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => {
const gotSubgoals = await trpc.iep.getSubgoals.query({
goal_id: goal1!.goal_id,
});
t.is(gotSubgoals.length, 2);
t.is(gotSubgoals.length, 3);

const gotSubgoal = await trpc.iep.getSubgoal.query({
subgoal_id: subgoal2Id,
Expand All @@ -88,6 +103,124 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => {
);
});

test("addTask - no duplicate subgoal_id + assigned_id combo", async (t) => {
const { trpc, seed } = await getTestServer(t, {
authenticateAs: "case_manager",
});

const para_id = seed.para.user_id;

const iep = await trpc.student.addIep.mutate({
student_id: seed.student.student_id,
start_date: new Date("2023-01-01"),
end_date: new Date("2023-12-31"),
});

const goal1 = await trpc.iep.addGoal.mutate({
iep_id: iep.iep_id,
description: "goal 1",
category: "writing",
});

const subgoal1 = await trpc.iep.addSubgoal.mutate({
goal_id: goal1!.goal_id,
status: "Complete",
description: "subgoal 1",
setup: "",
instructions: "",
materials: "materials",
target_level: 100,
baseline_level: 20,
metric_name: "words",
attempts_per_trial: 10,
number_of_trials: 30,
});
const subgoal1Id = subgoal1!.subgoal_id;

await trpc.iep.addTask.mutate({
subgoal_id: subgoal1Id,
assignee_id: para_id,
due_date: new Date("2023-12-31"),
trial_count: 5,
});

const error = await t.throwsAsync(async () => {
await trpc.iep.addTask.mutate({
subgoal_id: subgoal1Id,
assignee_id: para_id,
due_date: new Date("2024-03-31"),
trial_count: 1,
});
});

t.is(
error?.message,
"Task already exists: This subgoal has already been assigned to the same para"
);
});

test("assignTaskToParas - no duplicate subgoal_id + para_id combo", async (t) => {
const { trpc, seed } = await getTestServer(t, {
authenticateAs: "case_manager",
});

const para_1 = seed.para;

const para_2 = await trpc.para.createPara.mutate({
first_name: "Foo",
last_name: "Bar",
email: "[email protected]",
});

const iep = await trpc.student.addIep.mutate({
student_id: seed.student.student_id,
start_date: new Date("2023-01-01"),
end_date: new Date("2023-12-31"),
});

const goal1 = await trpc.iep.addGoal.mutate({
iep_id: iep.iep_id,
description: "goal 1",
category: "writing",
});

const subgoal1 = await trpc.iep.addSubgoal.mutate({
goal_id: goal1!.goal_id,
status: "Complete",
description: "subgoal 1",
setup: "",
instructions: "",
materials: "materials",
target_level: 100,
baseline_level: 20,
metric_name: "words",
attempts_per_trial: 10,
number_of_trials: 30,
});
const subgoal1Id = subgoal1!.subgoal_id;

await trpc.iep.assignTaskToParas.mutate({
subgoal_id: subgoal1Id,
para_ids: [para_1.user_id],
due_date: new Date("2023-12-31"),
trial_count: 5,
});

const error = await t.throwsAsync(async () => {
await trpc.iep.assignTaskToParas.mutate({
subgoal_id: subgoal1Id,
para_ids: [para_1.user_id, para_2.user_id],
due_date: new Date("2024-03-31"),
trial_count: 1,
});
});

t.is(
error?.message,
"Task already exists: This subgoal has already been assigned to one or more of these paras"
);
});

test("add benchmark - check full schema", async (t) => {
const { trpc, seed } = await getTestServer(t, {
authenticateAs: "case_manager",
Expand Down
26 changes: 26 additions & 0 deletions src/backend/routers/iep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,19 @@ export const iep = router({
.mutation(async (req) => {
const { subgoal_id, assignee_id, due_date, trial_count } = req.input;

const existingTask = await req.ctx.db
.selectFrom("task")
.where("subgoal_id", "=", subgoal_id)
.where("assignee_id", "=", assignee_id)
.selectAll()
.executeTakeFirst();

if (existingTask) {
throw new Error(
"Task already exists: This subgoal has already been assigned to the same para"
);
}

const result = await req.ctx.db
.insertInto("task")
.values({
Expand All @@ -160,6 +173,19 @@ export const iep = router({
.mutation(async (req) => {
const { subgoal_id, para_ids, due_date, trial_count } = req.input;

const existingTasks = await req.ctx.db
.selectFrom("task")
.where("subgoal_id", "=", subgoal_id)
.where("assignee_id", "in", para_ids)
.selectAll()
.execute();

if (existingTasks.length > 0) {
throw new Error(
"Task already exists: This subgoal has already been assigned to one or more of these paras"
);
}

const result = await req.ctx.db
.insertInto("task")
.values(
Expand Down
44 changes: 31 additions & 13 deletions src/components/benchmarks/BenchmarkAssignmentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,12 @@ export const BenchmarkAssignmentModal = (
subgoal_id: props.benchmark_id,
});

const [errorMessage, setErrorMessage] = useState<string>("");

const assignTaskToPara = trpc.iep.assignTaskToParas.useMutation();

const handleParaToggle = (paraId: string) => () => {
setErrorMessage("");
setSelectedParaIds((prev) => {
if (prev.includes(paraId)) {
return prev.filter((id) => id !== paraId);
Expand All @@ -69,6 +72,7 @@ export const BenchmarkAssignmentModal = (
const handleClose = () => {
props.onClose();
setSelectedParaIds([]);
setErrorMessage("");
setCurrentModalSelection("PARA_SELECTION");
};

Expand All @@ -90,19 +94,27 @@ export const BenchmarkAssignmentModal = (
setCurrentModalSelection(nextStep);
} else {
// Reached end, save
await assignTaskToPara.mutateAsync({
subgoal_id: props.benchmark_id,
para_ids: selectedParaIds,
due_date:
assignmentDuration.type === "until_date"
? assignmentDuration.date
: undefined,
trial_count:
assignmentDuration.type === "minimum_number_of_collections"
? assignmentDuration.minimumNumberOfCollections
: undefined,
});
handleClose();
try {
await assignTaskToPara.mutateAsync({
subgoal_id: props.benchmark_id,
para_ids: selectedParaIds,
due_date:
assignmentDuration.type === "until_date"
? assignmentDuration.date
: undefined,
trial_count:
assignmentDuration.type === "minimum_number_of_collections"
? assignmentDuration.minimumNumberOfCollections
: undefined,
});
handleClose();
} catch (err) {
// TODO: issue #450
console.log(err);
if (err instanceof Error) {
setErrorMessage(err.message);
}
}
}
};

Expand Down Expand Up @@ -173,6 +185,12 @@ export const BenchmarkAssignmentModal = (
/>
</Box>
)}

{errorMessage && (
<Box className={$benchmark.benchmarkDescriptionBox}>
{errorMessage}
</Box>
)}
<DialogActions>
{currentModalSelection !== STEPS[0] && (
<Button
Expand Down

0 comments on commit aea652d

Please sign in to comment.