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
2 changes: 1 addition & 1 deletion test/Areas/Directory/VMACSServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void Deserialize_ReturnsItem_WhenPayloadMatches()

Assert.NotNull(query);
Assert.NotNull(query.item);
Assert.Equal("Doe, John", query.item!.Name?.Single());
Assert.Equal("Doe, John", query.item.Name?.Single());
}

[Fact]
Expand Down
8 changes: 4 additions & 4 deletions test/CMS/CMSContentControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public async Task GetContentBlock_ReturnsBlock_WhenFound()
var result = await _controller.GetContentBlock(5, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(5, result.Value!.ContentBlockId);
Assert.Equal(5, result.Value.ContentBlockId);
}

[Fact]
Expand Down Expand Up @@ -273,7 +273,7 @@ public async Task CreateContentBlock_ReturnsBlock_OnSuccess()
var result = await _controller.CreateContentBlock(request, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(7, result.Value!.ContentBlockId);
Assert.Equal(7, result.Value.ContentBlockId);
}

[Fact]
Expand All @@ -286,7 +286,7 @@ public async Task CreateContentBlock_ReturnsValidationProblem_OnArgumentExceptio
var result = await _controller.CreateContentBlock(request, TestContext.Current.CancellationToken);

Assert.IsType<ObjectResult>(result.Result);
var problem = (ObjectResult)result.Result!;
var problem = (ObjectResult)result.Result;
Assert.IsType<ValidationProblemDetails>(problem.Value);
}

