Skip to content

TD-3838 Reworks query to use coalesce to avoid throwing exception #2562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 7, 2024
Merged

TD-3838 Reworks query to use coalesce to avoid throwing exception #2562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af13dee
TD3838 Reworks query to use coalesce to avoid throwing exception
kevwhitt-hee Mar 6, 2024
1f359b6
TD-3838 Returns null int instead of zero and comments id manipulation…
kevwhitt-hee Mar 6, 2024
95ec05a
Deprecates id manipulation tests
kevwhitt-hee Mar 6, 2024
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
10 changes: 5 additions & 5 deletions DigitalLearningSolutions.Data/DataServices/CourseContentService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,19 +269,19 @@ FROM Sessions AS S1 WITH (NOLOCK)
{
try
{
return connection.QueryFirst<int>(
@"SELECT ProgressId
FROM Progress
return connection.QueryFirst<int?>(
@"SELECT COALESCE((SELECT ProgressID
FROM Progress
WHERE CandidateID = @candidateId
AND CustomisationID = @customisationId
AND SystemRefreshed = 0
AND RemovedDate IS NULL",
AND RemovedDate IS NULL), NULL) AS ProgressId",
new { candidateId, customisationId }
);
}
catch (InvalidOperationException)
{
return null;
return 0;
}
}
}
Expand Down
87 changes: 44 additions & 43 deletions DigitalLearningSolutions.Web.Tests/Controllers/LearningMenu/IndexTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,49 +241,50 @@ public void Index_unable_to_enrol_should_not_StartOrUpdate_course_sessions()
// Then
A.CallTo(() => sessionService.StartOrUpdateDelegateSession(A<int>._, A<int>._, A<ISession>._)).MustNotHaveHappened();
}

[Test]
public void Index_detects_id_manipulation_no_progress_id()
{
// Given
var expectedCourseContent = CourseContentHelper.CreateDefaultCourseContent(CustomisationId);
A.CallTo(() => courseContentService.GetCourseContent(CandidateId, CustomisationId))
.Returns(expectedCourseContent);
A.CallTo(() => courseContentService.GetOrCreateProgressId(CandidateId, CustomisationId, CentreId)).Returns(10);
A.CallTo(() => courseContentService.GetProgressId(CandidateId, CustomisationId)).Returns(null);

// When
var result = controller.Index(CustomisationId);

// Then
result.Should()
.BeRedirectToActionResult()
.WithControllerName("LearningSolutions")
.WithActionName("StatusCode")
.WithRouteValue("code", 404);
}

[Test]
public void Index_detects_id_manipulation_self_register_false()
{
// Given
var expectedCourseContent = CourseContentHelper.CreateDefaultCourseContent(CustomisationId);
A.CallTo(() => courseContentService.GetCourseContent(CandidateId, CustomisationId))
.Returns(expectedCourseContent);
A.CallTo(() => courseContentService.GetOrCreateProgressId(CandidateId, CustomisationId, CentreId)).Returns(10);
A.CallTo(() => courseContentService.GetProgressId(CandidateId, CustomisationId)).Returns(null);
A.CallTo(() => courseDataService.GetSelfRegister(CustomisationId)).Returns(false);

// When
var result = controller.Index(CustomisationId);

// Then
result.Should()
.BeRedirectToActionResult()
.WithControllerName("LearningSolutions")
.WithActionName("StatusCode")
.WithRouteValue("code", 404);
}
//Deprecated in response to TD-3838 - a bug caused by this id manipulation detection functionality

//[Test]
//public void Index_detects_id_manipulation_no_progress_id()
//{
// // Given
// var expectedCourseContent = CourseContentHelper.CreateDefaultCourseContent(CustomisationId);
// A.CallTo(() => courseContentService.GetCourseContent(CandidateId, CustomisationId))
// .Returns(expectedCourseContent);
// A.CallTo(() => courseContentService.GetOrCreateProgressId(CandidateId, CustomisationId, CentreId)).Returns(10);
// A.CallTo(() => courseContentService.GetProgressId(CandidateId, CustomisationId)).Returns(null);

// // When
// var result = controller.Index(CustomisationId);

// // Then
// result.Should()
// .BeRedirectToActionResult()
// .WithControllerName("LearningSolutions")
// .WithActionName("StatusCode")
// .WithRouteValue("code", 404);
//}

//[Test]
//public void Index_detects_id_manipulation_self_register_false()
//{
// // Given
// var expectedCourseContent = CourseContentHelper.CreateDefaultCourseContent(CustomisationId);
// A.CallTo(() => courseContentService.GetCourseContent(CandidateId, CustomisationId))
// .Returns(expectedCourseContent);
// A.CallTo(() => courseContentService.GetOrCreateProgressId(CandidateId, CustomisationId, CentreId)).Returns(10);
// A.CallTo(() => courseContentService.GetProgressId(CandidateId, CustomisationId)).Returns(null);
// A.CallTo(() => courseDataService.GetSelfRegister(CustomisationId)).Returns(false);

// // When
// var result = controller.Index(CustomisationId);

// // Then
// result.Should()
// .BeRedirectToActionResult()
// .WithControllerName("LearningSolutions")
// .WithActionName("StatusCode")
// .WithRouteValue("code", 404);
//}

[Test]
public void Index_not_detects_id_manipulation_self_register_true()
Expand Down
10 changes: 6 additions & 4 deletions DigitalLearningSolutions.Web/Controllers/LearningMenuController/LearningMenuController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,11 @@ public IActionResult Index(int customisationId)
var sectionId = courseContent.Sections.First().Id;
return RedirectToAction("Section", "LearningMenu", new { customisationId, sectionId });
}
if (UniqueIdManipulationDetected(candidateId, customisationId))
{
return RedirectToAction("StatusCode", "LearningSolutions", new { code = 404 });
}
// Unique Id Manipulation Detection is being disabled as part of work on TD-3838 - a bug created by its introduction
//if (UniqueIdManipulationDetected(candidateId, customisationId))
//{
// return RedirectToAction("StatusCode", "LearningSolutions", new { code = 404 });
//}
var progressId = courseContentService.GetOrCreateProgressId(candidateId, customisationId, centreId);
if (progressId == null)
{
Expand All @@ -97,6 +98,7 @@ public IActionResult Index(int customisationId)
$"Candidate id: {candidateId}, customisation id: {customisationId}, centre id: {centreId}");
return RedirectToAction("StatusCode", "LearningSolutions", new { code = 404 });
}

if (sessionService.StartOrUpdateDelegateSession(candidateId, customisationId, HttpContext.Session) > 0)
{
courseContentService.UpdateProgress(progressId.Value);
Expand Down