Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace DigitalLearningSolutions.Data.Tests.DataServices
using DigitalLearningSolutions.Data.Tests.TestHelpers;
using FakeItEasy;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.Extensions.Logging;
using NUnit.Framework;

Expand Down Expand Up @@ -218,14 +219,14 @@ public void GetNumberOfActiveCoursesAtCentre_with_filtered_category_returns_expe
}

[Test]
public void GetCourseStatisticsAtCentreForCategoryID_should_return_course_statistics_correctly()
public void GetCourseStatisticsAtCentreForAdminCategoryId_should_return_course_statistics_correctly()
{
// Given
const int centreId = 101;
const int categoryId = 0;

// When
var result = courseDataService.GetCourseStatisticsAtCentreForCategoryId(centreId, categoryId).ToList();
var result = courseDataService.GetCourseStatisticsAtCentreForAdminCategoryId(centreId, categoryId).ToList();

// Then
var expectedFirstCourse = new CourseStatistics
Expand All @@ -234,6 +235,7 @@ public void GetCourseStatisticsAtCentreForCategoryID_should_return_course_statis
CentreId = 101,
Active = false,
AllCentres = false,
ApplicationId = 1,
ApplicationName = "Entry Level - Win XP, Office 2003/07 OLD",
CustomisationName = "Standard",
DelegateCount = 25,
Expand All @@ -251,7 +253,7 @@ public void GetCourseStatisticsAtCentreForCategoryID_should_return_course_statis
}

[Test]
public void GetCourseDetailsByIdAtCentreForCategoryId_should_return_course_details_correctly()
public void GetCourseDetailsForAdminCategoryId_should_return_course_details_correctly()
{
// Given
const int customisationId = 100;
Expand All @@ -266,7 +268,7 @@ public void GetCourseDetailsByIdAtCentreForCategoryId_should_return_course_detai

// When
var result =
courseDataService.GetCourseDetails(customisationId, centreId, categoryId)!;
courseDataService.GetCourseDetailsForAdminCategoryId(customisationId, centreId, categoryId)!;
// Overwrite the created time as it is populated by a default constraint and not consistent over different databases
result.CreatedDate = fixedCreationDateTime;

Expand Down Expand Up @@ -316,5 +318,30 @@ public void GetDelegateCoursesAttemptStats_should_return_delegate_course_info_co
totalAttempts.Should().Be(23);
attemptsPassed.Should().Be(11);
}