Expand All @@ -313,7 +313,7 @@ public async Task UpdateContentBlock_ReturnsBlock_OnSuccess()
var result = await _controller.UpdateContentBlock(3, request, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(3, result.Value!.ContentBlockId);
Assert.Equal(3, result.Value.ContentBlockId);
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion test/CMS/CMSFilesControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public async Task GetFile_ReturnsFile_WhenFound()
var result = await _controller.GetFile(guid, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(guid, result.Value!.FileGuid);
Assert.Equal(guid, result.Value.FileGuid);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions test/CMS/CMSLeftNavControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public async Task GetMenu_ReturnsMenu_WhenFound()
var result = await _controller.GetMenu(5, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(5, result.Value!.LeftNavMenuId);
Assert.Equal(5, result.Value.LeftNavMenuId);
}

[Fact]
Expand Down Expand Up @@ -96,7 +96,7 @@ public async Task UpdateMenu_ReturnsMenu_OnSuccess()
var result = await _controller.UpdateMenu(5, request, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Equal(5, result.Value!.LeftNavMenuId);
Assert.Equal(5, result.Value.LeftNavMenuId);
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion test/CMS/CMSOptionsControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public async Task SearchPeople_ReturnsEmpty_WhenSearchBelowMinimumLength(string?
var result = await _controller.SearchPeople(search!, TestContext.Current.CancellationToken);

Assert.NotNull(result.Value);
Assert.Empty(result.Value!);
Assert.Empty(result.Value);
}

[Fact]
Expand Down
8 changes: 4 additions & 4 deletions test/ClinicalScheduler/PermissionsControllerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ public async Task GetUserPermissions_WithValidUser_ReturnsOkWithPermissions()
var okResult = Assert.IsType<OkObjectResult>(result.Result);
dynamic? response = okResult.Value;
Assert.Equal(TestUserMothraId, response!.user.mothraId);
Assert.Equal(TestUserDisplayName, response!.user.displayName);
Assert.False(response!.permissions.hasManagePermission);
Assert.Equal(1, response!.permissions.editableServiceCount);
Assert.Single(response!.editableServices);
Assert.Equal(TestUserDisplayName, response.user.displayName);
Assert.False(response.permissions.hasManagePermission);
Assert.Equal(1, response.permissions.editableServiceCount);
Assert.Single(response.editableServices);
}

[Fact]
Expand Down
3 changes: 1 addition & 2 deletions test/Effort/HarvestTimeParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ public void ParseTimeString_ValidTimeFormats_ParsesCorrectly(string input, int e
{
var result = HarvestTimeParser.ParseTimeString(input);

Assert.NotNull(result);
var value = result!.Value;
var value = Assert.IsType<TimeSpan>(result);
Assert.Equal(expectedHour, value.Hours);
Assert.Equal(expectedMinute, value.Minutes);
}
Expand Down
2 changes: 1 addition & 1 deletion test/HealthChecks/HealthCheckCollectorTokenHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public async Task SendAsync_StampsTokenHeader()

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.NotNull(recorder.LastRequest);
var values = recorder.LastRequest!.Headers.GetValues(HealthCheckCollectorAuth.HeaderName).ToList();
var values = recorder.LastRequest.Headers.GetValues(HealthCheckCollectorAuth.HeaderName).ToList();
Assert.Single(values);
Assert.Equal(HealthCheckCollectorAuth.Token, values[0]);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Scheduler/ScheduledJobRunnerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public async Task RunAsync_ResolvesJobByIdAndStampsSchedulerActor()

Assert.True(capturing.Ran);
Assert.NotNull(capturing.CapturedContext);
Assert.Equal(ScheduledJobContext.SchedulerActor, capturing.CapturedContext!.ModBy);
Assert.Equal(ScheduledJobContext.SchedulerActor, capturing.CapturedContext.ModBy);
}

[Fact]
Expand Down
6 changes: 3 additions & 3 deletions test/Students/EmergencyContactControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public async Task GetStudentContactDetail_StudentCannotViewOtherRecord_ReturnsFo
var result = await _controller.GetStudentContactDetail(999);

Assert.IsType<ObjectResult>(result.Result);
var objectResult = (ObjectResult)result.Result!;
var objectResult = (ObjectResult)result.Result;
Assert.Equal(403, objectResult.StatusCode);
}

Expand Down Expand Up @@ -662,7 +662,7 @@ public void GetStudentContactDetail_HasAuthorizeAttributeButNoPermissionGate()
var method = typeof(EmergencyContactController).GetMethod(nameof(EmergencyContactController.GetStudentContactDetail));
Assert.NotNull(method);

var permissionAttrs = method!.GetCustomAttributes(typeof(PermissionAttribute), false);
var permissionAttrs = method.GetCustomAttributes(typeof(PermissionAttribute), false);
Assert.Empty(permissionAttrs);

var authorizeAttrs = method.GetCustomAttributes(typeof(AuthorizeAttribute), false);
Expand All @@ -676,7 +676,7 @@ public void CanEdit_HasAuthorizeAttributeButNoPermissionGate()
var method = typeof(EmergencyContactController).GetMethod(nameof(EmergencyContactController.CanEdit));
Assert.NotNull(method);

var permissionAttrs = method!.GetCustomAttributes(typeof(PermissionAttribute), false);
var permissionAttrs = method.GetCustomAttributes(typeof(PermissionAttribute), false);
Assert.Empty(permissionAttrs);

var authorizeAttrs = method.GetCustomAttributes(typeof(AuthorizeAttribute), false);
Expand Down
2 changes: 1 addition & 1 deletion test/Students/EmergencyContactServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1387,7 +1387,7 @@ public async Task GetStudentContactReportAsync_IncludesContactDetails()
Assert.Equal("95616", student.Zip);
Assert.True(student.ContactPermanent);
Assert.NotNull(student.EmergencyContact);
Assert.Equal("Jane Doe", student.EmergencyContact!.Name);
Assert.Equal("Jane Doe", student.EmergencyContact.Name);
}
}

Expand Down
16 changes: 14 additions & 2 deletions web/Areas/CMS/Services/CmsFilePathSafety.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ public static class CmsFilePathSafety
"LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"
};

private static readonly Regex DisallowedFileNameChars = new(@"[^a-zA-Z0-9._\- ]", RegexOptions.Compiled);
// Bounds matching so a pathological filename cannot pin a request thread.
private static readonly Regex DisallowedFileNameChars =
new(@"[^a-zA-Z0-9._\- ]", RegexOptions.Compiled, TimeSpan.FromSeconds(1));

/// <summary>
/// Returns a filename safe to use in a Content-Disposition response header.
Expand All @@ -46,7 +48,17 @@ public static string SanitizeDownloadName(string? userInput)
}

var fileNamePart = StripPathComponents(userInput);
var filtered = DisallowedFileNameChars.Replace(fileNamePart, string.Empty).Trim();

