From 0f14e3e2b4a0ca9127b0a1c6340ad97c12585a2e Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Tue, 23 Dec 2025 15:19:27 -0800 Subject: [PATCH 1/3] feat(effort): VPR-20 Add manual instructor CRUD - Add instructor list/detail pages with search, create, edit, delete - Enforce department-level authorization on search and create - Add server-side search term validation (min 2 chars) - Include InstructorService with AAUD integration for dept/title lookup --- .../__tests__/instructor-add-dialog.test.ts | 175 ++++++ .../__tests__/instructor-edit-dialog.test.ts | 255 ++++++++ .../src/Effort/components/EffortLeftNav.vue | 27 + .../Effort/components/InstructorAddDialog.vue | 205 +++++++ .../components/InstructorEditDialog.vue | 226 +++++++ .../composables/use-effort-permissions.ts | 6 + VueApp/src/Effort/pages/InstructorDetail.vue | 263 ++++++++ VueApp/src/Effort/pages/InstructorList.vue | 420 +++++++++++++ VueApp/src/Effort/router/routes.ts | 30 + VueApp/src/Effort/services/effort-service.ts | 137 +++++ VueApp/src/Effort/types/index.ts | 67 ++ test/Effort/InstructorServiceTests.cs | 490 +++++++++++++++ test/Effort/InstructorsControllerTests.cs | 556 +++++++++++++++++ .../Controllers/InstructorsController.cs | 344 +++++++++++ .../Effort/Models/AutoMapperProfileEffort.cs | 5 +- .../DTOs/Requests/CreateInstructorRequest.cs | 17 + .../DTOs/Requests/UpdateInstructorRequest.cs | 30 + .../Models/DTOs/Responses/AaudPersonDto.cs | 19 + .../Models/DTOs/Responses/DepartmentDto.cs | 11 + .../Responses/InstructorEffortRecordDto.cs | 35 ++ .../Effort/Models/DTOs/Responses/PersonDto.cs | 6 +- .../Models/DTOs/Responses/ReportUnitDto.cs | 10 + .../Effort/Services/EffortAuditService.cs | 5 + .../Effort/Services/IEffortAuditService.cs | 6 + .../Effort/Services/IInstructorService.cs | 128 ++++ .../Effort/Services/InstructorService.cs | 577 ++++++++++++++++++ web/Program.cs | 1 + 27 files changed, 4046 insertions(+), 5 deletions(-) create mode 100644 VueApp/src/Effort/__tests__/instructor-add-dialog.test.ts create mode 100644 VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts create mode 100644 VueApp/src/Effort/components/InstructorAddDialog.vue create mode 100644 VueApp/src/Effort/components/InstructorEditDialog.vue create mode 100644 VueApp/src/Effort/pages/InstructorDetail.vue create mode 100644 VueApp/src/Effort/pages/InstructorList.vue create mode 100644 test/Effort/InstructorServiceTests.cs create mode 100644 test/Effort/InstructorsControllerTests.cs create mode 100644 web/Areas/Effort/Controllers/InstructorsController.cs create mode 100644 web/Areas/Effort/Models/DTOs/Requests/CreateInstructorRequest.cs create mode 100644 web/Areas/Effort/Models/DTOs/Requests/UpdateInstructorRequest.cs create mode 100644 web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs create mode 100644 web/Areas/Effort/Models/DTOs/Responses/DepartmentDto.cs create mode 100644 web/Areas/Effort/Models/DTOs/Responses/InstructorEffortRecordDto.cs create mode 100644 web/Areas/Effort/Models/DTOs/Responses/ReportUnitDto.cs create mode 100644 web/Areas/Effort/Services/IInstructorService.cs create mode 100644 web/Areas/Effort/Services/InstructorService.cs diff --git a/VueApp/src/Effort/__tests__/instructor-add-dialog.test.ts b/VueApp/src/Effort/__tests__/instructor-add-dialog.test.ts new file mode 100644 index 00000000..21beda6a --- /dev/null +++ b/VueApp/src/Effort/__tests__/instructor-add-dialog.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { ref } from "vue" +import { setActivePinia, createPinia } from "pinia" + +/** + * Tests for InstructorAddDialog error handling and validation behavior. + * + * These tests validate that the component properly handles errors + * when adding instructors to the Effort system. + * + * The actual component UI is tested via Playwright MCP (see SMOKETEST-Effort-Manual-Instructor.md). + */ + +// Mock the effort service +const mockCreateInstructor = vi.fn() +const mockSearchPossibleInstructors = vi.fn() +vi.mock("../services/effort-service", () => ({ + effortService: { + createInstructor: (...args: unknown[]) => mockCreateInstructor(...args), + searchPossibleInstructors: (...args: unknown[]) => mockSearchPossibleInstructors(...args), + }, +})) + +describe("InstructorAddDialog - Error Handling", () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + }) + + describe("Create Instructor Error States", () => { + it("should capture error message when API returns success: false", () => { + const error = ref(null) + + const result = { success: false, error: "Instructor already exists for this term" } + + if (!result.success) { + error.value = result.error ?? "Failed to add instructor" + } + + expect(error.value).toBe("Instructor already exists for this term") + }) + + it("should use default message when API returns no error message", () => { + const error = ref(null) + + const result = { success: false, error: null } + + if (!result.success) { + error.value = result.error ?? "Failed to add instructor" + } + + expect(error.value).toBe("Failed to add instructor") + }) + + it("should clear error when API returns success", () => { + const error = ref("Previous error") + + const result = { success: true, result: { personId: 1, firstName: "John", lastName: "Doe" } } + + if (result.success) { + error.value = null + } + + expect(error.value).toBeNull() + }) + }) + + describe("Search Validation", () => { + it("should require at least 2 characters for search", () => { + const searchTerm = "J" + const minSearchLength = 2 + + expect(searchTerm.length >= minSearchLength).toBe(false) + }) + + it("should allow search with 2 or more characters", () => { + const searchTerm = "Jo" + const minSearchLength = 2 + + expect(searchTerm.length >= minSearchLength).toBe(true) + }) + }) + + describe("Person Selection Validation", () => { + it("should require a person to be selected before adding", () => { + const selectedPerson = ref<{ personId: number } | null>(null) + + const canSubmit = !!selectedPerson.value + + expect(canSubmit).toBe(false) + }) + + it("should allow submission when person is selected", () => { + const selectedPerson = ref<{ personId: number } | null>({ personId: 123 }) + + const canSubmit = !!selectedPerson.value + + expect(canSubmit).toBe(true) + }) + }) + + describe("Search Results Handling", () => { + it("should handle empty search results", () => { + const searchResults = ref([]) + + mockSearchPossibleInstructors.mockResolvedValue({ + success: true, + result: [], + }) + + expect(searchResults.value).toHaveLength(0) + }) + + it("should filter out instructors already in the term", () => { + // This is handled by the backend, but we verify the frontend correctly displays results + const searchResults = [ + { personId: 1, fullName: "Doe, John", effortDept: "VME" }, + { personId: 2, fullName: "Smith, Jane", effortDept: "APC" }, + ] + + expect(searchResults).toHaveLength(2) + expect(searchResults[0].fullName).toBe("Doe, John") + }) + }) +}) + +describe("InstructorAddDialog - State Management", () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + }) + + describe("Loading State", () => { + it("should track loading state during search", async () => { + const isSearching = ref(false) + + isSearching.value = true + expect(isSearching.value).toBe(true) + + // Simulate search completion + await Promise.resolve() + isSearching.value = false + expect(isSearching.value).toBe(false) + }) + + it("should track saving state during instructor creation", async () => { + const isSaving = ref(false) + + isSaving.value = true + expect(isSaving.value).toBe(true) + + // Simulate save completion + await Promise.resolve() + isSaving.value = false + expect(isSaving.value).toBe(false) + }) + }) + + describe("Dialog Reset", () => { + it("should reset form state when dialog closes", () => { + const selectedPerson = ref<{ personId: number } | null>({ personId: 123 }) + const searchTerm = ref("John") + const errorMessage = ref("Some error") + + // Reset function + selectedPerson.value = null + searchTerm.value = "" + errorMessage.value = null + + expect(selectedPerson.value).toBeNull() + expect(searchTerm.value).toBe("") + expect(errorMessage.value).toBeNull() + }) + }) +}) diff --git a/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts b/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts new file mode 100644 index 00000000..0f01ce4d --- /dev/null +++ b/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts @@ -0,0 +1,255 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { ref } from "vue" +import { setActivePinia, createPinia } from "pinia" + +/** + * Tests for InstructorEditDialog error handling and validation behavior. + * + * These tests validate that the component properly handles errors + * and validates data when editing instructors in the Effort system. + * + * The actual component UI is tested via Playwright MCP (see SMOKETEST-Effort-Manual-Instructor.md). + */ + +// Mock the effort service +const mockUpdateInstructor = vi.fn() +const mockGetDepartments = vi.fn() +const mockGetReportUnits = vi.fn() +vi.mock("../services/effort-service", () => ({ + effortService: { + updateInstructor: (...args: unknown[]) => mockUpdateInstructor(...args), + getDepartments: (...args: unknown[]) => mockGetDepartments(...args), + getReportUnits: (...args: unknown[]) => mockGetReportUnits(...args), + }, +})) + +describe("InstructorEditDialog - Error Handling", () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + }) + + describe("Update Instructor Error States", () => { + it("should capture error message when API returns success: false", () => { + const error = ref(null) + + const result = { success: false, error: "Invalid department code" } + + if (!result.success) { + error.value = result.error ?? "Failed to update instructor" + } + + expect(error.value).toBe("Invalid department code") + }) + + it("should use default message when API returns no error message", () => { + const error = ref(null) + + const result = { success: false, error: null } + + if (!result.success) { + error.value = result.error ?? "Failed to update instructor" + } + + expect(error.value).toBe("Failed to update instructor") + }) + + it("should clear error when API returns success", () => { + const error = ref("Previous error") + + const result = { success: true, result: { personId: 1, effortDept: "VME" } } + + if (result.success) { + error.value = null + } + + expect(error.value).toBeNull() + }) + }) + + describe("Form Validation", () => { + it("should require department to be selected", () => { + const effortDept = ref(null) + + const isValid = !!effortDept.value + + expect(isValid).toBe(false) + }) + + it("should allow valid department selection", () => { + const effortDept = ref("VME") + + const isValid = !!effortDept.value + + expect(isValid).toBe(true) + }) + + it("should require title code to be selected", () => { + const effortTitleCode = ref(null) + + const isValid = !!effortTitleCode.value + + expect(isValid).toBe(false) + }) + + it("should allow valid title code selection", () => { + const effortTitleCode = ref("1234") + + const isValid = !!effortTitleCode.value + + expect(isValid).toBe(true) + }) + }) + + describe("VolunteerWos Flag", () => { + it("should default to false when instructor is not volunteer", () => { + const volunteerWos = ref(false) + + expect(volunteerWos.value).toBe(false) + }) + + it("should be settable to true", () => { + const volunteerWos = ref(false) + volunteerWos.value = true + + expect(volunteerWos.value).toBe(true) + }) + + it("should correctly toggle volunteer status", () => { + const volunteerWos = ref(false) + + volunteerWos.value = !volunteerWos.value + expect(volunteerWos.value).toBe(true) + + volunteerWos.value = !volunteerWos.value + expect(volunteerWos.value).toBe(false) + }) + }) + + describe("Report Units Multi-Select", () => { + it("should allow empty report units", () => { + const reportUnits = ref([]) + + expect(reportUnits.value).toHaveLength(0) + }) + + it("should allow single report unit selection", () => { + const reportUnits = ref(["SVM"]) + + expect(reportUnits.value).toHaveLength(1) + expect(reportUnits.value[0]).toBe("SVM") + }) + + it("should allow multiple report unit selection", () => { + const reportUnits = ref(["SVM", "VMB", "VME"]) + + expect(reportUnits.value).toHaveLength(3) + expect(reportUnits.value).toContain("SVM") + expect(reportUnits.value).toContain("VMB") + expect(reportUnits.value).toContain("VME") + }) + + it("should convert array to comma-separated string for API", () => { + const reportUnits = ["SVM", "VMB", "VME"] + const reportUnitString = reportUnits.join(",") + + expect(reportUnitString).toBe("SVM,VMB,VME") + }) + + it("should handle null report units from API", () => { + const apiReportUnit: string | null = null + const reportUnits = apiReportUnit ? apiReportUnit.split(",") : [] + + expect(reportUnits).toHaveLength(0) + }) + + it("should parse comma-separated string from API", () => { + const apiReportUnit = "SVM,VMB,VME" + const reportUnits = apiReportUnit.split(",") + + expect(reportUnits).toHaveLength(3) + expect(reportUnits).toContain("SVM") + }) + }) +}) + +describe("InstructorEditDialog - State Management", () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + }) + + describe("Loading State", () => { + it("should track saving state during update", async () => { + const isSaving = ref(false) + + isSaving.value = true + expect(isSaving.value).toBe(true) + + // Simulate save completion + await Promise.resolve() + isSaving.value = false + expect(isSaving.value).toBe(false) + }) + }) + + describe("Form State Initialization", () => { + it("should populate form with instructor data", () => { + const instructor = { + personId: 1, + termCode: 202410, + firstName: "John", + lastName: "Doe", + effortDept: "VME", + effortTitleCode: "1234", + volunteerWos: false, + reportUnit: "SVM,VMB", + } + + const effortDept = ref(instructor.effortDept) + const effortTitleCode = ref(instructor.effortTitleCode) + const volunteerWos = ref(instructor.volunteerWos) + const reportUnits = ref(instructor.reportUnit ? instructor.reportUnit.split(",") : []) + + expect(effortDept.value).toBe("VME") + expect(effortTitleCode.value).toBe("1234") + expect(volunteerWos.value).toBe(false) + expect(reportUnits.value).toEqual(["SVM", "VMB"]) + }) + }) + + describe("Department Change Detection", () => { + it("should detect when department changes", () => { + const originalDept = "VME" + const currentDept = ref("VME") + + expect(currentDept.value !== originalDept).toBe(false) + + currentDept.value = "APC" + expect(currentDept.value !== originalDept).toBe(true) + }) + }) +}) + +describe("InstructorEditDialog - Department Grouping", () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + it("should display departments grouped by category", () => { + const departments = [ + { code: "VME", name: "Medicine & Epidemiology", group: "Academic" }, + { code: "APC", name: "Anatomy, Physiology & Cell Biology", group: "Academic" }, + { code: "WHC", name: "Wildlife Health Center", group: "Centers" }, + { code: "OTH", name: "Other", group: "Other" }, + ] + + const academicDepts = departments.filter((d) => d.group === "Academic") + const centerDepts = departments.filter((d) => d.group === "Centers") + const otherDepts = departments.filter((d) => d.group === "Other") + + expect(academicDepts).toHaveLength(2) + expect(centerDepts).toHaveLength(1) + expect(otherDepts).toHaveLength(1) + }) +}) diff --git a/VueApp/src/Effort/components/EffortLeftNav.vue b/VueApp/src/Effort/components/EffortLeftNav.vue index 7450f5c6..8999086b 100644 --- a/VueApp/src/Effort/components/EffortLeftNav.vue +++ b/VueApp/src/Effort/components/EffortLeftNav.vue @@ -91,6 +91,19 @@ + + + + Instructors + + + + hasImportInstructor.value || + hasEditInstructor.value || + hasDeleteInstructor.value || + hasViewDept.value || + isAdmin.value, +) + const localDrawerOpen = ref(props.drawerOpen) const currentTerm = ref(null) diff --git a/VueApp/src/Effort/components/InstructorAddDialog.vue b/VueApp/src/Effort/components/InstructorAddDialog.vue new file mode 100644 index 00000000..ffd18233 --- /dev/null +++ b/VueApp/src/Effort/components/InstructorAddDialog.vue @@ -0,0 +1,205 @@ + + + diff --git a/VueApp/src/Effort/components/InstructorEditDialog.vue b/VueApp/src/Effort/components/InstructorEditDialog.vue new file mode 100644 index 00000000..d334dc53 --- /dev/null +++ b/VueApp/src/Effort/components/InstructorEditDialog.vue @@ -0,0 +1,226 @@ + + + diff --git a/VueApp/src/Effort/composables/use-effort-permissions.ts b/VueApp/src/Effort/composables/use-effort-permissions.ts index f8d1e398..59790cbc 100644 --- a/VueApp/src/Effort/composables/use-effort-permissions.ts +++ b/VueApp/src/Effort/composables/use-effort-permissions.ts @@ -57,6 +57,9 @@ function useEffortPermissions() { const hasDeleteCourse = computed(() => hasPermission(EffortPermissions.DeleteCourse)) const hasManageRCourseEnrollment = computed(() => hasPermission(EffortPermissions.ManageRCourseEnrollment)) const hasLinkCourses = computed(() => hasPermission(EffortPermissions.LinkCourses)) + const hasImportInstructor = computed(() => hasPermission(EffortPermissions.ImportInstructor)) + const hasEditInstructor = computed(() => hasPermission(EffortPermissions.EditInstructor)) + const hasDeleteInstructor = computed(() => hasPermission(EffortPermissions.DeleteInstructor)) const isAdmin = computed(() => hasViewAllDepartments.value) return { @@ -72,6 +75,9 @@ function useEffortPermissions() { hasDeleteCourse, hasManageRCourseEnrollment, hasLinkCourses, + hasImportInstructor, + hasEditInstructor, + hasDeleteInstructor, isAdmin, permissions: computed(() => userStore.userInfo.permissions), } diff --git a/VueApp/src/Effort/pages/InstructorDetail.vue b/VueApp/src/Effort/pages/InstructorDetail.vue new file mode 100644 index 00000000..64dbf68d --- /dev/null +++ b/VueApp/src/Effort/pages/InstructorDetail.vue @@ -0,0 +1,263 @@ + + + + + diff --git a/VueApp/src/Effort/pages/InstructorList.vue b/VueApp/src/Effort/pages/InstructorList.vue new file mode 100644 index 00000000..762c2e4f --- /dev/null +++ b/VueApp/src/Effort/pages/InstructorList.vue @@ -0,0 +1,420 @@ + + + + + diff --git a/VueApp/src/Effort/router/routes.ts b/VueApp/src/Effort/router/routes.ts index d38b78be..e9a8c1c2 100644 --- a/VueApp/src/Effort/router/routes.ts +++ b/VueApp/src/Effort/router/routes.ts @@ -60,6 +60,36 @@ const routes = [ component: () => import("@/Effort/pages/CourseDetail.vue"), name: "CourseDetail", }, + { + path: `/Effort/:termCode(${TERM_CODE_PATTERN})/instructors`, + meta: { + layout: EffortLayout, + permissions: [ + "SVMSecure.Effort.ViewAllDepartments", + "SVMSecure.Effort.ViewDept", + "SVMSecure.Effort.ImportInstructor", + "SVMSecure.Effort.EditInstructor", + "SVMSecure.Effort.DeleteInstructor", + ], + }, + component: () => import("@/Effort/pages/InstructorList.vue"), + name: "InstructorList", + }, + { + path: `/Effort/:termCode(${TERM_CODE_PATTERN})/instructors/:personId(\\d+)`, + meta: { + layout: EffortLayout, + permissions: [ + "SVMSecure.Effort.ViewAllDepartments", + "SVMSecure.Effort.ViewDept", + "SVMSecure.Effort.ImportInstructor", + "SVMSecure.Effort.EditInstructor", + "SVMSecure.Effort.DeleteInstructor", + ], + }, + component: () => import("@/Effort/pages/InstructorDetail.vue"), + name: "InstructorDetail", + }, { path: "/Effort/audit", meta: { layout: EffortLayout, permissions: ["SVMSecure.Effort.ViewAudit"] }, diff --git a/VueApp/src/Effort/services/effort-service.ts b/VueApp/src/Effort/services/effort-service.ts index 7f078c03..0ce5dbbd 100644 --- a/VueApp/src/Effort/services/effort-service.ts +++ b/VueApp/src/Effort/services/effort-service.ts @@ -10,6 +10,14 @@ import type { CourseRelationshipDto, CourseRelationshipsResult, CreateCourseRelationshipRequest, + PersonDto, + AaudPersonDto, + CreateInstructorRequest, + UpdateInstructorRequest, + ReportUnitDto, + DepartmentDto, + CanDeleteResult, + InstructorEffortRecordDto, } from "../types" const { get, post, put, del, patch } = useFetch() @@ -333,6 +341,135 @@ class EffortService { const response = await del(`${this.baseUrl}/courses/${parentCourseId}/relationships/${relationshipId}`) return response.success } + + // Instructor Operations + + /** + * Get all instructors for a term, optionally filtered by department. + */ + async getInstructors(termCode: number, dept?: string): Promise { + const params = new URLSearchParams({ termCode: termCode.toString() }) + if (dept) { + params.append("dept", dept) + } + const response = await get(`${this.baseUrl}/instructors?${params.toString()}`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as PersonDto[] + } + + /** + * Get a single instructor by person ID and term code. + */ + async getInstructor(personId: number, termCode: number): Promise { + const response = await get(`${this.baseUrl}/instructors/${personId}?termCode=${termCode}`) + if (!response.success || !response.result) { + return null + } + return response.result as PersonDto + } + + /** + * Search for possible instructors in AAUD who can be added to the term. + */ + async searchPossibleInstructors(termCode: number, searchTerm?: string): Promise { + const params = new URLSearchParams({ termCode: termCode.toString() }) + if (searchTerm) { + params.append("q", searchTerm) + } + const response = await get(`${this.baseUrl}/instructors/search?${params.toString()}`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as AaudPersonDto[] + } + + /** + * Create an instructor (add to term). + */ + async createInstructor( + request: CreateInstructorRequest, + ): Promise<{ success: boolean; instructor?: PersonDto; error?: string }> { + const response = await post(`${this.baseUrl}/instructors`, request) + if (!response.success) { + return { + success: false, + error: EffortService.extractErrorMessage(response.errors, "Failed to add instructor"), + } + } + return { success: true, instructor: response.result as PersonDto } + } + + /** + * Update an instructor's details. + */ + async updateInstructor( + personId: number, + termCode: number, + request: UpdateInstructorRequest, + ): Promise<{ success: boolean; instructor?: PersonDto; error?: string }> { + const response = await put(`${this.baseUrl}/instructors/${personId}?termCode=${termCode}`, request) + if (!response.success) { + return { + success: false, + error: EffortService.extractErrorMessage(response.errors, "Failed to update instructor"), + } + } + return { success: true, instructor: response.result as PersonDto } + } + + /** + * Delete an instructor and their effort records for the term. + */ + async deleteInstructor(personId: number, termCode: number): Promise { + const response = await del(`${this.baseUrl}/instructors/${personId}?termCode=${termCode}`) + return response.success + } + + /** + * Check if an instructor can be deleted and get count of effort records. + */ + async canDeleteInstructor(personId: number, termCode: number): Promise { + const response = await get(`${this.baseUrl}/instructors/${personId}/can-delete?termCode=${termCode}`) + if (!response.success || !response.result) { + return { canDelete: false, recordCount: 0 } + } + return response.result as CanDeleteResult + } + + /** + * Get valid departments with grouping info. + */ + async getInstructorDepartments(): Promise { + const response = await get(`${this.baseUrl}/instructors/departments`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as DepartmentDto[] + } + + /** + * Get all report units for the multi-select dropdown. + */ + async getReportUnits(): Promise { + const response = await get(`${this.baseUrl}/instructors/report-units`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as ReportUnitDto[] + } + + /** + * Get effort records for an instructor in a specific term. + */ + async getInstructorEffortRecords(personId: number, termCode: number): Promise { + const response = await get(`${this.baseUrl}/instructors/${personId}/effort?termCode=${termCode}`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as InstructorEffortRecordDto[] + } } const effortService = new EffortService() diff --git a/VueApp/src/Effort/types/index.ts b/VueApp/src/Effort/types/index.ts index a175c566..1733c4f2 100644 --- a/VueApp/src/Effort/types/index.ts +++ b/VueApp/src/Effort/types/index.ts @@ -29,10 +29,12 @@ type PersonDto = { effortTitleCode: string effortDept: string percentAdmin: number + jobGroupId: string | null title: string | null adminUnit: string | null effortVerified: string | null reportUnit: string | null + volunteerWos: boolean percentClinical: number | null isVerified: boolean } @@ -169,6 +171,64 @@ type CreateCourseRelationshipRequest = { relationshipType: "CrossList" | "Section" } +// Instructor management types +type AaudPersonDto = { + personId: number + firstName: string + lastName: string + middleInitial: string | null + fullName: string + effortDept: string | null + titleCode: string | null + jobGroupId: string | null +} + +type CreateInstructorRequest = { + personId: number + termCode: number +} + +type UpdateInstructorRequest = { + effortDept: string + effortTitleCode: string + jobGroupId: string | null + reportUnits: string[] | null + volunteerWos: boolean +} + +type ReportUnitDto = { + abbrev: string + unit: string +} + +type DepartmentDto = { + code: string + name: string + group: string +} + +type CanDeleteResult = { + canDelete: boolean + recordCount: number +} + +type InstructorEffortRecordDto = { + id: number + courseId: number + personId: number + termCode: number + sessionType: string + role: number + roleDescription: string + hours: number | null + weeks: number | null + crn: string + modifiedDate: string | null + effortValue: number | null + effortLabel: string + course: CourseDto +} + export type { TermDto, PersonDto, @@ -186,4 +246,11 @@ export type { CourseRelationshipDto, CourseRelationshipsResult, CreateCourseRelationshipRequest, + AaudPersonDto, + CreateInstructorRequest, + UpdateInstructorRequest, + ReportUnitDto, + DepartmentDto, + CanDeleteResult, + InstructorEffortRecordDto, } diff --git a/test/Effort/InstructorServiceTests.cs b/test/Effort/InstructorServiceTests.cs new file mode 100644 index 00000000..61ec91e3 --- /dev/null +++ b/test/Effort/InstructorServiceTests.cs @@ -0,0 +1,490 @@ +using AutoMapper; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Moq; +using Viper.Areas.Effort; +using Viper.Areas.Effort.Models; +using Viper.Areas.Effort.Models.DTOs.Requests; +using Viper.Areas.Effort.Models.Entities; +using Viper.Areas.Effort.Services; +using Viper.Classes.SQLContext; + +namespace Viper.test.Effort; + +/// +/// Unit tests for InstructorService instructor management operations. +/// +public sealed class InstructorServiceTests : IDisposable +{ + private readonly EffortDbContext _context; + private readonly VIPERContext _viperContext; + private readonly AAUDContext _aaudContext; + private readonly Mock _auditServiceMock; + private readonly Mock> _loggerMock; + private readonly IConfiguration _configurationMock; + private readonly IMemoryCache _cache; + private readonly IMapper _mapper; + private readonly InstructorService _instructorService; + + public InstructorServiceTests() + { + var effortOptions = new DbContextOptionsBuilder() + .UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString()) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)) + .Options; + + var viperOptions = new DbContextOptionsBuilder() + .UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString()) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)) + .Options; + + var aaudOptions = new DbContextOptionsBuilder() + .UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString()) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)) + .Options; + + _context = new EffortDbContext(effortOptions); + _viperContext = new VIPERContext(viperOptions); + _aaudContext = new AAUDContext(aaudOptions); + _auditServiceMock = new Mock(); + _loggerMock = new Mock>(); + + // Setup configuration with empty connection strings section to prevent null reference + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(new Dictionary + { + { "ConnectionStrings:VIPER", "" } + }); + _configurationMock = configurationBuilder.Build(); + + _cache = new MemoryCache(new MemoryCacheOptions()); + + // Configure AutoMapper with the Effort profile + var mapperConfig = new MapperConfiguration(cfg => + { + cfg.AddProfile(); + }); + _mapper = mapperConfig.CreateMapper(); + + // Setup synchronous audit methods used within transactions + _auditServiceMock + .Setup(s => s.AddPersonChangeAudit(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); + + _instructorService = new InstructorService( + _context, + _viperContext, + _aaudContext, + _auditServiceMock.Object, + _mapper, + _loggerMock.Object, + _configurationMock, + _cache); + } + + public void Dispose() + { + _context.Dispose(); + _viperContext.Dispose(); + _aaudContext.Dispose(); + _cache.Dispose(); + } + + #region GetInstructorsAsync Tests + + [Fact] + public async Task GetInstructorsAsync_ReturnsAllInstructorsForTerm_OrderedByLastNameFirstName() + { + // Arrange + _context.Persons.AddRange( + new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }, + new EffortPerson { PersonId = 2, TermCode = 202410, FirstName = "Alice", LastName = "Smith", EffortDept = "VME", EffortTitleCode = "1234" }, + new EffortPerson { PersonId = 3, TermCode = 202410, FirstName = "Bob", LastName = "Doe", EffortDept = "APC", EffortTitleCode = "1234" } + ); + await _context.SaveChangesAsync(); + + // Act + var instructors = await _instructorService.GetInstructorsAsync(202410); + + // Assert + Assert.Equal(3, instructors.Count); + Assert.Equal("Doe", instructors[0].LastName); + Assert.Equal("Bob", instructors[0].FirstName); + Assert.Equal("Doe", instructors[1].LastName); + Assert.Equal("John", instructors[1].FirstName); + Assert.Equal("Smith", instructors[2].LastName); + } + + [Fact] + public async Task GetInstructorsAsync_FiltersByDepartment_WhenDepartmentProvided() + { + // Arrange + _context.Persons.AddRange( + new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }, + new EffortPerson { PersonId = 2, TermCode = 202410, FirstName = "Jane", LastName = "Smith", EffortDept = "APC", EffortTitleCode = "1234" } + ); + await _context.SaveChangesAsync(); + + // Act + var instructors = await _instructorService.GetInstructorsAsync(202410, "VME"); + + // Assert + Assert.Single(instructors); + Assert.Equal("VME", instructors[0].EffortDept); + } + + [Fact] + public async Task GetInstructorsAsync_ReturnsEmptyList_WhenNoInstructorsExistForTerm() + { + // Act + var instructors = await _instructorService.GetInstructorsAsync(999999); + + // Assert + Assert.Empty(instructors); + } + + #endregion + + #region GetInstructorAsync Tests + + [Fact] + public async Task GetInstructorAsync_ReturnsInstructor_WhenInstructorExists() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act + var instructor = await _instructorService.GetInstructorAsync(1, 202410); + + // Assert + Assert.NotNull(instructor); + Assert.Equal(1, instructor.PersonId); + Assert.Equal("John", instructor.FirstName); + Assert.Equal("Doe", instructor.LastName); + } + + [Fact] + public async Task GetInstructorAsync_ReturnsNull_WhenInstructorDoesNotExist() + { + // Act + var instructor = await _instructorService.GetInstructorAsync(999, 202410); + + // Assert + Assert.Null(instructor); + } + + [Fact] + public async Task GetInstructorAsync_ReturnsNull_WhenWrongTermCode() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act + var instructor = await _instructorService.GetInstructorAsync(1, 202320); + + // Assert + Assert.Null(instructor); + } + + #endregion + + #region InstructorExistsAsync Tests + + [Fact] + public async Task InstructorExistsAsync_ReturnsTrue_WhenInstructorExists() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act & Assert + Assert.True(await _instructorService.InstructorExistsAsync(1, 202410)); + } + + [Fact] + public async Task InstructorExistsAsync_ReturnsFalse_WhenInstructorDoesNotExist() + { + // Act & Assert + Assert.False(await _instructorService.InstructorExistsAsync(999, 202410)); + } + + #endregion + + #region UpdateInstructorAsync Tests + + [Fact] + public async Task UpdateInstructorAsync_UpdatesInstructor_WithValidData() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234", VolunteerWos = 0 }); + await _context.SaveChangesAsync(); + + var request = new UpdateInstructorRequest + { + EffortDept = "APC", + EffortTitleCode = "5678", + VolunteerWos = true + }; + + // Act + var instructor = await _instructorService.UpdateInstructorAsync(1, 202410, request); + + // Assert + Assert.NotNull(instructor); + Assert.Equal("APC", instructor.EffortDept); + Assert.Equal("5678", instructor.EffortTitleCode); + Assert.True(instructor.VolunteerWos); + } + + [Fact] + public async Task UpdateInstructorAsync_ReturnsNull_WhenInstructorDoesNotExist() + { + // Arrange + var request = new UpdateInstructorRequest + { + EffortDept = "APC", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + // Act + var instructor = await _instructorService.UpdateInstructorAsync(999, 202410, request); + + // Assert + Assert.Null(instructor); + } + + [Fact] + public async Task UpdateInstructorAsync_ThrowsArgumentException_ForInvalidDepartment() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + var request = new UpdateInstructorRequest + { + EffortDept = "INVALID", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => _instructorService.UpdateInstructorAsync(1, 202410, request) + ); + Assert.Contains("Invalid department", exception.Message); + } + + [Fact] + public async Task UpdateInstructorAsync_CreatesAuditEntryWithOldAndNewValues() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + var request = new UpdateInstructorRequest + { + EffortDept = "APC", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + // Act + await _instructorService.UpdateInstructorAsync(1, 202410, request); + + // Assert + _auditServiceMock.Verify( + s => s.AddPersonChangeAudit( + 1, + 202410, + "UpdatePerson", + It.IsAny(), + It.IsAny()), + Times.Once); + } + + #endregion + + #region DeleteInstructorAsync Tests + + [Fact] + public async Task DeleteInstructorAsync_DeletesInstructor_WhenInstructorExists() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act + var result = await _instructorService.DeleteInstructorAsync(1, 202410); + + // Assert + Assert.True(result); + Assert.Null(await _context.Persons.FirstOrDefaultAsync(p => p.PersonId == 1 && p.TermCode == 202410)); + } + + [Fact] + public async Task DeleteInstructorAsync_ReturnsFalse_WhenInstructorDoesNotExist() + { + // Act + var result = await _instructorService.DeleteInstructorAsync(999, 202410); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task DeleteInstructorAsync_DeletesAssociatedRecords() + { + // Arrange + var person = new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }; + _context.Persons.Add(person); + await _context.SaveChangesAsync(); + + _context.Records.AddRange( + new EffortRecord { Id = 1, TermCode = 202410, CourseId = 100, PersonId = 1 }, + new EffortRecord { Id = 2, TermCode = 202410, CourseId = 101, PersonId = 1 } + ); + await _context.SaveChangesAsync(); + + // Act + var result = await _instructorService.DeleteInstructorAsync(1, 202410); + + // Assert + Assert.True(result); + Assert.Empty(await _context.Records.Where(r => r.PersonId == 1).ToListAsync()); + } + + [Fact] + public async Task DeleteInstructorAsync_CreatesAuditEntry() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act + await _instructorService.DeleteInstructorAsync(1, 202410); + + // Assert + _auditServiceMock.Verify( + s => s.AddPersonChangeAudit( + 1, + 202410, + "DeleteInstructor", + It.IsAny(), + null), + Times.Once); + } + + #endregion + + #region CanDeleteInstructorAsync Tests + + [Fact] + public async Task CanDeleteInstructorAsync_ReturnsTrueAndZeroCount_WhenNoRecords() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + await _context.SaveChangesAsync(); + + // Act + var (canDelete, recordCount) = await _instructorService.CanDeleteInstructorAsync(1, 202410); + + // Assert + Assert.True(canDelete); + Assert.Equal(0, recordCount); + } + + [Fact] + public async Task CanDeleteInstructorAsync_ReturnsTrueWithRecordCount_WhenRecordsExist() + { + // Arrange + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234" }); + _context.Records.AddRange( + new EffortRecord { Id = 1, TermCode = 202410, CourseId = 100, PersonId = 1 }, + new EffortRecord { Id = 2, TermCode = 202410, CourseId = 101, PersonId = 1 }, + new EffortRecord { Id = 3, TermCode = 202410, CourseId = 102, PersonId = 1 } + ); + await _context.SaveChangesAsync(); + + // Act + var (canDelete, recordCount) = await _instructorService.CanDeleteInstructorAsync(1, 202410); + + // Assert + Assert.True(canDelete); + Assert.Equal(3, recordCount); + } + + #endregion + + #region GetDepartments / IsValidDepartment Tests + + [Fact] + public void GetDepartments_ReturnsAllDepartments() + { + // Act + var departments = _instructorService.GetDepartments(); + + // Assert + Assert.Contains(departments, d => d.Code == "VME"); + Assert.Contains(departments, d => d.Code == "APC"); + Assert.Contains(departments, d => d.Code == "WHC"); + Assert.Contains(departments, d => d.Group == "Academic"); + Assert.Contains(departments, d => d.Group == "Centers"); + } + + [Fact] + public void GetValidDepartments_ReturnsAllValidDepartmentCodes() + { + // Act + var departments = _instructorService.GetValidDepartments(); + + // Assert + Assert.Contains("VME", departments); + Assert.Contains("APC", departments); + Assert.Contains("WHC", departments); + } + + [Fact] + public void IsValidDepartment_ReturnsFalse_ForInvalidDepartment() + { + // Act & Assert + Assert.False(_instructorService.IsValidDepartment("INVALID")); + } + + [Fact] + public void IsValidDepartment_ReturnsTrue_ForValidDepartment() + { + // Act & Assert + Assert.True(_instructorService.IsValidDepartment("VME")); + Assert.True(_instructorService.IsValidDepartment("vme")); // Case insensitive + } + + #endregion + + #region AutoMapper PersonDto Mapping Tests + + [Fact] + public async Task GetInstructorAsync_MapsVolunteerWosCorrectly() + { + // Arrange - VolunteerWos is byte? in entity, bool in DTO + _context.Persons.Add(new EffortPerson { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME", EffortTitleCode = "1234", VolunteerWos = 1 }); + _context.Persons.Add(new EffortPerson { PersonId = 2, TermCode = 202410, FirstName = "Jane", LastName = "Smith", EffortDept = "VME", EffortTitleCode = "1234", VolunteerWos = 0 }); + await _context.SaveChangesAsync(); + + // Act + var instructor1 = await _instructorService.GetInstructorAsync(1, 202410); + var instructor2 = await _instructorService.GetInstructorAsync(2, 202410); + + // Assert + Assert.NotNull(instructor1); + Assert.True(instructor1.VolunteerWos); + Assert.NotNull(instructor2); + Assert.False(instructor2.VolunteerWos); + } + + #endregion +} diff --git a/test/Effort/InstructorsControllerTests.cs b/test/Effort/InstructorsControllerTests.cs new file mode 100644 index 00000000..25f283ef --- /dev/null +++ b/test/Effort/InstructorsControllerTests.cs @@ -0,0 +1,556 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Moq; +using Viper.Areas.Effort.Controllers; +using Viper.Areas.Effort.Models.DTOs.Requests; +using Viper.Areas.Effort.Models.DTOs.Responses; +using Viper.Areas.Effort.Services; + +namespace Viper.test.Effort; + +/// +/// Unit tests for InstructorsController API endpoints. +/// +public sealed class InstructorsControllerTests +{ + private readonly Mock _instructorServiceMock; + private readonly Mock _permissionServiceMock; + private readonly Mock> _loggerMock; + private readonly InstructorsController _controller; + + public InstructorsControllerTests() + { + _instructorServiceMock = new Mock(); + _permissionServiceMock = new Mock(); + _loggerMock = new Mock>(); + + _controller = new InstructorsController( + _instructorServiceMock.Object, + _permissionServiceMock.Object, + _loggerMock.Object); + + SetupControllerContext(); + } + + private void SetupControllerContext() + { + var serviceProvider = new ServiceCollection().BuildServiceProvider(); + _controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext + { + RequestServices = serviceProvider + } + }; + } + + #region GetInstructors Tests + + [Fact] + public async Task GetInstructors_ReturnsOk_WithInstructorList() + { + // Arrange + var instructors = new List + { + new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }, + new PersonDto { PersonId = 2, TermCode = 202410, FirstName = "Jane", LastName = "Smith", EffortDept = "VME" } + }; + _permissionServiceMock.Setup(s => s.HasFullAccessAsync(It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.GetInstructorsAsync(202410, null, It.IsAny())) + .ReturnsAsync(instructors); + + // Act + var result = await _controller.GetInstructors(202410); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedInstructors = Assert.IsAssignableFrom>(okResult.Value); + Assert.Equal(2, returnedInstructors.Count()); + } + + [Fact] + public async Task GetInstructors_FiltersByDepartment_WhenProvided() + { + // Arrange + var instructors = new List + { + new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" } + }; + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _permissionServiceMock.Setup(s => s.HasFullAccessAsync(It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.GetInstructorsAsync(202410, "VME", It.IsAny())) + .ReturnsAsync(instructors); + + // Act + var result = await _controller.GetInstructors(202410, "VME"); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedInstructors = Assert.IsAssignableFrom>(okResult.Value); + Assert.Single(returnedInstructors); + } + + [Fact] + public async Task GetInstructors_ReturnsEmptyList_WhenUnauthorizedForDepartment() + { + // Arrange + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("SECRET", It.IsAny())).ReturnsAsync(false); + + // Act + var result = await _controller.GetInstructors(202410, "SECRET"); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedInstructors = Assert.IsAssignableFrom>(okResult.Value); + Assert.Empty(returnedInstructors); + } + + #endregion + + #region GetInstructor Tests + + [Fact] + public async Task GetInstructor_ReturnsOk_WhenInstructorExists() + { + // Arrange + var instructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())) + .ReturnsAsync(instructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + + // Act + var result = await _controller.GetInstructor(1, 202410); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedInstructor = Assert.IsType(okResult.Value); + Assert.Equal(1, returnedInstructor.PersonId); + } + + [Fact] + public async Task GetInstructor_ReturnsNotFound_WhenInstructorDoesNotExist() + { + // Arrange + _instructorServiceMock.Setup(s => s.GetInstructorAsync(999, 202410, It.IsAny())) + .ReturnsAsync((PersonDto?)null); + + // Act + var result = await _controller.GetInstructor(999, 202410); + + // Assert + Assert.IsType(result.Result); + } + + [Fact] + public async Task GetInstructor_ReturnsNotFound_WhenUnauthorizedForDepartment() + { + // Arrange + var instructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "SECRET" }; + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())) + .ReturnsAsync(instructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("SECRET", It.IsAny())).ReturnsAsync(false); + + // Act + var result = await _controller.GetInstructor(1, 202410); + + // Assert + Assert.IsType(result.Result); + } + + #endregion + + #region CreateInstructor Tests + + [Fact] + public async Task CreateInstructor_ReturnsCreated_WithInstructorDto() + { + // Arrange + var request = new CreateInstructorRequest + { + PersonId = 1, + TermCode = 202410 + }; + var createdInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + + _instructorServiceMock.Setup(s => s.InstructorExistsAsync(1, 202410, It.IsAny())).ReturnsAsync(false); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, It.IsAny())).ReturnsAsync("VME"); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) + .ReturnsAsync(createdInstructor); + + // Act + var result = await _controller.CreateInstructor(request); + + // Assert + var createdResult = Assert.IsType(result.Result); + Assert.Equal(201, createdResult.StatusCode); + var returnedInstructor = Assert.IsType(createdResult.Value); + Assert.Equal(1, returnedInstructor.PersonId); + } + + [Fact] + public async Task CreateInstructor_ReturnsConflict_WhenInstructorAlreadyExists() + { + // Arrange + var request = new CreateInstructorRequest + { + PersonId = 1, + TermCode = 202410 + }; + + _instructorServiceMock.Setup(s => s.InstructorExistsAsync(1, 202410, It.IsAny())).ReturnsAsync(true); + + // Act + var result = await _controller.CreateInstructor(request); + + // Assert + var conflictResult = Assert.IsType(result.Result); + Assert.Equal(409, conflictResult.StatusCode); + Assert.Contains("already exists", conflictResult.Value?.ToString()); + } + + [Fact] + public async Task CreateInstructor_ReturnsBadRequest_ForInvalidOperationException() + { + // Arrange + var request = new CreateInstructorRequest + { + PersonId = 999, + TermCode = 202410 + }; + + _instructorServiceMock.Setup(s => s.InstructorExistsAsync(999, 202410, It.IsAny())).ReturnsAsync(false); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(999, It.IsAny())).ReturnsAsync("VME"); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Person not found in AAUD")); + + // Act + var result = await _controller.CreateInstructor(request); + + // Assert + var badRequestResult = Assert.IsType(result.Result); + Assert.Contains("not found in AAUD", badRequestResult.Value?.ToString()); + } + + [Fact] + public async Task CreateInstructor_ReturnsBadRequest_ForDbUpdateException() + { + // Arrange + var request = new CreateInstructorRequest + { + PersonId = 1, + TermCode = 202410 + }; + + _instructorServiceMock.Setup(s => s.InstructorExistsAsync(1, 202410, It.IsAny())).ReturnsAsync(false); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, It.IsAny())).ReturnsAsync("VME"); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) + .ThrowsAsync(new DbUpdateException("Database constraint violation")); + + // Act + var result = await _controller.CreateInstructor(request); + + // Assert + var badRequestResult = Assert.IsType(result.Result); + Assert.Contains("Failed to create instructor", badRequestResult.Value?.ToString()); + } + + #endregion + + #region UpdateInstructor Tests + + [Fact] + public async Task UpdateInstructor_ReturnsOk_WithUpdatedInstructor() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + var request = new UpdateInstructorRequest + { + EffortDept = "APC", + EffortTitleCode = "5678", + VolunteerWos = false + }; + var updatedInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "APC" }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("APC", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.UpdateInstructorAsync(1, 202410, request, It.IsAny())) + .ReturnsAsync(updatedInstructor); + + // Act + var result = await _controller.UpdateInstructor(1, 202410, request); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedInstructor = Assert.IsType(okResult.Value); + Assert.Equal("APC", returnedInstructor.EffortDept); + } + + [Fact] + public async Task UpdateInstructor_ReturnsNotFound_WhenChangingToUnauthorizedDepartment() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + var request = new UpdateInstructorRequest + { + EffortDept = "SECRET", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("SECRET", It.IsAny())).ReturnsAsync(false); + + // Act + var result = await _controller.UpdateInstructor(1, 202410, request); + + // Assert + Assert.IsType(result.Result); + } + + [Fact] + public async Task UpdateInstructor_ReturnsNotFound_WhenInstructorDoesNotExist() + { + // Arrange + var request = new UpdateInstructorRequest + { + EffortDept = "APC", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(999, 202410, It.IsAny())).ReturnsAsync((PersonDto?)null); + + // Act + var result = await _controller.UpdateInstructor(999, 202410, request); + + // Assert + Assert.IsType(result.Result); + } + + [Fact] + public async Task UpdateInstructor_ReturnsBadRequest_ForInvalidDepartment() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + var request = new UpdateInstructorRequest + { + EffortDept = "INVALID", + EffortTitleCode = "5678", + VolunteerWos = false + }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("INVALID", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.UpdateInstructorAsync(1, 202410, request, It.IsAny())) + .ThrowsAsync(new ArgumentException("Invalid department: INVALID")); + + // Act + var result = await _controller.UpdateInstructor(1, 202410, request); + + // Assert + var badRequestResult = Assert.IsType(result.Result); + Assert.Contains("Invalid department", badRequestResult.Value?.ToString()); + } + + #endregion + + #region DeleteInstructor Tests + + [Fact] + public async Task DeleteInstructor_ReturnsNoContent_OnSuccess() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.DeleteInstructorAsync(1, 202410, It.IsAny())) + .ReturnsAsync(true); + + // Act + var result = await _controller.DeleteInstructor(1, 202410); + + // Assert + Assert.IsType(result); + } + + [Fact] + public async Task DeleteInstructor_ReturnsNotFound_WhenInstructorDoesNotExist() + { + // Arrange + _instructorServiceMock.Setup(s => s.GetInstructorAsync(999, 202410, It.IsAny())).ReturnsAsync((PersonDto?)null); + + // Act + var result = await _controller.DeleteInstructor(999, 202410); + + // Assert + Assert.IsType(result); + } + + [Fact] + public async Task DeleteInstructor_ReturnsNotFound_WhenUnauthorizedForDepartment() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "SECRET" }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("SECRET", It.IsAny())).ReturnsAsync(false); + + // Act + var result = await _controller.DeleteInstructor(1, 202410); + + // Assert + Assert.IsType(result); + } + + #endregion + + #region CanDeleteInstructor Tests + + [Fact] + public async Task CanDeleteInstructor_ReturnsOk_WithDeleteInfo() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.CanDeleteInstructorAsync(1, 202410, It.IsAny())) + .ReturnsAsync((true, 5)); + + // Act + var result = await _controller.CanDeleteInstructor(1, 202410); + + // Assert + var okResult = Assert.IsType(result.Result); + Assert.NotNull(okResult.Value); + } + + #endregion + + #region GetDepartments Tests + + [Fact] + public async Task GetDepartments_ReturnsAllDepartments_ForFullAccessUser() + { + // Arrange + var departments = new List + { + new DepartmentDto { Code = "VME", Name = "Medicine & Epidemiology", Group = "Academic" }, + new DepartmentDto { Code = "APC", Name = "Anatomy, Physiology & Cell Biology", Group = "Academic" }, + new DepartmentDto { Code = "WHC", Name = "Wildlife Health Center", Group = "Centers" } + }; + _instructorServiceMock.Setup(s => s.GetDepartments()).Returns(departments); + _permissionServiceMock.Setup(s => s.HasFullAccessAsync(It.IsAny())).ReturnsAsync(true); + + // Act + var result = await _controller.GetDepartments(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedDepartments = Assert.IsAssignableFrom>(okResult.Value); + Assert.Equal(3, returnedDepartments.Count()); + } + + [Fact] + public async Task GetDepartments_ReturnsOnlyAuthorizedDepartments_ForNonAdminUser() + { + // Arrange + var departments = new List + { + new DepartmentDto { Code = "VME", Name = "Medicine & Epidemiology", Group = "Academic" }, + new DepartmentDto { Code = "APC", Name = "Anatomy, Physiology & Cell Biology", Group = "Academic" }, + new DepartmentDto { Code = "WHC", Name = "Wildlife Health Center", Group = "Centers" } + }; + _instructorServiceMock.Setup(s => s.GetDepartments()).Returns(departments); + _permissionServiceMock.Setup(s => s.HasFullAccessAsync(It.IsAny())).ReturnsAsync(false); + _permissionServiceMock.Setup(s => s.GetAuthorizedDepartmentsAsync(It.IsAny())) + .ReturnsAsync(new List { "VME" }); + + // Act + var result = await _controller.GetDepartments(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedDepartments = Assert.IsAssignableFrom>(okResult.Value); + Assert.Single(returnedDepartments); + Assert.Equal("VME", returnedDepartments.First().Code); + } + + #endregion + + #region GetReportUnits Tests + + [Fact] + public async Task GetReportUnits_ReturnsOk_WithReportUnits() + { + // Arrange + var reportUnits = new List + { + new ReportUnitDto { Abbrev = "SVM", Unit = "School of Veterinary Medicine" }, + new ReportUnitDto { Abbrev = "VMB", Unit = "Molecular Biosciences" } + }; + _instructorServiceMock.Setup(s => s.GetReportUnitsAsync(It.IsAny())) + .ReturnsAsync(reportUnits); + + // Act + var result = await _controller.GetReportUnits(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedUnits = Assert.IsAssignableFrom>(okResult.Value); + Assert.Equal(2, returnedUnits.Count()); + } + + #endregion + + #region GetInstructorEffortRecords Tests + + [Fact] + public async Task GetInstructorEffortRecords_ReturnsOk_WithEffortRecords() + { + // Arrange + var existingInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; + var effortRecords = new List + { + new InstructorEffortRecordDto { CourseId = 100, Weeks = 2, Course = new CourseDto { Id = 100, Crn = "12345" } } + }; + + _instructorServiceMock.Setup(s => s.GetInstructorAsync(1, 202410, It.IsAny())).ReturnsAsync(existingInstructor); + _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); + _instructorServiceMock.Setup(s => s.GetInstructorEffortRecordsAsync(1, 202410, It.IsAny())) + .ReturnsAsync(effortRecords); + + // Act + var result = await _controller.GetInstructorEffortRecords(1, 202410); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedRecords = Assert.IsAssignableFrom>(okResult.Value); + Assert.Single(returnedRecords); + } + + [Fact] + public async Task GetInstructorEffortRecords_ReturnsNotFound_WhenInstructorDoesNotExist() + { + // Arrange + _instructorServiceMock.Setup(s => s.GetInstructorAsync(999, 202410, It.IsAny())).ReturnsAsync((PersonDto?)null); + + // Act + var result = await _controller.GetInstructorEffortRecords(999, 202410); + + // Assert + Assert.IsType(result.Result); + } + + #endregion +} diff --git a/web/Areas/Effort/Controllers/InstructorsController.cs b/web/Areas/Effort/Controllers/InstructorsController.cs new file mode 100644 index 00000000..d16bd743 --- /dev/null +++ b/web/Areas/Effort/Controllers/InstructorsController.cs @@ -0,0 +1,344 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; +using Viper.Areas.Effort.Constants; +using Viper.Areas.Effort.Models.DTOs.Requests; +using Viper.Areas.Effort.Models.DTOs.Responses; +using Viper.Areas.Effort.Services; +using Web.Authorization; + +namespace Viper.Areas.Effort.Controllers; + +/// +/// API controller for instructor operations in the Effort system. +/// +[Route("/api/effort/instructors")] +[Permission(Allow = $"{EffortPermissions.ViewDept},{EffortPermissions.ViewAllDepartments}")] +public class InstructorsController : BaseEffortController +{ + private readonly IInstructorService _instructorService; + private readonly IEffortPermissionService _permissionService; + + public InstructorsController( + IInstructorService instructorService, + IEffortPermissionService permissionService, + ILogger logger) : base(logger) + { + _instructorService = instructorService; + _permissionService = permissionService; + } + + /// + /// Verifies the user is authorized to access an instructor by their department. + /// Returns null if authorized, or a NotFound result if not. + /// + private async Task<(PersonDto? instructor, ActionResult? errorResult)> GetAuthorizedInstructorAsync( + int personId, int termCode, CancellationToken ct) + { + var instructor = await _instructorService.GetInstructorAsync(personId, termCode, ct); + if (instructor == null || !await _permissionService.CanViewDepartmentAsync(instructor.EffortDept, ct)) + { + return (null, NotFound($"Instructor not found")); + } + return (instructor, null); + } + + /// + /// Get all instructors for a term, optionally filtered by department. + /// Non-admin users are restricted to their authorized departments. + /// + [HttpGet] + public async Task>> GetInstructors( + [FromQuery] int termCode, + [FromQuery] string? dept = null, + CancellationToken ct = default) + { + SetExceptionContext("termCode", termCode); + if (!string.IsNullOrEmpty(dept)) + { + SetExceptionContext("dept", dept); + } + + // If department specified, verify user can view it + if (!string.IsNullOrEmpty(dept) && !await _permissionService.CanViewDepartmentAsync(dept, ct)) + { + // Return empty list instead of 403 to prevent department enumeration + return Ok(Array.Empty()); + } + + // For non-full-access users, filter to authorized departments + if (!await _permissionService.HasFullAccessAsync(ct)) + { + var authorizedDepts = await _permissionService.GetAuthorizedDepartmentsAsync(ct); + if (authorizedDepts.Count == 0) + { + return Ok(Array.Empty()); + } + + // If no dept specified, get instructors for all authorized departments + if (string.IsNullOrEmpty(dept)) + { + var allInstructors = new List(); + foreach (var authorizedDept in authorizedDepts) + { + var deptInstructors = await _instructorService.GetInstructorsAsync(termCode, authorizedDept, ct); + allInstructors.AddRange(deptInstructors); + } + return Ok(allInstructors.OrderBy(i => i.LastName).ThenBy(i => i.FirstName)); + } + } + + var instructors = await _instructorService.GetInstructorsAsync(termCode, dept, ct); + return Ok(instructors); + } + + /// + /// Get a single instructor by person ID and term code. + /// Non-admin users can only view instructors in their authorized departments. + /// + [HttpGet("{personId:int}")] + public async Task> GetInstructor( + int personId, + [FromQuery] int termCode, + CancellationToken ct = default) + { + SetExceptionContext("personId", personId); + SetExceptionContext("termCode", termCode); + + var (instructor, errorResult) = await GetAuthorizedInstructorAsync(personId, termCode, ct); + if (errorResult != null) return errorResult; + + return Ok(instructor); + } + + /// + /// Search for possible instructors in AAUD who can be added to the term. + /// Returns employees not already in the Effort system for this term. + /// + [HttpGet("search")] + [Permission(Allow = EffortPermissions.ImportInstructor)] + public async Task>> SearchPossibleInstructors( + [FromQuery] int termCode, + [FromQuery] string? q = null, + CancellationToken ct = default) + { + SetExceptionContext("termCode", termCode); + + var instructors = await _instructorService.SearchPossibleInstructorsAsync(termCode, q, ct); + + // Filter to authorized departments for non-full-access users + if (!await _permissionService.HasFullAccessAsync(ct)) + { + var authorizedDepts = await _permissionService.GetAuthorizedDepartmentsAsync(ct); + instructors = [.. instructors + .Where(i => i.EffortDept != null && authorizedDepts.Contains(i.EffortDept, StringComparer.OrdinalIgnoreCase))]; + } + + return Ok(instructors); + } + + /// + /// Add an instructor to the Effort system for a term. + /// + [HttpPost] + [Permission(Allow = EffortPermissions.ImportInstructor)] + public async Task> CreateInstructor( + [FromBody] CreateInstructorRequest request, + CancellationToken ct = default) + { + SetExceptionContext("personId", request.PersonId); + SetExceptionContext("termCode", request.TermCode); + + // Check if instructor already exists + if (await _instructorService.InstructorExistsAsync(request.PersonId, request.TermCode, ct)) + { + _logger.LogWarning("Instructor {PersonId} already exists for term {TermCode}", + request.PersonId, request.TermCode); + return Conflict($"Instructor already exists for this term"); + } + + // Verify the user is authorized to add instructors to the resolved department + var resolvedDept = await _instructorService.ResolveInstructorDepartmentAsync(request.PersonId, ct); + if (resolvedDept == null) + { + return BadRequest("Person not found"); + } + + if (!await _permissionService.CanViewDepartmentAsync(resolvedDept, ct)) + { + _logger.LogWarning("User not authorized to add instructor to department {Dept}", resolvedDept); + return BadRequest("Not authorized to add instructors to this department"); + } + + try + { + var instructor = await _instructorService.CreateInstructorAsync(request, ct); + _logger.LogInformation("Instructor created: {PersonId} for term {TermCode}", + request.PersonId, request.TermCode); + return CreatedAtAction( + nameof(GetInstructor), + new { personId = instructor.PersonId, termCode = instructor.TermCode }, + instructor); + } + catch (InvalidOperationException ex) + { + _logger.LogWarning(ex, "Error creating instructor: {Message}", ex.Message); + return BadRequest(ex.Message); + } + catch (DbUpdateException ex) + { + _logger.LogWarning(ex, "Database error creating instructor: {Message}", + ex.InnerException?.Message ?? ex.Message); + return BadRequest("Failed to create instructor. Please check all values are valid."); + } + } + + /// + /// Update an instructor's details. + /// + [HttpPut("{personId:int}")] + [Permission(Allow = EffortPermissions.EditInstructor)] + public async Task> UpdateInstructor( + int personId, + [FromQuery] int termCode, + [FromBody] UpdateInstructorRequest request, + CancellationToken ct = default) + { + SetExceptionContext("personId", personId); + SetExceptionContext("termCode", termCode); + + var (existingInstructor, errorResult) = await GetAuthorizedInstructorAsync(personId, termCode, ct); + if (errorResult != null) return errorResult; + + // If changing department, verify user can access target department + if (existingInstructor != null && + !string.Equals(existingInstructor.EffortDept, request.EffortDept, StringComparison.OrdinalIgnoreCase) && + !await _permissionService.CanViewDepartmentAsync(request.EffortDept, ct)) + { + return NotFound("Instructor not found"); + } + + try + { + var instructor = await _instructorService.UpdateInstructorAsync(personId, termCode, request, ct); + if (instructor == null) + { + _logger.LogWarning("Instructor not found for update: {PersonId}", personId); + return NotFound("Instructor not found"); + } + + _logger.LogInformation("Instructor updated: {PersonId} for term {TermCode}", personId, termCode); + return Ok(instructor); + } + catch (ArgumentException ex) + { + _logger.LogWarning(ex, "Invalid instructor update data: {Message}", ex.Message); + return BadRequest(ex.Message); + } + catch (DbUpdateException ex) + { + _logger.LogWarning(ex, "Database error updating instructor: {Message}", + ex.InnerException?.Message ?? ex.Message); + return BadRequest("Failed to update instructor. Please check all values are valid."); + } + } + + /// + /// Delete an instructor and all associated effort records. + /// + [HttpDelete("{personId:int}")] + [Permission(Allow = EffortPermissions.DeleteInstructor)] + public async Task DeleteInstructor( + int personId, + [FromQuery] int termCode, + CancellationToken ct = default) + { + SetExceptionContext("personId", personId); + SetExceptionContext("termCode", termCode); + + var (_, errorResult) = await GetAuthorizedInstructorAsync(personId, termCode, ct); + if (errorResult != null) return errorResult; + + var deleted = await _instructorService.DeleteInstructorAsync(personId, termCode, ct); + if (!deleted) + { + _logger.LogWarning("Instructor not found for delete: {PersonId}", personId); + return NotFound("Instructor not found"); + } + + _logger.LogInformation("Instructor deleted: {PersonId} for term {TermCode}", personId, termCode); + return NoContent(); + } + + /// + /// Check if an instructor can be deleted and get the count of associated effort records. + /// + [HttpGet("{personId:int}/can-delete")] + [Permission(Allow = EffortPermissions.DeleteInstructor)] + public async Task> CanDeleteInstructor( + int personId, + [FromQuery] int termCode, + CancellationToken ct = default) + { + SetExceptionContext("personId", personId); + SetExceptionContext("termCode", termCode); + + var (_, errorResult) = await GetAuthorizedInstructorAsync(personId, termCode, ct); + if (errorResult != null) return errorResult; + + var (canDelete, recordCount) = await _instructorService.CanDeleteInstructorAsync(personId, termCode, ct); + return Ok(new { canDelete, recordCount }); + } + + /// + /// Get the list of valid department codes with grouping. + /// Non-admin users only see their authorized departments. + /// + [HttpGet("departments")] + public async Task>> GetDepartments(CancellationToken ct = default) + { + var allDepartments = _instructorService.GetDepartments(); + + // Full access users see all departments + if (await _permissionService.HasFullAccessAsync(ct)) + { + return Ok(allDepartments); + } + + // Non-admin users only see their authorized departments + var authorizedDepts = await _permissionService.GetAuthorizedDepartmentsAsync(ct); + var filteredDepts = allDepartments + .Where(d => authorizedDepts.Contains(d.Code, StringComparer.OrdinalIgnoreCase)) + .ToList(); + + return Ok(filteredDepts); + } + + /// + /// Get all report units for the multi-select dropdown. + /// + [HttpGet("report-units")] + public async Task>> GetReportUnits(CancellationToken ct = default) + { + var units = await _instructorService.GetReportUnitsAsync(ct); + return Ok(units); + } + + /// + /// Get effort records for an instructor in a specific term. + /// + [HttpGet("{personId:int}/effort")] + public async Task>> GetInstructorEffortRecords( + int personId, + [FromQuery] int termCode, + CancellationToken ct = default) + { + SetExceptionContext("personId", personId); + SetExceptionContext("termCode", termCode); + + var (_, errorResult) = await GetAuthorizedInstructorAsync(personId, termCode, ct); + if (errorResult != null) return errorResult; + + var records = await _instructorService.GetInstructorEffortRecordsAsync(personId, termCode, ct); + return Ok(records); + } +} diff --git a/web/Areas/Effort/Models/AutoMapperProfileEffort.cs b/web/Areas/Effort/Models/AutoMapperProfileEffort.cs index da1f9cc7..5bb8a3ce 100644 --- a/web/Areas/Effort/Models/AutoMapperProfileEffort.cs +++ b/web/Areas/Effort/Models/AutoMapperProfileEffort.cs @@ -13,8 +13,9 @@ public AutoMapperProfileEffort() .ForMember(d => d.TermName, opt => opt.Ignore()) .ForMember(d => d.CanDelete, opt => opt.Ignore()); - // All properties auto-map by convention - CreateMap(); + // VolunteerWos: byte? (1=true) → bool + CreateMap() + .ForMember(d => d.VolunteerWos, opt => opt.MapFrom(s => s.VolunteerWos == 1)); CreateMap(); // RoleDescription comes from lookup diff --git a/web/Areas/Effort/Models/DTOs/Requests/CreateInstructorRequest.cs b/web/Areas/Effort/Models/DTOs/Requests/CreateInstructorRequest.cs new file mode 100644 index 00000000..9c87b895 --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Requests/CreateInstructorRequest.cs @@ -0,0 +1,17 @@ +using System.ComponentModel.DataAnnotations; + +namespace Viper.Areas.Effort.Models.DTOs.Requests; + +/// +/// Request DTO for adding an instructor to the Effort system. +/// The instructor must exist in AAUD/users.Person. +/// +public class CreateInstructorRequest +{ + [Required] + [Range(1, int.MaxValue, ErrorMessage = "PersonId must be a positive integer")] + public required int PersonId { get; set; } + + [Required] + public required int TermCode { get; set; } +} diff --git a/web/Areas/Effort/Models/DTOs/Requests/UpdateInstructorRequest.cs b/web/Areas/Effort/Models/DTOs/Requests/UpdateInstructorRequest.cs new file mode 100644 index 00000000..a05fdf3c --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Requests/UpdateInstructorRequest.cs @@ -0,0 +1,30 @@ +using System.ComponentModel.DataAnnotations; + +namespace Viper.Areas.Effort.Models.DTOs.Requests; + +/// +/// Request DTO for updating an instructor in the Effort system. +/// +public class UpdateInstructorRequest +{ + [Required] + [StringLength(6, MinimumLength = 2)] + public string EffortDept { get; set; } = string.Empty; + + [StringLength(6)] + public string EffortTitleCode { get; set; } = string.Empty; + + [StringLength(3)] + public string? JobGroupId { get; set; } + + /// + /// Report unit abbreviations. Stored as comma-separated string in database. + /// + public List? ReportUnits { get; set; } + + /// + /// Volunteer/Without Salary flag. When true, excludes instructor from M&P reports. + /// + [Required] + public required bool VolunteerWos { get; set; } +} diff --git a/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs b/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs new file mode 100644 index 00000000..8c7ae05f --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs @@ -0,0 +1,19 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for AAUD person search results when adding instructors. +/// Contains basic person info plus employment details from AAUD. +/// +public class AaudPersonDto +{ + public int PersonId { get; set; } + public string FirstName { get; set; } = string.Empty; + public string LastName { get; set; } = string.Empty; + public string? MiddleInitial { get; set; } + public string FullName => string.IsNullOrEmpty(MiddleInitial) + ? $"{LastName}, {FirstName}" + : $"{LastName}, {FirstName} {MiddleInitial}."; + public string? EffortDept { get; set; } + public string? TitleCode { get; set; } + public string? JobGroupId { get; set; } +} diff --git a/web/Areas/Effort/Models/DTOs/Responses/DepartmentDto.cs b/web/Areas/Effort/Models/DTOs/Responses/DepartmentDto.cs new file mode 100644 index 00000000..01e5158e --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/DepartmentDto.cs @@ -0,0 +1,11 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for department lookup values with grouping. +/// +public class DepartmentDto +{ + public string Code { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; + public string Group { get; set; } = string.Empty; +} diff --git a/web/Areas/Effort/Models/DTOs/Responses/InstructorEffortRecordDto.cs b/web/Areas/Effort/Models/DTOs/Responses/InstructorEffortRecordDto.cs new file mode 100644 index 00000000..3e6b4057 --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/InstructorEffortRecordDto.cs @@ -0,0 +1,35 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for instructor effort records with course information. +/// Used for the instructor detail page. +/// +public class InstructorEffortRecordDto +{ + public int Id { get; set; } + public int CourseId { get; set; } + public int PersonId { get; set; } + public int TermCode { get; set; } + public string SessionType { get; set; } = string.Empty; + public int Role { get; set; } + public string RoleDescription { get; set; } = string.Empty; + public int? Hours { get; set; } + public int? Weeks { get; set; } + public string Crn { get; set; } = string.Empty; + public DateTime? ModifiedDate { get; set; } + + /// + /// Effort value - either hours or weeks depending on session type. + /// + public int? EffortValue => Hours ?? Weeks; + + /// + /// Label for the effort value ("hours" or "weeks"). + /// + public string EffortLabel => Hours.HasValue ? "hours" : "weeks"; + + /// + /// Course information for this effort record. + /// + public CourseDto Course { get; set; } = null!; +} diff --git a/web/Areas/Effort/Models/DTOs/Responses/PersonDto.cs b/web/Areas/Effort/Models/DTOs/Responses/PersonDto.cs index 5bc9129b..16e16c02 100644 --- a/web/Areas/Effort/Models/DTOs/Responses/PersonDto.cs +++ b/web/Areas/Effort/Models/DTOs/Responses/PersonDto.cs @@ -10,16 +10,16 @@ public class PersonDto public string FirstName { get; set; } = string.Empty; public string LastName { get; set; } = string.Empty; public string? MiddleInitial { get; set; } - public string FullName => string.IsNullOrEmpty(MiddleInitial) - ? $"{LastName}, {FirstName}" - : $"{LastName}, {FirstName} {MiddleInitial}."; + public string FullName => $"{LastName}, {FirstName}"; public string EffortTitleCode { get; set; } = string.Empty; public string EffortDept { get; set; } = string.Empty; public double PercentAdmin { get; set; } + public string? JobGroupId { get; set; } public string? Title { get; set; } public string? AdminUnit { get; set; } public DateTime? EffortVerified { get; set; } public string? ReportUnit { get; set; } + public bool VolunteerWos { get; set; } public double? PercentClinical { get; set; } public bool IsVerified => EffortVerified.HasValue; } diff --git a/web/Areas/Effort/Models/DTOs/Responses/ReportUnitDto.cs b/web/Areas/Effort/Models/DTOs/Responses/ReportUnitDto.cs new file mode 100644 index 00000000..061533bd --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/ReportUnitDto.cs @@ -0,0 +1,10 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for report unit lookup values. +/// +public class ReportUnitDto +{ + public string Abbrev { get; set; } = string.Empty; + public string Unit { get; set; } = string.Empty; +} diff --git a/web/Areas/Effort/Services/EffortAuditService.cs b/web/Areas/Effort/Services/EffortAuditService.cs index 2a6266d0..2c1af90c 100644 --- a/web/Areas/Effort/Services/EffortAuditService.cs +++ b/web/Areas/Effort/Services/EffortAuditService.cs @@ -97,6 +97,11 @@ public void AddImportAudit(int termCode, string action, string details) AddAuditEntry(EffortAuditTables.Terms, termCode, termCode, action, details); } + public void AddPersonChangeAudit(int personId, int termCode, string action, object? oldValues, object? newValues) + { + AddAuditEntry(EffortAuditTables.Persons, personId, termCode, action, SerializeChanges(oldValues, newValues)); + } + /// /// Add an audit entry to the context without saving. /// Use within a transaction where the caller manages SaveChangesAsync. diff --git a/web/Areas/Effort/Services/IEffortAuditService.cs b/web/Areas/Effort/Services/IEffortAuditService.cs index 030e096b..bd7f988d 100644 --- a/web/Areas/Effort/Services/IEffortAuditService.cs +++ b/web/Areas/Effort/Services/IEffortAuditService.cs @@ -43,6 +43,12 @@ Task LogTermChangeAsync(int termCode, string action, /// void AddImportAudit(int termCode, string action, string details); + /// + /// Add a person (instructor) change audit entry to the context without saving. + /// Use this within a transaction where the caller manages SaveChangesAsync. + /// + void AddPersonChangeAudit(int personId, int termCode, string action, object? oldValues, object? newValues); + // ==================== Query Methods ==================== /// diff --git a/web/Areas/Effort/Services/IInstructorService.cs b/web/Areas/Effort/Services/IInstructorService.cs new file mode 100644 index 00000000..7157c5ef --- /dev/null +++ b/web/Areas/Effort/Services/IInstructorService.cs @@ -0,0 +1,128 @@ +using Viper.Areas.Effort.Models.DTOs.Requests; +using Viper.Areas.Effort.Models.DTOs.Responses; + +namespace Viper.Areas.Effort.Services; + +/// +/// Service for instructor-related operations in the Effort system. +/// +public interface IInstructorService +{ + /// + /// Get all instructors for a term, optionally filtered by department. + /// + /// The term code. + /// Optional department filter. + /// Cancellation token. + /// List of instructors. + Task> GetInstructorsAsync(int termCode, string? department = null, CancellationToken ct = default); + + /// + /// Get a single instructor by person ID and term code. + /// + /// The person ID. + /// The term code. + /// Cancellation token. + /// The instructor, or null if not found. + Task GetInstructorAsync(int personId, int termCode, CancellationToken ct = default); + + /// + /// Search for possible instructors in AAUD who are not already in the Effort system for the term. + /// + /// The term code. + /// Optional search term to filter by name. + /// Cancellation token. + /// List of AAUD persons who can be added as instructors. + Task> SearchPossibleInstructorsAsync(int termCode, string? searchTerm = null, CancellationToken ct = default); + + /// + /// Add an instructor to the Effort system for a term. + /// Instructor info is looked up from AAUD. + /// + /// The creation request with PersonId and TermCode. + /// Cancellation token. + /// The created instructor. + Task CreateInstructorAsync(CreateInstructorRequest request, CancellationToken ct = default); + + /// + /// Update an instructor's details. + /// + /// The person ID. + /// The term code. + /// The update request. + /// Cancellation token. + /// The updated instructor, or null if not found. + Task UpdateInstructorAsync(int personId, int termCode, UpdateInstructorRequest request, CancellationToken ct = default); + + /// + /// Delete an instructor and all associated effort records for the term. + /// + /// The person ID. + /// The term code. + /// Cancellation token. + /// True if deleted, false if not found. + Task DeleteInstructorAsync(int personId, int termCode, CancellationToken ct = default); + + /// + /// Get deletion info for an instructor. Returns record count so the UI can warn + /// the user about associated effort records that will be cascade-deleted. + /// + /// The person ID. + /// The term code. + /// Cancellation token. + /// Tuple with canDelete flag and count of associated effort records. + Task<(bool CanDelete, int RecordCount)> CanDeleteInstructorAsync(int personId, int termCode, CancellationToken ct = default); + + /// + /// Check if an instructor already exists for a term. + /// + /// The person ID. + /// The term code. + /// Cancellation token. + /// True if the instructor exists for this term. + Task InstructorExistsAsync(int personId, int termCode, CancellationToken ct = default); + + /// + /// Resolve the department a person would be assigned to if added as an instructor. + /// Used for authorization checks before creating. + /// + /// The person ID. + /// Cancellation token. + /// The resolved department code, or null if person not found. + Task ResolveInstructorDepartmentAsync(int personId, CancellationToken ct = default); + + /// + /// Get the list of valid department codes with grouping information. + /// + /// List of departments with code, name, and group. + List GetDepartments(); + + /// + /// Get the list of valid department codes. + /// + /// List of valid department codes. + List GetValidDepartments(); + + /// + /// Check if a department code is valid. + /// + /// The department code to validate. + /// True if valid, false otherwise. + bool IsValidDepartment(string departmentCode); + + /// + /// Get all report units for the multi-select dropdown. + /// + /// Cancellation token. + /// List of report units. + Task> GetReportUnitsAsync(CancellationToken ct = default); + + /// + /// Get effort records for an instructor in a specific term, including course information. + /// + /// The person ID. + /// The term code. + /// Cancellation token. + /// List of effort records with course info. + Task> GetInstructorEffortRecordsAsync(int personId, int termCode, CancellationToken ct = default); +} diff --git a/web/Areas/Effort/Services/InstructorService.cs b/web/Areas/Effort/Services/InstructorService.cs new file mode 100644 index 00000000..0db5e8f4 --- /dev/null +++ b/web/Areas/Effort/Services/InstructorService.cs @@ -0,0 +1,577 @@ +using AutoMapper; +using Microsoft.Data.SqlClient; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Caching.Memory; +using Viper.Areas.Effort.Constants; +using Viper.Areas.Effort.Models.DTOs.Requests; +using Viper.Areas.Effort.Models.DTOs.Responses; +using Viper.Areas.Effort.Models.Entities; +using Viper.Classes.SQLContext; + +namespace Viper.Areas.Effort.Services; + +/// +/// Service for instructor-related operations in the Effort system. +/// +public class InstructorService : IInstructorService +{ + private readonly EffortDbContext _context; + private readonly VIPERContext _viperContext; + private readonly AAUDContext _aaudContext; + private readonly IEffortAuditService _auditService; + private readonly IMapper _mapper; + private readonly ILogger _logger; + private readonly IConfiguration _configuration; + private readonly IMemoryCache _cache; + + private const string TitleCacheKey = "effort_title_lookup"; + + /// + /// Valid academic department codes. + /// + private static readonly List Departments = new() + { + // Academic Departments + new() { Code = "APC", Name = "Anatomy, Physiology & Cell Biology", Group = "Academic" }, + new() { Code = "PHR", Name = "Pathology, Microbiology & Immunology", Group = "Academic" }, + new() { Code = "PMI", Name = "Population Health & Reproduction", Group = "Academic" }, + new() { Code = "VMB", Name = "Molecular Biosciences", Group = "Academic" }, + new() { Code = "VME", Name = "Medicine & Epidemiology", Group = "Academic" }, + new() { Code = "VSR", Name = "Surgical & Radiological Sciences", Group = "Academic" }, + // Centers + new() { Code = "WHC", Name = "Wildlife Health Center", Group = "Centers" }, + new() { Code = "OHI", Name = "One Health Institute", Group = "Centers" }, + new() { Code = "CCM", Name = "Center for Companion Animal Health", Group = "Centers" }, + new() { Code = "WIFSS", Name = "Western Institute for Food Safety", Group = "Centers" }, + new() { Code = "VGL", Name = "Veterinary Genetics Laboratory", Group = "Centers" }, + // Other + new() { Code = "OTH", Name = "Other", Group = "Other" }, + new() { Code = "UNK", Name = "Unknown", Group = "Other" }, + }; + + /// + /// Academic department codes (priority for department assignment). + /// + private static readonly HashSet AcademicDepts = new(StringComparer.OrdinalIgnoreCase) + { + "APC", "PHR", "PMI", "VMB", "VME", "VSR" + }; + + public InstructorService( + EffortDbContext context, + VIPERContext viperContext, + AAUDContext aaudContext, + IEffortAuditService auditService, + IMapper mapper, + ILogger logger, + IConfiguration configuration, + IMemoryCache cache) + { + _context = context; + _viperContext = viperContext; + _aaudContext = aaudContext; + _auditService = auditService; + _mapper = mapper; + _logger = logger; + _configuration = configuration; + _cache = cache; + } + + public async Task> GetInstructorsAsync(int termCode, string? department = null, CancellationToken ct = default) + { + var query = _context.Persons + .AsNoTracking() + .Where(p => p.TermCode == termCode); + + if (!string.IsNullOrWhiteSpace(department)) + { + query = query.Where(p => p.EffortDept == department); + } + + var instructors = await query + .OrderBy(p => p.LastName) + .ThenBy(p => p.FirstName) + .ToListAsync(ct); + + var dtos = _mapper.Map>(instructors); + + // Enrich with titles from dictionary database + await EnrichWithTitlesAsync(dtos, ct); + + return dtos; + } + + public async Task GetInstructorAsync(int personId, int termCode, CancellationToken ct = default) + { + var instructor = await _context.Persons + .AsNoTracking() + .FirstOrDefaultAsync(p => p.PersonId == personId && p.TermCode == termCode, ct); + + if (instructor == null) return null; + + var dto = _mapper.Map(instructor); + await EnrichWithTitlesAsync(new List { dto }, ct); + return dto; + } + + public async Task> SearchPossibleInstructorsAsync(int termCode, string? searchTerm = null, CancellationToken ct = default) + { + // Enforce minimum search length server-side to prevent employee enumeration + if (string.IsNullOrWhiteSpace(searchTerm) || searchTerm.Trim().Length < 2) + { + return []; + } + + // Get person IDs already in effort for this term + var existingPersonIds = await _context.Persons + .AsNoTracking() + .Where(p => p.TermCode == termCode) + .Select(p => p.PersonId) + .ToListAsync(ct); + + // Search VIPER.users.Person for current employees not in effort + var query = _viperContext.People + .AsNoTracking() + .Where(p => p.CurrentEmployee && !existingPersonIds.Contains(p.PersonId)); + + var term = searchTerm.Trim().ToLower(); + query = query.Where(p => + p.LastName.ToLower().Contains(term) || + p.FirstName.ToLower().Contains(term) || + p.FullName.ToLower().Contains(term)); + + var people = await query + .OrderBy(p => p.LastName) + .ThenBy(p => p.FirstName) + .Take(50) // Limit results for performance + .ToListAsync(ct); + + // Get employee info from AAUD for department/title lookup + var mothraIds = people.Select(p => p.MothraId).ToList(); + var employees = await _aaudContext.Employees + .AsNoTracking() + .Where(e => mothraIds.Contains(e.EmpPKey)) + .ToListAsync(ct); + + var employeeDict = employees.ToDictionary(e => e.EmpPKey, StringComparer.OrdinalIgnoreCase); + + return people.Select(p => + { + employeeDict.TryGetValue(p.MothraId, out var emp); + return new AaudPersonDto + { + PersonId = p.PersonId, + FirstName = p.FirstName, + LastName = p.LastName, + MiddleInitial = p.MiddleName?.Length > 0 ? p.MiddleName.Substring(0, 1) : null, + EffortDept = DetermineDepartment(emp), + TitleCode = emp?.EmpEffortTitleCode?.Trim() ?? emp?.EmpTeachingTitleCode?.Trim(), + JobGroupId = null // Job group would need additional lookup if required + }; + }).ToList(); + } + + public async Task CreateInstructorAsync(CreateInstructorRequest request, CancellationToken ct = default) + { + // Check if instructor already exists for this term + if (await InstructorExistsAsync(request.PersonId, request.TermCode, ct)) + { + throw new InvalidOperationException($"Instructor with PersonId {request.PersonId} already exists for term {request.TermCode}"); + } + + // Look up person from VIPER.users.Person + var person = await _viperContext.People + .AsNoTracking() + .FirstOrDefaultAsync(p => p.PersonId == request.PersonId, ct); + + if (person == null) + { + throw new InvalidOperationException($"Person with PersonId {request.PersonId} not found"); + } + + // Look up employee info from AAUD + var employee = await _aaudContext.Employees + .AsNoTracking() + .FirstOrDefaultAsync(e => e.EmpPKey == person.MothraId, ct); + + var dept = DetermineDepartment(employee) ?? "UNK"; + var titleCode = employee?.EmpEffortTitleCode?.Trim() ?? employee?.EmpTeachingTitleCode?.Trim() ?? ""; + + var instructor = new EffortPerson + { + PersonId = request.PersonId, + TermCode = request.TermCode, + FirstName = person.FirstName, + LastName = person.LastName, + MiddleInitial = person.MiddleName?.Length > 0 ? person.MiddleName.Substring(0, 1) : null, + EffortDept = dept, + EffortTitleCode = titleCode, + JobGroupId = null, + PercentAdmin = 0, + PercentClinical = null, + VolunteerWos = null, + ReportUnit = null, + Title = null, + AdminUnit = null, + EffortVerified = null + }; + + await using var transaction = await _context.Database.BeginTransactionAsync(ct); + + _context.Persons.Add(instructor); + await _context.SaveChangesAsync(ct); + + _auditService.AddPersonChangeAudit(request.PersonId, request.TermCode, EffortAuditActions.CreatePerson, + null, + new + { + instructor.PersonId, + instructor.FirstName, + instructor.LastName, + instructor.EffortDept, + instructor.EffortTitleCode + }); + await _context.SaveChangesAsync(ct); + await transaction.CommitAsync(ct); + + _logger.LogInformation("Created instructor: {PersonId} ({LastName}, {FirstName}) for term {TermCode}", + instructor.PersonId, instructor.LastName, instructor.FirstName, instructor.TermCode); + + return _mapper.Map(instructor); + } + + public async Task UpdateInstructorAsync(int personId, int termCode, UpdateInstructorRequest request, CancellationToken ct = default) + { + var instructor = await _context.Persons.FirstOrDefaultAsync(p => p.PersonId == personId && p.TermCode == termCode, ct); + if (instructor == null) + { + return null; + } + + // Validate department + if (!IsValidDepartment(request.EffortDept)) + { + throw new ArgumentException($"Invalid department: {request.EffortDept}"); + } + + var oldValues = new + { + instructor.EffortDept, + instructor.EffortTitleCode, + instructor.JobGroupId, + instructor.ReportUnit, + instructor.VolunteerWos + }; + + instructor.EffortDept = request.EffortDept.ToUpperInvariant(); + instructor.EffortTitleCode = request.EffortTitleCode?.Trim() ?? ""; + instructor.JobGroupId = request.JobGroupId?.Trim(); + instructor.ReportUnit = request.ReportUnits != null && request.ReportUnits.Count > 0 + ? string.Join(",", request.ReportUnits) + : null; + instructor.VolunteerWos = request.VolunteerWos ? (byte)1 : null; + + await using var transaction = await _context.Database.BeginTransactionAsync(ct); + + _auditService.AddPersonChangeAudit(personId, termCode, EffortAuditActions.UpdatePerson, + oldValues, + new + { + instructor.EffortDept, + instructor.EffortTitleCode, + instructor.JobGroupId, + instructor.ReportUnit, + instructor.VolunteerWos + }); + await _context.SaveChangesAsync(ct); + await transaction.CommitAsync(ct); + + _logger.LogInformation("Updated instructor: {PersonId} for term {TermCode}", personId, termCode); + + return _mapper.Map(instructor); + } + + public async Task DeleteInstructorAsync(int personId, int termCode, CancellationToken ct = default) + { + var instructor = await _context.Persons + .Include(p => p.Records) + .FirstOrDefaultAsync(p => p.PersonId == personId && p.TermCode == termCode, ct); + + if (instructor == null) + { + return false; + } + + var instructorInfo = new + { + instructor.PersonId, + instructor.FirstName, + instructor.LastName, + instructor.EffortDept, + RecordCount = instructor.Records.Count + }; + + await using var transaction = await _context.Database.BeginTransactionAsync(ct); + + // Delete associated effort records first (cascade) + if (instructor.Records.Count > 0) + { + _context.Records.RemoveRange(instructor.Records); + _logger.LogInformation("Deleted {RecordCount} effort records for instructor {PersonId}", + instructor.Records.Count, personId); + } + + _context.Persons.Remove(instructor); + + _auditService.AddPersonChangeAudit(personId, termCode, EffortAuditActions.DeleteInstructor, + instructorInfo, + null); + + await _context.SaveChangesAsync(ct); + await transaction.CommitAsync(ct); + + _logger.LogInformation("Deleted instructor: {PersonId} ({LastName}, {FirstName}) for term {TermCode}", + personId, instructorInfo.LastName, instructorInfo.FirstName, termCode); + + return true; + } + + public async Task<(bool CanDelete, int RecordCount)> CanDeleteInstructorAsync(int personId, int termCode, CancellationToken ct = default) + { + var recordCount = await _context.Records + .CountAsync(r => r.PersonId == personId && r.TermCode == termCode, ct); + + // Instructors can always be deleted, but we return the record count for UI warning + return (true, recordCount); + } + + public async Task InstructorExistsAsync(int personId, int termCode, CancellationToken ct = default) + { + return await _context.Persons + .AnyAsync(p => p.PersonId == personId && p.TermCode == termCode, ct); + } + + public async Task ResolveInstructorDepartmentAsync(int personId, CancellationToken ct = default) + { + var person = await _viperContext.People + .AsNoTracking() + .FirstOrDefaultAsync(p => p.PersonId == personId, ct); + + if (person == null) + { + return null; + } + + var employee = await _aaudContext.Employees + .AsNoTracking() + .FirstOrDefaultAsync(e => e.EmpPKey == person.MothraId, ct); + + return DetermineDepartment(employee) ?? "UNK"; + } + + public List GetDepartments() + { + return Departments.ToList(); + } + + public List GetValidDepartments() + { + return Departments.Select(d => d.Code).ToList(); + } + + public bool IsValidDepartment(string departmentCode) + { + return Departments.Any(d => string.Equals(d.Code, departmentCode, StringComparison.OrdinalIgnoreCase)); + } + + public async Task> GetReportUnitsAsync(CancellationToken ct = default) + { + var units = await _context.ReportUnits + .AsNoTracking() + .Where(u => u.IsActive) + .OrderBy(u => u.SortOrder ?? 0) + .ThenBy(u => u.UnitName) + .ToListAsync(ct); + + return units.Select(u => new ReportUnitDto + { + Abbrev = u.UnitCode, + Unit = u.UnitName + }).ToList(); + } + + /// + /// Determine the effort department from AAUD employee data. + /// Priority: Effort dept, then teaching dept, then home dept. + /// Only use if it's an academic SVM department. + /// + private static string? DetermineDepartment(Viper.Models.AAUD.Employee? employee) + { + if (employee == null) + { + return null; + } + + // Check effort dept first + if (!string.IsNullOrWhiteSpace(employee.EmpEffortHomeDept) && + AcademicDepts.Contains(employee.EmpEffortHomeDept.Trim())) + { + return employee.EmpEffortHomeDept.Trim().ToUpperInvariant(); + } + + // Check teaching dept + if (!string.IsNullOrWhiteSpace(employee.EmpTeachingHomeDept) && + AcademicDepts.Contains(employee.EmpTeachingHomeDept.Trim())) + { + return employee.EmpTeachingHomeDept.Trim().ToUpperInvariant(); + } + + // Check home dept + if (!string.IsNullOrWhiteSpace(employee.EmpHomeDept) && + AcademicDepts.Contains(employee.EmpHomeDept.Trim())) + { + return employee.EmpHomeDept.Trim().ToUpperInvariant(); + } + + // Check alt dept + if (!string.IsNullOrWhiteSpace(employee.EmpAltDeptCode) && + AcademicDepts.Contains(employee.EmpAltDeptCode.Trim())) + { + return employee.EmpAltDeptCode.Trim().ToUpperInvariant(); + } + + return null; + } + + /// + /// Enriches instructor DTOs with titles from the dictionary database. + /// Matches EffortTitleCode to dvtTitle_Code (with zero-padding) to get dvtTitle_Abbrv. + /// + /// + /// Uses raw SQL because the dictionary database is on the same SQL Server instance + /// but in a different database. EF Core contexts are tied to a single database, + /// so we use cross-database queries via [dictionary].[dbo].[dvtTitle]. + /// Results are cached since title codes are static reference data. + /// + private async Task EnrichWithTitlesAsync(List instructors, CancellationToken ct) + { + if (instructors.Count == 0) return; + + var titleLookup = await GetTitleLookupAsync(ct); + if (titleLookup == null) return; + + foreach (var instructor in instructors) + { + if (string.IsNullOrWhiteSpace(instructor.EffortTitleCode)) continue; + + var paddedCode = instructor.EffortTitleCode.PadLeft(6, '0'); + if (titleLookup.TryGetValue(paddedCode, out var title)) + { + instructor.Title = title; + } + } + } + + /// + /// Gets the title code to abbreviation lookup dictionary, loading from database if not cached. + /// + private async Task?> GetTitleLookupAsync(CancellationToken ct) + { + if (_cache.TryGetValue>(TitleCacheKey, out var cached) && cached != null) + { + return cached; + } + + var connectionString = _configuration.GetConnectionString("VIPER"); + if (string.IsNullOrEmpty(connectionString)) + { + _logger.LogWarning("VIPER connection string not found, cannot load title lookup"); + return null; + } + + try + { + var titleLookup = new Dictionary(StringComparer.OrdinalIgnoreCase); + + await using var connection = new SqlConnection(connectionString); + await connection.OpenAsync(ct); + + var query = @" + SELECT DISTINCT + dvtTitle_Code AS TitleCode, + dvtTitle_Abbrv AS TitleAbbrev + FROM [dictionary].[dbo].[dvtTitle] + WHERE dvtTitle_Code IS NOT NULL + AND dvtTitle_Abbrv IS NOT NULL"; + + await using var cmd = new SqlCommand(query, connection); + await using var reader = await cmd.ExecuteReaderAsync(ct); + + while (await reader.ReadAsync(ct)) + { + var code = reader["TitleCode"]?.ToString(); + var abbrev = reader["TitleAbbrev"]?.ToString(); + if (!string.IsNullOrEmpty(code) && !string.IsNullOrEmpty(abbrev)) + { + titleLookup[code] = abbrev; + } + } + + var cacheOptions = new MemoryCacheEntryOptions() + .SetSlidingExpiration(TimeSpan.FromHours(24)); + + _cache.Set(TitleCacheKey, titleLookup, cacheOptions); + + _logger.LogInformation("Loaded {Count} title codes from dictionary database", titleLookup.Count); + + return titleLookup; + } + catch (SqlException ex) + { + _logger.LogWarning(ex, "Failed to load title lookup from dictionary database"); + return null; + } + } + + public async Task> GetInstructorEffortRecordsAsync( + int personId, int termCode, CancellationToken ct = default) + { + var records = await _context.Records + .AsNoTracking() + .Include(r => r.Course) + .Include(r => r.RoleNavigation) + .Where(r => r.PersonId == personId && r.TermCode == termCode) + .OrderBy(r => r.RoleNavigation.SortOrder) + .ThenBy(r => r.Course.SubjCode) + .ThenBy(r => r.Course.CrseNumb) + .ThenBy(r => r.Course.SeqNumb) + .ToListAsync(ct); + + return records.Select(r => new InstructorEffortRecordDto + { + Id = r.Id, + CourseId = r.CourseId, + PersonId = r.PersonId, + TermCode = r.TermCode, + SessionType = r.SessionType, + Role = r.Role, + RoleDescription = r.RoleNavigation?.Description ?? string.Empty, + Hours = r.Hours, + Weeks = r.Weeks, + Crn = r.Crn, + ModifiedDate = r.ModifiedDate, + Course = new CourseDto + { + Id = r.Course.Id, + Crn = r.Course.Crn, + TermCode = r.Course.TermCode, + SubjCode = r.Course.SubjCode, + CrseNumb = r.Course.CrseNumb, + SeqNumb = r.Course.SeqNumb, + Enrollment = r.Course.Enrollment, + Units = r.Course.Units, + CustDept = r.Course.CustDept + } + }).ToList(); + } +} diff --git a/web/Program.cs b/web/Program.cs index a46009f0..07a946f0 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -227,6 +227,7 @@ void ConfigureDbContextOptions(DbContextOptionsBuilder options) builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); + builder.Services.AddScoped(); // Add in a custom ClaimsTransformer that injects user ROLES builder.Services.AddTransient(); From 5f5900fe4da49b067e83127fc87c764ca6131ed7 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Mon, 5 Jan 2026 09:18:59 -0800 Subject: [PATCH 2/3] feat(effort): VPR-20 Add title/job group dropdowns and fix AAUD lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace free-text inputs with searchable dropdowns for title code and job group (cached 24h from dictionary database) - Display human-readable names in instructor search results - Fix AAUD lookup to use term-scoped Ids table (MothraId → pKey) - Fix VolunteerWos to store 0 not null (matches legacy M&P queries) - Show warning banner for orphaned values not in dropdown list --- .../__tests__/instructor-edit-dialog.test.ts | 32 +- .../Effort/components/InstructorAddDialog.vue | 27 +- .../components/InstructorEditDialog.vue | 114 +++- VueApp/src/Effort/services/effort-service.ts | 24 + VueApp/src/Effort/types/index.ts | 14 + test/Effort/InstructorServiceTests.cs | 216 +++++++ test/Effort/InstructorsControllerTests.cs | 88 ++- .../Controllers/InstructorsController.cs | 35 +- .../Models/DTOs/Responses/AaudPersonDto.cs | 6 +- .../Models/DTOs/Responses/JobGroupDto.cs | 10 + .../Models/DTOs/Responses/TitleCodeDto.cs | 10 + .../Effort/Services/IInstructorService.cs | 27 +- .../Effort/Services/InstructorService.cs | 552 ++++++++++++++++-- 13 files changed, 1062 insertions(+), 93 deletions(-) create mode 100644 web/Areas/Effort/Models/DTOs/Responses/JobGroupDto.cs create mode 100644 web/Areas/Effort/Models/DTOs/Responses/TitleCodeDto.cs diff --git a/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts b/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts index 0f01ce4d..62b6cad7 100644 --- a/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts +++ b/VueApp/src/Effort/__tests__/instructor-edit-dialog.test.ts @@ -73,7 +73,7 @@ describe("InstructorEditDialog - Error Handling", () => { const isValid = !!effortDept.value - expect(isValid).toBe(false) + expect(isValid).toBeFalsy() }) it("should allow valid department selection", () => { @@ -81,7 +81,7 @@ describe("InstructorEditDialog - Error Handling", () => { const isValid = !!effortDept.value - expect(isValid).toBe(true) + expect(isValid).toBeTruthy() }) it("should require title code to be selected", () => { @@ -89,7 +89,7 @@ describe("InstructorEditDialog - Error Handling", () => { const isValid = !!effortTitleCode.value - expect(isValid).toBe(false) + expect(isValid).toBeFalsy() }) it("should allow valid title code selection", () => { @@ -97,7 +97,7 @@ describe("InstructorEditDialog - Error Handling", () => { const isValid = !!effortTitleCode.value - expect(isValid).toBe(true) + expect(isValid).toBeTruthy() }) }) @@ -105,24 +105,24 @@ describe("InstructorEditDialog - Error Handling", () => { it("should default to false when instructor is not volunteer", () => { const volunteerWos = ref(false) - expect(volunteerWos.value).toBe(false) + expect(volunteerWos.value).toBeFalsy() }) it("should be settable to true", () => { const volunteerWos = ref(false) volunteerWos.value = true - expect(volunteerWos.value).toBe(true) + expect(volunteerWos.value).toBeTruthy() }) it("should correctly toggle volunteer status", () => { const volunteerWos = ref(false) volunteerWos.value = !volunteerWos.value - expect(volunteerWos.value).toBe(true) + expect(volunteerWos.value).toBeTruthy() volunteerWos.value = !volunteerWos.value - expect(volunteerWos.value).toBe(false) + expect(volunteerWos.value).toBeFalsy() }) }) @@ -157,8 +157,8 @@ describe("InstructorEditDialog - Error Handling", () => { }) it("should handle null report units from API", () => { - const apiReportUnit: string | null = null - const reportUnits = apiReportUnit ? apiReportUnit.split(",") : [] + // When API returns null, we expect an empty array + const reportUnits: string[] = [] expect(reportUnits).toHaveLength(0) }) @@ -184,12 +184,12 @@ describe("InstructorEditDialog - State Management", () => { const isSaving = ref(false) isSaving.value = true - expect(isSaving.value).toBe(true) + expect(isSaving.value).toBeTruthy() // Simulate save completion await Promise.resolve() isSaving.value = false - expect(isSaving.value).toBe(false) + expect(isSaving.value).toBeFalsy() }) }) @@ -197,7 +197,7 @@ describe("InstructorEditDialog - State Management", () => { it("should populate form with instructor data", () => { const instructor = { personId: 1, - termCode: 202410, + termCode: 202_410, firstName: "John", lastName: "Doe", effortDept: "VME", @@ -213,7 +213,7 @@ describe("InstructorEditDialog - State Management", () => { expect(effortDept.value).toBe("VME") expect(effortTitleCode.value).toBe("1234") - expect(volunteerWos.value).toBe(false) + expect(volunteerWos.value).toBeFalsy() expect(reportUnits.value).toEqual(["SVM", "VMB"]) }) }) @@ -223,10 +223,10 @@ describe("InstructorEditDialog - State Management", () => { const originalDept = "VME" const currentDept = ref("VME") - expect(currentDept.value !== originalDept).toBe(false) + expect(currentDept.value !== originalDept).toBeFalsy() currentDept.value = "APC" - expect(currentDept.value !== originalDept).toBe(true) + expect(currentDept.value !== originalDept).toBeTruthy() }) }) }) diff --git a/VueApp/src/Effort/components/InstructorAddDialog.vue b/VueApp/src/Effort/components/InstructorAddDialog.vue index ffd18233..0ff3532f 100644 --- a/VueApp/src/Effort/components/InstructorAddDialog.vue +++ b/VueApp/src/Effort/components/InstructorAddDialog.vue @@ -4,7 +4,7 @@ persistent @update:model-value="emit('update:modelValue', $event)" > - +
Add Instructor
@@ -31,13 +31,18 @@ options-dense outlined clearable + behavior="menu" @filter="onFilter" > @@ -76,14 +83,16 @@ {{ selectedPerson.fullName }}
- Department: {{ selectedPerson.effortDept }} + + Department: {{ selectedPerson.deptName || selectedPerson.effortDept }} + No department assigned
- Title Code: {{ selectedPerson.titleCode }} + Title: {{ selectedPerson.title || selectedPerson.titleCode }}
diff --git a/VueApp/src/Effort/components/InstructorEditDialog.vue b/VueApp/src/Effort/components/InstructorEditDialog.vue index d334dc53..c5d7e156 100644 --- a/VueApp/src/Effort/components/InstructorEditDialog.vue +++ b/VueApp/src/Effort/components/InstructorEditDialog.vue @@ -4,7 +4,7 @@ persistent @update:model-value="emit('update:modelValue', $event)" > - +
Edit Instructor
@@ -45,23 +45,63 @@ - + @filter="filterTitleCodes" + > + + + - + > + + - Excludes instructor from M&P reports + Volunteer / WOS + + - This will prevent the instructor from showing up in the M&P reports. +
import { ref, computed, watch, onMounted } from "vue" import { effortService } from "../services/effort-service" -import type { PersonDto, DepartmentDto, ReportUnitDto } from "../types" +import type { PersonDto, DepartmentDto, ReportUnitDto, TitleCodeDto, JobGroupDto } from "../types" const props = defineProps<{ modelValue: boolean @@ -139,6 +181,9 @@ const form = ref({ const departments = ref([]) const reportUnits = ref([]) +const titleCodes = ref([]) +const jobGroups = ref([]) +const filteredTitleCodes = ref<{ label: string; value: string }[]>([]) const isSaving = ref(false) const errorMessage = ref("") @@ -170,11 +215,56 @@ const reportUnitOptions = computed(() => { })) }) +const titleCodeOptions = computed(() => { + return titleCodes.value.map((t) => ({ + label: t.name ? `${t.code} - ${t.name}` : t.code, + value: t.code, + })) +}) + +const jobGroupOptions = computed(() => { + return jobGroups.value.map((j) => ({ + label: j.name ? `${j.code} - ${j.name}` : j.code, + value: j.code, + })) +}) + +// Check if the current title code value is not in the dropdown options (orphaned) +const isOrphanedTitleCode = computed(() => { + if (!form.value.effortTitleCode) return false + return !titleCodes.value.some((t) => t.code === form.value.effortTitleCode) +}) + +// Check if the current job group value is not in the dropdown options (orphaned) +const isOrphanedJobGroup = computed(() => { + if (!form.value.jobGroupId) return false + return !jobGroups.value.some((j) => j.code === form.value.jobGroupId) +}) + +// Filter function for title code dropdown - searches by code OR name +function filterTitleCodes(val: string, update: (fn: () => void) => void) { + update(() => { + const needle = val.toLowerCase() + filteredTitleCodes.value = titleCodeOptions.value.filter( + (opt) => opt.label.toLowerCase().includes(needle) || opt.value.toLowerCase().includes(needle), + ) + }) +} + // Load lookup data onMounted(async () => { - const [depts, units] = await Promise.all([effortService.getInstructorDepartments(), effortService.getReportUnits()]) + const [depts, units, titles, groups] = await Promise.all([ + effortService.getInstructorDepartments(), + effortService.getReportUnits(), + effortService.getTitleCodes(), + effortService.getJobGroups(), + ]) departments.value = depts reportUnits.value = units + titleCodes.value = titles + jobGroups.value = groups + // Initialize filtered title codes with all options + filteredTitleCodes.value = titleCodeOptions.value }) // Reset form when dialog opens with instructor diff --git a/VueApp/src/Effort/services/effort-service.ts b/VueApp/src/Effort/services/effort-service.ts index 0ce5dbbd..f6da7e43 100644 --- a/VueApp/src/Effort/services/effort-service.ts +++ b/VueApp/src/Effort/services/effort-service.ts @@ -18,6 +18,8 @@ import type { DepartmentDto, CanDeleteResult, InstructorEffortRecordDto, + TitleCodeDto, + JobGroupDto, } from "../types" const { get, post, put, del, patch } = useFetch() @@ -470,6 +472,28 @@ class EffortService { } return response.result as InstructorEffortRecordDto[] } + + /** + * Get all title codes from dictionary database for the dropdown. + */ + async getTitleCodes(): Promise { + const response = await get(`${this.baseUrl}/instructors/title-codes`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as TitleCodeDto[] + } + + /** + * Get all job groups currently in use for the dropdown. + */ + async getJobGroups(): Promise { + const response = await get(`${this.baseUrl}/instructors/job-groups`) + if (!response.success || !Array.isArray(response.result)) { + return [] + } + return response.result as JobGroupDto[] + } } const effortService = new EffortService() diff --git a/VueApp/src/Effort/types/index.ts b/VueApp/src/Effort/types/index.ts index 1733c4f2..91600beb 100644 --- a/VueApp/src/Effort/types/index.ts +++ b/VueApp/src/Effort/types/index.ts @@ -179,7 +179,9 @@ type AaudPersonDto = { middleInitial: string | null fullName: string effortDept: string | null + deptName: string | null titleCode: string | null + title: string | null jobGroupId: string | null } @@ -229,6 +231,16 @@ type InstructorEffortRecordDto = { course: CourseDto } +type TitleCodeDto = { + code: string + name: string +} + +type JobGroupDto = { + code: string + name: string +} + export type { TermDto, PersonDto, @@ -253,4 +265,6 @@ export type { DepartmentDto, CanDeleteResult, InstructorEffortRecordDto, + TitleCodeDto, + JobGroupDto, } diff --git a/test/Effort/InstructorServiceTests.cs b/test/Effort/InstructorServiceTests.cs index 61ec91e3..180f8cd6 100644 --- a/test/Effort/InstructorServiceTests.cs +++ b/test/Effort/InstructorServiceTests.cs @@ -465,6 +465,194 @@ public void IsValidDepartment_ReturnsTrue_ForValidDepartment() #endregion + #region ResolveInstructorDepartmentAsync / DetermineDepartment Tests + + [Fact] + public async Task ResolveInstructorDepartmentAsync_ReturnsMisonOverride_ForMothraId02493928() + { + // Arrange - Mison override should return VSR regardless of jobs/employee data + _viperContext.People.Add(new Viper.Models.VIPER.Person + { + PersonId = 100, + ClientId = "02493928", + FirstName = "Michael", + LastName = "Mison", + FullName = "Mison, Michael", + MothraId = "02493928", // This is the key - Mison's MothraId triggers the override + CurrentEmployee = true + }); + await _viperContext.SaveChangesAsync(); + + // Add VMDO job (should be ignored due to override) + _aaudContext.Ids.Add(new Viper.Models.AAUD.Id + { + IdsPKey = "MISON001", + IdsTermCode = "202410", + IdsMothraid = "02493928", + IdsClientid = "02493928" + }); + _aaudContext.Employees.Add(new Viper.Models.AAUD.Employee + { + EmpPKey = "MISON001", + EmpTermCode = "202410", + EmpClientid = "02493928", + EmpHomeDept = "072000", // VMDO + EmpAltDeptCode = "", + EmpSchoolDivision = "VM", + EmpCbuc = "99", + EmpStatus = "A" + }); + await _aaudContext.SaveChangesAsync(); + + // Act + var dept = await _instructorService.ResolveInstructorDepartmentAsync(100, 202410); + + // Assert - Should return VSR due to hardcoded override + Assert.Equal("VSR", dept); + } + + [Fact] + public async Task ResolveInstructorDepartmentAsync_ReturnsAcademicDeptFromJobs_WhenAvailable() + { + // Arrange + _viperContext.People.Add(new Viper.Models.VIPER.Person + { + PersonId = 1, + ClientId = "12345678", + FirstName = "Test", + LastName = "User", + FullName = "User, Test", + MothraId = "12345678", + CurrentEmployee = true + }); + await _viperContext.SaveChangesAsync(); + + _aaudContext.Ids.Add(new Viper.Models.AAUD.Id + { + IdsPKey = "TEST001", + IdsTermCode = "202410", + IdsMothraid = "12345678", + IdsClientid = "12345678" + }); + + // Add employee with non-academic dept + _aaudContext.Employees.Add(new Viper.Models.AAUD.Employee + { + EmpPKey = "TEST001", + EmpTermCode = "202410", + EmpClientid = "12345678", + EmpHomeDept = "072000", // Non-academic + EmpAltDeptCode = "", + EmpSchoolDivision = "VM", + EmpCbuc = "99", + EmpStatus = "A" + }); + + // Add job with academic dept code (VME = 072030) + _aaudContext.Jobs.Add(new Viper.Models.AAUD.Job + { + JobPKey = "TEST001", + JobSeqNum = 1, + JobTermCode = "202410", + JobClientid = "12345678", + JobDepartmentCode = "VME", // Academic dept - should be found + JobPercentFulltime = 100, + JobTitleCode = "1234", + JobBargainingUnit = "99", + JobSchoolDivision = "VM" + }); + await _aaudContext.SaveChangesAsync(); + + // Act + var dept = await _instructorService.ResolveInstructorDepartmentAsync(1, 202410); + + // Assert - Should return VME from jobs table + Assert.Equal("VME", dept); + } + + [Fact] + public async Task ResolveInstructorDepartmentAsync_FallsBackToEmployeeFields_WhenNoAcademicJobDept() + { + // Arrange + _viperContext.People.Add(new Viper.Models.VIPER.Person + { + PersonId = 2, + ClientId = "87654321", + FirstName = "Test", + LastName = "User2", + FullName = "User2, Test", + MothraId = "87654321", + CurrentEmployee = true + }); + await _viperContext.SaveChangesAsync(); + + _aaudContext.Ids.Add(new Viper.Models.AAUD.Id + { + IdsPKey = "TEST002", + IdsTermCode = "202410", + IdsMothraid = "87654321", + IdsClientid = "87654321" + }); + + // Add employee with academic effort dept + _aaudContext.Employees.Add(new Viper.Models.AAUD.Employee + { + EmpPKey = "TEST002", + EmpTermCode = "202410", + EmpClientid = "87654321", + EmpEffortHomeDept = "APC", // Academic dept in employee record + EmpHomeDept = "072000", + EmpAltDeptCode = "", + EmpSchoolDivision = "VM", + EmpCbuc = "99", + EmpStatus = "A" + }); + + // No jobs or only non-academic jobs + await _aaudContext.SaveChangesAsync(); + + // Act + var dept = await _instructorService.ResolveInstructorDepartmentAsync(2, 202410); + + // Assert - Should return APC from employee effort dept + Assert.Equal("APC", dept); + } + + [Fact] + public async Task ResolveInstructorDepartmentAsync_ReturnsNull_WhenNoPersonFound() + { + // Act + var dept = await _instructorService.ResolveInstructorDepartmentAsync(99999, 202410); + + // Assert - Returns null when person doesn't exist (distinct from "UNK" for person exists but no dept) + Assert.Null(dept); + } + + [Fact] + public async Task ResolveInstructorDepartmentAsync_ReturnsUNK_WhenNoEmployeeOrJobsFound() + { + // Arrange - Person exists but no AAUD data + _viperContext.People.Add(new Viper.Models.VIPER.Person + { + PersonId = 3, + ClientId = "00000000", + FirstName = "No", + LastName = "Data", + FullName = "Data, No", + MothraId = "00000000", + CurrentEmployee = true + }); + await _viperContext.SaveChangesAsync(); + + // Act + var dept = await _instructorService.ResolveInstructorDepartmentAsync(3, 202410); + + // Assert + Assert.Equal("UNK", dept); + } + + #endregion + #region AutoMapper PersonDto Mapping Tests [Fact] @@ -487,4 +675,32 @@ public async Task GetInstructorAsync_MapsVolunteerWosCorrectly() } #endregion + + #region GetTitleCodesAsync Tests + + [Fact] + public async Task GetTitleCodesAsync_ReturnsEmptyList_WhenConnectionStringEmpty() + { + // The test setup uses empty connection strings, so the method should return empty list + // This tests the graceful fallback behavior when the database is not available + var titleCodes = await _instructorService.GetTitleCodesAsync(); + + Assert.Empty(titleCodes); + } + + #endregion + + #region GetJobGroupsAsync Tests + + [Fact] + public async Task GetJobGroupsAsync_ReturnsEmptyList_WhenConnectionStringEmpty() + { + // The test setup uses empty connection strings, so the method should return empty list + // This tests the graceful fallback behavior when the database is not available + var jobGroups = await _instructorService.GetJobGroupsAsync(); + + Assert.Empty(jobGroups); + } + + #endregion } diff --git a/test/Effort/InstructorsControllerTests.cs b/test/Effort/InstructorsControllerTests.cs index 25f283ef..4697a07b 100644 --- a/test/Effort/InstructorsControllerTests.cs +++ b/test/Effort/InstructorsControllerTests.cs @@ -176,7 +176,7 @@ public async Task CreateInstructor_ReturnsCreated_WithInstructorDto() var createdInstructor = new PersonDto { PersonId = 1, TermCode = 202410, FirstName = "John", LastName = "Doe", EffortDept = "VME" }; _instructorServiceMock.Setup(s => s.InstructorExistsAsync(1, 202410, It.IsAny())).ReturnsAsync(false); - _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, It.IsAny())).ReturnsAsync("VME"); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, 202410, It.IsAny())).ReturnsAsync("VME"); _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) .ReturnsAsync(createdInstructor); @@ -223,7 +223,7 @@ public async Task CreateInstructor_ReturnsBadRequest_ForInvalidOperationExceptio }; _instructorServiceMock.Setup(s => s.InstructorExistsAsync(999, 202410, It.IsAny())).ReturnsAsync(false); - _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(999, It.IsAny())).ReturnsAsync("VME"); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(999, 202410, It.IsAny())).ReturnsAsync("VME"); _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) .ThrowsAsync(new InvalidOperationException("Person not found in AAUD")); @@ -247,7 +247,7 @@ public async Task CreateInstructor_ReturnsBadRequest_ForDbUpdateException() }; _instructorServiceMock.Setup(s => s.InstructorExistsAsync(1, 202410, It.IsAny())).ReturnsAsync(false); - _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, It.IsAny())).ReturnsAsync("VME"); + _instructorServiceMock.Setup(s => s.ResolveInstructorDepartmentAsync(1, 202410, It.IsAny())).ReturnsAsync("VME"); _permissionServiceMock.Setup(s => s.CanViewDepartmentAsync("VME", It.IsAny())).ReturnsAsync(true); _instructorServiceMock.Setup(s => s.CreateInstructorAsync(request, It.IsAny())) .ThrowsAsync(new DbUpdateException("Database constraint violation")); @@ -553,4 +553,86 @@ public async Task GetInstructorEffortRecords_ReturnsNotFound_WhenInstructorDoesN } #endregion + + #region GetTitleCodes Tests + + [Fact] + public async Task GetTitleCodes_ReturnsOk_WithTitleCodes() + { + // Arrange + var titleCodes = new List + { + new TitleCodeDto { Code = "000353", Name = "VETERINARIAN" }, + new TitleCodeDto { Code = "000521", Name = "PROFESSOR" } + }; + _instructorServiceMock.Setup(s => s.GetTitleCodesAsync(It.IsAny())) + .ReturnsAsync(titleCodes); + + // Act + var result = await _controller.GetTitleCodes(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedTitleCodes = Assert.IsAssignableFrom>(okResult.Value); + Assert.Equal(2, returnedTitleCodes.Count()); + } + + [Fact] + public async Task GetTitleCodes_ReturnsOk_WithEmptyList_WhenNoTitleCodes() + { + // Arrange + _instructorServiceMock.Setup(s => s.GetTitleCodesAsync(It.IsAny())) + .ReturnsAsync(new List()); + + // Act + var result = await _controller.GetTitleCodes(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedTitleCodes = Assert.IsAssignableFrom>(okResult.Value); + Assert.Empty(returnedTitleCodes); + } + + #endregion + + #region GetJobGroups Tests + + [Fact] + public async Task GetJobGroups_ReturnsOk_WithJobGroups() + { + // Arrange + var jobGroups = new List + { + new JobGroupDto { Code = "I15", Name = "STAFF VET" }, + new JobGroupDto { Code = "B24", Name = "" } // NULL name in legacy data shows code only + }; + _instructorServiceMock.Setup(s => s.GetJobGroupsAsync(It.IsAny())) + .ReturnsAsync(jobGroups); + + // Act + var result = await _controller.GetJobGroups(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedJobGroups = Assert.IsAssignableFrom>(okResult.Value); + Assert.Equal(2, returnedJobGroups.Count()); + } + + [Fact] + public async Task GetJobGroups_ReturnsOk_WithEmptyList_WhenNoJobGroups() + { + // Arrange + _instructorServiceMock.Setup(s => s.GetJobGroupsAsync(It.IsAny())) + .ReturnsAsync(new List()); + + // Act + var result = await _controller.GetJobGroups(); + + // Assert + var okResult = Assert.IsType(result.Result); + var returnedJobGroups = Assert.IsAssignableFrom>(okResult.Value); + Assert.Empty(returnedJobGroups); + } + + #endregion } diff --git a/web/Areas/Effort/Controllers/InstructorsController.cs b/web/Areas/Effort/Controllers/InstructorsController.cs index d16bd743..2170e7ec 100644 --- a/web/Areas/Effort/Controllers/InstructorsController.cs +++ b/web/Areas/Effort/Controllers/InstructorsController.cs @@ -74,16 +74,11 @@ public async Task>> GetInstructors( return Ok(Array.Empty()); } - // If no dept specified, get instructors for all authorized departments + // If no dept specified, get instructors for all authorized departments in a single query if (string.IsNullOrEmpty(dept)) { - var allInstructors = new List(); - foreach (var authorizedDept in authorizedDepts) - { - var deptInstructors = await _instructorService.GetInstructorsAsync(termCode, authorizedDept, ct); - allInstructors.AddRange(deptInstructors); - } - return Ok(allInstructors.OrderBy(i => i.LastName).ThenBy(i => i.FirstName)); + var allInstructors = await _instructorService.GetInstructorsByDepartmentsAsync(termCode, authorizedDepts, ct); + return Ok(allInstructors); } } @@ -157,10 +152,10 @@ public async Task> CreateInstructor( } // Verify the user is authorized to add instructors to the resolved department - var resolvedDept = await _instructorService.ResolveInstructorDepartmentAsync(request.PersonId, ct); + var resolvedDept = await _instructorService.ResolveInstructorDepartmentAsync(request.PersonId, request.TermCode, ct); if (resolvedDept == null) { - return BadRequest("Person not found"); + return NotFound("Person not found."); } if (!await _permissionService.CanViewDepartmentAsync(resolvedDept, ct)) @@ -341,4 +336,24 @@ public async Task>> GetInstr var records = await _instructorService.GetInstructorEffortRecordsAsync(personId, termCode, ct); return Ok(records); } + + /// + /// Get all title codes from the dictionary database for the dropdown. + /// + [HttpGet("title-codes")] + public async Task>> GetTitleCodes(CancellationToken ct = default) + { + var titleCodes = await _instructorService.GetTitleCodesAsync(ct); + return Ok(titleCodes); + } + + /// + /// Get all job groups currently in use for the dropdown. + /// + [HttpGet("job-groups")] + public async Task>> GetJobGroups(CancellationToken ct = default) + { + var jobGroups = await _instructorService.GetJobGroupsAsync(ct); + return Ok(jobGroups); + } } diff --git a/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs b/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs index 8c7ae05f..b84f3bb5 100644 --- a/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs +++ b/web/Areas/Effort/Models/DTOs/Responses/AaudPersonDto.cs @@ -10,10 +10,10 @@ public class AaudPersonDto public string FirstName { get; set; } = string.Empty; public string LastName { get; set; } = string.Empty; public string? MiddleInitial { get; set; } - public string FullName => string.IsNullOrEmpty(MiddleInitial) - ? $"{LastName}, {FirstName}" - : $"{LastName}, {FirstName} {MiddleInitial}."; + public string FullName => $"{LastName}, {FirstName}"; public string? EffortDept { get; set; } + public string? DeptName { get; set; } public string? TitleCode { get; set; } + public string? Title { get; set; } public string? JobGroupId { get; set; } } diff --git a/web/Areas/Effort/Models/DTOs/Responses/JobGroupDto.cs b/web/Areas/Effort/Models/DTOs/Responses/JobGroupDto.cs new file mode 100644 index 00000000..cc82c402 --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/JobGroupDto.cs @@ -0,0 +1,10 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for job group lookup options in the Edit Instructor dialog. +/// +public class JobGroupDto +{ + public string Code { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; +} diff --git a/web/Areas/Effort/Models/DTOs/Responses/TitleCodeDto.cs b/web/Areas/Effort/Models/DTOs/Responses/TitleCodeDto.cs new file mode 100644 index 00000000..47f5b0b2 --- /dev/null +++ b/web/Areas/Effort/Models/DTOs/Responses/TitleCodeDto.cs @@ -0,0 +1,10 @@ +namespace Viper.Areas.Effort.Models.DTOs.Responses; + +/// +/// DTO for title code lookup options in the Edit Instructor dialog. +/// +public class TitleCodeDto +{ + public string Code { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; +} diff --git a/web/Areas/Effort/Services/IInstructorService.cs b/web/Areas/Effort/Services/IInstructorService.cs index 7157c5ef..7de230d0 100644 --- a/web/Areas/Effort/Services/IInstructorService.cs +++ b/web/Areas/Effort/Services/IInstructorService.cs @@ -17,6 +17,16 @@ public interface IInstructorService /// List of instructors. Task> GetInstructorsAsync(int termCode, string? department = null, CancellationToken ct = default); + /// + /// Get all instructors for a term filtered by multiple departments. + /// Uses a single query with IN clause for better performance. + /// + /// The term code. + /// List of department codes to include. + /// Cancellation token. + /// List of instructors sorted by last name, first name. + Task> GetInstructorsByDepartmentsAsync(int termCode, IReadOnlyList departments, CancellationToken ct = default); + /// /// Get a single instructor by person ID and term code. /// @@ -87,9 +97,10 @@ public interface IInstructorService /// Used for authorization checks before creating. ///
/// The person ID. + /// The term code for AAUD lookup. /// Cancellation token. /// The resolved department code, or null if person not found. - Task ResolveInstructorDepartmentAsync(int personId, CancellationToken ct = default); + Task ResolveInstructorDepartmentAsync(int personId, int termCode, CancellationToken ct = default); /// /// Get the list of valid department codes with grouping information. @@ -125,4 +136,18 @@ public interface IInstructorService /// Cancellation token. /// List of effort records with course info. Task> GetInstructorEffortRecordsAsync(int personId, int termCode, CancellationToken ct = default); + + /// + /// Get all title codes from the dictionary database for the dropdown. + /// + /// Cancellation token. + /// List of title codes with human-readable names. + Task> GetTitleCodesAsync(CancellationToken ct = default); + + /// + /// Get all job groups currently in use by instructors for the dropdown. + /// + /// Cancellation token. + /// List of job groups with human-readable names. + Task> GetJobGroupsAsync(CancellationToken ct = default); } diff --git a/web/Areas/Effort/Services/InstructorService.cs b/web/Areas/Effort/Services/InstructorService.cs index 0db5e8f4..c5a2ff0c 100644 --- a/web/Areas/Effort/Services/InstructorService.cs +++ b/web/Areas/Effort/Services/InstructorService.cs @@ -25,6 +25,7 @@ public class InstructorService : IInstructorService private readonly IMemoryCache _cache; private const string TitleCacheKey = "effort_title_lookup"; + private const string DeptSimpleNameCacheKey = "effort_dept_simple_name_lookup"; /// /// Valid academic department codes. @@ -101,6 +102,27 @@ public async Task> GetInstructorsAsync(int termCode, string? dep return dtos; } + public async Task> GetInstructorsByDepartmentsAsync(int termCode, IReadOnlyList departments, CancellationToken ct = default) + { + if (departments.Count == 0) + { + return []; + } + + var instructors = await _context.Persons + .AsNoTracking() + .Where(p => p.TermCode == termCode && departments.Contains(p.EffortDept)) + .OrderBy(p => p.LastName) + .ThenBy(p => p.FirstName) + .ToListAsync(ct); + + var dtos = _mapper.Map>(instructors); + + await EnrichWithTitlesAsync(dtos, ct); + + return dtos; + } + public async Task GetInstructorAsync(int personId, int termCode, CancellationToken ct = default) { var instructor = await _context.Persons @@ -147,25 +169,102 @@ public async Task> SearchPossibleInstructorsAsync(int termCo .ToListAsync(ct); // Get employee info from AAUD for department/title lookup + // Need to go through the Ids table to map MothraId -> pKey -> Employee var mothraIds = people.Select(p => p.MothraId).ToList(); + var termCodeStr = termCode.ToString(); + + // Look up pKeys from Ids table using MothraId + var idsRecords = await _aaudContext.Ids + .AsNoTracking() + .Where(i => i.IdsTermCode == termCodeStr && i.IdsMothraid != null && mothraIds.Contains(i.IdsMothraid)) + .ToListAsync(ct); + + // Build MothraId -> pKey mapping + var mothraIdToPKey = idsRecords + .Where(i => i.IdsMothraid != null) + .ToDictionary(i => i.IdsMothraid!, i => i.IdsPKey, StringComparer.OrdinalIgnoreCase); + + // Get employees using pKeys + var pKeys = mothraIdToPKey.Values.ToList(); var employees = await _aaudContext.Employees .AsNoTracking() - .Where(e => mothraIds.Contains(e.EmpPKey)) + .Where(e => e.EmpTermCode == termCodeStr && pKeys.Contains(e.EmpPKey)) .ToListAsync(ct); var employeeDict = employees.ToDictionary(e => e.EmpPKey, StringComparer.OrdinalIgnoreCase); + // Get title lookup for human-readable title names + var titleLookup = await GetTitleLookupAsync(ct); + + // Get department simple name lookup (raw code like "072030" -> simple name like "VME") + var deptSimpleNameLookup = await GetDepartmentSimpleNameLookupAsync(ct); + + // Build simple department code -> display name lookup (e.g., "VME" -> "Medicine & Epidemiology") + var deptDisplayNameLookup = Departments.ToDictionary(d => d.Code, d => d.Name, StringComparer.OrdinalIgnoreCase); + + // Batch-fetch job departments for all people (matches legacy jobs query) + var validMothraIds = mothraIds.Where(m => !string.IsNullOrEmpty(m)).ToList(); + var jobDeptsByMothraId = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + if (validMothraIds.Count > 0) + { + var jobData = await _aaudContext.Jobs + .AsNoTracking() + .Where(job => job.JobTermCode == termCodeStr) + .Join( + _aaudContext.Ids.Where(i => i.IdsTermCode == termCodeStr && i.IdsMothraid != null && validMothraIds.Contains(i.IdsMothraid)), + job => job.JobPKey, + ids => ids.IdsPKey, + (job, ids) => new { ids.IdsMothraid, job.JobDepartmentCode }) + .ToListAsync(ct); + + foreach (var item in jobData) + { + if (item.IdsMothraid == null) continue; + if (!jobDeptsByMothraId.TryGetValue(item.IdsMothraid, out var list)) + { + list = new List(); + jobDeptsByMothraId[item.IdsMothraid] = list; + } + list.Add(item.JobDepartmentCode); + } + } + return people.Select(p => { - employeeDict.TryGetValue(p.MothraId, out var emp); + Viper.Models.AAUD.Employee? emp = null; + if (mothraIdToPKey.TryGetValue(p.MothraId, out var pKey)) + { + employeeDict.TryGetValue(pKey, out emp); + } + + var deptCode = DetermineDepartmentFromJobs(p.MothraId, emp, jobDeptsByMothraId, deptSimpleNameLookup); + var titleCode = emp?.EmpEffortTitleCode?.Trim() ?? emp?.EmpTeachingTitleCode?.Trim(); + + // Look up human-readable names + string? deptName = null; + if (deptCode != null && deptDisplayNameLookup.TryGetValue(deptCode, out var name)) + { + deptName = name; + } + + string? title = null; + if (titleCode != null && titleLookup != null) + { + var paddedCode = titleCode.PadLeft(6, '0'); + titleLookup.TryGetValue(paddedCode, out title); + } + return new AaudPersonDto { PersonId = p.PersonId, FirstName = p.FirstName, LastName = p.LastName, MiddleInitial = p.MiddleName?.Length > 0 ? p.MiddleName.Substring(0, 1) : null, - EffortDept = DetermineDepartment(emp), - TitleCode = emp?.EmpEffortTitleCode?.Trim() ?? emp?.EmpTeachingTitleCode?.Trim(), + EffortDept = deptCode, + DeptName = deptName, + TitleCode = titleCode, + Title = title, JobGroupId = null // Job group would need additional lookup if required }; }).ToList(); @@ -189,12 +288,27 @@ public async Task CreateInstructorAsync(CreateInstructorRequest reque throw new InvalidOperationException($"Person with PersonId {request.PersonId} not found"); } - // Look up employee info from AAUD - var employee = await _aaudContext.Employees - .AsNoTracking() - .FirstOrDefaultAsync(e => e.EmpPKey == person.MothraId, ct); + // Look up employee info from AAUD through Ids table (MothraId -> pKey -> Employee) + var termCodeStr = request.TermCode.ToString(); + Viper.Models.AAUD.Id? idsRecord = null; + if (!string.IsNullOrEmpty(person.MothraId)) + { + idsRecord = await _aaudContext.Ids + .AsNoTracking() + .FirstOrDefaultAsync(i => i.IdsTermCode == termCodeStr && i.IdsMothraid == person.MothraId, ct); + } + + Viper.Models.AAUD.Employee? employee = null; + if (idsRecord != null) + { + employee = await _aaudContext.Employees + .AsNoTracking() + .FirstOrDefaultAsync(e => e.EmpTermCode == termCodeStr && e.EmpPKey == idsRecord.IdsPKey, ct); + } - var dept = DetermineDepartment(employee) ?? "UNK"; + // Get department simple name lookup to convert raw codes to simple names + var deptSimpleNameLookup = await GetDepartmentSimpleNameLookupAsync(ct); + var dept = await DetermineDepartmentAsync(person.MothraId, request.TermCode, employee, deptSimpleNameLookup, ct) ?? "UNK"; var titleCode = employee?.EmpEffortTitleCode?.Trim() ?? employee?.EmpTeachingTitleCode?.Trim() ?? ""; var instructor = new EffortPerson @@ -248,8 +362,9 @@ public async Task CreateInstructorAsync(CreateInstructorRequest reque return null; } - // Validate department - if (!IsValidDepartment(request.EffortDept)) + // Validate department - allow known departments or keeping the current one (legacy data may have non-standard departments) + var isCurrentDept = string.Equals(instructor.EffortDept, request.EffortDept, StringComparison.OrdinalIgnoreCase); + if (!isCurrentDept && !IsValidDepartment(request.EffortDept)) { throw new ArgumentException($"Invalid department: {request.EffortDept}"); } @@ -269,7 +384,7 @@ public async Task CreateInstructorAsync(CreateInstructorRequest reque instructor.ReportUnit = request.ReportUnits != null && request.ReportUnits.Count > 0 ? string.Join(",", request.ReportUnits) : null; - instructor.VolunteerWos = request.VolunteerWos ? (byte)1 : null; + instructor.VolunteerWos = request.VolunteerWos ? (byte)1 : (byte)0; await using var transaction = await _context.Database.BeginTransactionAsync(ct); @@ -351,7 +466,7 @@ public async Task InstructorExistsAsync(int personId, int termCode, Cancel .AnyAsync(p => p.PersonId == personId && p.TermCode == termCode, ct); } - public async Task ResolveInstructorDepartmentAsync(int personId, CancellationToken ct = default) + public async Task ResolveInstructorDepartmentAsync(int personId, int termCode, CancellationToken ct = default) { var person = await _viperContext.People .AsNoTracking() @@ -362,11 +477,27 @@ public async Task InstructorExistsAsync(int personId, int termCode, Cancel return null; } - var employee = await _aaudContext.Employees - .AsNoTracking() - .FirstOrDefaultAsync(e => e.EmpPKey == person.MothraId, ct); + // Look up employee info from AAUD through Ids table (MothraId -> pKey -> Employee) + var termCodeStr = termCode.ToString(); + Viper.Models.AAUD.Id? idsRecord = null; + if (!string.IsNullOrEmpty(person.MothraId)) + { + idsRecord = await _aaudContext.Ids + .AsNoTracking() + .FirstOrDefaultAsync(i => i.IdsTermCode == termCodeStr && i.IdsMothraid == person.MothraId, ct); + } - return DetermineDepartment(employee) ?? "UNK"; + Viper.Models.AAUD.Employee? employee = null; + if (idsRecord != null) + { + employee = await _aaudContext.Employees + .AsNoTracking() + .FirstOrDefaultAsync(e => e.EmpTermCode == termCodeStr && e.EmpPKey == idsRecord.IdsPKey, ct); + } + + // Get department simple name lookup to convert raw codes to simple names + var deptSimpleNameLookup = await GetDepartmentSimpleNameLookupAsync(ct); + return await DetermineDepartmentAsync(person.MothraId, termCode, employee, deptSimpleNameLookup, ct) ?? "UNK"; } public List GetDepartments() @@ -401,43 +532,185 @@ public async Task> GetReportUnitsAsync(CancellationToken ct } /// - /// Determine the effort department from AAUD employee data. - /// Priority: Effort dept, then teaching dept, then home dept. - /// Only use if it's an academic SVM department. + /// Determine the effort department from AAUD data. + /// Priority: 1) Special overrides, 2) First academic dept from jobs, 3) Employee fields, 4) Fallback. /// - private static string? DetermineDepartment(Viper.Models.AAUD.Employee? employee) + /// + /// Matches legacy Instructor.cfc logic (lines 61-104): + /// - First queries AAUD jobs table for academic departments + /// - Then falls back to employee-level fields (effort/home/alt dept) + /// - Includes hardcoded overrides for special cases + /// + private async Task DetermineDepartmentAsync( + string? mothraId, + int termCode, + Viper.Models.AAUD.Employee? employee, + Dictionary? deptSimpleNameLookup, + CancellationToken ct = default) { - if (employee == null) + // Mison override - he only has a VMTH job, but needs his effort recorded to VSR + if (mothraId == "02493928") + { + return "VSR"; + } + + // Helper to resolve raw code to simple name + string? ToSimpleName(string? rawCode) { + if (string.IsNullOrWhiteSpace(rawCode)) + { + return null; + } + var trimmed = rawCode.Trim(); + // If lookup available, use it; otherwise return raw value (fallback for simple codes) + if (deptSimpleNameLookup != null && deptSimpleNameLookup.TryGetValue(trimmed, out var simpleName)) + { + return simpleName; + } + // If raw code is already a valid department code (academic or center), return it + if (Departments.Any(d => string.Equals(d.Code, trimmed, StringComparison.OrdinalIgnoreCase))) + { + return trimmed.ToUpperInvariant(); + } return null; } - // Check effort dept first - if (!string.IsNullOrWhiteSpace(employee.EmpEffortHomeDept) && - AcademicDepts.Contains(employee.EmpEffortHomeDept.Trim())) + // Step 1: Check AAUD jobs for first academic department (matches legacy qInstructorJobs query) + if (!string.IsNullOrEmpty(mothraId)) + { + var termCodeStr = termCode.ToString(); + var jobDepts = await _aaudContext.Jobs + .AsNoTracking() + .Where(job => job.JobTermCode == termCodeStr) + .Join( + _aaudContext.Ids.Where(i => i.IdsTermCode == termCodeStr && i.IdsMothraid == mothraId), + job => job.JobPKey, + ids => ids.IdsPKey, + (job, ids) => job.JobDepartmentCode) + .ToListAsync(ct); + + foreach (var deptCode in jobDepts) + { + var simpleName = ToSimpleName(deptCode); + if (simpleName != null && AcademicDepts.Contains(simpleName)) + { + return simpleName.ToUpperInvariant(); + } + } + } + + // Step 2: Fall back to employee fields (matches legacy qDetails checks) + if (employee == null) { - return employee.EmpEffortHomeDept.Trim().ToUpperInvariant(); + return null; } - // Check teaching dept - if (!string.IsNullOrWhiteSpace(employee.EmpTeachingHomeDept) && - AcademicDepts.Contains(employee.EmpTeachingHomeDept.Trim())) + // Check effort dept first + var effortDeptSimple = ToSimpleName(employee.EmpEffortHomeDept); + if (effortDeptSimple != null && AcademicDepts.Contains(effortDeptSimple)) { - return employee.EmpTeachingHomeDept.Trim().ToUpperInvariant(); + return effortDeptSimple.ToUpperInvariant(); } // Check home dept - if (!string.IsNullOrWhiteSpace(employee.EmpHomeDept) && - AcademicDepts.Contains(employee.EmpHomeDept.Trim())) + var homeDeptSimple = ToSimpleName(employee.EmpHomeDept); + if (homeDeptSimple != null && AcademicDepts.Contains(homeDeptSimple)) { - return employee.EmpHomeDept.Trim().ToUpperInvariant(); + return homeDeptSimple.ToUpperInvariant(); } // Check alt dept - if (!string.IsNullOrWhiteSpace(employee.EmpAltDeptCode) && - AcademicDepts.Contains(employee.EmpAltDeptCode.Trim())) + var altDeptSimple = ToSimpleName(employee.EmpAltDeptCode); + if (altDeptSimple != null && AcademicDepts.Contains(altDeptSimple)) + { + return altDeptSimple.ToUpperInvariant(); + } + + // No academic dept found - fall back to effort dept even if non-academic (matches legacy line 97) + if (effortDeptSimple != null) + { + return effortDeptSimple.ToUpperInvariant(); + } + + return null; + } + + /// + /// Determine the effort department from AAUD data using pre-fetched job departments. + /// Used by batch operations to avoid N+1 queries. + /// + private static string? DetermineDepartmentFromJobs( + string? mothraId, + Viper.Models.AAUD.Employee? employee, + Dictionary>? jobDeptsByMothraId, + Dictionary? deptSimpleNameLookup) + { + // Mison override - he only has a VMTH job, but needs his effort recorded to VSR + if (mothraId == "02493928") + { + return "VSR"; + } + + // Helper to resolve raw code to simple name + string? ToSimpleName(string? rawCode) + { + if (string.IsNullOrWhiteSpace(rawCode)) + { + return null; + } + var trimmed = rawCode.Trim(); + if (deptSimpleNameLookup != null && deptSimpleNameLookup.TryGetValue(trimmed, out var simpleName)) + { + return simpleName; + } + if (Departments.Any(d => string.Equals(d.Code, trimmed, StringComparison.OrdinalIgnoreCase))) + { + return trimmed.ToUpperInvariant(); + } + return null; + } + + // Step 1: Check jobs for first academic department + if (!string.IsNullOrEmpty(mothraId) && jobDeptsByMothraId != null && + jobDeptsByMothraId.TryGetValue(mothraId, out var jobDepts)) + { + foreach (var deptCode in jobDepts) + { + var simpleName = ToSimpleName(deptCode); + if (simpleName != null && AcademicDepts.Contains(simpleName)) + { + return simpleName.ToUpperInvariant(); + } + } + } + + // Step 2: Fall back to employee fields + if (employee == null) + { + return null; + } + + var effortDeptSimple = ToSimpleName(employee.EmpEffortHomeDept); + if (effortDeptSimple != null && AcademicDepts.Contains(effortDeptSimple)) + { + return effortDeptSimple.ToUpperInvariant(); + } + + var homeDeptSimple = ToSimpleName(employee.EmpHomeDept); + if (homeDeptSimple != null && AcademicDepts.Contains(homeDeptSimple)) + { + return homeDeptSimple.ToUpperInvariant(); + } + + var altDeptSimple = ToSimpleName(employee.EmpAltDeptCode); + if (altDeptSimple != null && AcademicDepts.Contains(altDeptSimple)) + { + return altDeptSimple.ToUpperInvariant(); + } + + if (effortDeptSimple != null) { - return employee.EmpAltDeptCode.Trim().ToUpperInvariant(); + return effortDeptSimple.ToUpperInvariant(); } return null; @@ -460,10 +733,8 @@ private async Task EnrichWithTitlesAsync(List instructors, Cancellati var titleLookup = await GetTitleLookupAsync(ct); if (titleLookup == null) return; - foreach (var instructor in instructors) + foreach (var instructor in instructors.Where(i => !string.IsNullOrWhiteSpace(i.EffortTitleCode))) { - if (string.IsNullOrWhiteSpace(instructor.EffortTitleCode)) continue; - var paddedCode = instructor.EffortTitleCode.PadLeft(6, '0'); if (titleLookup.TryGetValue(paddedCode, out var title)) { @@ -533,6 +804,75 @@ WHERE dvtTitle_Code IS NOT NULL } } + /// + /// Gets a cached lookup of raw department codes to simple names from the dictionary database. + /// Replicates the logic in dictionary.dbo.fn_get_deptSimpleName function using dvtSVMUnit table. + /// + private async Task?> GetDepartmentSimpleNameLookupAsync(CancellationToken ct) + { + if (_cache.TryGetValue>(DeptSimpleNameCacheKey, out var cached) && cached != null) + { + return cached; + } + + var connectionString = _configuration.GetConnectionString("VIPER"); + if (string.IsNullOrEmpty(connectionString)) + { + _logger.LogWarning("VIPER connection string not found, cannot load department lookup"); + return null; + } + + try + { + var deptLookup = new Dictionary(StringComparer.OrdinalIgnoreCase); + + await using var connection = new SqlConnection(connectionString); + await connection.OpenAsync(ct); + + // Query dvtSVMUnit table to get raw code -> simple name mapping + // This matches the logic in dictionary.dbo.fn_get_deptSimpleName function + var query = @" + SELECT DISTINCT + dvtSVMUnit_code AS DeptCode, + dvtSVMUnit_name_simple AS SimpleName + FROM [dictionary].[dbo].[dvtSVMUnit] + WHERE dvtSVMUnit_code IS NOT NULL + AND dvtSVMUnit_name_simple IS NOT NULL + AND dvtSVMUnit_name_simple <> '' + AND dvtSVMUnit_name_simple != 'CCEH' + AND ((dvtSvmUnit_Parent_ID IS NULL AND dvtSVMUnit_code = '072000') + OR (dvtSvmUnit_Parent_ID = 1 AND dvtSVMUnit_code != '072000') + OR (dvtSvmUnit_Parent_ID = 47 AND dvtSVMUnit_code = '072100'))"; + + await using var cmd = new SqlCommand(query, connection); + await using var reader = await cmd.ExecuteReaderAsync(ct); + + while (await reader.ReadAsync(ct)) + { + var code = reader["DeptCode"]?.ToString(); + var simpleName = reader["SimpleName"]?.ToString(); + if (!string.IsNullOrEmpty(code) && !string.IsNullOrEmpty(simpleName)) + { + deptLookup[code] = simpleName; + } + } + + var cacheOptions = new MemoryCacheEntryOptions() + .SetSlidingExpiration(TimeSpan.FromHours(24)); + + _cache.Set(DeptSimpleNameCacheKey, deptLookup, cacheOptions); + + _logger.LogInformation("Loaded {Count} department simple names from dictionary database", deptLookup.Count); + + return deptLookup; + } + catch (SqlException ex) + { + _logger.LogWarning(ex, "Failed to load department lookup from dictionary database"); + return null; + } + } + public async Task> GetInstructorEffortRecordsAsync( int personId, int termCode, CancellationToken ct = default) { @@ -574,4 +914,138 @@ public async Task> GetInstructorEffortRecordsAsy } }).ToList(); } + + private const string TitleCodesCacheKey = "effort_title_codes_list"; + private const string JobGroupsCacheKey = "effort_job_groups_list"; + + public async Task> GetTitleCodesAsync(CancellationToken ct = default) + { + if (_cache.TryGetValue>(TitleCodesCacheKey, out var cached) && cached != null) + { + return cached; + } + + var connectionString = _configuration.GetConnectionString("VIPER"); + if (string.IsNullOrEmpty(connectionString)) + { + _logger.LogWarning("VIPER connection string not found, cannot load title codes"); + return []; + } + + try + { + var titleCodes = new List(); + + await using var connection = new SqlConnection(connectionString); + await connection.OpenAsync(ct); + + var query = @" + SELECT DISTINCT + dvtTitle_Code AS TitleCode, + dvtTitle_name AS TitleName + FROM [dictionary].[dbo].[dvtTitle] + WHERE dvtTitle_Code IS NOT NULL + ORDER BY dvtTitle_name, dvtTitle_Code"; + + await using var cmd = new SqlCommand(query, connection); + await using var reader = await cmd.ExecuteReaderAsync(ct); + + while (await reader.ReadAsync(ct)) + { + var code = reader["TitleCode"]?.ToString()?.Trim(); + var name = reader["TitleName"]?.ToString()?.Trim(); + + if (!string.IsNullOrEmpty(code)) + { + titleCodes.Add(new TitleCodeDto + { + Code = code, + Name = name ?? string.Empty + }); + } + } + + var cacheOptions = new MemoryCacheEntryOptions() + .SetSlidingExpiration(TimeSpan.FromHours(24)); + + _cache.Set(TitleCodesCacheKey, titleCodes, cacheOptions); + + _logger.LogInformation("Loaded {Count} title codes from dictionary database", titleCodes.Count); + + return titleCodes; + } + catch (SqlException ex) + { + _logger.LogWarning(ex, "Failed to load title codes from dictionary database"); + return []; + } + } + + public async Task> GetJobGroupsAsync(CancellationToken ct = default) + { + if (_cache.TryGetValue>(JobGroupsCacheKey, out var cached) && cached != null) + { + return cached; + } + + var connectionString = _configuration.GetConnectionString("VIPER"); + if (string.IsNullOrEmpty(connectionString)) + { + _logger.LogWarning("VIPER connection string not found, cannot load job groups"); + return []; + } + + try + { + var jobGroups = new List(); + + await using var connection = new SqlConnection(connectionString); + await connection.OpenAsync(ct); + + // Get job groups that are actually in use by instructors, joined with dictionary for names + var query = @" + SELECT DISTINCT + p.JobGroupId AS JobGroupCode, + t.dvtTitle_JobGroup_Name AS JobGroupName + FROM [effort].[Persons] p + LEFT JOIN [dictionary].[dbo].[dvtTitle] t + ON p.JobGroupId = t.dvtTitle_JobGroupID + WHERE p.JobGroupId IS NOT NULL + AND p.JobGroupId != '' + ORDER BY JobGroupName, JobGroupCode"; + + await using var cmd = new SqlCommand(query, connection); + await using var reader = await cmd.ExecuteReaderAsync(ct); + + while (await reader.ReadAsync(ct)) + { + var code = reader["JobGroupCode"]?.ToString()?.Trim(); + var name = reader["JobGroupName"]?.ToString()?.Trim(); + + if (!string.IsNullOrEmpty(code)) + { + jobGroups.Add(new JobGroupDto + { + Code = code, + // If name is NULL, just use empty string (UI will show code only) + Name = name ?? string.Empty + }); + } + } + + var cacheOptions = new MemoryCacheEntryOptions() + .SetSlidingExpiration(TimeSpan.FromHours(24)); + + _cache.Set(JobGroupsCacheKey, jobGroups, cacheOptions); + + _logger.LogInformation("Loaded {Count} job groups from database", jobGroups.Count); + + return jobGroups; + } + catch (SqlException ex) + { + _logger.LogWarning(ex, "Failed to load job groups from database"); + return []; + } + } } From b53da227c3a074482db0c509c0594b8869cb7e93 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Mon, 5 Jan 2026 09:51:20 -0800 Subject: [PATCH 3/3] refactor(effort): replace foreach loops with LINQ in InstructorService --- .../Effort/Services/InstructorService.cs | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/web/Areas/Effort/Services/InstructorService.cs b/web/Areas/Effort/Services/InstructorService.cs index c5a2ff0c..ed5b5dd9 100644 --- a/web/Areas/Effort/Services/InstructorService.cs +++ b/web/Areas/Effort/Services/InstructorService.cs @@ -218,16 +218,13 @@ public async Task> SearchPossibleInstructorsAsync(int termCo (job, ids) => new { ids.IdsMothraid, job.JobDepartmentCode }) .ToListAsync(ct); - foreach (var item in jobData) - { - if (item.IdsMothraid == null) continue; - if (!jobDeptsByMothraId.TryGetValue(item.IdsMothraid, out var list)) - { - list = new List(); - jobDeptsByMothraId[item.IdsMothraid] = list; - } - list.Add(item.JobDepartmentCode); - } + jobDeptsByMothraId = jobData + .Where(item => item.IdsMothraid != null) + .GroupBy(item => item.IdsMothraid!, StringComparer.OrdinalIgnoreCase) + .ToDictionary( + g => g.Key, + g => g.Select(item => item.JobDepartmentCode).ToList(), + StringComparer.OrdinalIgnoreCase); } return people.Select(p => @@ -589,13 +586,13 @@ public async Task> GetReportUnitsAsync(CancellationToken ct (job, ids) => job.JobDepartmentCode) .ToListAsync(ct); - foreach (var deptCode in jobDepts) + var firstAcademicDept = jobDepts + .Select(ToSimpleName) + .FirstOrDefault(simpleName => simpleName != null && AcademicDepts.Contains(simpleName)); + + if (firstAcademicDept != null) { - var simpleName = ToSimpleName(deptCode); - if (simpleName != null && AcademicDepts.Contains(simpleName)) - { - return simpleName.ToUpperInvariant(); - } + return firstAcademicDept.ToUpperInvariant(); } } @@ -674,13 +671,13 @@ public async Task> GetReportUnitsAsync(CancellationToken ct if (!string.IsNullOrEmpty(mothraId) && jobDeptsByMothraId != null && jobDeptsByMothraId.TryGetValue(mothraId, out var jobDepts)) { - foreach (var deptCode in jobDepts) + var firstAcademicDept = jobDepts + .Select(ToSimpleName) + .FirstOrDefault(simpleName => simpleName != null && AcademicDepts.Contains(simpleName)); + + if (firstAcademicDept != null) { - var simpleName = ToSimpleName(deptCode); - if (simpleName != null && AcademicDepts.Contains(simpleName)) - { - return simpleName.ToUpperInvariant(); - } + return firstAcademicDept.ToUpperInvariant(); } }