diff --git a/test/Effort/ClinicalImportServiceTests.cs b/test/Effort/ClinicalImportServiceTests.cs index b140e3c88..8d7d50391 100644 --- a/test/Effort/ClinicalImportServiceTests.cs +++ b/test/Effort/ClinicalImportServiceTests.cs @@ -73,6 +73,10 @@ public ClinicalImportServiceTests() .BatchResolveDepartmentsAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(ci => ci.Arg>().ToDictionary(id => id, _ => "VME")); + _instructorServiceMock + .GetExcludedTitleCodesAsync(Arg.Any()) + .Returns(new HashSet(StringComparer.OrdinalIgnoreCase)); + _service = new ClinicalImportService( _context, _viperContext, diff --git a/test/Effort/EffortRecordServiceTests.cs b/test/Effort/EffortRecordServiceTests.cs index 3d9835c54..df1c5eae7 100644 --- a/test/Effort/EffortRecordServiceTests.cs +++ b/test/Effort/EffortRecordServiceTests.cs @@ -1,6 +1,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using NSubstitute.ExceptionExtensions; using Viper.Areas.Effort; @@ -71,6 +72,7 @@ public EffortRecordServiceTests() courseClassificationService, _rCourseServiceMock, _userHelperMock, + Options.Create(new EffortSettings { AutoCreateGenericRCourse = true }), _loggerMock); SeedTestData(); @@ -1490,6 +1492,55 @@ await _rCourseServiceMock.Received(1).CreateRCourseEffortRecordAsync( Arg.Any()); } + [Fact] + public async Task CreateEffortRecordAsync_DoesNotCallRCourseService_WhenAutoCreateDisabled() + { + // Arrange - first non-R-course record, but generic R-course auto-create disabled + var serviceWithRCourseDisabled = new EffortRecordService( + _context, + _rapsContext, + _auditServiceMock, + _instructorServiceMock, + new CourseClassificationService(), + _rCourseServiceMock, + _userHelperMock, + Options.Create(new EffortSettings { AutoCreateGenericRCourse = false }), + _loggerMock); + + var newPersonId = 202; + _context.Persons.Add(new EffortPerson + { + PersonId = newPersonId, + TermCode = TestTermCode, + FirstName = "Disabled", + LastName = "Instructor", + EffortDept = "VME" + }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var request = new CreateEffortRecordRequest + { + PersonId = newPersonId, + TermCode = TestTermCode, + CourseId = TestCourseId, // VET 410 - not an R-course + EffortTypeId = "LEC", + RoleId = 1, + EffortValue = 40 + }; + + // Act + var (result, _) = await serviceWithRCourseDisabled.CreateEffortRecordAsync(request, TestContext.Current.CancellationToken); + + // Assert - record created, but no generic R-course auto-created + Assert.NotNull(result); + await _rCourseServiceMock.DidNotReceive().CreateRCourseEffortRecordAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + } + [Fact] public async Task CreateEffortRecordAsync_DoesNotCallRCourseService_WhenSecondNonRCourseAdded() { diff --git a/test/Effort/HarvestServiceTests.cs b/test/Effort/HarvestServiceTests.cs index 8b8931f81..5ad3a0340 100644 --- a/test/Effort/HarvestServiceTests.cs +++ b/test/Effort/HarvestServiceTests.cs @@ -1,8 +1,10 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using Viper.Areas.Effort; +using Viper.Areas.Effort.Constants; using Viper.Areas.Effort.Models.DTOs.Responses; using Viper.Areas.Effort.Models.Entities; using Viper.Areas.Effort.Services; @@ -34,6 +36,11 @@ public sealed class HarvestServiceTests : IDisposable private readonly ILogger _loggerMock; private readonly HarvestService _harvestService; + // R-course auto-creation enabled for the shared service so existing R-course tests + // exercise the generation path. Toggle-off behavior is covered by its own test. + private static readonly IOptions RCourseEnabledSettings = + Options.Create(new EffortSettings { AutoCreateGenericRCourse = true }); + private const int TestTermCode = 202410; public HarvestServiceTests() @@ -93,6 +100,8 @@ public HarvestServiceTests() .GetTitleCodesAsync(Arg.Any()).Returns(new List()); _instructorServiceMock .GetDepartmentSimpleNameLookupAsync(Arg.Any()).Returns(new Dictionary()); + _instructorServiceMock + .GetExcludedTitleCodesAsync(Arg.Any()).Returns(new HashSet(StringComparer.OrdinalIgnoreCase)); // Setup audit service mock for harvest operations _auditServiceMock .ClearAuditForTermAsync(Arg.Any(), Arg.Any()).Returns(Task.CompletedTask); @@ -123,6 +132,7 @@ public HarvestServiceTests() _instructorServiceMock, _rCourseServiceMock, _clinicalImportServiceMock, + RCourseEnabledSettings, _loggerMock); } @@ -745,6 +755,7 @@ public async Task ExecuteHarvestAsync_CallsRCourseService_ForEligibleInstructors _instructorServiceMock, _rCourseServiceMock, _clinicalImportServiceMock, + RCourseEnabledSettings, _loggerMock); // Act - Run harvest (R-course detection uses inline EndsWith("R") logic) @@ -762,6 +773,83 @@ await _rCourseServiceMock.Received(1).CreateRCourseEffortRecordAsync( Arg.Any()); } + [Fact] + public async Task ExecuteHarvestAsync_DoesNotCallRCourseService_WhenAutoCreateDisabled() + { + // Arrange - same eligible-instructor setup as the enabled case + _context.Terms.Add(new EffortTerm { TermCode = TestTermCode }); + _context.EffortTypes.Add(new EffortType + { + Id = "LEC", + Description = "Lecture", + AllowedOnRCourses = true, + IsActive = true + }); + await _context.SaveChangesAsync(TestContext.Current.CancellationToken); + + var testPhase = new TestDataHarvestPhase(_context, TestTermCode); + var disabledSettings = Options.Create(new EffortSettings { AutoCreateGenericRCourse = false }); + + var harvestService = new HarvestService( + new List { testPhase }, + _context, + _viperContext, + _coursesContext, + _crestContext, + _aaudContext, + _dictionaryContext, + _auditServiceMock, + _termServiceMock, + _instructorServiceMock, + _rCourseServiceMock, + _clinicalImportServiceMock, + disabledSettings, + _loggerMock); + + // Act + var result = await harvestService.ExecuteHarvestAsync(TestTermCode, modifiedBy: 123, ct: TestContext.Current.CancellationToken); + + // Assert - harvest succeeds but the generic R-course is not created + Assert.True(result.Success); + await _rCourseServiceMock.DidNotReceive().CreateRCourseEffortRecordAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + } + + [Fact] + public void ApplyDirectorCustodialDeptFallback_InheritsDirectorDept_OnlyForNonAcademicCourses() + { + // Arrange — three non-CREST courses: UNK with academic director, already-academic, + // and UNK with a non-academic director. + var courses = new List + { + new() { Crn = "10001", CustDept = "UNK", Source = EffortConstants.SourceNonCrest }, + new() { Crn = "10002", CustDept = "PMI", Source = EffortConstants.SourceNonCrest }, + new() { Crn = "10003", CustDept = "UNK", Source = EffortConstants.SourceNonCrest }, + }; + var effort = new List + { + new() { Crn = "10001", MothraId = "AAA", RoleId = EffortConstants.DirectorRoleId }, + new() { Crn = "10003", MothraId = "CCC", RoleId = EffortConstants.DirectorRoleId }, + }; + var batchDepts = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["AAA"] = "VME", + ["CCC"] = "UNK", + }; + + // Act + NonCrestHarvestPhase.ApplyDirectorCustodialDeptFallback(courses, effort, batchDepts); + + // Assert + Assert.Equal("VME", courses[0].CustDept); // inherited from academic IOR + Assert.Equal("PMI", courses[1].CustDept); // already academic — unchanged + Assert.Equal("UNK", courses[2].CustDept); // IOR dept not academic — unchanged + } + /// /// Creates a HarvestService that uses a single custom phase for testing. /// @@ -780,6 +868,7 @@ private HarvestService CreateServiceWithPhase(IHarvestPhase phase) _instructorServiceMock, _rCourseServiceMock, _clinicalImportServiceMock, + RCourseEnabledSettings, _loggerMock); } diff --git a/test/Effort/InstructorServiceTests.cs b/test/Effort/InstructorServiceTests.cs index d59e42bfa..13deadc4b 100644 --- a/test/Effort/InstructorServiceTests.cs +++ b/test/Effort/InstructorServiceTests.cs @@ -1113,6 +1113,75 @@ public async Task BatchResolveDepartmentsAsync_ReturnsOverrideDept_WhenMothraIdH Assert.Equal(expectedDept, result[overrideMothraId]); } + [Fact] + public async Task BatchResolveDepartmentsAsync_ReturnsVme_ForChristineJohnson_WithNoAcademicJob() + { + // Christine Johnson (00129082) has no academic-department job in AAUD and a + // non-academic home/effort dept (072016), so the dept cannot be auto-derived. + // The override pins her to VME. + const string mothraId = "00129082"; + _aaudContext.Ids.Add(new Id + { + IdsPKey = "CJOHNSON01", + IdsTermCode = "202410", + IdsMothraid = mothraId, + IdsClientid = mothraId + }); + _aaudContext.Employees.Add(new Employee + { + EmpPKey = "CJOHNSON01", + EmpTermCode = "202410", + EmpClientid = mothraId, + EmpHomeDept = "072016", + EmpAltDeptCode = "", + EmpEffortHomeDept = "072016", + EmpSchoolDivision = "VM", + EmpCbuc = "99", + EmpStatus = "A" + }); + await _aaudContext.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Act + var result = await _instructorService.BatchResolveDepartmentsAsync([mothraId], 202410, TestContext.Current.CancellationToken); + + // Assert + Assert.Equal("VME", result[mothraId]); + } + + [Fact] + public async Task BatchResolveDepartmentsAsync_ReturnsVsr_ForMichaelMison_WithNonAcademicJob() + { + // Michael Mison (02493928) only has a VMTH job (non-academic); the override + // records his effort to VSR regardless of his AAUD job/employee depts. + const string mothraId = "02493928"; + _aaudContext.Ids.Add(new Id + { + IdsPKey = "MISON00001", + IdsTermCode = "202410", + IdsMothraid = mothraId, + IdsClientid = mothraId + }); + _aaudContext.Employees.Add(new Employee + { + EmpPKey = "MISON00001", + EmpTermCode = "202410", + EmpClientid = mothraId, + EmpHomeDept = "072000", + EmpAltDeptCode = "", + EmpEffortHomeDept = "072000", + EmpSchoolDivision = "VM", + EmpCbuc = "99", + EmpStatus = "A" + }); + await _aaudContext.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Act + var result = await _instructorService.BatchResolveDepartmentsAsync([mothraId], 202410, TestContext.Current.CancellationToken); + + // Assert + Assert.Equal("VSR", result[mothraId]); + } + [Fact] public async Task BatchResolveDepartmentsAsync_ReturnsAcademicDeptFromJobs_WhenAvailable() { diff --git a/web/Areas/Effort/Constants/EffortConstants.cs b/web/Areas/Effort/Constants/EffortConstants.cs index e04ec1d31..be4c39bb4 100644 --- a/web/Areas/Effort/Constants/EffortConstants.cs +++ b/web/Areas/Effort/Constants/EffortConstants.cs @@ -19,7 +19,11 @@ public static class EffortConstants /// public static readonly FrozenDictionary DepartmentOverrides = new Dictionary { - ["02493928"] = "VSR" + // Michael Mison — only has a VMTH job, but effort is recorded to VSR. + ["02493928"] = "VSR", + // Christine Johnson — no academic-department job in AAUD (home/effort dept 072016), + // so the dept cannot be auto-derived; effort is recorded to VME. + ["00129082"] = "VME" }.ToFrozenDictionary(); /// diff --git a/web/Areas/Effort/EffortSettings.cs b/web/Areas/Effort/EffortSettings.cs index 82529a5da..873a60a66 100644 --- a/web/Areas/Effort/EffortSettings.cs +++ b/web/Areas/Effort/EffortSettings.cs @@ -14,4 +14,11 @@ public class EffortSettings /// Number of days instructors have to respond to verification emails. /// public int VerificationReplyDays { get; set; } = 7; + + /// + /// When true, the harvest and on-demand effort entry auto-create the generic + /// "RES 000R-001" resident-teaching course (CRN "RESID") and its effort records. + /// Defaults to false — the placeholder course is not currently in use. + /// + public bool AutoCreateGenericRCourse { get; set; } } diff --git a/web/Areas/Effort/Services/ClinicalImportService.cs b/web/Areas/Effort/Services/ClinicalImportService.cs index fb180b653..a4dc4b301 100644 --- a/web/Areas/Effort/Services/ClinicalImportService.cs +++ b/web/Areas/Effort/Services/ClinicalImportService.cs @@ -69,6 +69,11 @@ public class ClinicalImportService : IClinicalImportService /// private Dictionary? _jobGroupLookup; + /// + /// Lazily loaded set of emeritus/recall title codes to exclude from import. + /// + private HashSet? _excludedTitleCodes; + public ClinicalImportService( EffortDbContext context, VIPERContext viperContext, @@ -117,10 +122,14 @@ public async Task ValidateImportableInstructorsAsync( _jobGroupLookup ??= titleCodes.ToDictionary(t => t.Code, t => t.JobGroupId, StringComparer.OrdinalIgnoreCase); } - // Build lookup of MothraId → titleCode for those with valid title codes + HashSet excludedTitleCodes = _excludedTitleCodes ??= await _instructorService.GetExcludedTitleCodesAsync(ct); + + // Build lookup of MothraId → titleCode for those with valid, non-excluded title codes + // (emeritus/recall appointments are excluded from harvest). var titleCodeByMothraId = aaudImportInfo .Where(a => !string.IsNullOrEmpty(a.TitleCode?.Trim()) - && _titleLookup.ContainsKey(a.TitleCode!.Trim())) + && _titleLookup.ContainsKey(a.TitleCode!.Trim()) + && !excludedTitleCodes.Contains(a.TitleCode!.Trim())) .GroupBy(a => a.MothraId, StringComparer.OrdinalIgnoreCase) .ToDictionary(g => g.Key, g => g.First().TitleCode!.Trim(), StringComparer.OrdinalIgnoreCase); @@ -250,7 +259,7 @@ public async Task ExecuteImportAsync(int termCode, Clin var auditDetails = $"Clinical import ({mode}): {result.RecordsAdded} added, {result.RecordsUpdated} updated, {result.RecordsDeleted} deleted, {result.RecordsSkipped} skipped"; if (result.SkippedInstructors.Count > 0) { - auditDetails += $". Skipped instructors (no AAUD record or invalid title): {string.Join(", ", result.SkippedInstructors)}"; + auditDetails += $". Skipped instructors (no AAUD record, invalid title, or excluded appointment): {string.Join(", ", result.SkippedInstructors)}"; } _auditService.AddImportAudit(termCode, EffortAuditActions.ImportClinical, auditDetails); @@ -326,7 +335,7 @@ public async Task ExecuteImportWithProgressAsync( var auditDetails = $"Clinical import ({mode}): {result.RecordsAdded} added, {result.RecordsUpdated} updated, {result.RecordsDeleted} deleted, {result.RecordsSkipped} skipped"; if (result.SkippedInstructors.Count > 0) { - auditDetails += $". Skipped instructors (no AAUD record or invalid title): {string.Join(", ", result.SkippedInstructors)}"; + auditDetails += $". Skipped instructors (no AAUD record, invalid title, or excluded appointment): {string.Join(", ", result.SkippedInstructors)}"; } _auditService.AddImportAudit(termCode, EffortAuditActions.ImportClinical, auditDetails); @@ -1092,10 +1101,12 @@ private async Task EnsureEffortPersonAsync( _jobGroupLookup ??= titleCodes.ToDictionary(t => t.Code, t => t.JobGroupId, StringComparer.OrdinalIgnoreCase); } - if (string.IsNullOrEmpty(titleCode) || !_titleLookup.ContainsKey(titleCode)) + var excludedTitleCodes = _excludedTitleCodes ??= await _instructorService.GetExcludedTitleCodesAsync(ct); + + if (string.IsNullOrEmpty(titleCode) || !_titleLookup.ContainsKey(titleCode) || excludedTitleCodes.Contains(titleCode)) { _logger.LogWarning( - "Skipping clinical instructor {MothraId}: no AAUD record or invalid title code '{TitleCode}'", + "Skipping clinical instructor {MothraId}: no AAUD record, invalid title, or excluded (emeritus/recall) title code '{TitleCode}'", mothraId, titleCode); _processedMothraIds[mothraId] = false; return false; diff --git a/web/Areas/Effort/Services/EffortRecordService.cs b/web/Areas/Effort/Services/EffortRecordService.cs index f8ad6b845..142b03e9b 100644 --- a/web/Areas/Effort/Services/EffortRecordService.cs +++ b/web/Areas/Effort/Services/EffortRecordService.cs @@ -1,4 +1,5 @@ using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; using Viper.Areas.Effort.Constants; using Viper.Areas.Effort.Exceptions; using Viper.Areas.Effort.Models.DTOs; @@ -22,6 +23,7 @@ public class EffortRecordService : IEffortRecordService private readonly ICourseClassificationService _courseClassificationService; private readonly IRCourseService _rCourseService; private readonly IUserHelper _userHelper; + private readonly EffortSettings _settings; private readonly ILogger _logger; public EffortRecordService( @@ -32,6 +34,7 @@ public EffortRecordService( ICourseClassificationService courseClassificationService, IRCourseService rCourseService, IUserHelper userHelper, + IOptions settings, ILogger logger) { _context = context; @@ -41,6 +44,7 @@ public EffortRecordService( _courseClassificationService = courseClassificationService; _rCourseService = rCourseService; _userHelper = userHelper; + _settings = settings.Value; _logger = logger; } @@ -687,6 +691,11 @@ private void ValidateEffortTypeForCourse(EffortType effortType, EffortCourse cou /// private async Task TryCreateRCourseForInstructorAsync(int personId, int termCode, int modifiedBy, CancellationToken ct) { + if (!_settings.AutoCreateGenericRCourse) + { + return; + } + // Count how many non-R-course effort records this instructor has in this term var nonRCourseCount = await _context.Records .AsNoTracking() diff --git a/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs b/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs index a839c815b..f9279c21b 100644 --- a/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs +++ b/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs @@ -117,6 +117,7 @@ public override async Task GeneratePreviewAsync(HarvestContext context, Cancella .ToDictionary(g => g.Key, g => g.First().JobGroupId, StringComparer.OrdinalIgnoreCase); context.DeptSimpleNameLookup ??= await context.InstructorService.GetDepartmentSimpleNameLookupAsync(ct); + var excludedTitleCodes = context.ExcludedTitleCodes ??= await context.InstructorService.GetExcludedTitleCodesAsync(ct); // Get VIPER person IDs for the instructors var mothraIds = crestInstructors.Select(i => i.MothraId).Distinct().ToList(); @@ -126,6 +127,8 @@ public override async Task GeneratePreviewAsync(HarvestContext context, Cancella .Select(p => new { p.PersonId, p.MothraId }) .ToDictionaryAsync(p => p.MothraId ?? "", p => p.PersonId, ct); + var excludedInstructors = new List(); + foreach (var instructor in crestInstructors) { if (!personDetails.TryGetValue(instructor.MothraId, out var personId)) @@ -137,6 +140,16 @@ public override async Task GeneratePreviewAsync(HarvestContext context, Cancella } var titleDesc = context.TitleLookup.TryGetValue(instructor.TitleCode, out var desc) ? desc : instructor.TitleCode; + + // Exclude emeritus/recall appointments from harvest. Their effort records are + // skipped automatically because BuildCrestEffortRecordsAsync only emits records + // for instructors present in CrestInstructors. + if (excludedTitleCodes.Contains(instructor.TitleCode)) + { + excludedInstructors.Add($"{instructor.LastName}, {instructor.FirstName} ({titleDesc})"); + continue; + } + // HomeDept is already fully resolved by BatchResolveDepartmentsAsync var dept = instructor.HomeDept; @@ -154,6 +167,8 @@ public override async Task GeneratePreviewAsync(HarvestContext context, Cancella }); } + AddEmeritusExclusionWarning(context, PhaseName, excludedInstructors); + // Step 5: Build effort records await BuildCrestEffortRecordsAsync(context, courseOfferings, blockCourseIds, ct); @@ -405,6 +420,12 @@ private async Task BuildCrestEffortRecordsAsync( // Build effort records var effortByPersonCourse = new Dictionary(); + // Index instructors by MothraId once; effort records are only created for persons + // present in CrestInstructors (valid AAUD data, title code, and VIPER person record). + var instructorByMothraId = context.Preview.CrestInstructors + .GroupBy(i => i.MothraId, StringComparer.OrdinalIgnoreCase) + .ToDictionary(g => g.Key, g => g.First(), StringComparer.OrdinalIgnoreCase); + var offeringAssignments = courseOfferings .Join( offerPersons, @@ -422,10 +443,7 @@ private async Task BuildCrestEffortRecordsAsync( continue; } - // Only create effort records for persons who are in CrestInstructors - // (those with valid AAUD data, title code, and VIPER person record) - var instructor = context.Preview.CrestInstructors.FirstOrDefault(i => i.MothraId == mothraId); - if (instructor == null) continue; + if (!instructorByMothraId.TryGetValue(mothraId, out var instructor)) continue; var personName = instructor.FullName; diff --git a/web/Areas/Effort/Services/Harvest/HarvestContext.cs b/web/Areas/Effort/Services/Harvest/HarvestContext.cs index 046d27ee2..c26518f32 100644 --- a/web/Areas/Effort/Services/Harvest/HarvestContext.cs +++ b/web/Areas/Effort/Services/Harvest/HarvestContext.cs @@ -54,6 +54,11 @@ public sealed class HarvestContext /// public Dictionary? DeptSimpleNameLookup { get; set; } + /// + /// Effort title codes to exclude from harvest (emeritus/recall appointments). + /// + public HashSet? ExcludedTitleCodes { get; set; } + /// /// Warnings accumulated during harvest. /// diff --git a/web/Areas/Effort/Services/Harvest/HarvestPhaseBase.cs b/web/Areas/Effort/Services/Harvest/HarvestPhaseBase.cs index e56e04f0d..68d7930c5 100644 --- a/web/Areas/Effort/Services/Harvest/HarvestPhaseBase.cs +++ b/web/Areas/Effort/Services/Harvest/HarvestPhaseBase.cs @@ -285,6 +285,36 @@ protected static bool IsSemesterTerm(int termCode) #endregion + #region Exclusion Helpers + + /// + /// Record a single warning listing instructors excluded from harvest because their + /// effort title is an emeritus/recall appointment. Added to both the preview warnings + /// (shown before commit) and the execution warnings (shown in the result). + /// + protected static void AddEmeritusExclusionWarning( + HarvestContext ctx, + string phaseName, + IReadOnlyCollection excludedNames) + { + if (excludedNames.Count == 0) + { + return; + } + + var warning = new HarvestWarning + { + Phase = phaseName, + Message = $"{excludedNames.Count} emeritus/recall instructor(s) excluded from harvest", + Details = string.Join("; ", excludedNames) + }; + + ctx.Warnings.Add(warning); + ctx.Preview.Warnings.Add(warning); + } + + #endregion + #region Department Resolution /// diff --git a/web/Areas/Effort/Services/Harvest/NonCrestHarvestPhase.cs b/web/Areas/Effort/Services/Harvest/NonCrestHarvestPhase.cs index a9d86ea22..affa82fa4 100644 --- a/web/Areas/Effort/Services/Harvest/NonCrestHarvestPhase.cs +++ b/web/Areas/Effort/Services/Harvest/NonCrestHarvestPhase.cs @@ -265,6 +265,7 @@ private async Task BuildNonCrestInstructorsAndEffortAsync( // Batch-resolve departments using full resolution chain (jobs → employee fields → fallback) var batchDepts = await context.InstructorService.BatchResolveDepartmentsAsync(nonCrestMothraIds, context.TermCode, ct); + var excludedTitleCodes = context.ExcludedTitleCodes ??= await context.InstructorService.GetExcludedTitleCodesAsync(ct); // CRN-keyed lookup for SubjCode/CrseNumb resolution from POA rows (POA has no unit info). // Variable-unit CRNs produce multiple allNonCrestCourses rows; take the first since @@ -274,6 +275,10 @@ private async Task BuildNonCrestInstructorsAndEffortAsync( .ToDictionary(g => g.Key, g => g.First()); // Create instructor previews and effort records + var excludedMothraIds = new HashSet(StringComparer.OrdinalIgnoreCase); + var excludedInstructors = new List(); + var addedInstructorMothraIds = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var poa in poaEntries) { var pidmStr = poa.PoaPidm; @@ -298,8 +303,19 @@ private async Task BuildNonCrestInstructorsAndEffortAsync( var titleDesc = context.TitleLookup.TryGetValue(titleCode, out var desc) ? desc : titleCode; + // Exclude emeritus/recall appointments from harvest (skips both the instructor + // and the effort record built below). + if (excludedTitleCodes.Contains(titleCode)) + { + if (excludedMothraIds.Add(mothraId)) + { + excludedInstructors.Add($"{fullName} ({titleDesc})"); + } + continue; + } + // Add instructor if not already added - skip if no valid VIPER person record - if (!context.Preview.NonCrestInstructors.Any(i => i.MothraId == mothraId)) + if (!addedInstructorMothraIds.Contains(mothraId)) { if (!viperPersonLookup.TryGetValue(mothraId, out var personId)) { @@ -309,6 +325,7 @@ private async Task BuildNonCrestInstructorsAndEffortAsync( continue; } + addedInstructorMothraIds.Add(mothraId); context.Preview.NonCrestInstructors.Add(new HarvestPersonPreview { MothraId = mothraId, @@ -342,9 +359,56 @@ private async Task BuildNonCrestInstructorsAndEffortAsync( }); } + AddEmeritusExclusionWarning(context, PhaseName, excludedInstructors); + + // Restore legacy IOR custodial-dept fallback (legacy Import.cfm Phase 2): when a + // non-CREST course's own Banner dept doesn't resolve to an academic department, + // inherit the custodial dept from its Director/IOR when that instructor's resolved + // department is academic. Without this, such courses harvest as "UNK". + ApplyDirectorCustodialDeptFallback( + context.Preview.NonCrestCourses, context.Preview.NonCrestEffort, batchDepts); + // Sort instructors var sortedInstructors = context.Preview.NonCrestInstructors.OrderBy(i => i.FullName).ToList(); context.Preview.NonCrestInstructors.Clear(); context.Preview.NonCrestInstructors.AddRange(sortedInstructors); } + + /// + /// For non-CREST courses whose custodial dept is not one of the academic departments, + /// overwrite it with the Director/IOR's resolved department when that is academic. + /// Mirrors legacy Import.cfm Phase 2 ("look to the dept of the IOR"). Applies to all + /// unit variants of a CRN. + /// + internal static void ApplyDirectorCustodialDeptFallback( + List courses, + List effort, + Dictionary batchDepts) + { + var academicDepts = EffortConstants.AcademicDepartments.ToHashSet(StringComparer.OrdinalIgnoreCase); + + // Map each CRN to its Director's department, keeping only academic departments. + var directorDeptByCrn = effort + .Where(e => e.RoleId == EffortConstants.DirectorRoleId && !string.IsNullOrWhiteSpace(e.Crn)) + .Select(e => new { e.Crn, Dept = batchDepts.GetValueOrDefault(e.MothraId, "UNK") }) + .Where(x => academicDepts.Contains(x.Dept)) + .GroupBy(x => x.Crn, StringComparer.OrdinalIgnoreCase) + // OrderBy makes the choice deterministic when a CRN has multiple academic IOR depts. + .ToDictionary(g => g.Key, g => g.OrderBy(x => x.Dept).First().Dept, StringComparer.OrdinalIgnoreCase); + + if (directorDeptByCrn.Count == 0) + { + return; + } + + foreach (var course in courses + .Where(c => c.Source == EffortConstants.SourceNonCrest && !academicDepts.Contains(c.CustDept))) + { + if (!string.IsNullOrWhiteSpace(course.Crn) && + directorDeptByCrn.TryGetValue(course.Crn, out var iorDept)) + { + course.CustDept = iorDept; + } + } + } } diff --git a/web/Areas/Effort/Services/HarvestService.cs b/web/Areas/Effort/Services/HarvestService.cs index 9470d4d24..474841072 100644 --- a/web/Areas/Effort/Services/HarvestService.cs +++ b/web/Areas/Effort/Services/HarvestService.cs @@ -1,5 +1,6 @@ using System.Threading.Channels; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; using Viper.Areas.Effort.Constants; using Viper.Areas.Effort.Models.DTOs.Responses; using Viper.Areas.Effort.Services.Harvest; @@ -25,6 +26,7 @@ public class HarvestService : IHarvestService private readonly IInstructorService _instructorService; private readonly IRCourseService _rCourseService; private readonly IClinicalImportService _clinicalImportService; + private readonly EffortSettings _settings; private readonly ILogger _logger; public HarvestService( @@ -40,6 +42,7 @@ public HarvestService( IInstructorService instructorService, IRCourseService rCourseService, IClinicalImportService clinicalImportService, + IOptions settings, ILogger logger) { _phases = phases; @@ -54,6 +57,7 @@ public HarvestService( _instructorService = instructorService; _rCourseService = rCourseService; _clinicalImportService = clinicalImportService; + _settings = settings.Value; _logger = logger; } @@ -347,6 +351,14 @@ await progressChannel.WriteAsync(new HarvestProgressEvent /// private async Task GenerateRCoursesForEligibleInstructorsAsync(int termCode, int modifiedBy, CancellationToken ct) { + if (!_settings.AutoCreateGenericRCourse) + { + _logger.LogInformation( + "Skipping generic R-course generation for term {TermCode} (AutoCreateGenericRCourse disabled)", + termCode); + return; + } + // Find all instructors in the term who have at least one non-R-course effort record var eligibleInstructors = await _context.Records .AsNoTracking() diff --git a/web/Areas/Effort/Services/IInstructorService.cs b/web/Areas/Effort/Services/IInstructorService.cs index 4795b5b97..7e20d0fe3 100644 --- a/web/Areas/Effort/Services/IInstructorService.cs +++ b/web/Areas/Effort/Services/IInstructorService.cs @@ -167,6 +167,14 @@ public interface IInstructorService /// Dictionary mapping raw codes to simple names, or null if unavailable. Task?> GetDepartmentSimpleNameLookupAsync(CancellationToken ct = default); + /// + /// Get a cached set of effort title codes (dvtTitle_Code values) whose title name denotes + /// an emeritus or recall appointment, to be excluded from harvest. Empty set if unavailable. + /// + /// Cancellation token. + /// Set of title codes whose names match "EMERITUS" or "RECALL" (e.g. the codes for "PROF EMERITUS" or "RECALL FACULTY"). + Task> GetExcludedTitleCodesAsync(CancellationToken ct = default); + /// /// Resolve departments for multiple instructors in a single batch. /// Uses the full resolution chain (override → jobs → employee fields → fallback). diff --git a/web/Areas/Effort/Services/InstructorService.cs b/web/Areas/Effort/Services/InstructorService.cs index 6aabe4923..301c41333 100644 --- a/web/Areas/Effort/Services/InstructorService.cs +++ b/web/Areas/Effort/Services/InstructorService.cs @@ -29,6 +29,7 @@ public class InstructorService : IInstructorService private const string TitleCacheKey = "effort_title_lookup"; private const string DeptSimpleNameCacheKey = "effort_dept_simple_name_lookup"; + private const string ExcludedTitleCodesCacheKey = "effort_excluded_title_codes"; /// /// Valid academic department codes. @@ -1239,6 +1240,56 @@ private async Task EnrichWithTitlesAsync(List instructors, Cancellati } } + public async Task> GetExcludedTitleCodesAsync(CancellationToken ct = default) + { + if (_cache.TryGetValue>(ExcludedTitleCodesCacheKey, out var cached) && cached != null) + { + // Return a copy so callers cannot mutate the shared cached instance. + return new HashSet(cached, StringComparer.OrdinalIgnoreCase); + } + + var excluded = new HashSet(StringComparer.OrdinalIgnoreCase); + + try + { + // Emeritus and recall appointments are excluded from harvest. Match by title name + // in dictionary.dbo.dvtTitle (e.g., "PROF EMERITUS(WOS)", "RECALL FACULTY"). + // ToLower() makes the match case-insensitive: it translates to SQL LOWER(...) LIKE + // and also evaluates correctly under the in-memory provider used in tests. + var codes = await _dictionaryContext.Titles + .AsNoTracking() + .Where(t => t.Code != null && t.Name != null && + (t.Name.ToLower().Contains("emerit") || t.Name.ToLower().Contains("recall"))) + .Select(t => t.Code!.Trim()) + .ToListAsync(ct); + + foreach (var code in codes.Where(c => !string.IsNullOrEmpty(c))) + { + excluded.Add(code); + } + + var cacheOptions = new MemoryCacheEntryOptions() + .SetSlidingExpiration(TimeSpan.FromHours(24)); + + _cache.Set(ExcludedTitleCodesCacheKey, excluded, cacheOptions); + + _logger.LogInformation("Loaded {Count} excluded (emeritus/recall) title codes from dictionary database", excluded.Count); + } + catch (InvalidOperationException ex) + { + // Fail open: if the lookup is unavailable, harvest proceeds without exclusions + // rather than failing outright. + _logger.LogWarning(ex, "Failed to load excluded title codes from dictionary database"); + } + catch (DbException ex) + { + _logger.LogWarning(ex, "Failed to load excluded title codes from dictionary database"); + } + + // Return a copy so the cached instance is never exposed to callers. + return new HashSet(excluded, StringComparer.OrdinalIgnoreCase); + } + public async Task> GetInstructorEffortRecordsAsync( int personId, int termCode, CancellationToken ct = default) {