string filtered;
try
{
filtered = DisallowedFileNameChars.Replace(fileNamePart, string.Empty).Trim();
}
catch (RegexMatchTimeoutException)
{
// Fail safe: an unfiltered name must never reach the response header.
return DefaultDownloadName;
}

// Reject names that collapse to only dots/spaces: ".", ".." etc. would
// become "..zip" after the suffix step, which is traversal-shaped.
Expand Down
4 changes: 2 additions & 2 deletions web/Areas/CTS/Controllers/BundleCompetencyController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ public BundleCompetencyController(VIPERContext context)

private bool BundleExists(int bundleId)
{
return context.Bundles.Where(b => b.BundleId == bundleId).Any();
return context.Bundles.AsNoTracking().Any(b => b.BundleId == bundleId);
}

private bool CompetencyExists(int competencyId)
{
return context.Competencies.Where(b => b.CompetencyId == competencyId).Any();
return context.Competencies.AsNoTracking().Any(b => b.CompetencyId == competencyId);
}

[HttpGet]
Expand Down
8 changes: 4 additions & 4 deletions web/Areas/CTS/Controllers/BundleCompetencyGroupController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ public BundleCompetencyGroupController(VIPERContext context)

private bool BundleExists(int bundleId)
{
return context.Bundles.Where(b => b.BundleId == bundleId).Any();
return context.Bundles.AsNoTracking().Any(b => b.BundleId == bundleId);
}

private bool SameNameExists(int bundleId, string name, int? bundleCompetencyGroupId = null)
{
return context.BundleCompetencyGroups
.Where(g => g.BundleId == bundleId
.AsNoTracking()
.Any(g => g.BundleId == bundleId
&& g.Name == name
&& (bundleCompetencyGroupId == null || bundleCompetencyGroupId != g.BundleCompetencyGroupId))
.Any();
&& (bundleCompetencyGroupId == null || bundleCompetencyGroupId != g.BundleCompetencyGroupId));
}

[HttpGet]
Expand Down
2 changes: 1 addition & 1 deletion web/Areas/CTS/Models/CompetencyBundleAssociationDto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public CompetencyBundleAssociationDto(Competency c)
.Where(bc => bc.Bundle != null)
.Select(bc => new BundleInfoDto
{
BundleId = bc.Bundle!.BundleId,
BundleId = bc.Bundle.BundleId,
Name = bc.Bundle.Name,
Clinical = bc.Bundle.Clinical,
Assessment = bc.Bundle.Assessment,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status403Forbidden)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
public async Task<IActionResult> GetClinicianSchedule(string mothraId, [FromQuery] int? year = null)

Check warning on line 126 in web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs

View workflow job for this annotation

GitHub Actions / Backend Tests

'GetClinicianSchedule' has a cyclomatic complexity of '29'. Rewrite or refactor the code to decrease its complexity below '26'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1502)
{
var correlationId = HttpContext.TraceIdentifier ?? Guid.NewGuid().ToString();

Expand Down Expand Up @@ -289,7 +289,7 @@
isPrimaryEvaluator = schedule.Evaluator
};
})
.OrderBy(r => r!.name) // Sort rotations alphabetically
.OrderBy(r => r.name) // Sort rotations alphabetically
.Cast<dynamic>()
.ToArray()
: Array.Empty<dynamic>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,36 @@ public async Task<IActionResult> AddInstructor(
correlationId));
}

// ValidateRequestAsync above already guarantees both are present. Capturing them
// as non-null locals makes every later use provable rather than asserted.
if (request.RotationId is not { } rotationId || request.GradYear is not { } gradYear)
{
return BadRequest(new ErrorResponse(
ErrorCodes.ValidationError,
"Please check your input and try again.",
correlationId));
}

// Step 2: Check permissions - include own schedule check
if (!await CheckPermissionsForAddAsync(request.RotationId!.Value, request.MothraId!, correlationId, cancellationToken))
if (!await CheckPermissionsForAddAsync(rotationId, request.MothraId, correlationId, cancellationToken))
{
return Forbid();
}

// Step 3: Check for conflicts and build warning message
var warningMessage = await BuildConflictWarningAsync(
request.MothraId!,
request.MothraId,
request.WeekIds,
request.GradYear!.Value,
request.RotationId!.Value,
gradYear,
rotationId,
correlationId,
cancellationToken);

