diff --git a/frontend/src/tests/ddah-tests.js b/frontend/src/tests/ddah-tests.js index b6db3f104..ecb985969 100644 --- a/frontend/src/tests/ddah-tests.js +++ b/frontend/src/tests/ddah-tests.js @@ -87,8 +87,9 @@ export function ddahTests(api) { it("get all ddahs associated with a session", async () => { let resp = await apiGET(`/admin/sessions/${session.id}/ddahs`); expect(resp).toHaveStatus("success"); - // Originally only one ddah is seeded - expect(resp.payload.length).toEqual(1); + // Originally two ddahs are seeded and + // another one is added in this test suite + expect(resp.payload.length).toEqual(3); expect(resp.payload).toContainObject(ddah); }); diff --git a/frontend/src/tests/instructor-permission-test.js b/frontend/src/tests/instructor-permission-test.js index 0cbb54e64..aa4df5a45 100644 --- a/frontend/src/tests/instructor-permission-test.js +++ b/frontend/src/tests/instructor-permission-test.js @@ -46,55 +46,6 @@ export function instructorsPermissionTests(api) { expect(respSwitchBackUser).toHaveStatus("success"); } - /** - * Updates a position to include instructor with `instructorId` and - * creates a DDAH for one of the assignments realted to that - * instructor. - * - * @param instructorId: int - the unique Id of an instructor - * - * @returns {Promise} - */ - async function createDdahWithFixedDuties(instructorId) { - // We first need to update position to include our instructor - const existingPosition = databaseSeeder.seededData.positions[0]; - const positionWithInstructor = { - ...existingPosition, - instructor_ids: [...existingPosition.instructor_ids, instructorId], - }; - let resp = await apiPOST(`/admin/positions`, positionWithInstructor); - expect(resp).toHaveStatus("success"); - - // We then proceed to create a DDAH for that position - resp = await apiGET(`/instructor/sessions/${session.id}/assignments`); - expect(resp).toHaveStatus("success"); - expect(resp.payload.length).toBeGreaterThan(0); - const newDdah = { - assignment_id: resp.payload[0].id, - duties: [ - { - order: 2, - hours: 25, - description: "marking:Marking the midterm", - }, - { - order: 1, - hours: 4, - description: "training:Initial training", - }, - { - order: 3, - hours: 40, - description: "contact:Running tutorials", - }, - ], - }; - - resp = await apiPOST(`/admin/ddahs`, newDdah); - expect(resp).toHaveStatus("success"); - return resp.payload; - } - beforeAll(async () => { await databaseSeeder.seed(api); await databaseSeeder.seedForInstructors(api); @@ -116,7 +67,7 @@ export function instructorsPermissionTests(api) { existingContractTemplateId = resp.payload[0].id; const instructorOnlyUserData = { - utorid: "instructor_only_test_user_utorid", + ...databaseSeeder.seededData.instructors[3], roles: ["instructor"], }; @@ -524,43 +475,212 @@ export function instructorsPermissionTests(api) { await restoreDefaultUser(); }); - it("fetch Ddahs", async () => { - // If a user is not in Instructors table, it is not considered an instructor - // for the purpose of fetching DDAHs - so we create one - const newInstructor = { - first_name: "Jane", - last_name: "Smith", - email: "jane.smith@gmail.com", - utorid: instructorUser.utorid, - }; - let resp = await apiPOST(`/admin/instructors`, newInstructor); - expect(resp).toHaveStatus("success"); + describe("Ddah permissions", () => { + let position = databaseSeeder.seededData.positions[3]; + let assignment; + let assignmentInstructorCantAccess; + + beforeAll(async () => { + await restoreDefaultUser(); + + // Create another assignment (with no DDAH) + // for position associated with our instructor + const newAssignment = { + applicant_id: databaseSeeder.seededData.applicant.id, + position_id: position.id, + start_date: "2019-09-02T00:00:00.000Z", + end_date: "2019-12-31T00:00:00.000Z", + }; + let resp = await apiPOST("/admin/assignments", newAssignment); + expect(resp).toHaveStatus("success"); + expect(resp.payload).toBeDefined(); + assignment = resp.payload; + + // Create another assignment (with no DDAH) + // for position associated with other instructor + const otherPosition = databaseSeeder.seededData.positions[4]; + const otherAssignment = { + note: "", + applicant_id: databaseSeeder.seededData.applicant.id, + position_id: otherPosition.id, + start_date: "2019-09-02T00:00:00.000Z", + end_date: "2019-12-31T00:00:00.000Z", + }; + resp = await apiPOST("/admin/assignments", otherAssignment); + expect(resp).toHaveStatus("success"); + assignmentInstructorCantAccess = resp.payload; + }); - // Get newly created instructor and create a DDAH for them - resp = await apiGET(`/admin/instructors`); - expect(resp).toHaveStatus("success"); - const instructorId = resp.payload.find( - (instructor) => instructor.utorid === instructorUser.utorid - )?.id; - expect(instructorId).toBeDefined(); - const newDDAH = await createDdahWithFixedDuties(instructorId); + beforeEach(async () => { + await switchToInstructorOnlyUser(); + }); - // Test the DDAH is fetched properly - await switchToInstructorOnlyUser(); - resp = await apiGET(`/instructor/sessions/${session.id}/ddahs`); - expect(resp).toHaveStatus("success"); - expect(resp.payload).toHaveLength(1); - expect(resp.payload[0]).toStrictEqual(newDDAH); - }); + it("fetch Ddahs associated with self only", async () => { + // Our instructor should only be able to fetch ddahs associated with them + const resp = await apiGET( + `/instructor/sessions/${session.id}/ddahs` + ); + expect(resp).toHaveStatus("success"); + + // Only one DDAH is intially seeded for this instructor + expect(resp.payload).toHaveLength(1); + + // We get duties returned sorted in ascending order, so + // we need to sort the seeded ones before comparing + const sortedSeededDuties = databaseSeeder.seededData.ddahs[0].duties.sort( + (first, second) => first.order - second.order + ); + const firstDdah = resp.payload[0]; + expect(firstDdah.duties).toEqual(sortedSeededDuties); + }); + + it("create a Ddah for an assignment associated with self", async () => { + const ddahToCreate = { + // This assignment has no DDAH initially + assignment_id: assignment.id, + duties: [ + { + order: 1, + hours: 20, + description: "other:Facilitating workshops", + }, + { + order: 2, + hours: 50, + description: "other:Lecture support", + }, + ], + }; + let resp = await apiPOST(`/instructor/ddahs`, ddahToCreate); + expect(resp).toHaveStatus("success"); + expect(resp.payload).toEqual(expect.objectContaining(ddahToCreate)); + + // Test we can fetch both DDAHs now: the seeded one and the newly created one + resp = await apiGET(`/instructor/sessions/${session.id}/ddahs`); + expect(resp).toHaveStatus("success"); + expect(resp.payload).toHaveLength(2); + }); - it.todo("fetch Ddahs a position associated with self"); - it.todo("fetch Ddahs an assignment associated with self"); - it.todo("cannot fetch Ddahs for assignment not associated with self"); - it.todo("create a Ddah for an assignment associated with self"); - it.todo("update a Ddah for an assignment associated with self"); - it.todo( - "cannot set approved_date/accepted_date/revised_date/emailed_ate/signature for a Ddah associated with self" - ); - it.todo("cannot create a Ddah for an assignment not associated with self"); - it.todo("cannot update a Ddah for an assignment not associated with self"); + it("update a Ddah for an assignment associated with self", async () => { + // Fetch the first assignment associated with our instructor + let resp = await apiGET( + `/instructor/sessions/${session.id}/assignments` + ); + expect(resp).toHaveStatus("success"); + const assignment_id = resp.payload[0].id; + + // Update the DDAH and check it was updated correctly + const updatedDdah = { + assignment_id, + duties: [ + { + order: 1, + hours: 20, + description: "marking:Test Marking", + }, + { + order: 2, + hours: 50, + description: "other:Additional duties", + }, + ], + }; + resp = await apiPOST(`/instructor/ddahs`, updatedDdah); + expect(resp).toHaveStatus("success"); + expect(resp.payload).toEqual(expect.objectContaining(updatedDdah)); + }); + + // This test indeed fails and allows to set these fields, check Issue #608 + it.skip("cannot set approved_date/accepted_date/revised_date/emailed_date/signature for a Ddah associated with self", async () => { + // Fetch the DDAH related to instructor's assignment + let resp = await apiGET(`/instructor/sessions/${session.id}/ddahs`); + expect(resp).toHaveStatus("success"); + const originalDdah = resp.payload.find( + (ddah) => ddah.assignment_id === assignment.id + ); + + // Update the DDAH and check it was updated correctly + const ddahWithRestrictedFields = { + assignment_id: assignment.id, + duties: [ + { + order: 1, + hours: 40, + description: "marking:Test Marking", + }, + { + order: 2, + hours: 30, + description: "other:Additional duties", + }, + ], + approved_date: new Date().toISOString(), + accepted_date: "2020-09-02T00:00:00.000Z", + revised_date: "2020-09-01T00:00:00.000Z", + emailed_date: "2020-08-28T00:00:00.000Z", + signature: "Harry Potter", + }; + resp = await apiPOST(`/instructor/ddahs`, ddahWithRestrictedFields); + expect(resp).toHaveStatus("success"); + expect(resp.payload).toEqual(expect.objectContaining(originalDdah)); + + resp = await apiGET(`/instructor/sessions/${session.id}/ddahs`); + // still a success, but does not change the underlying data + expect(resp).toHaveStatus("success"); + const newDdah = resp.payload.find( + (ddah) => ddah.assignment_id === assignment.id + ); + expect(newDdah).toEqual(expect.objectContaining(originalDdah)); + }); + + it("cannot create a Ddah for an assignment not associated with self", async () => { + const ddahToCreate = { + // This assignment has no DDAH initially + assignment_id: assignmentInstructorCantAccess.id, + duties: [ + { + order: 1, + hours: 20, + description: "other:Facilitating workshops", + }, + ], + }; + let resp = await apiPOST(`/instructor/ddahs`, ddahToCreate); + expect(resp).toHaveStatus("error"); + }); + + it("cannot update a Ddah for an assignment not associated with self", async () => { + // First we add the DDAH to the assignment + // associated with another instructor + await restoreDefaultUser(); + const ddahToCreate = { + assignment_id: assignmentInstructorCantAccess.id, + duties: [ + { + order: 1, + hours: 30, + description: "contact:Practice", + }, + ], + }; + let resp = await apiPOST(`/admin/ddahs`, ddahToCreate); + expect(resp).toHaveStatus("success"); + + // Then we try to update the existing DDAH with our instructor + await switchToInstructorOnlyUser(); + const ddahToUpdate = { + // This assignment has a DDAH initially + assignment_id: assignmentInstructorCantAccess.id, + duties: [ + { + order: 1, + hours: 20, + description: "other:Facilitating workshops", + }, + ], + }; + resp = await apiPOST(`/instructor/ddahs`, ddahToUpdate); + expect(resp).toHaveStatus("error"); + }); + }); } diff --git a/frontend/src/tests/setup.js b/frontend/src/tests/setup.js index a6d8c131f..6a7401042 100644 --- a/frontend/src/tests/setup.js +++ b/frontend/src/tests/setup.js @@ -66,6 +66,18 @@ class DatabaseSeeder { email: "megan.miller@utoronto.ca", utorid: "millerm", }, + { + last_name: "Lucas", + first_name: "George", + email: "george.lucas@utoronto.ca", + utorid: "lucasg", + }, + { + last_name: "Bell", + first_name: "Jordan", + email: "jordan.bell@utoronto.ca", + utorid: "belljo", + }, ], applicants: [ { @@ -147,6 +159,22 @@ class DatabaseSeeder { instructor_ids: [], instructor_utorids: [], }, + { + position_code: "CSC555Y1", + position_title: "Computer Networks", + hours_per_assignment: 70, + contract_template_id: null, + instructor_ids: [], + instructor_utorids: ["lucasg", "millerm"], + }, + { + position_code: "ECO101H1F", + position_title: "Microeconomics", + hours_per_assignment: 50, + contract_template_id: null, + instructor_ids: [], + instructor_utorids: ["millerm", "belljo"], + }, ], assignments: [ { @@ -173,6 +201,58 @@ class DatabaseSeeder { position_code: "MAT235H1F", applicant_utorid: "weasleyr", }, + { + _temp_id: 1, + position_code: "CSC555Y1", + applicant_utorid: "molinat", + }, + { + _temp_id: 2, + position_code: "ECO101H1F", + applicant_utorid: "brownd", + }, + ], + ddahs: [ + { + _temp_id: 1, + duties: [ + { + order: 2, + hours: 18, + description: "marking:Marking midterms", + }, + { + order: 1, + hours: 2, + description: "training:Initial TA training", + }, + { + order: 3, + hours: 50, + description: "contact:Tutorials", + }, + ], + }, + { + _temp_id: 2, + duties: [ + { + order: 2, + hours: 18, + description: "marking:Marking midterms", + }, + { + order: 1, + hours: 2, + description: "training:Initial TA training", + }, + { + order: 3, + hours: 50, + description: "contact:Tutorials", + }, + ], + }, ], }; } @@ -400,6 +480,9 @@ async function seedDatabaseForInstructors( } // Assignment + // We will keep track of the inserted assignments for the purpose + // of creating DDAHs for some of them later + const processedAssignments = []; for (const assignment of seeded.assignments) { // The seed data is written in terms of position codes // and applicant utorids, so we need to mangle the data @@ -431,5 +514,49 @@ async function seedDatabaseForInstructors( resp = await apiPOST(`/admin/assignments`, assignment); expect(resp).toHaveStatus("success"); Object.assign(assignment, resp.payload); + processedAssignments.push(assignment); + } + + // DDAH + for (const ddah of seeded.ddahs) { + // _temp_id in ddahs should correspond to _temp_id in assignments + if (!ddah._temp_id) { + throw new Error( + `Inconsistency in seed data: could not create ddah without _temp_id` + ); + } + + // The seed data is written in terms of position codes + // and applicant utorids, since unique applicant can apply to + // at most one position + const { id: assignment_id } = + processedAssignments.find( + (assignment) => assignment._temp_id === ddah._temp_id + ) || {}; + if (!assignment_id) { + throw new Error( + `Inconsistency in seed data: could not find assignment with _temp_id ${ddah._temp_id}` + ); + } + + const newDdah = { + ...ddah, + assignment_id, + }; + let resp = await apiPOST(`/admin/ddahs`, newDdah); + expect(resp).toHaveStatus("success"); + } + + // Cleanup + for (const assignment of seeded.assignments) { + if (assignment._temp_id) { + const { temp_id, ...cleanAssignment } = assignment; + Object.assign(assignment, cleanAssignment); + } + } + + for (const ddah of seeded.ddahs) { + const { temp_id, ...cleanDdah } = ddah; + Object.assign(ddah, cleanDdah); } }