[Test]
public void GetCoursesAtCentreForAdminCategoryId_returns_expected_values()
{
// Given
var expectedFirstCourse = new Course
{
CustomisationId = 1,
CentreId = 2,
ApplicationId = 1,
ApplicationName = "Entry Level - Win XP, Office 2003/07 OLD",
CustomisationName = "Standard",
Active = false
};

// When
var result = courseDataService.GetCoursesAtCentreForAdminCategoryId(2, 0).ToList();

// Then
using (new AssertionScope())
{
result.Should().HaveCount(69);
result.First(c => c.CustomisationId == 1).Should().BeEquivalentTo(expectedFirstCourse);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
namespace DigitalLearningSolutions.Data.Tests.DataServices
{
using System;
using System.Linq;
using DigitalLearningSolutions.Data.DataServices;
using DigitalLearningSolutions.Data.Models.CourseDelegates;
using DigitalLearningSolutions.Data.Tests.TestHelpers;
using FluentAssertions;
using FluentAssertions.Execution;
using NUnit.Framework;

public class CourseDelegatesDataServiceTests
{
private ICourseDelegatesDataService courseDelegatesDataService = null!;

[SetUp]
public void Setup()
{
var connection = ServiceTestHelper.GetDatabaseConnection();
courseDelegatesDataService = new CourseDelegatesDataService(connection);
}

[Test]
public void GetDelegatesOnCourse_returns_expected_values()
{
// Given
var expectedFirstRecord = new CourseDelegate
{
Active = true,
CandidateNumber = "PC97",
CompleteBy = null,
DelegateId = 32926,
EmailAddress = "erpock.hs@5bntu",
Enrolled = new DateTime(2012, 07, 02, 13, 30, 37, 807),
FirstName = "xxxxx",
LastName = "xxxx",
LastUpdated = new DateTime(2012, 07, 31, 10, 18, 39, 993),
Locked = false,
ProgressId = 18395,
RemovedDate = null,
Completed = null,
AllAttempts = 0,
AttemptsPassed = 0
};

// When
var result = courseDelegatesDataService.GetDelegatesOnCourse(1, 2).ToList();

// Then
using (new AssertionScope())
{
result.Should().HaveCount(3);
result.First().Should().BeEquivalentTo(expectedFirstRecord);
}
}
}
}
29 changes: 0 additions & 29 deletions DigitalLearningSolutions.Data.Tests/Models/CourseStatisticTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,34 +33,5 @@ public void InProgressCount_should_be_calculated_from_total_delegates_and_total_
// Then
courseStatistics.InProgressCount.Should().Be(42);
}

[Test]
public void Course_name_should_be_application_name_if_customisation_name_is_null()
{
// When
var courseStatistics = new CourseStatistics
{
ApplicationName = "Test application",
CustomisationName = null,
};

// Then
courseStatistics.CourseName.Should().BeEquivalentTo("Test application");
}

[Test]
public void Course_name_should_include_customisation_name_if_it_is_not_null()
{
// When
var courseStatistics = new CourseStatistics
{
ApplicationName = "Test application",
CustomisationName = "customisation",
};

// Then
courseStatistics.CourseName.Should().BeEquivalentTo("Test application - customisation");
}

}
}
37 changes: 37 additions & 0 deletions DigitalLearningSolutions.Data.Tests/Models/User/CourseTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
namespace DigitalLearningSolutions.Data.Tests.Models.User
{
using DigitalLearningSolutions.Data.Models.Courses;
using FluentAssertions;
using NUnit.Framework;

public class CourseTests
{
[Test]
public void Course_name_should_be_application_name_if_customisation_name_is_null()
{
// When
var courseStatistics = new Course
{
ApplicationName = "Test application",
CustomisationName = string.Empty
};

// Then
courseStatistics.CourseName.Should().BeEquivalentTo("Test application");
}

[Test]
public void Course_name_should_include_customisation_name_if_it_is_not_null()
{
// When
var courseStatistics = new Course
{
ApplicationName = "Test application",
CustomisationName = "customisation"
};

// Then
courseStatistics.CourseName.Should().BeEquivalentTo("Test application - customisation");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
namespace DigitalLearningSolutions.Data.Tests.Services
{
using System.Collections.Generic;
using DigitalLearningSolutions.Data.DataServices;
using DigitalLearningSolutions.Data.Models.CourseDelegates;
using DigitalLearningSolutions.Data.Models.Courses;
using DigitalLearningSolutions.Data.Services;
using DigitalLearningSolutions.Data.Tests.TestHelpers;
using FakeItEasy;
using FluentAssertions;
using FluentAssertions.Execution;
using NUnit.Framework;

public class CourseDelegatesServiceTests
{
private ICourseDataService courseDataService = null!;
private ICourseDelegatesDataService courseDelegatesDataService = null!;
private ICourseDelegatesService courseDelegatesService = null!;

[SetUp]
public void Setup()
{
courseDataService = A.Fake<ICourseDataService>();
courseDelegatesDataService = A.Fake<ICourseDelegatesDataService>();

courseDelegatesService = new CourseDelegatesService(
courseDataService,
courseDelegatesDataService
);
}

[Test]
public void GetCoursesAndCourseDelegatesForCentre_populates_course_delegates_data()
{
// Given
const int centreId = 2;
const int categoryId = 1;
A.CallTo(() => courseDataService.GetCoursesAtCentreForAdminCategoryId(centreId, categoryId))
.Returns(new List<Course> { new Course { CustomisationId = 1 } });
A.CallTo(() => courseDelegatesDataService.GetDelegatesOnCourse(A<int>._, A<int>._))
.Returns(new List<CourseDelegate> { new CourseDelegate() });

// When
var result = courseDelegatesService.GetCoursesAndCourseDelegatesForCentre(
centreId,
categoryId,
null
);

// Then
using (new AssertionScope())
{
result.Courses.Should().HaveCount(1);
result.Delegates.Should().HaveCount(1);
result.CustomisationId.Should().Be(1);
}
}

[Test]
public void GetCoursesAndCourseDelegatesForCentre_contains_empty_lists_with_no_courses_in_category()
{
// Given
A.CallTo(() => courseDataService.GetCoursesAtCentreForAdminCategoryId(2, 7)).Returns(new List<Course>());

// When
var result = courseDelegatesService.GetCoursesAndCourseDelegatesForCentre(2, 7, null);

// Then
using (new AssertionScope())
{
A.CallTo(() => courseDelegatesDataService.GetDelegatesOnCourse(A<int>._, A<int>._))
.MustNotHaveHappened();
result.Courses.Should().BeEmpty();
result.Delegates.Should().BeEmpty();
result.CustomisationId.Should().BeNull();
}
}

[Test]
public void GetCoursesAndCourseDelegatesForCentre_uses_passed_in_customisation_id()
{
// Given
const int customisationId = 2;
const int centreId = 2;
const int categoryId = 1;
A.CallTo(() => courseDataService.GetCoursesAtCentreForAdminCategoryId(centreId, categoryId))
.Returns(new List<Course> { new Course { CustomisationId = 1 } });
A.CallTo(() => courseDelegatesDataService.GetDelegatesOnCourse(A<int>._, A<int>._))
.Returns(new List<CourseDelegate> { new CourseDelegate() });

// When
var result = courseDelegatesService.GetCoursesAndCourseDelegatesForCentre(
centreId,
categoryId,
customisationId
);

// Then
A.CallTo(() => courseDelegatesDataService.GetDelegatesOnCourse(customisationId, centreId))
.MustHaveHappened();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class CourseServiceTests
public void Setup()
{
courseDataService = A.Fake<ICourseDataService>();
A.CallTo(() => courseDataService.GetCourseStatisticsAtCentreForCategoryId(CentreId, AdminCategoryId))
A.CallTo(() => courseDataService.GetCourseStatisticsAtCentreForAdminCategoryId(CentreId, AdminCategoryId))
.Returns(GetSampleCourses());
courseAdminFieldsService = A.Fake<ICourseAdminFieldsService>();
courseService = new CourseService(courseDataService, courseAdminFieldsService);
Expand Down
34 changes: 30 additions & 4 deletions DigitalLearningSolutions.Data/DataServices/CourseDataService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ public interface ICourseDataService
void RemoveCurrentCourse(int progressId, int candidateId);
void EnrolOnSelfAssessment(int selfAssessmentId, int candidateId);
int GetNumberOfActiveCoursesAtCentreForCategory(int centreId, int categoryId);
IEnumerable<CourseStatistics> GetCourseStatisticsAtCentreForCategoryId(int centreId, int categoryId);
IEnumerable<CourseStatistics> GetCourseStatisticsAtCentreForAdminCategoryId(int centreId, int categoryId);
IEnumerable<DelegateCourseInfo> GetDelegateCoursesInfo(int delegateId);
(int totalAttempts, int attemptsPassed) GetDelegateCourseAttemptStats(int delegateId, int customisationId);
CourseDetails? GetCourseDetails(int customisationId, int centreId, int categoryId);
CourseDetails? GetCourseDetailsForAdminCategoryId(int customisationId, int centreId, int categoryId);
IEnumerable<Course> GetCoursesAtCentreForAdminCategoryId(int centreId, int categoryId);
}

public class CourseDataService : ICourseDataService
Expand Down Expand Up @@ -171,14 +172,17 @@ FROM Customisations AS c
);
}

public IEnumerable<CourseStatistics> GetCourseStatisticsAtCentreForCategoryId(int centreId, int categoryId)
// Admins have a non-nullable category ID where 0 = all. This is why we have the
// @categoryId = 0 in the WHERE clause, to prevent filtering on category ID when it is 0
public IEnumerable<CourseStatistics> GetCourseStatisticsAtCentreForAdminCategoryId(int centreId, int categoryId)
{
return connection.Query<CourseStatistics>(
@$"SELECT
cu.CustomisationID,
cu.CentreID,
cu.Active,
cu.AllCentres,
ap.ApplicationId,
ap.ApplicationName,
cu.CustomisationName,
{DelegateCountQuery},
Expand Down Expand Up @@ -252,7 +256,9 @@ FROM AssessAttempts aa
);
}

public CourseDetails? GetCourseDetails(int customisationId, int centreId, int categoryId)
// Admins have a non-nullable category ID where 0 = all. This is why we have the
// @categoryId = 0 in the WHERE clause, to prevent filtering on category ID when it is 0
public CourseDetails? GetCourseDetailsForAdminCategoryId(int customisationId, int centreId, int categoryId)
{
return connection.Query<CourseDetails>(
@$"SELECT
Expand Down Expand Up @@ -298,5 +304,25 @@ AND ap.ArchivedDate IS NULL
new { customisationId, centreId, categoryId }
).FirstOrDefault();
}

// Admins have a non-nullable category ID where 0 = all. This is why we have the
// @categoryId = 0 in the WHERE clause, to prevent filtering on category ID when it is 0
public IEnumerable<Course> GetCoursesAtCentreForAdminCategoryId(int centreId, int categoryId)
{
return connection.Query<Course>(
@"SELECT
c.CustomisationID,
c.CentreID,
c.ApplicationID,
a.ApplicationName,
c.CustomisationName,
c.Active
FROM Customisations AS c
JOIN Applications AS a on a.ApplicationID = c.ApplicationID
WHERE (CentreID = @centreId OR CentreID = 0)
AND (a.CourseCategoryID = @categoryId OR @categoryId = 0)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd still prefer the categoryId to be null with no filter rather than "0" which can be ambiguous. What do you think?

Copy link
Contributor Author

@AlexJacksonDS AlexJacksonDS Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admins don't have a nullable CategoryId. I think we should keep this behaviour consistent with the AdminUser records so it doesn't cause confusion unless we want to go through and change everything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think in this query the "categoryId" is effectively a filter on category id, which intuitively would be:

  • Null = no filter
  • Value = filter applied

new { centreId, categoryId }
);
}
}
}
Loading