// Step 4: Add instructor through service layer
var response = await ProcessAddInstructorAsync(
request,
rotationId,
gradYear,
warningMessage,
correlationId,
cancellationToken);
Expand Down Expand Up @@ -176,16 +188,18 @@ private async Task<bool> CheckPermissionsForAddAsync(

private async Task<AddInstructorResponse> ProcessAddInstructorAsync(
AddInstructorRequest request,
int rotationId,
int gradYear,
string? warningMessage,
string correlationId,
CancellationToken cancellationToken)
{
// Add instructor to schedule
var createdSchedules = await _scheduleEditService.AddInstructorAsync(
request.MothraId!,
request.RotationId!.Value,
request.MothraId,
rotationId,
request.WeekIds,
request.GradYear!.Value,
gradYear,
request.IsPrimaryEvaluator,
cancellationToken);

Expand All @@ -198,7 +212,7 @@ private async Task<AddInstructorResponse> ProcessAddInstructorAsync(
}

_logger.LogInformation("Successfully added instructor to rotation {RotationId} for {WeekCount} weeks (CorrelationId: {CorrelationId})",
request.RotationId!.Value, request.WeekIds.Length, correlationId);
rotationId, request.WeekIds.Length, correlationId);

return new AddInstructorResponse
{
Expand Down Expand Up @@ -388,8 +402,18 @@ public async Task<IActionResult> SetPrimaryEvaluator(
correlationId));
}

// [Required] plus the ModelState check above already guarantee this. Capturing it
// as a non-null local makes every later use provable rather than asserted.
if (request.IsPrimary is not { } isPrimary)
{
return BadRequest(new ErrorResponse(
ErrorCodes.ValidationError,
"Please check your input and try again.",
correlationId));
}

var (success, previousPrimaryName) = await _scheduleEditService.SetPrimaryEvaluatorAsync(
instructorScheduleId, request.IsPrimary!.Value, cancellationToken, request.RequiresPrimaryEvaluator);
instructorScheduleId, isPrimary, cancellationToken, request.RequiresPrimaryEvaluator);

if (!success)
{
Expand All @@ -399,14 +423,14 @@ public async Task<IActionResult> SetPrimaryEvaluator(
correlationId));
}

var action = request.IsPrimary!.Value ? "set as" : "removed as";
var action = isPrimary ? "set as" : "removed as";
_logger.LogInformation("Successfully {Action} primary evaluator for instructor schedule {ScheduleId} (CorrelationId: {CorrelationId})",
action, instructorScheduleId, correlationId);

return Ok(new
{
message = $"Instructor successfully {action} primary evaluator",
isPrimaryEvaluator = request.IsPrimary!.Value,
isPrimaryEvaluator = isPrimary,
previousPrimaryName
});
}
Expand Down
4 changes: 3 additions & 1 deletion web/Areas/Computing/Services/BiorenderStudentLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ public async Task<List<BiorenderStudent>> GetBiorenderStudentInfo(List<string> e
);
}

// GetSingleStudent returns a constructed BiorenderStudent on every path, so there is
// nothing to filter out here.
var taskResults = await Task.WhenAll(resultList);
return taskResults.Where(t => t != null).ToList()!;
return taskResults.ToList();
}

/// <summary>
Expand Down
8 changes: 5 additions & 3 deletions web/Areas/Effort/Controllers/ClinicalImportController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,10 @@ public async Task StreamImport(
// Create a channel for progress events
var channel = Channel.CreateUnbounded<ClinicalImportProgressEvent>();

// Start the import in a background task.
// Don't pass ct to Task.Run — cancellation is handled cooperatively inside the lambda.
// Start the import in a background task. CancellationToken.None is deliberate: a token
// here only stops the task ever starting, which would skip the lambda's own catch and
// leave the channel un-completed, hanging the SSE reader below. Cancellation is handled
// cooperatively via ct inside the lambda instead.
var importTask = Task.Run(async () =>
{
try
Expand Down Expand Up @@ -160,7 +162,7 @@ public async Task StreamImport(
{
channel.Writer.TryComplete();
}
});
}, CancellationToken.None);

try
{
Expand Down
Loading
Loading