Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions test/Effort/ClinicalImportServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public ClinicalImportServiceTests()
.BatchResolveDepartmentsAsync(Arg.Any<List<string>>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
.Returns(ci => ci.Arg<List<string>>().ToDictionary(id => id, _ => "VME"));

_instructorServiceMock
.GetExcludedTitleCodesAsync(Arg.Any<CancellationToken>())
.Returns(new HashSet<string>(StringComparer.OrdinalIgnoreCase));

_service = new ClinicalImportService(
_context,
_viperContext,
Expand Down
51 changes: 51 additions & 0 deletions test/Effort/EffortRecordServiceTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -71,6 +72,7 @@ public EffortRecordServiceTests()
courseClassificationService,
_rCourseServiceMock,
_userHelperMock,
Options.Create(new EffortSettings { AutoCreateGenericRCourse = true }),
_loggerMock);

SeedTestData();
Expand Down Expand Up @@ -1490,6 +1492,55 @@ await _rCourseServiceMock.Received(1).CreateRCourseEffortRecordAsync(
Arg.Any<CancellationToken>());
}

[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<int>(),
Arg.Any<int>(),
Arg.Any<int>(),
Arg.Any<RCourseCreationContext>(),
Arg.Any<CancellationToken>());
}

[Fact]
public async Task CreateEffortRecordAsync_DoesNotCallRCourseService_WhenSecondNonRCourseAdded()
{
Expand Down
89 changes: 89 additions & 0 deletions test/Effort/HarvestServiceTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -34,6 +36,11 @@ public sealed class HarvestServiceTests : IDisposable
private readonly ILogger<HarvestService> _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<EffortSettings> RCourseEnabledSettings =
Options.Create(new EffortSettings { AutoCreateGenericRCourse = true });

private const int TestTermCode = 202410;

public HarvestServiceTests()
Expand Down Expand Up @@ -93,6 +100,8 @@ public HarvestServiceTests()
.GetTitleCodesAsync(Arg.Any<CancellationToken>()).Returns(new List<TitleCodeDto>());
_instructorServiceMock
.GetDepartmentSimpleNameLookupAsync(Arg.Any<CancellationToken>()).Returns(new Dictionary<string, string>());
_instructorServiceMock
.GetExcludedTitleCodesAsync(Arg.Any<CancellationToken>()).Returns(new HashSet<string>(StringComparer.OrdinalIgnoreCase));
// Setup audit service mock for harvest operations
_auditServiceMock
.ClearAuditForTermAsync(Arg.Any<int>(), Arg.Any<CancellationToken>()).Returns(Task.CompletedTask);
Expand Down Expand Up @@ -123,6 +132,7 @@ public HarvestServiceTests()
_instructorServiceMock,
_rCourseServiceMock,
_clinicalImportServiceMock,
RCourseEnabledSettings,
_loggerMock);
}

Expand Down Expand Up @@ -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)
Expand All @@ -762,6 +773,83 @@ await _rCourseServiceMock.Received(1).CreateRCourseEffortRecordAsync(
Arg.Any<CancellationToken>());
}

[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<IHarvestPhase> { 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<int>(),
Arg.Any<int>(),
Arg.Any<int>(),
Arg.Any<RCourseCreationContext>(),
Arg.Any<CancellationToken>());
}

[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<HarvestCoursePreview>
{
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<HarvestRecordPreview>
{
new() { Crn = "10001", MothraId = "AAA", RoleId = EffortConstants.DirectorRoleId },
new() { Crn = "10003", MothraId = "CCC", RoleId = EffortConstants.DirectorRoleId },
};
var batchDepts = new Dictionary<string, string>(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
}

/// <summary>
/// Creates a HarvestService that uses a single custom phase for testing.
/// </summary>
Expand All @@ -780,6 +868,7 @@ private HarvestService CreateServiceWithPhase(IHarvestPhase phase)
_instructorServiceMock,
_rCourseServiceMock,
_clinicalImportServiceMock,
RCourseEnabledSettings,
_loggerMock);
}

Expand Down
69 changes: 69 additions & 0 deletions test/Effort/InstructorServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
6 changes: 5 additions & 1 deletion web/Areas/Effort/Constants/EffortConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ public static class EffortConstants
/// </summary>
public static readonly FrozenDictionary<string, string> DepartmentOverrides = new Dictionary<string, string>
{
["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();

/// <summary>
Expand Down
7 changes: 7 additions & 0 deletions web/Areas/Effort/EffortSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,11 @@ public class EffortSettings
/// Number of days instructors have to respond to verification emails.
/// </summary>
public int VerificationReplyDays { get; set; } = 7;

/// <summary>
/// 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.
/// </summary>
public bool AutoCreateGenericRCourse { get; set; }
}
23 changes: 17 additions & 6 deletions web/Areas/Effort/Services/ClinicalImportService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public class ClinicalImportService : IClinicalImportService
/// </summary>
private Dictionary<string, string?>? _jobGroupLookup;

/// <summary>
/// Lazily loaded set of emeritus/recall title codes to exclude from import.
/// </summary>
private HashSet<string>? _excludedTitleCodes;

public ClinicalImportService(
EffortDbContext context,
VIPERContext viperContext,
Expand Down Expand Up @@ -117,10 +122,14 @@ public async Task<ImportValidationResult> ValidateImportableInstructorsAsync(
_jobGroupLookup ??= titleCodes.ToDictionary(t => t.Code, t => t.JobGroupId, StringComparer.OrdinalIgnoreCase);
}

// Build lookup of MothraId → titleCode for those with valid title codes
HashSet<string> 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);

Expand Down Expand Up @@ -250,7 +259,7 @@ public async Task<ClinicalImportResultDto> 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);

Expand Down Expand Up @@ -326,7 +335,7 @@ public async Task<ClinicalImportResultDto> 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);

Expand Down Expand Up @@ -1092,10 +1101,12 @@ private async Task<bool> 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);
Comment thread
rlorenzo marked this conversation as resolved.
_processedMothraIds[mothraId] = false;
Comment thread
rlorenzo marked this conversation as resolved.
return false;
Expand Down
Loading
Loading