From 089d01bfb8193033b9ac6ff4ce48695a2f8dc668 Mon Sep 17 00:00:00 2001 From: OliZor Date: Mon, 20 Sep 2021 11:46:31 +0100 Subject: [PATCH 01/15] HEEDLS-438 Change casing of page titles --- .../TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml | 2 +- .../CourseSetup/AdminFields/EditAdminField.cshtml | 2 +- .../Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml index 905e9cc5ae..ee3ba8cdad 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml @@ -10,7 +10,7 @@ @{ var errorHasOccurred = !ViewData.ModelState.IsValid; - ViewData["Title"] = "Add Course Admin Field"; + ViewData["Title"] = "Add course admin field"; ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml index ff25d44b59..f14f72a6d7 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml @@ -10,7 +10,7 @@ @{ var errorHasOccurred = !ViewData.ModelState.IsValid; - ViewData["Title"] = "Edit Course Admin Field"; + ViewData["Title"] = "Edit course admin field"; ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml index cfbac37c20..c8bdb1f0c9 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml @@ -6,7 +6,7 @@ @{ - ViewData["Title"] = "Manage Course Admin Fields"; + ViewData["Title"] = "Manage course admin fields"; ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{Configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; From a5398cbe275d58334738ad4d4a81a9c61e65ffc0 Mon Sep 17 00:00:00 2001 From: OliZor Date: Tue, 21 Sep 2021 11:53:55 +0100 Subject: [PATCH 02/15] HEEDLS-438 Return NotFound if user cannot access course --- .../DataServices/CourseDataService.cs | 29 ++++++++++++----- .../Services/CourseService.cs | 15 ++++++++- .../CourseSetup/AdminFieldsController.cs | 32 ++++++++++++++++++- 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs index 7a9e8ed55f..9a1e7ecda1 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs @@ -23,6 +23,7 @@ public interface ICourseDataService CourseNameInfo? GetCourseNameAndApplication(int customisationId); CourseDetails? GetCourseDetailsForAdminCategoryId(int customisationId, int centreId, int categoryId); IEnumerable GetCentrallyManagedAndCentreCourses(int centreId, int? categoryId); + (int centreId, int categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId); } public class CourseDataService : ICourseDataService @@ -30,7 +31,7 @@ public class CourseDataService : ICourseDataService private const string DelegateCountQuery = @"(SELECT COUNT(pr.CandidateID) FROM dbo.Progress AS pr - INNER JOIN dbo.Candidates AS can ON can.CandidateID = pr.CandidateID + INNER JOIN dbo.Candidates AS can ON can.CandidateID = pr.CandidateID WHERE pr.CustomisationID = cu.CustomisationID AND can.CentreID = @centreId AND RemovedDate IS NULL) AS DelegateCount"; @@ -38,7 +39,7 @@ FROM dbo.Progress AS pr private const string CompletedCountQuery = @"(SELECT COUNT(pr.CandidateID) FROM dbo.Progress AS pr - INNER JOIN dbo.Candidates AS can ON can.CandidateID = pr.CandidateID + INNER JOIN dbo.Candidates AS can ON can.CandidateID = pr.CandidateID WHERE pr.CustomisationID = cu.CustomisationID AND pr.Completed IS NOT NULL AND can.CentreID = @centreId) AS CompletedCount"; @@ -167,7 +168,7 @@ public int GetNumberOfActiveCoursesAtCentreForCategory(int centreId, int adminCa @"SELECT COUNT(*) FROM Customisations AS c JOIN Applications AS a on a.ApplicationID = c.ApplicationID - WHERE Active = 1 AND CentreID = @centreId + WHERE Active = 1 AND CentreID = @centreId AND (a.CourseCategoryID = @adminCategoryId OR @adminCategoryId = 0)", new { centreId, adminCategoryId } ); @@ -199,7 +200,7 @@ FROM dbo.Customisations AS cu INNER JOIN dbo.Applications AS ap ON ap.ApplicationID = ca.ApplicationID INNER JOIN dbo.CourseCategories AS cc ON cc.CourseCategoryID = ap.CourseCategoryID INNER JOIN dbo.CourseTopics AS ct ON ct.CourseTopicID = ap.CourseTopicId - WHERE (ap.CourseCategoryID = @categoryId OR @categoryId = 0) + WHERE (ap.CourseCategoryID = @categoryId OR @categoryId = 0) AND (cu.CentreID = @centreId OR (cu.AllCentres = 1 AND ca.Active = 1)) AND ca.CentreID = @centreId AND ap.ArchivedDate IS NULL", @@ -298,9 +299,9 @@ FROM dbo.Customisations AS cu LEFT JOIN dbo.Customisations AS refreshToCu ON refreshToCu.CustomisationID = cu.RefreshToCustomisationId LEFT JOIN dbo.Applications AS refreshToAp ON refreshToAp.ApplicationID = refreshToCu.ApplicationID WHERE - (ap.CourseCategoryID = @categoryId OR @categoryId = 0) + (ap.CourseCategoryID = @categoryId OR @categoryId = 0) AND cu.CentreID = @centreId - AND ap.ArchivedDate IS NULL + AND ap.ArchivedDate IS NULL AND cu.CustomisationID = @customisationId", new { customisationId, centreId, categoryId } ).FirstOrDefault(); @@ -309,9 +310,9 @@ AND ap.ArchivedDate IS NULL public CourseNameInfo? GetCourseNameAndApplication(int customisationId) { var names = connection.QueryFirstOrDefault( - @"SELECT cu.CustomisationId, cu.CustomisationName, ap.ApplicationName + @"SELECT cu.CustomisationName, ap.ApplicationName FROM Customisations cu - JOIN Applications ap ON cu.ApplicationId = ap.ApplicationId + JOIN Applications ap ON cu.ApplicationId = ap.ApplicationId WHERE cu.CustomisationId = @customisationId", new { customisationId } ); @@ -345,5 +346,17 @@ FROM Customisations AS c new { centreId, categoryId } ); } + + public (int centreId, int categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId) + { + return connection.QueryFirstOrDefault<(int, int)>( + @"SELECT c.CentreID, + a.CategoryID + FROM Customisations AS c + JOIN Applications AS a on a.ApplicationID = c.ApplicationID + WHERE CustomisationID = @customisationId", + new { customisationId } + ); + } } } diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 4a37781e7b..2557052f9c 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -10,12 +10,13 @@ public interface ICourseService public IEnumerable GetTopCourseStatistics(int centreId, int categoryId); public IEnumerable GetCentreSpecificCourseStatistics(int centreId, int categoryId); public IEnumerable GetAllCoursesForDelegate(int delegateId, int centreId); + public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int categoryId); } public class CourseService : ICourseService { - private readonly ICourseDataService courseDataService; private readonly ICourseAdminFieldsService courseAdminFieldsService; + private readonly ICourseDataService courseDataService; public CourseService(ICourseDataService courseDataService, ICourseAdminFieldsService courseAdminFieldsService) { @@ -52,5 +53,17 @@ public IEnumerable GetAllCoursesForDelegate(int delegateI } ); } + + public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int categoryId) + { + var ids = courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); + + if (centreId != ids.centreId || categoryId != ids.categoryId) + { + return false; + } + + return true; + } } } diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index c5de15c3b5..0df5e30fc7 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -28,22 +28,28 @@ public class AdminFieldsController : Controller private static readonly DateTimeOffset CookieExpiry = DateTimeOffset.UtcNow.AddDays(7); private readonly ICourseAdminFieldsDataService courseAdminFieldsDataService; private readonly ICourseAdminFieldsService courseAdminFieldsService; + private readonly ICourseService courseService; public AdminFieldsController( ICourseAdminFieldsService courseAdminFieldsService, - ICourseAdminFieldsDataService courseAdminFieldsDataService + ICourseAdminFieldsDataService courseAdminFieldsDataService, + ICourseService courseService ) { this.courseAdminFieldsService = courseAdminFieldsService; this.courseAdminFieldsDataService = courseAdminFieldsDataService; + this.courseService = courseService; } [HttpGet] [Route("{customisationId}/AdminFields")] public IActionResult Index(int customisationId) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; + var courseAdminFields = courseAdminFieldsService.GetCustomPromptsForCourse( customisationId, centreId, @@ -67,8 +73,11 @@ public IActionResult EditAdminFieldStart(int customisationId, int promptNumber) [Route("{customisationId}/AdminFields/{promptNumber:int}/Edit")] public IActionResult EditAdminField(int customisationId, int promptNumber) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; + var courseAdminField = courseAdminFieldsService.GetCustomPromptsForCourse( customisationId, centreId, @@ -106,6 +115,8 @@ public IActionResult EditAdminField(EditAdminFieldViewModel model, string action [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( @@ -160,6 +171,8 @@ public IActionResult AddAdminFieldNew(int customisationId) [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminField(int customisationId) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var addAdminFieldData = TempData.Peek()!; SetViewBagAdminFieldNameOptions(addAdminFieldData.AddModel.AdminFieldId); @@ -200,6 +213,8 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminFieldAnswersBulk(int customisationId) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( data.AddModel.OptionsString, @@ -238,6 +253,8 @@ BulkAdminFieldAnswersViewModel model [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Remove")] public IActionResult RemoveAdminField(int customisationId, int promptNumber) { + ReturnNotFoundIfUserCannotAccessCourse(customisationId); + var answerCount = courseAdminFieldsDataService.GetAnswerCountForCourseAdminField(customisationId, promptNumber); @@ -518,5 +535,18 @@ private void ValidateBulkOptionsString(string? optionsString) ); } } + + private NotFoundResult? ReturnNotFoundIfUserCannotAccessCourse(int customisationId) + { + var centreId = User.GetCentreId(); + var categoryId = User.GetAdminCategoryId()!; + + if (!courseService.VerifyUserCanAccessCourse(customisationId, centreId, categoryId.Value)) + { + return NotFound(); + } + + return null; + } } } From 04239d8353c6931be17f8f3da46865b5d78bbb12 Mon Sep 17 00:00:00 2001 From: OliZor Date: Tue, 21 Sep 2021 12:32:07 +0100 Subject: [PATCH 03/15] HEEDLS-438 Refactor VerifyUserCanAccessCourse --- .../DataServices/CourseDataService.cs | 6 +++--- DigitalLearningSolutions.Data/Services/CourseService.cs | 2 +- .../CourseSetup/AdminFieldsControllerTests.cs | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs index 9a1e7ecda1..d7034b98fe 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs @@ -23,7 +23,7 @@ public interface ICourseDataService CourseNameInfo? GetCourseNameAndApplication(int customisationId); CourseDetails? GetCourseDetailsForAdminCategoryId(int customisationId, int centreId, int categoryId); IEnumerable GetCentrallyManagedAndCentreCourses(int centreId, int? categoryId); - (int centreId, int categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId); + (int? centreId, int? categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId); } public class CourseDataService : ICourseDataService @@ -347,11 +347,11 @@ FROM Customisations AS c ); } - public (int centreId, int categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId) + public (int? centreId, int? categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId) { return connection.QueryFirstOrDefault<(int, int)>( @"SELECT c.CentreID, - a.CategoryID + a.CourseCategoryID FROM Customisations AS c JOIN Applications AS a on a.ApplicationID = c.ApplicationID WHERE CustomisationID = @customisationId", diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 2557052f9c..0e8d62fdad 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -58,7 +58,7 @@ public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int cat { var ids = courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); - if (centreId != ids.centreId || categoryId != ids.categoryId) + if (centreId != ids.centreId || (categoryId != ids.categoryId && categoryId != 0)) { return false; } diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index ecd224e07d..51c12f4100 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -23,12 +23,13 @@ public class AdminFieldsControllerTests A.Fake(); private readonly ICourseAdminFieldsService courseAdminFieldsService = A.Fake(); + private readonly ICourseService courseService = A.Fake(); private AdminFieldsController controller = null!; [SetUp] public void Setup() { - controller = new AdminFieldsController(courseAdminFieldsService, courseAdminFieldsDataService) + controller = new AdminFieldsController(courseAdminFieldsService, courseAdminFieldsDataService, courseService) .WithDefaultContext() .WithMockUser(true, 101) .WithMockTempData(); From 8bba4ffd910380e2b8a75ccdff834f0bb401d0d9 Mon Sep 17 00:00:00 2001 From: OliZor Date: Tue, 21 Sep 2021 15:47:50 +0100 Subject: [PATCH 04/15] HEEDLS-438 Add tests for verifying user can access course --- .../DataServices/CourseDataServiceTests.cs | 11 ++++++ .../Services/CourseServiceTests.cs | 16 ++++++++ .../Services/CourseService.cs | 10 ++--- .../CourseSetup/AdminFieldsControllerTests.cs | 38 ++++++++++++++++++- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs index 676963a8f0..d76ec999a1 100644 --- a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs @@ -364,5 +364,16 @@ public void GetCentrallyManagedAndCentreCourses_returns_expected_values() result.Should().HaveCount(260); result.First().Should().BeEquivalentTo(expectedFirstCourse); } + + [Test] + public void GetCentreIdAndCategoryIdForCourse_returns_expected_values() + { + // When + var (centreId, categoryId) = courseDataService.GetCentreIdAndCategoryIdForCourse(1); + + // Then + centreId.Should().Be(2); + categoryId.Should().Be(2); + } } } diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs index a2f8a38d5e..4fa1837741 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs @@ -149,5 +149,21 @@ public void GetAllCoursesForDelegate_should_not_fetch_attempt_stats_if_course_no results[0].DelegateCourseInfo.Should().BeEquivalentTo(info); results[0].AttemptStats.Should().Be((0, 0)); } + + [Test] + public void VerifyUserCanAccessCourse_should_call_correct_data_service_method() + { + // Given + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .Returns((2, 2)); + + // When + var result = courseService.VerifyUserCanAccessCourse(1, 2, 2); + + // Then + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .MustHaveHappened(1, Times.Exactly); + result.Should().Be(true); + } } } diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 0e8d62fdad..9fba5b1423 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -56,14 +56,10 @@ public IEnumerable GetAllCoursesForDelegate(int delegateI public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int categoryId) { - var ids = courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); + var (courseCentreId, courseCategoryId) = + courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); - if (centreId != ids.centreId || (categoryId != ids.categoryId && categoryId != 0)) - { - return false; - } - - return true; + return centreId == courseCentreId && (categoryId == courseCategoryId || categoryId == 0); } } } diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 51c12f4100..2a41896076 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -29,18 +29,46 @@ public class AdminFieldsControllerTests [SetUp] public void Setup() { - controller = new AdminFieldsController(courseAdminFieldsService, courseAdminFieldsDataService, courseService) + controller = new AdminFieldsController( + courseAdminFieldsService, + courseAdminFieldsDataService, + courseService + ) .WithDefaultContext() .WithMockUser(true, 101) .WithMockTempData(); } + [Test] + public void All_admin_field_pages_return_not_found_if_user_cannot_access_course() + { + // Given + A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + .Returns(false); + + // When + var indexResult = controller.Index(2); + var editResult = controller.EditAdminField(2, 1); + var editBulkResult = controller.EditAdminFieldBulk(2, 1); + var addResult = controller.AddAdminField(2); + var removeResult = controller.RemoveAdminField(2, 1); + + // Then + indexResult.Should().BeNotFoundResult(); + editResult.Should().BeNotFoundResult(); + editBulkResult.Should().BeNotFoundResult(); + addResult.Should().BeNotFoundResult(); + removeResult.Should().BeNotFoundResult(); + } + [Test] public void AdminFields_returns_AdminFields_page_when_appropriate_course_found() { // Given var samplePrompt1 = CustomPromptsTestHelper.GetDefaultCustomPrompt(1, "System Access Granted", "Yes\r\nNo"); var customPrompts = new List { samplePrompt1 }; + A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + .Returns(true); A.CallTo(() => courseAdminFieldsService.GetCustomPromptsForCourse(A._, A._, A._)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFields(customPrompts)); @@ -195,6 +223,8 @@ public void AddAdminField_post_updates_temp_data_and_redirects() var expectedPromptModel = new AddAdminFieldViewModel(1); var initialTempData = new AddAdminFieldData(expectedPromptModel); controller.TempData.Set(initialTempData); + A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + .Returns(true); // When var result = controller.AddAdminField(1); @@ -213,6 +243,8 @@ public void AddAdminField_save_clears_temp_data_and_redirects_to_index() var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); + A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + .Returns(true); A.CallTo( () => courseAdminFieldsService.AddCustomPromptToCourse( 100, @@ -457,6 +489,10 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() [Test] public void RemoveAdminField_removes_admin_field_with_no_user_answers() { + // Given + A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + .Returns(true); + // When var result = controller.RemoveAdminField(100, 2); From dc2497e7b99c612b7331fe1c7b33095e5b858c68 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 23 Sep 2021 16:09:44 +0100 Subject: [PATCH 05/15] HEEDLS-438 Rename VerifyAdminUserCanAccessCourse and add extra tests --- .../Services/CourseServiceTests.cs | 35 ++++++++++++++++- .../Services/CourseService.cs | 4 +- .../CourseSetup/AdminFieldsControllerTests.cs | 10 ++--- .../CourseSetup/AdminFieldsController.cs | 39 ++++++++++++------- 4 files changed, 66 insertions(+), 22 deletions(-) diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs index 4fa1837741..4f6a6cfc68 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs @@ -151,19 +151,50 @@ public void GetAllCoursesForDelegate_should_not_fetch_attempt_stats_if_course_no } [Test] - public void VerifyUserCanAccessCourse_should_call_correct_data_service_method() + public void VerifyAdminUserCanAccessCourse_should_call_correct_data_service_method() { // Given A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) .Returns((2, 2)); // When - var result = courseService.VerifyUserCanAccessCourse(1, 2, 2); + var result = courseService.VerifyAdminUserCanAccessCourse(1, 2, 2); // Then A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) .MustHaveHappened(1, Times.Exactly); result.Should().Be(true); } + + [Test] + public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_centreId() + { + // Given + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .Returns((2, 2)); + + // When + var result = courseService.VerifyAdminUserCanAccessCourse(1, 1, 2); + + // Then + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .MustHaveHappened(1, Times.Exactly); + result.Should().Be(false); + } + + [Test] + public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_categoryId() + { + // Given + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .Returns((2, 2)); + + // When + var result = courseService.VerifyAdminUserCanAccessCourse(1, 2, 1); + // Then + A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + .MustHaveHappened(1, Times.Exactly); + result.Should().Be(false); + } } } diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 9fba5b1423..76ca69c5da 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -10,7 +10,7 @@ public interface ICourseService public IEnumerable GetTopCourseStatistics(int centreId, int categoryId); public IEnumerable GetCentreSpecificCourseStatistics(int centreId, int categoryId); public IEnumerable GetAllCoursesForDelegate(int delegateId, int centreId); - public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int categoryId); + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int categoryId); } public class CourseService : ICourseService @@ -54,7 +54,7 @@ public IEnumerable GetAllCoursesForDelegate(int delegateI ); } - public bool VerifyUserCanAccessCourse(int customisationId, int centreId, int categoryId) + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int categoryId) { var (courseCentreId, courseCategoryId) = courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 2a41896076..64867a30ce 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -43,7 +43,7 @@ public void Setup() public void All_admin_field_pages_return_not_found_if_user_cannot_access_course() { // Given - A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(false); // When @@ -67,7 +67,7 @@ public void AdminFields_returns_AdminFields_page_when_appropriate_course_found() // Given var samplePrompt1 = CustomPromptsTestHelper.GetDefaultCustomPrompt(1, "System Access Granted", "Yes\r\nNo"); var customPrompts = new List { samplePrompt1 }; - A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(true); A.CallTo(() => courseAdminFieldsService.GetCustomPromptsForCourse(A._, A._, A._)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFields(customPrompts)); @@ -223,7 +223,7 @@ public void AddAdminField_post_updates_temp_data_and_redirects() var expectedPromptModel = new AddAdminFieldViewModel(1); var initialTempData = new AddAdminFieldData(expectedPromptModel); controller.TempData.Set(initialTempData); - A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(true); // When @@ -243,7 +243,7 @@ public void AddAdminField_save_clears_temp_data_and_redirects_to_index() var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); - A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(true); A.CallTo( () => courseAdminFieldsService.AddCustomPromptToCourse( @@ -490,7 +490,7 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() public void RemoveAdminField_removes_admin_field_with_no_user_answers() { // Given - A.CallTo(() => courseService.VerifyUserCanAccessCourse(A._, A._, A._)) + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(true); // When diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index 0df5e30fc7..16fb2340f4 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -45,7 +45,10 @@ ICourseService courseService [Route("{customisationId}/AdminFields")] public IActionResult Index(int customisationId) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; @@ -73,7 +76,10 @@ public IActionResult EditAdminFieldStart(int customisationId, int promptNumber) [Route("{customisationId}/AdminFields/{promptNumber:int}/Edit")] public IActionResult EditAdminField(int customisationId, int promptNumber) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; @@ -115,7 +121,10 @@ public IActionResult EditAdminField(EditAdminFieldViewModel model, string action [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var data = TempData.Peek()!; @@ -171,7 +180,10 @@ public IActionResult AddAdminFieldNew(int customisationId) [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminField(int customisationId) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var addAdminFieldData = TempData.Peek()!; @@ -213,7 +225,10 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminFieldAnswersBulk(int customisationId) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( @@ -253,7 +268,10 @@ BulkAdminFieldAnswersViewModel model [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Remove")] public IActionResult RemoveAdminField(int customisationId, int promptNumber) { - ReturnNotFoundIfUserCannotAccessCourse(customisationId); + if (!VerifyAdminUserCanAccessCourse(customisationId)) + { + return NotFound(); + } var answerCount = courseAdminFieldsDataService.GetAnswerCountForCourseAdminField(customisationId, promptNumber); @@ -536,17 +554,12 @@ private void ValidateBulkOptionsString(string? optionsString) } } - private NotFoundResult? ReturnNotFoundIfUserCannotAccessCourse(int customisationId) + private bool VerifyAdminUserCanAccessCourse(int customisationId) { var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; - if (!courseService.VerifyUserCanAccessCourse(customisationId, centreId, categoryId.Value)) - { - return NotFound(); - } - - return null; + return courseService.VerifyAdminUserCanAccessCourse(customisationId, centreId, categoryId.Value); } } } From 68d91fbbba3bd61bcd9191c889185972eaeca941 Mon Sep 17 00:00:00 2001 From: OliZor Date: Tue, 28 Sep 2021 13:29:52 +0100 Subject: [PATCH 06/15] HEEDLS-438 Create service filter for VerifyAdminUserCanAccessCourse --- .../Services/CourseServiceTests.cs | 6 +- .../AddCourseAdminFieldsAccessibilityTests.cs | 2 + ...EditCourseAdminFieldsAccessibilityTests.cs | 2 + .../CourseSetup/AdminFieldsControllerTests.cs | 51 ++--------- .../VerifyAdminUserCanAccessCourseTests.cs | 90 +++++++++++++++++++ .../CourseSetup/AdminFieldsController.cs | 84 ++++++----------- .../VerifyAdminUserCanAccessCourse.cs | 38 ++++++++ DigitalLearningSolutions.Web/Startup.cs | 1 + .../AdminFields/BulkAdminFieldAnswers.cshtml | 3 +- 9 files changed, 173 insertions(+), 104 deletions(-) create mode 100644 DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs create mode 100644 DigitalLearningSolutions.Web/ServiceFilter/VerifyAdminUserCanAccessCourse.cs diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs index 4f6a6cfc68..0aad9452c5 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs @@ -163,7 +163,7 @@ public void VerifyAdminUserCanAccessCourse_should_call_correct_data_service_meth // Then A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) .MustHaveHappened(1, Times.Exactly); - result.Should().Be(true); + result.Should().BeTrue(); } [Test] @@ -179,7 +179,7 @@ public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incor // Then A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) .MustHaveHappened(1, Times.Exactly); - result.Should().Be(false); + result.Should().BeFalse(); } [Test] @@ -194,7 +194,7 @@ public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incor // Then A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) .MustHaveHappened(1, Times.Exactly); - result.Should().Be(false); + result.Should().BeFalse(); } } } diff --git a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs index 937b808544..5bfb65339d 100644 --- a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs +++ b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs @@ -35,10 +35,12 @@ public void AddAdminField_journey_has_no_accessibility_errors() Driver.ClickButtonByText("Submit"); ValidatePageHeading("Add course admin field"); + var bulkPostResult = new AxeBuilder(Driver).Analyze(); addPageResult.Violations.Should().BeEmpty(); bulkAdditionResult.Violations.Should().BeEmpty(); addAnswersResult.Violations.Should().BeEmpty(); + bulkPostResult.Violations.Should().BeEmpty(); } private void AddAnswer(string answerString) diff --git a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs index d76ea148db..4fb6eacd19 100644 --- a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs +++ b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs @@ -29,9 +29,11 @@ public void EditAdminField_journey_has_no_accessibility_errors() Driver.ClickButtonByText("Submit"); ValidatePageHeading("Edit course admin field"); + var bulkPostResult = new AxeBuilder(Driver).Analyze(); editPageResult.Violations.Should().BeEmpty(); bulkAdditionResult.Violations.Should().BeEmpty(); + bulkPostResult.Violations.Should().BeEmpty(); } } } diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 64867a30ce..64f3472c3d 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -23,7 +23,6 @@ public class AdminFieldsControllerTests A.Fake(); private readonly ICourseAdminFieldsService courseAdminFieldsService = A.Fake(); - private readonly ICourseService courseService = A.Fake(); private AdminFieldsController controller = null!; [SetUp] @@ -31,44 +30,19 @@ public void Setup() { controller = new AdminFieldsController( courseAdminFieldsService, - courseAdminFieldsDataService, - courseService + courseAdminFieldsDataService ) .WithDefaultContext() .WithMockUser(true, 101) .WithMockTempData(); } - [Test] - public void All_admin_field_pages_return_not_found_if_user_cannot_access_course() - { - // Given - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(false); - - // When - var indexResult = controller.Index(2); - var editResult = controller.EditAdminField(2, 1); - var editBulkResult = controller.EditAdminFieldBulk(2, 1); - var addResult = controller.AddAdminField(2); - var removeResult = controller.RemoveAdminField(2, 1); - - // Then - indexResult.Should().BeNotFoundResult(); - editResult.Should().BeNotFoundResult(); - editBulkResult.Should().BeNotFoundResult(); - addResult.Should().BeNotFoundResult(); - removeResult.Should().BeNotFoundResult(); - } - [Test] public void AdminFields_returns_AdminFields_page_when_appropriate_course_found() { // Given var samplePrompt1 = CustomPromptsTestHelper.GetDefaultCustomPrompt(1, "System Access Granted", "Yes\r\nNo"); var customPrompts = new List { samplePrompt1 }; - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(true); A.CallTo(() => courseAdminFieldsService.GetCustomPromptsForCourse(A._, A._, A._)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFields(customPrompts)); @@ -95,7 +69,7 @@ public void PostEditAdminField_save_calls_correct_methods() ).DoesNothing(); // When - var result = controller.EditAdminField(model, action); + var result = controller.EditAdminField(model, action, 1, 1); // Then A.CallTo( @@ -124,7 +98,7 @@ public void PostEditAdminField_add_configures_new_answer() ).DoesNothing(); // When - var result = controller.EditAdminField(model, action); + var result = controller.EditAdminField(model, action, 1, 1); // Then using (new AssertionScope()) @@ -142,7 +116,7 @@ public void PostEditAdminField_delete_removes_configured_answer() const string action = "delete0"; // When - var result = controller.EditAdminField(model, action); + var result = controller.EditAdminField(model, action, 1, 1); // Then using (new AssertionScope()) @@ -160,7 +134,7 @@ public void PostAdminField_bulk_sets_up_temp_data_and_redirects() const string action = "bulk"; // When - var result = controller.EditAdminField(model, action); + var result = controller.EditAdminField(model, action, 1, 1); // Then using (new AssertionScope()) @@ -178,7 +152,7 @@ public void PostEditAdminField_returns_error_with_unexpected_action() const string action = "deletetest"; // When - var result = controller.EditAdminField(model, action); + var result = controller.EditAdminField(model, action, 1, 1); // Then result.Should().BeStatusCodeResult().WithStatusCode(500); @@ -223,8 +197,6 @@ public void AddAdminField_post_updates_temp_data_and_redirects() var expectedPromptModel = new AddAdminFieldViewModel(1); var initialTempData = new AddAdminFieldData(expectedPromptModel); controller.TempData.Set(initialTempData); - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(true); // When var result = controller.AddAdminField(1); @@ -243,8 +215,6 @@ public void AddAdminField_save_clears_temp_data_and_redirects_to_index() var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(true); A.CallTo( () => courseAdminFieldsService.AddCustomPromptToCourse( 100, @@ -359,8 +329,7 @@ public void AddAdminField_adds_answer_without_admin_field_selected() const string action = "addPrompt"; // When - var result = - controller.AddAdminField(1, initialViewModel, action); + controller.AddAdminField(1, initialViewModel, action); // Then using (new AssertionScope()) @@ -476,7 +445,7 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() controller.TempData.Set(initialTempData); // When - var result = controller.AddAdminFieldAnswersBulk(inputViewModel); + var result = controller.AddAdminFieldAnswersBulk(inputViewModel, 1); // Then using (new AssertionScope()) @@ -489,10 +458,6 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() [Test] public void RemoveAdminField_removes_admin_field_with_no_user_answers() { - // Given - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(true); - // When var result = controller.RemoveAdminField(100, 2); diff --git a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs new file mode 100644 index 0000000000..033e2f0fcd --- /dev/null +++ b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs @@ -0,0 +1,90 @@ +namespace DigitalLearningSolutions.Web.Tests.ServiceFilter +{ + using System.Collections.Generic; + using DigitalLearningSolutions.Data.Services; + using DigitalLearningSolutions.Web.Controllers; + using DigitalLearningSolutions.Web.ServiceFilter; + using DigitalLearningSolutions.Web.Tests.ControllerHelpers; + using FakeItEasy; + using FluentAssertions.AspNetCore.Mvc; + using Microsoft.AspNetCore.Http; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Abstractions; + using Microsoft.AspNetCore.Mvc.Filters; + using Microsoft.AspNetCore.Routing; + using Microsoft.Extensions.Configuration; + using NUnit.Framework; + + public class VerifyAdminUserCanAccessCourseTests + { + private readonly ICourseService courseService = A.Fake(); + + [Test] + public void Returns_NotFound_if_service_returns_false() + { + // Given + var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() + .WithMockUser(true, 101); + var context = new ActionExecutingContext( + new ActionContext( + new DefaultHttpContext(), + new RouteData(new RouteValueDictionary()), + new ActionDescriptor() + ), + new List(), + new Dictionary(), + homeController + ); + context.RouteData.Values["customisationId"] = 2; + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) + .Returns(false); + + // When + new VerifyAdminUserCanAccessCourse(courseService).OnActionExecuting(context); + + // Then + A.CallTo( + () => courseService.VerifyAdminUserCanAccessCourse( + A._, + A._, + A._ + ) + ).MustHaveHappened(); + context.Result.Should().BeNotFoundResult(); + } + + [Test] + public void Does_not_return_NotFound_if_service_returns_true() + { + // Given + var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() + .WithMockUser(true, 101); + var context = new ActionExecutingContext( + new ActionContext( + new DefaultHttpContext(), + new RouteData(new RouteValueDictionary()), + new ActionDescriptor() + ), + new List(), + new Dictionary(), + homeController + ); + context.RouteData.Values["customisationId"] = 24286; + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) + .Returns(true); + + // When + new VerifyAdminUserCanAccessCourse(courseService).OnActionExecuting(context); + + // Then + A.CallTo( + () => courseService.VerifyAdminUserCanAccessCourse( + A._, + A._, + A._ + ) + ).MustHaveHappened(); + context.Result.Should().BeNull(); + } + } +} diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index 16fb2340f4..a480236eff 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -28,28 +28,21 @@ public class AdminFieldsController : Controller private static readonly DateTimeOffset CookieExpiry = DateTimeOffset.UtcNow.AddDays(7); private readonly ICourseAdminFieldsDataService courseAdminFieldsDataService; private readonly ICourseAdminFieldsService courseAdminFieldsService; - private readonly ICourseService courseService; public AdminFieldsController( ICourseAdminFieldsService courseAdminFieldsService, - ICourseAdminFieldsDataService courseAdminFieldsDataService, - ICourseService courseService + ICourseAdminFieldsDataService courseAdminFieldsDataService ) { this.courseAdminFieldsService = courseAdminFieldsService; this.courseAdminFieldsDataService = courseAdminFieldsDataService; - this.courseService = courseService; } [HttpGet] - [Route("{customisationId}/AdminFields")] + [Route("{customisationId:int}/AdminFields")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] public IActionResult Index(int customisationId) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; @@ -64,7 +57,7 @@ public IActionResult Index(int customisationId) } [HttpGet] - [Route("{customisationId}/AdminFields/{promptNumber:int}/Edit/Start")] + [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Start")] public IActionResult EditAdminFieldStart(int customisationId, int promptNumber) { TempData.Clear(); @@ -73,14 +66,10 @@ public IActionResult EditAdminFieldStart(int customisationId, int promptNumber) } [HttpGet] - [Route("{customisationId}/AdminFields/{promptNumber:int}/Edit")] + [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] public IActionResult EditAdminField(int customisationId, int promptNumber) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var centreId = User.GetCentreId(); var categoryId = User.GetAdminCategoryId()!; @@ -99,8 +88,14 @@ public IActionResult EditAdminField(int customisationId, int promptNumber) } [HttpPost] - [Route("{customisationId}/AdminFields/{promptNumber:int}/Edit")] - public IActionResult EditAdminField(EditAdminFieldViewModel model, string action) + [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] + public IActionResult EditAdminField( + EditAdminFieldViewModel model, + string action, + int customisationId, + int promptNumber + ) { if (action.StartsWith(DeleteAction) && TryGetAnswerIndexFromDeleteAction(action, out var index)) { @@ -118,14 +113,10 @@ public IActionResult EditAdminField(EditAdminFieldViewModel model, string action [HttpGet] [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Bulk")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( @@ -152,7 +143,7 @@ BulkAdminFieldAnswersViewModel model } var editData = TempData.Peek()!; - editData.EditModel!.OptionsString = + editData.EditModel.OptionsString = NewlineSeparatedStringListHelper.RemoveEmptyOptions(model.OptionsString); TempData.Set(editData); @@ -177,14 +168,10 @@ public IActionResult AddAdminFieldNew(int customisationId) [HttpGet] [Route("{customisationId:int}/AdminFields/Add")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminField(int customisationId) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var addAdminFieldData = TempData.Peek()!; SetViewBagAdminFieldNameOptions(addAdminFieldData.AddModel.AdminFieldId); @@ -195,15 +182,11 @@ public IActionResult AddAdminField(int customisationId) } [HttpPost] - [Route("{customisationId}/AdminFields/Add")] + [Route("{customisationId:int}/AdminFields/Add")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel model, string action) { - if (customisationId != model.CustomisationId) - { - return new StatusCodeResult(500); - } - UpdateTempDataWithCoursePromptModelValues(model); if (action.StartsWith(DeleteAction) && TryGetAnswerIndexFromDeleteAction(action, out var index)) @@ -221,15 +204,11 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m } [HttpGet] - [Route("{customisationId}/AdminFields/Add/Bulk")] + [Route("{customisationId:int}/AdminFields/Add/Bulk")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminFieldAnswersBulk(int customisationId) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( data.AddModel.OptionsString, @@ -241,10 +220,12 @@ public IActionResult AddAdminFieldAnswersBulk(int customisationId) } [HttpPost] - [Route("{customisationId}/AdminFields/Add/Bulk")] + [Route("{customisationId:int}/AdminFields/Add/Bulk")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminFieldAnswersBulk( - BulkAdminFieldAnswersViewModel model + BulkAdminFieldAnswersViewModel model, + int customisationId ) { ValidateBulkOptionsString(model.OptionsString); @@ -266,13 +247,9 @@ BulkAdminFieldAnswersViewModel model [HttpGet] [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Remove")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] public IActionResult RemoveAdminField(int customisationId, int promptNumber) { - if (!VerifyAdminUserCanAccessCourse(customisationId)) - { - return NotFound(); - } - var answerCount = courseAdminFieldsDataService.GetAnswerCountForCourseAdminField(customisationId, promptNumber); @@ -291,6 +268,7 @@ public IActionResult RemoveAdminField(int customisationId, int promptNumber) [HttpPost] [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Remove")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] public IActionResult RemoveAdminField(int customisationId, int promptNumber, RemoveAdminFieldViewModel model) { if (!model.Confirm) @@ -553,13 +531,5 @@ private void ValidateBulkOptionsString(string? optionsString) ); } } - - private bool VerifyAdminUserCanAccessCourse(int customisationId) - { - var centreId = User.GetCentreId(); - var categoryId = User.GetAdminCategoryId()!; - - return courseService.VerifyAdminUserCanAccessCourse(customisationId, centreId, categoryId.Value); - } } } diff --git a/DigitalLearningSolutions.Web/ServiceFilter/VerifyAdminUserCanAccessCourse.cs b/DigitalLearningSolutions.Web/ServiceFilter/VerifyAdminUserCanAccessCourse.cs new file mode 100644 index 0000000000..5ee27d7bff --- /dev/null +++ b/DigitalLearningSolutions.Web/ServiceFilter/VerifyAdminUserCanAccessCourse.cs @@ -0,0 +1,38 @@ +namespace DigitalLearningSolutions.Web.ServiceFilter +{ + using DigitalLearningSolutions.Data.Services; + using DigitalLearningSolutions.Web.Helpers; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Filters; + + public class VerifyAdminUserCanAccessCourse : IActionFilter + { + private readonly ICourseService courseService; + + public VerifyAdminUserCanAccessCourse( + ICourseService courseService + ) + { + this.courseService = courseService; + } + + public void OnActionExecuted(ActionExecutedContext context) { } + + public void OnActionExecuting(ActionExecutingContext context) + { + if (!(context.Controller is Controller controller)) + { + return; + } + + var centreId = controller.User.GetCentreId(); + var categoryId = controller.User.GetAdminCategoryId()!; + var customisationId = int.Parse(context.RouteData.Values["customisationId"].ToString()!); + + if (!courseService.VerifyAdminUserCanAccessCourse(customisationId, centreId, categoryId.Value)) + { + context.Result = new NotFoundResult(); + } + } + } +} diff --git a/DigitalLearningSolutions.Web/Startup.cs b/DigitalLearningSolutions.Web/Startup.cs index 9e48b6ac08..e9e089ed02 100644 --- a/DigitalLearningSolutions.Web/Startup.cs +++ b/DigitalLearningSolutions.Web/Startup.cs @@ -246,6 +246,7 @@ private static void RegisterWebServiceFilters(IServiceCollection services) services.AddScoped>(); services.AddScoped>(); services.AddScoped>(); + services.AddScoped(); } public void Configure(IApplicationBuilder app, IMigrationRunner migrationRunner, IFeatureManager featureManager) diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml index 92912749c4..e282d78670 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml @@ -33,6 +33,7 @@
+ - + From 292303bec07ea4dafb52e62924a71abd1e8565d3 Mon Sep 17 00:00:00 2001 From: OliZor Date: Wed, 29 Sep 2021 12:00:38 +0100 Subject: [PATCH 07/15] HEEDLS-438 Remove categoryId from courseAdminFields methods, and change data service method for Service Filter to return a bool --- .../CourseAdminFieldsDataServiceTests.cs | 6 ++-- .../DataServices/CourseDataServiceTests.cs | 8 ++--- .../Services/CourseAdminFieldsServiceTests.cs | 13 ++++---- .../Services/CourseServiceTests.cs | 24 +++++++------- .../CourseAdminFieldsDataService.cs | 6 ++-- .../DataServices/CourseDataService.cs | 23 ++++++++----- .../Services/CourseAdminFieldsService.cs | 27 ++++++--------- .../Services/CourseService.cs | 10 +++--- .../CourseSetup/AdminFieldsControllerTests.cs | 12 ++++--- .../CourseSetup/AdminFieldsController.cs | 33 ++++++++++++++----- 10 files changed, 86 insertions(+), 76 deletions(-) diff --git a/DigitalLearningSolutions.Data.Tests/DataServices/CourseAdminFieldsDataServiceTests.cs b/DigitalLearningSolutions.Data.Tests/DataServices/CourseAdminFieldsDataServiceTests.cs index 6a9b46cc6d..b20b3c1e87 100644 --- a/DigitalLearningSolutions.Data.Tests/DataServices/CourseAdminFieldsDataServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/DataServices/CourseAdminFieldsDataServiceTests.cs @@ -35,7 +35,7 @@ public void GetCourseAdminFields_returns_populated_CourseAdminFieldsResult() ); // When - var returnedCourseAdminFieldsResult = courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0); + var returnedCourseAdminFieldsResult = courseAdminFieldsDataService.GetCourseAdminFields(100, 101); // Then returnedCourseAdminFieldsResult.Should().BeEquivalentTo(expectedCourseAdminFieldsResult); @@ -52,7 +52,7 @@ public void UpdateCustomPromptForCourse_correctly_updates_custom_prompt() // When courseAdminFieldsDataService.UpdateCustomPromptForCourse(100, 1, 1, options); - var courseAdminFields = courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0); + var courseAdminFields = courseAdminFieldsDataService.GetCourseAdminFields(100, 101); // Then using (new AssertionScope()) @@ -87,7 +87,7 @@ public void UpdateCustomPromptForCourse_correctly_adds_custom_prompt() // When courseAdminFieldsDataService.UpdateCustomPromptForCourse(100, 3, 1, options); - var courseCustomPrompts = courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 2); + var courseCustomPrompts = courseAdminFieldsDataService.GetCourseAdminFields(100, 101); var customPrompt = courseAdminFieldsDataService.GetCoursePromptsAlphabetical() .Single(c => c.id == 1) .name; diff --git a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs index d76ec999a1..871b47a884 100644 --- a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs @@ -9,7 +9,6 @@ namespace DigitalLearningSolutions.Data.Tests.DataServices using DigitalLearningSolutions.Data.Tests.TestHelpers; using FakeItEasy; using FluentAssertions; - using FluentAssertions.Execution; using Microsoft.Extensions.Logging; using NUnit.Framework; @@ -366,14 +365,13 @@ public void GetCentrallyManagedAndCentreCourses_returns_expected_values() } [Test] - public void GetCentreIdAndCategoryIdForCourse_returns_expected_values() + public void DoesCourseExistAtCentre_returns_true_if_course_exists() { // When - var (centreId, categoryId) = courseDataService.GetCentreIdAndCategoryIdForCourse(1); + var result = courseDataService.DoesCourseExistAtCentre(100, 101, null); // Then - centreId.Should().Be(2); - categoryId.Should().Be(2); + result.Should().Be(true); } } } diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseAdminFieldsServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseAdminFieldsServiceTests.cs index 52912fef57..6ecfb8f9a7 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseAdminFieldsServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseAdminFieldsServiceTests.cs @@ -35,11 +35,11 @@ public void GetCustomPromptsForCourse_Returns_Populated_CourseAdminFields() var expectedPrompt2 = CustomPromptsTestHelper.GetDefaultCustomPrompt(2, "Priority Access"); var customPrompts = new List { expectedPrompt1, expectedPrompt2 }; var expectedCourseAdminFields = CustomPromptsTestHelper.GetDefaultCourseAdminFields(customPrompts); - A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0)) + A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFieldsResult()); // When - var result = courseAdminFieldsService.GetCustomPromptsForCourse(100, 101, 0); + var result = courseAdminFieldsService.GetCustomPromptsForCourse(100, 101); // Then result.Should().BeEquivalentTo(expectedCourseAdminFields); @@ -63,7 +63,7 @@ public void GetCustomPromptsWithAnswersForCourse_Returns_Populated_List_of_Custo answer: answer2 ); var expected = new List { expected1, expected2 }; - A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0)) + A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFieldsResult()); var delegateCourseInfo = new DelegateCourseInfo { Answer1 = answer1, Answer2 = answer2 }; @@ -112,11 +112,11 @@ public void AddCustomPromptToCourse_adds_prompt_to_course_at_next_prompt_number( ( () => courseAdminFieldsDataService.UpdateCustomPromptForCourse(100, A._, A._, null) ).DoesNothing(); - A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0)) + A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFieldsResult()); // When - var result = courseAdminFieldsService.AddCustomPromptToCourse(100, 101, 0, 3, null); + var result = courseAdminFieldsService.AddCustomPromptToCourse(100, 101, 3, null); // Then A.CallTo @@ -134,7 +134,7 @@ public void AddCustomPromptToCourse_does_not_add_prompt_if_course_has_all_prompt ( () => courseAdminFieldsDataService.UpdateCustomPromptForCourse(100, A._, A._, null) ).DoesNothing(); - A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101, 0)) + A.CallTo(() => courseAdminFieldsDataService.GetCourseAdminFields(100, 101)) .Returns( CustomPromptsTestHelper.GetDefaultCourseAdminFieldsResult( "System Access Granted", @@ -149,7 +149,6 @@ public void AddCustomPromptToCourse_does_not_add_prompt_if_course_has_all_prompt var result = courseAdminFieldsService.AddCustomPromptToCourse( 100, 101, - 0, 3, "Adding a fourth prompt" ); diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs index 0aad9452c5..612289c71d 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs @@ -109,8 +109,7 @@ public void GetAllCoursesForDelegate_should_call_correct_data_service_and_helper () => courseAdminFieldsService.GetCustomPromptsWithAnswersForCourse( info, customisationId, - CentreId, - 0 + CentreId ) ).MustHaveHappened(1, Times.Exactly); A.CallTo(() => courseDataService.GetDelegateCourseAttemptStats(delegateId, customisationId)) @@ -140,8 +139,7 @@ public void GetAllCoursesForDelegate_should_not_fetch_attempt_stats_if_course_no () => courseAdminFieldsService.GetCustomPromptsWithAnswersForCourse( info, customisationId, - CentreId, - 0 + CentreId ) ).MustHaveHappened(1, Times.Exactly); A.CallTo(() => courseDataService.GetDelegateCourseAttemptStats(A._, A._)).MustNotHaveHappened(); @@ -154,14 +152,14 @@ public void GetAllCoursesForDelegate_should_not_fetch_attempt_stats_if_course_no public void VerifyAdminUserCanAccessCourse_should_call_correct_data_service_method() { // Given - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) - .Returns((2, 2)); + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) + .Returns(true); // When var result = courseService.VerifyAdminUserCanAccessCourse(1, 2, 2); // Then - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) .MustHaveHappened(1, Times.Exactly); result.Should().BeTrue(); } @@ -170,14 +168,14 @@ public void VerifyAdminUserCanAccessCourse_should_call_correct_data_service_meth public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_centreId() { // Given - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) - .Returns((2, 2)); + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) + .Returns(false); // When var result = courseService.VerifyAdminUserCanAccessCourse(1, 1, 2); // Then - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) .MustHaveHappened(1, Times.Exactly); result.Should().BeFalse(); } @@ -186,13 +184,13 @@ public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incor public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_categoryId() { // Given - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) - .Returns((2, 2)); + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) + .Returns(false); // When var result = courseService.VerifyAdminUserCanAccessCourse(1, 2, 1); // Then - A.CallTo(() => courseDataService.GetCentreIdAndCategoryIdForCourse(A._)) + A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) .MustHaveHappened(1, Times.Exactly); result.Should().BeFalse(); } diff --git a/DigitalLearningSolutions.Data/DataServices/CourseAdminFieldsDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseAdminFieldsDataService.cs index f1a0e29fcd..5cf115918c 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseAdminFieldsDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseAdminFieldsDataService.cs @@ -8,7 +8,7 @@ public interface ICourseAdminFieldsDataService { - CourseAdminFieldsResult? GetCourseAdminFields(int customisationId, int centreId, int categoryId); + CourseAdminFieldsResult? GetCourseAdminFields(int customisationId, int centreId); void UpdateCustomPromptForCourse(int customisationId, int promptNumber, string? options); @@ -37,7 +37,7 @@ public CourseAdminFieldsDataService(IDbConnection connection) this.connection = connection; } - public CourseAdminFieldsResult GetCourseAdminFields(int customisationId, int centreId, int categoryId) + public CourseAdminFieldsResult GetCourseAdminFields(int customisationId, int centreId) { var result = connection.Query( @"SELECT @@ -63,7 +63,7 @@ LEFT JOIN CoursePrompts AS cp3 WHERE cu.CentreID = @centreId AND ap.ArchivedDate IS NULL AND cu.CustomisationID = @customisationId", - new { customisationId, centreId, categoryId } + new { customisationId, centreId } ).Single(); return result; diff --git a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs index d7034b98fe..ac44c06c4f 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs @@ -23,7 +23,7 @@ public interface ICourseDataService CourseNameInfo? GetCourseNameAndApplication(int customisationId); CourseDetails? GetCourseDetailsForAdminCategoryId(int customisationId, int centreId, int categoryId); IEnumerable GetCentrallyManagedAndCentreCourses(int centreId, int? categoryId); - (int? centreId, int? categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId); + bool DoesCourseExistAtCentre(int customisationId, int centreId, int? categoryId); } public class CourseDataService : ICourseDataService @@ -347,15 +347,20 @@ FROM Customisations AS c ); } - public (int? centreId, int? categoryId) GetCentreIdAndCategoryIdForCourse(int customisationId) + public bool DoesCourseExistAtCentre(int customisationId, int centreId, int? categoryId) { - return connection.QueryFirstOrDefault<(int, int)>( - @"SELECT c.CentreID, - a.CourseCategoryID - FROM Customisations AS c - JOIN Applications AS a on a.ApplicationID = c.ApplicationID - WHERE CustomisationID = @customisationId", - new { customisationId } + return connection.QueryFirstOrDefault( + @"SELECT CASE WHEN EXISTS ( + SELECT * + FROM Customisations AS c + JOIN Applications AS a on a.ApplicationID = c.ApplicationID + WHERE CustomisationID = @customisationId + AND c.CentreID = @centreId + AND (a.CourseCategoryID = 0 OR @categoryId IS NULL) + ) + THEN CAST(1 AS BIT) + ELSE CAST(0 AS BIT) END", + new { customisationId, centreId, categoryId } ); } } diff --git a/DigitalLearningSolutions.Data/Services/CourseAdminFieldsService.cs b/DigitalLearningSolutions.Data/Services/CourseAdminFieldsService.cs index 0b9d76eee4..03c7b8b6ac 100644 --- a/DigitalLearningSolutions.Data/Services/CourseAdminFieldsService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseAdminFieldsService.cs @@ -11,13 +11,12 @@ public interface ICourseAdminFieldsService { - public CourseAdminFields GetCustomPromptsForCourse(int customisationId, int centreId, int categoryId); + public CourseAdminFields GetCustomPromptsForCourse(int customisationId, int centreId); public List GetCustomPromptsWithAnswersForCourse( DelegateCourseInfo delegateCourseInfo, int customisationId, - int centreId, - int categoryId = 0 + int centreId ); public void UpdateCustomPromptForCourse(int customisationId, int promptId, string? options); @@ -27,7 +26,6 @@ public List GetCustomPromptsWithAnswersForCourse( public bool AddCustomPromptToCourse( int customisationId, int centreId, - int categoryId, int promptId, string? options ); @@ -53,11 +51,10 @@ ILogger logger public CourseAdminFields GetCustomPromptsForCourse( int customisationId, - int centreId, - int categoryId = 0 + int centreId ) { - var result = courseAdminFieldsDataService.GetCourseAdminFields(customisationId, centreId, categoryId); + var result = courseAdminFieldsDataService.GetCourseAdminFields(customisationId, centreId); return new CourseAdminFields( customisationId, centreId, @@ -68,11 +65,10 @@ public CourseAdminFields GetCustomPromptsForCourse( public List GetCustomPromptsWithAnswersForCourse( DelegateCourseInfo delegateCourseInfo, int customisationId, - int centreId, - int categoryId = 0 + int centreId ) { - var result = GetCourseCustomPromptsResultForCourse(customisationId, centreId, categoryId); + var result = GetCourseCustomPromptsResultForCourse(customisationId, centreId); return PopulateCustomPromptWithAnswerListFromCourseAdminFieldsResult(result, delegateCourseInfo); } @@ -90,15 +86,13 @@ public void UpdateCustomPromptForCourse(int customisationId, int promptId, strin public bool AddCustomPromptToCourse( int customisationId, int centreId, - int categoryId, int promptId, string? options ) { var courseAdminFields = GetCustomPromptsForCourse( customisationId, - centreId, - categoryId + centreId ); var promptNumber = GetNextPromptNumber(courseAdminFields); @@ -157,12 +151,11 @@ public string GetPromptName(int customisationId, int promptNumber) private CourseAdminFieldsResult? GetCourseCustomPromptsResultForCourse( int customisationId, - int centreId, - int categoryId + int centreId ) { - var result = courseAdminFieldsDataService.GetCourseAdminFields(customisationId, centreId, categoryId); - if (result == null || categoryId != 0 && result.CourseCategoryId != categoryId) + var result = courseAdminFieldsDataService.GetCourseAdminFields(customisationId, centreId); + if (result == null) { return null; } diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 76ca69c5da..43680adef0 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -10,7 +10,7 @@ public interface ICourseService public IEnumerable GetTopCourseStatistics(int centreId, int categoryId); public IEnumerable GetCentreSpecificCourseStatistics(int centreId, int categoryId); public IEnumerable GetAllCoursesForDelegate(int delegateId, int centreId); - public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int categoryId); + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int? categoryId); } public class CourseService : ICourseService @@ -54,12 +54,10 @@ public IEnumerable GetAllCoursesForDelegate(int delegateI ); } - public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int categoryId) + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int? categoryId) { - var (courseCentreId, courseCategoryId) = - courseDataService.GetCentreIdAndCategoryIdForCourse(customisationId); - - return centreId == courseCentreId && (categoryId == courseCategoryId || categoryId == 0); + var nullableCategoryId = categoryId == 0 ? null : categoryId; + return courseDataService.DoesCourseExistAtCentre(customisationId, centreId, nullableCategoryId); } } } diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 64f3472c3d..3733c36a27 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -23,6 +23,7 @@ public class AdminFieldsControllerTests A.Fake(); private readonly ICourseAdminFieldsService courseAdminFieldsService = A.Fake(); + private readonly ICourseService courseService = A.Fake(); private AdminFieldsController controller = null!; [SetUp] @@ -30,7 +31,8 @@ public void Setup() { controller = new AdminFieldsController( courseAdminFieldsService, - courseAdminFieldsDataService + courseAdminFieldsDataService, + courseService ) .WithDefaultContext() .WithMockUser(true, 101) @@ -43,7 +45,7 @@ public void AdminFields_returns_AdminFields_page_when_appropriate_course_found() // Given var samplePrompt1 = CustomPromptsTestHelper.GetDefaultCustomPrompt(1, "System Access Granted", "Yes\r\nNo"); var customPrompts = new List { samplePrompt1 }; - A.CallTo(() => courseAdminFieldsService.GetCustomPromptsForCourse(A._, A._, A._)) + A.CallTo(() => courseAdminFieldsService.GetCustomPromptsForCourse(A._, A._)) .Returns(CustomPromptsTestHelper.GetDefaultCourseAdminFields(customPrompts)); // When @@ -169,6 +171,9 @@ public void AdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() controller.TempData.Set(initialTempData); + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) + .Returns(true); + // When var result = controller.EditAdminFieldBulkPost(inputViewModel); @@ -219,7 +224,6 @@ public void AddAdminField_save_clears_temp_data_and_redirects_to_index() () => courseAdminFieldsService.AddCustomPromptToCourse( 100, 101, - 0, 1, "Test" ) @@ -249,7 +253,6 @@ public void AddAdminField_save_redirects_successfully_without_answers_configured () => courseAdminFieldsService.AddCustomPromptToCourse( 100, 101, - 0, 1, null ) @@ -279,7 +282,6 @@ public void AddAdminField_calls_service_and_redirects_to_error_on_failure() () => courseAdminFieldsService.AddCustomPromptToCourse( 100, 101, - 0, 1, "Test" ) diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index a480236eff..af41fc705c 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -28,14 +28,17 @@ public class AdminFieldsController : Controller private static readonly DateTimeOffset CookieExpiry = DateTimeOffset.UtcNow.AddDays(7); private readonly ICourseAdminFieldsDataService courseAdminFieldsDataService; private readonly ICourseAdminFieldsService courseAdminFieldsService; + private readonly ICourseService courseService; public AdminFieldsController( ICourseAdminFieldsService courseAdminFieldsService, - ICourseAdminFieldsDataService courseAdminFieldsDataService + ICourseAdminFieldsDataService courseAdminFieldsDataService, + ICourseService courseService ) { this.courseAdminFieldsService = courseAdminFieldsService; this.courseAdminFieldsDataService = courseAdminFieldsDataService; + this.courseService = courseService; } [HttpGet] @@ -44,12 +47,10 @@ ICourseAdminFieldsDataService courseAdminFieldsDataService public IActionResult Index(int customisationId) { var centreId = User.GetCentreId(); - var categoryId = User.GetAdminCategoryId()!; var courseAdminFields = courseAdminFieldsService.GetCustomPromptsForCourse( customisationId, - centreId, - categoryId.Value + centreId ); var model = new AdminFieldsViewModel(courseAdminFields.AdminFields, customisationId); @@ -75,8 +76,7 @@ public IActionResult EditAdminField(int customisationId, int promptNumber) var courseAdminField = courseAdminFieldsService.GetCustomPromptsForCourse( customisationId, - centreId, - categoryId.Value + centreId ).AdminFields .Single(cp => cp.CustomPromptNumber == promptNumber); @@ -130,12 +130,17 @@ public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) } [HttpPost] - [Route("AdminFieldsEdit/Bulk")] + [Route("AdminFields/Edit/Bulk")] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulkPost( BulkAdminFieldAnswersViewModel model ) { + if (!VerifyAdminUserCanAccessCourse(model.CustomisationId)) + { + return NotFound(); + } + ValidateBulkOptionsString(model.OptionsString); if (!ModelState.IsValid) { @@ -338,7 +343,6 @@ private IActionResult AddAdminFieldPostSave(AddAdminFieldViewModel model) if (courseAdminFieldsService.AddCustomPromptToCourse( model.CustomisationId, centreId, - categoryId.Value, model.AdminFieldId!.Value, model.OptionsString )) @@ -531,5 +535,18 @@ private void ValidateBulkOptionsString(string? optionsString) ); } } + + private bool VerifyAdminUserCanAccessCourse(int customisationId) + { + var centreId = User.GetCentreId(); + var categoryId = User.GetAdminCategoryId()!; + + if (courseService.VerifyAdminUserCanAccessCourse(customisationId, centreId, categoryId.Value)) + { + return true; + } + + return false; + } } } From 58934120371a1b63acfe189930527c2b56669431 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 13:50:16 +0100 Subject: [PATCH 08/15] HEEDLS-438 Remove CustomisationId from admin fields models, and other markups --- .../DataServices/CourseDataServiceTests.cs | 22 +++++- .../Services/CourseServiceTests.cs | 19 +---- .../DataServices/CourseDataService.cs | 2 +- .../AddCourseAdminFieldsAccessibilityTests.cs | 2 - .../CourseSetup/AdminFieldsControllerTests.cs | 66 ++++++++-------- .../VerifyAdminUserCanAccessCourseTests.cs | 51 ++++--------- .../CourseSetup/AdminFieldsController.cs | 76 +++++++------------ .../CourseSetup/AddAdminFieldViewModel.cs | 8 -- .../CourseSetup/AdminFieldAnswersViewModel.cs | 4 - .../BulkAdminFieldAnswersViewModel.cs | 8 +- .../CourseSetup/EditAdminFieldViewModel.cs | 5 +- .../CourseSetup/RemoveAdminFieldViewModel.cs | 5 +- .../AdminFields/AddAdminField.cshtml | 2 +- .../AdminFields/BulkAdminFieldAnswers.cshtml | 5 +- .../AdminFields/EditAdminField.cshtml | 2 +- .../AdminFields/RemoveAdminField.cshtml | 2 +- 16 files changed, 109 insertions(+), 170 deletions(-) diff --git a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs index 871b47a884..540f100cdf 100644 --- a/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/DataServices/CourseDataServiceTests.cs @@ -371,7 +371,27 @@ public void DoesCourseExistAtCentre_returns_true_if_course_exists() var result = courseDataService.DoesCourseExistAtCentre(100, 101, null); // Then - result.Should().Be(true); + result.Should().BeTrue(); + } + + [Test] + public void DoesCourseExistAtCentre_returns_false_if_course_does_not_exist_at_centre() + { + // When + var result = courseDataService.DoesCourseExistAtCentre(100, 2, 0); + + // Then + result.Should().BeFalse(); + } + + [Test] + public void DoesCourseExistAtCentre_returns_false_if_course_does_not_exist_with_categoryId() + { + // When + var result = courseDataService.DoesCourseExistAtCentre(100, 101, 99); + + // Then + result.Should().BeFalse(); } } } diff --git a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs index 612289c71d..497e1b97bf 100644 --- a/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs +++ b/DigitalLearningSolutions.Data.Tests/Services/CourseServiceTests.cs @@ -165,34 +165,19 @@ public void VerifyAdminUserCanAccessCourse_should_call_correct_data_service_meth } [Test] - public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_centreId() + public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_ids() { // Given A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) .Returns(false); // When - var result = courseService.VerifyAdminUserCanAccessCourse(1, 1, 2); + var result = courseService.VerifyAdminUserCanAccessCourse(1, 1, 1); // Then A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) .MustHaveHappened(1, Times.Exactly); result.Should().BeFalse(); } - - [Test] - public void VerifyAdminUserCanAccessCourse_should_return_return_false_with_incorrect_categoryId() - { - // Given - A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) - .Returns(false); - - // When - var result = courseService.VerifyAdminUserCanAccessCourse(1, 2, 1); - // Then - A.CallTo(() => courseDataService.DoesCourseExistAtCentre(A._, A._, A._)) - .MustHaveHappened(1, Times.Exactly); - result.Should().BeFalse(); - } } } diff --git a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs index ac44c06c4f..2469b2136a 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs @@ -349,7 +349,7 @@ FROM Customisations AS c public bool DoesCourseExistAtCentre(int customisationId, int centreId, int? categoryId) { - return connection.QueryFirstOrDefault( + return connection.ExecuteScalar( @"SELECT CASE WHEN EXISTS ( SELECT * FROM Customisations AS c diff --git a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs index 5bfb65339d..937b808544 100644 --- a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs +++ b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/AddCourseAdminFieldsAccessibilityTests.cs @@ -35,12 +35,10 @@ public void AddAdminField_journey_has_no_accessibility_errors() Driver.ClickButtonByText("Submit"); ValidatePageHeading("Add course admin field"); - var bulkPostResult = new AxeBuilder(Driver).Analyze(); addPageResult.Violations.Should().BeEmpty(); bulkAdditionResult.Violations.Should().BeEmpty(); addAnswersResult.Violations.Should().BeEmpty(); - bulkPostResult.Violations.Should().BeEmpty(); } private void AddAnswer(string answerString) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 3733c36a27..f966b40399 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -59,7 +59,7 @@ public void AdminFields_returns_AdminFields_page_when_appropriate_course_found() public void PostEditAdminField_save_calls_correct_methods() { // Given - var model = new EditAdminFieldViewModel(1, 1, "Test", "Options"); + var model = new EditAdminFieldViewModel(1, "Test", "Options"); const string action = "save"; A.CallTo( @@ -71,7 +71,7 @@ public void PostEditAdminField_save_calls_correct_methods() ).DoesNothing(); // When - var result = controller.EditAdminField(model, action, 1, 1); + var result = controller.EditAdminField(1, model, action); // Then A.CallTo( @@ -88,7 +88,7 @@ public void PostEditAdminField_save_calls_correct_methods() public void PostEditAdminField_add_configures_new_answer() { // Given - var model = new EditAdminFieldViewModel(1, 1, "Test", "Options"); + var model = new EditAdminFieldViewModel(1, "Test", "Options"); const string action = "addPrompt"; A.CallTo( @@ -100,7 +100,7 @@ public void PostEditAdminField_add_configures_new_answer() ).DoesNothing(); // When - var result = controller.EditAdminField(model, action, 1, 1); + var result = controller.EditAdminField(1, model, action); // Then using (new AssertionScope()) @@ -114,11 +114,11 @@ public void PostEditAdminField_add_configures_new_answer() public void PostEditAdminField_delete_removes_configured_answer() { // Given - var model = new EditAdminFieldViewModel(1, 1, "Test", "Test\r\nAnswer"); + var model = new EditAdminFieldViewModel(1, "Test", "Test\r\nAnswer"); const string action = "delete0"; // When - var result = controller.EditAdminField(model, action, 1, 1); + var result = controller.EditAdminField(1, model, action); // Then using (new AssertionScope()) @@ -132,11 +132,11 @@ public void PostEditAdminField_delete_removes_configured_answer() public void PostAdminField_bulk_sets_up_temp_data_and_redirects() { // Given - var model = new EditAdminFieldViewModel(1, 1, "Test", "Options"); + var model = new EditAdminFieldViewModel(1, "Test", "Options"); const string action = "bulk"; // When - var result = controller.EditAdminField(model, action, 1, 1); + var result = controller.EditAdminField(1, model, action); // Then using (new AssertionScope()) @@ -150,11 +150,11 @@ public void PostAdminField_bulk_sets_up_temp_data_and_redirects() public void PostEditAdminField_returns_error_with_unexpected_action() { // Given - var model = new EditAdminFieldViewModel(1, 1, "Test", "Options"); + var model = new EditAdminFieldViewModel(1, "Test", "Options"); const string action = "deletetest"; // When - var result = controller.EditAdminField(model, action, 1, 1); + var result = controller.EditAdminField(1, model, action); // Then result.Should().BeStatusCodeResult().WithStatusCode(500); @@ -164,9 +164,9 @@ public void PostEditAdminField_returns_error_with_unexpected_action() public void AdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() { // Given - var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1, 1); - var initialEditViewModel = new EditAdminFieldViewModel(1, 1, "Test", "Test"); - var expectedViewModel = new EditAdminFieldViewModel(1, 1, "Test", "Test\r\nAnswer"); + var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1); + var initialEditViewModel = new EditAdminFieldViewModel(1, "Test", "Test"); + var expectedViewModel = new EditAdminFieldViewModel(1, "Test", "Test\r\nAnswer"); var initialTempData = new EditAdminFieldData(initialEditViewModel); controller.TempData.Set(initialTempData); @@ -175,7 +175,7 @@ public void AdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() .Returns(true); // When - var result = controller.EditAdminFieldBulkPost(inputViewModel); + var result = controller.EditAdminFieldBulkPost(1, inputViewModel); // Then using (new AssertionScope()) @@ -199,7 +199,7 @@ public void AddAdminFieldNew_sets_new_temp_data() [Test] public void AddAdminField_post_updates_temp_data_and_redirects() { - var expectedPromptModel = new AddAdminFieldViewModel(1); + var expectedPromptModel = new AddAdminFieldViewModel(); var initialTempData = new AddAdminFieldData(expectedPromptModel); controller.TempData.Set(initialTempData); @@ -215,7 +215,7 @@ public void AddAdminField_post_updates_temp_data_and_redirects() public void AddAdminField_save_clears_temp_data_and_redirects_to_index() { // Given - var model = new AddAdminFieldViewModel(100, 1, "Test"); + var model = new AddAdminFieldViewModel(1, "Test"); const string action = "save"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -244,7 +244,7 @@ public void AddAdminField_save_clears_temp_data_and_redirects_to_index() public void AddAdminField_save_redirects_successfully_without_answers_configured() { // Given - var model = new AddAdminFieldViewModel(100, 1, null); + var model = new AddAdminFieldViewModel(1, null); const string action = "save"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -273,7 +273,7 @@ public void AddAdminField_save_redirects_successfully_without_answers_configured public void AddAdminField_calls_service_and_redirects_to_error_on_failure() { // Given - var model = new AddAdminFieldViewModel(100, 1, "Test"); + var model = new AddAdminFieldViewModel(1, "Test"); const string action = "save"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -300,11 +300,11 @@ public void AddAdminField_calls_service_and_redirects_to_error_on_failure() [Test] public void AddAdminField_add_configures_new_answer_and_updates_temp_data() { - var initialViewModel = new AddAdminFieldViewModel(1, 1, "Test", "Answer"); + var initialViewModel = new AddAdminFieldViewModel(1, "Test", "Answer"); var initialTempData = new AddAdminFieldData(initialViewModel); controller.TempData.Set(initialTempData); - var expectedViewModel = new AddAdminFieldViewModel(1, 1, "Test\r\nAnswer"); + var expectedViewModel = new AddAdminFieldViewModel(1, "Test\r\nAnswer"); const string action = "addPrompt"; // When @@ -323,11 +323,11 @@ public void AddAdminField_add_configures_new_answer_and_updates_temp_data() [Test] public void AddAdminField_adds_answer_without_admin_field_selected() { - var initialViewModel = new AddAdminFieldViewModel(1, null, null, "Answer"); + var initialViewModel = new AddAdminFieldViewModel(null, null, "Answer"); var initialTempData = new AddAdminFieldData(initialViewModel); controller.TempData.Set(initialTempData); - var expectedViewModel = new AddAdminFieldViewModel(1, null, "Answer"); + var expectedViewModel = new AddAdminFieldViewModel(null, "Answer"); const string action = "addPrompt"; // When @@ -344,7 +344,7 @@ public void AddAdminField_adds_answer_without_admin_field_selected() public void AddAdminField_delete_removes_configured_answer() { // Given - var model = new AddAdminFieldViewModel(1, 1, "Test\r\nAnswer"); + var model = new AddAdminFieldViewModel(1, "Test\r\nAnswer"); const string action = "delete0"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -363,7 +363,7 @@ public void AddAdminField_delete_removes_configured_answer() [Test] public void AddAdminField_removes_answer_without_admin_field_selected() { - var model = new AddAdminFieldViewModel(1, null, "Test\r\nAnswer"); + var model = new AddAdminFieldViewModel(null, "Test\r\nAnswer"); const string action = "delete0"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -383,7 +383,7 @@ public void AddAdminField_removes_answer_without_admin_field_selected() public void AddAdminField_bulk_sets_up_temp_data_and_redirects() { // Given - var model = new AddAdminFieldViewModel(1, 1, "Options"); + var model = new AddAdminFieldViewModel(1, "Options"); const string action = "bulk"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -403,7 +403,7 @@ public void AddAdminField_bulk_sets_up_temp_data_and_redirects() public void AddAdminField_bulk_redirects_without_admin_field_selected() { // Given - var model = new AddAdminFieldViewModel(1, null, "Options"); + var model = new AddAdminFieldViewModel(null, "Options"); const string action = "bulk"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -423,7 +423,7 @@ public void AddAdminField_bulk_redirects_without_admin_field_selected() public void AddAdminField_returns_error_with_unexpected_action() { // Given - var model = new AddAdminFieldViewModel(1); + var model = new AddAdminFieldViewModel(); const string action = "deletetest"; var initialTempData = new AddAdminFieldData(model); controller.TempData.Set(initialTempData); @@ -440,14 +440,14 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() { // Given var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1); - var initialAddViewModel = new AddAdminFieldViewModel(1, 1, "Test"); - var expectedViewModel = new AddAdminFieldViewModel(1, 1, "Test\r\nAnswer"); + var initialAddViewModel = new AddAdminFieldViewModel(1, "Test"); + var expectedViewModel = new AddAdminFieldViewModel(1, "Test\r\nAnswer"); var initialTempData = new AddAdminFieldData(initialAddViewModel); controller.TempData.Set(initialTempData); // When - var result = controller.AddAdminFieldAnswersBulk(inputViewModel, 1); + var result = controller.AddAdminFieldAnswersBulk(1, inputViewModel); // Then using (new AssertionScope()) @@ -471,7 +471,7 @@ public void RemoveAdminField_removes_admin_field_with_no_user_answers() public void RemoveAdminField_returns_remove_view_if_admin_field_has_user_answers() { // Given - var removeViewModel = new RemoveAdminFieldViewModel(100, "System Access Granted", 1); + var removeViewModel = new RemoveAdminFieldViewModel("System Access Granted", 1); // When var result = controller.RemoveAdminField(100, 1, removeViewModel); @@ -484,7 +484,7 @@ public void RemoveAdminField_returns_remove_view_if_admin_field_has_user_answers public void RemoveAdminField_does_not_remove_admin_field_without_confirmation() { // Given - var removeViewModel = new RemoveAdminFieldViewModel(100, "System Access Granted", 1); + var removeViewModel = new RemoveAdminFieldViewModel("System Access Granted", 1); removeViewModel.Confirm = false; var expectedErrorMessage = "You must confirm before deleting this field"; @@ -501,7 +501,7 @@ public void RemoveAdminField_does_not_remove_admin_field_without_confirmation() public void RemoveAdminField_removes_admin_field_with_confirmation_and_redirects() { // Given - var removeViewModel = new RemoveAdminFieldViewModel(100, "System Access Granted", 1); + var removeViewModel = new RemoveAdminFieldViewModel("System Access Granted", 1); removeViewModel.Confirm = true; // When diff --git a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs index 033e2f0fcd..3637131e0a 100644 --- a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs +++ b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs @@ -23,18 +23,7 @@ public class VerifyAdminUserCanAccessCourseTests public void Returns_NotFound_if_service_returns_false() { // Given - var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() - .WithMockUser(true, 101); - var context = new ActionExecutingContext( - new ActionContext( - new DefaultHttpContext(), - new RouteData(new RouteValueDictionary()), - new ActionDescriptor() - ), - new List(), - new Dictionary(), - homeController - ); + var context = SetupContext(); context.RouteData.Values["customisationId"] = 2; A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(false); @@ -43,13 +32,6 @@ public void Returns_NotFound_if_service_returns_false() new VerifyAdminUserCanAccessCourse(courseService).OnActionExecuting(context); // Then - A.CallTo( - () => courseService.VerifyAdminUserCanAccessCourse( - A._, - A._, - A._ - ) - ).MustHaveHappened(); context.Result.Should().BeNotFoundResult(); } @@ -57,6 +39,20 @@ public void Returns_NotFound_if_service_returns_false() public void Does_not_return_NotFound_if_service_returns_true() { // Given + var context = SetupContext(); + context.RouteData.Values["customisationId"] = 24286; + A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) + .Returns(true); + + // When + new VerifyAdminUserCanAccessCourse(courseService).OnActionExecuting(context); + + // Then + context.Result.Should().BeNull(); + } + + private ActionExecutingContext SetupContext() + { var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() .WithMockUser(true, 101); var context = new ActionExecutingContext( @@ -69,22 +65,7 @@ public void Does_not_return_NotFound_if_service_returns_true() new Dictionary(), homeController ); - context.RouteData.Values["customisationId"] = 24286; - A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) - .Returns(true); - - // When - new VerifyAdminUserCanAccessCourse(courseService).OnActionExecuting(context); - - // Then - A.CallTo( - () => courseService.VerifyAdminUserCanAccessCourse( - A._, - A._, - A._ - ) - ).MustHaveHappened(); - context.Result.Should().BeNull(); + return context; } } } diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index af41fc705c..45ab1cc7f5 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -72,7 +72,6 @@ public IActionResult EditAdminFieldStart(int customisationId, int promptNumber) public IActionResult EditAdminField(int customisationId, int promptNumber) { var centreId = User.GetCentreId(); - var categoryId = User.GetAdminCategoryId()!; var courseAdminField = courseAdminFieldsService.GetCustomPromptsForCourse( customisationId, @@ -82,7 +81,7 @@ public IActionResult EditAdminField(int customisationId, int promptNumber) var data = TempData.Get(); - var model = data?.EditModel ?? new EditAdminFieldViewModel(courseAdminField, customisationId); + var model = data?.EditModel ?? new EditAdminFieldViewModel(courseAdminField); return View(model); } @@ -91,10 +90,9 @@ public IActionResult EditAdminField(int customisationId, int promptNumber) [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] public IActionResult EditAdminField( - EditAdminFieldViewModel model, - string action, int customisationId, - int promptNumber + EditAdminFieldViewModel model, + string action ) { if (action.StartsWith(DeleteAction) && TryGetAnswerIndexFromDeleteAction(action, out var index)) @@ -104,9 +102,9 @@ int promptNumber return action switch { - SaveAction => EditAdminFieldPostSave(model), + SaveAction => EditAdminFieldPostSave(customisationId, model), AddPromptAction => AdminFieldAnswersPostAddPrompt(model), - BulkAction => EditAdminFieldBulk(model), + BulkAction => EditAdminFieldBulk(customisationId, model), _ => new StatusCodeResult(500) }; } @@ -122,7 +120,6 @@ public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) var model = new BulkAdminFieldAnswersViewModel( data.EditModel.OptionsString, false, - customisationId, promptNumber ); @@ -130,17 +127,14 @@ public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) } [HttpPost] - [Route("AdminFields/Edit/Bulk")] + [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Bulk")] + [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulkPost( + int customisationId, BulkAdminFieldAnswersViewModel model ) { - if (!VerifyAdminUserCanAccessCourse(model.CustomisationId)) - { - return NotFound(); - } - ValidateBulkOptionsString(model.OptionsString); if (!ModelState.IsValid) { @@ -154,7 +148,7 @@ BulkAdminFieldAnswersViewModel model return RedirectToAction( "EditAdminField", - new { customisationId = model.CustomisationId, promptNumber = model.PromptNumber } + new { customisationId, promptNumber = model.PromptNumber } ); } @@ -164,11 +158,11 @@ public IActionResult AddAdminFieldNew(int customisationId) { TempData.Clear(); - var model = new AddAdminFieldViewModel(customisationId); + var model = new AddAdminFieldViewModel(); SetAddAdminFieldTempData(model); - return RedirectToAction("AddAdminField", new { customisationId = model.CustomisationId }); + return RedirectToAction("AddAdminField", new { customisationId }); } [HttpGet] @@ -201,9 +195,9 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m return action switch { - SaveAction => AddAdminFieldPostSave(model), + SaveAction => AddAdminFieldPostSave(customisationId, model), AddPromptAction => AdminFieldAnswersPostAddPrompt(model), - BulkAction => AddAdminFieldBulk(model), + BulkAction => AddAdminFieldBulk(customisationId, model), _ => new StatusCodeResult(500) }; } @@ -217,8 +211,7 @@ public IActionResult AddAdminFieldAnswersBulk(int customisationId) var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( data.AddModel.OptionsString, - true, - customisationId + true ); return View("BulkAdminFieldAnswers", model); @@ -229,8 +222,8 @@ public IActionResult AddAdminFieldAnswersBulk(int customisationId) [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult AddAdminFieldAnswersBulk( - BulkAdminFieldAnswersViewModel model, - int customisationId + int customisationId, + BulkAdminFieldAnswersViewModel model ) { ValidateBulkOptionsString(model.OptionsString); @@ -246,7 +239,7 @@ int customisationId return RedirectToAction( "AddAdminField", - new { customisationId = model.CustomisationId } + new { customisationId } ); } @@ -266,7 +259,7 @@ public IActionResult RemoveAdminField(int customisationId, int promptNumber) var promptName = courseAdminFieldsService.GetPromptName(customisationId, promptNumber); - var model = new RemoveAdminFieldViewModel(customisationId, promptName, answerCount); + var model = new RemoveAdminFieldViewModel(promptName, answerCount); return View(model); } @@ -288,26 +281,26 @@ public IActionResult RemoveAdminField(int customisationId, int promptNumber, Rem return RemoveAdminFieldAndRedirect(customisationId, promptNumber); } - private IActionResult EditAdminFieldPostSave(EditAdminFieldViewModel model) + private IActionResult EditAdminFieldPostSave(int customisationId, EditAdminFieldViewModel model) { ModelState.ClearAllErrors(); courseAdminFieldsService.UpdateCustomPromptForCourse( - model.CustomisationId, + customisationId, model.PromptNumber, model.OptionsString ); - return RedirectToAction("Index", new { customisationId = model.CustomisationId }); + return RedirectToAction("Index", new { customisationId }); } - private IActionResult EditAdminFieldBulk(EditAdminFieldViewModel model) + private IActionResult EditAdminFieldBulk(int customisationId, EditAdminFieldViewModel model) { SetEditAdminFieldTempData(model); return RedirectToAction( "EditAdminFieldBulk", - new { customisationId = model.CustomisationId, promptNumber = model.PromptNumber } + new { customisationId, promptNumber = model.PromptNumber } ); } @@ -327,7 +320,7 @@ private void SetEditAdminFieldTempData(EditAdminFieldViewModel model) TempData.Set(data); } - private IActionResult AddAdminFieldPostSave(AddAdminFieldViewModel model) + private IActionResult AddAdminFieldPostSave(int customisationId, AddAdminFieldViewModel model) { ModelState.ClearErrorsForAllFieldsExcept(nameof(AddAdminFieldViewModel.AdminFieldId)); @@ -341,26 +334,26 @@ private IActionResult AddAdminFieldPostSave(AddAdminFieldViewModel model) var categoryId = User.GetAdminCategoryId()!; if (courseAdminFieldsService.AddCustomPromptToCourse( - model.CustomisationId, + customisationId, centreId, model.AdminFieldId!.Value, model.OptionsString )) { TempData.Clear(); - return RedirectToAction("Index", new { customisationId = model.CustomisationId }); + return RedirectToAction("Index", new { customisationId }); } return new StatusCodeResult(500); } - private IActionResult AddAdminFieldBulk(AddAdminFieldViewModel model) + private IActionResult AddAdminFieldBulk(int customisationId, AddAdminFieldViewModel model) { SetAddAdminFieldTempData(model); return RedirectToAction( "AddAdminFieldAnswersBulk", - new { customisationId = model.CustomisationId } + new { customisationId } ); } @@ -535,18 +528,5 @@ private void ValidateBulkOptionsString(string? optionsString) ); } } - - private bool VerifyAdminUserCanAccessCourse(int customisationId) - { - var centreId = User.GetCentreId(); - var categoryId = User.GetAdminCategoryId()!; - - if (courseService.VerifyAdminUserCanAccessCourse(customisationId, centreId, categoryId.Value)) - { - return true; - } - - return false; - } } } diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddAdminFieldViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddAdminFieldViewModel.cs index b3945ce773..7a9bbfc43e 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddAdminFieldViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddAdminFieldViewModel.cs @@ -9,20 +9,12 @@ public AddAdminFieldViewModel() IncludeAnswersTableCaption = true; } - public AddAdminFieldViewModel(int customisationId) - { - CustomisationId = customisationId; - IncludeAnswersTableCaption = true; - } - public AddAdminFieldViewModel( - int customisationId, int? adminFieldId, string? options, string? answer = null ) { - CustomisationId = customisationId; AdminFieldId = adminFieldId; OptionsString = options; IncludeAnswersTableCaption = true; diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AdminFieldAnswersViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AdminFieldAnswersViewModel.cs index 7daa6cf7be..32b9ec68f8 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AdminFieldAnswersViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AdminFieldAnswersViewModel.cs @@ -9,20 +9,16 @@ public class AdminFieldAnswersViewModel public AdminFieldAnswersViewModel() { } public AdminFieldAnswersViewModel( - int customisationId, string optionsString, string? answer = null, bool includeAnswersTableCaption = false ) { - CustomisationId = customisationId; OptionsString = optionsString; Answer = answer; IncludeAnswersTableCaption = includeAnswersTableCaption; } - public int CustomisationId { get; set; } - public string? OptionsString { get; set; } public List Options => NewlineSeparatedStringListHelper.SplitNewlineSeparatedList(OptionsString); diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs index 80567ea52d..748f9011f8 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs @@ -7,33 +7,27 @@ public BulkAdminFieldAnswersViewModel() { } public BulkAdminFieldAnswersViewModel( string? optionsString, bool isAddPromptJourney, - int customisationId, int promptNumber ) { OptionsString = optionsString; IsAddPromptJourney = isAddPromptJourney; - CustomisationId = customisationId; PromptNumber = promptNumber; } public BulkAdminFieldAnswersViewModel( string? optionsString, - bool isAddPromptJourney, - int customisationId + bool isAddPromptJourney ) { OptionsString = optionsString; IsAddPromptJourney = isAddPromptJourney; - CustomisationId = customisationId; } public string? OptionsString { get; set; } public bool IsAddPromptJourney { get; set; } - public int CustomisationId { get; set; } - public int PromptNumber { get; set; } } } diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/EditAdminFieldViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/EditAdminFieldViewModel.cs index c93a5c1a94..f9ffee776d 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/EditAdminFieldViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/EditAdminFieldViewModel.cs @@ -7,9 +7,8 @@ public class EditAdminFieldViewModel : AdminFieldAnswersViewModel { public EditAdminFieldViewModel() { } - public EditAdminFieldViewModel(CustomPrompt customPrompt, int customisationId) + public EditAdminFieldViewModel(CustomPrompt customPrompt) { - CustomisationId = customisationId; PromptNumber = customPrompt.CustomPromptNumber; Prompt = customPrompt.CustomPromptText; OptionsString = NewlineSeparatedStringListHelper.JoinNewlineSeparatedList(customPrompt.Options); @@ -17,13 +16,11 @@ public EditAdminFieldViewModel(CustomPrompt customPrompt, int customisationId) } public EditAdminFieldViewModel( - int customisationId, int customPromptNumber, string text, string? options ) { - CustomisationId = customisationId; PromptNumber = customPromptNumber; Prompt = text; OptionsString = options; diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/RemoveAdminFieldViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/RemoveAdminFieldViewModel.cs index 46560d2cac..77d1dbeb57 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/RemoveAdminFieldViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/RemoveAdminFieldViewModel.cs @@ -4,15 +4,12 @@ public class RemoveAdminFieldViewModel { public RemoveAdminFieldViewModel() { } - public RemoveAdminFieldViewModel(int customisationId, string promptName, int answerCount) + public RemoveAdminFieldViewModel(string promptName, int answerCount) { - CustomisationId = customisationId; PromptName = promptName; AnswerCount = answerCount; } - public int CustomisationId { get; set; } - public string? PromptName { get; set; } public bool Confirm { get; set; } diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml index ee3ba8cdad..c0380db9f6 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml @@ -14,7 +14,7 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var cancelLinkData = new Dictionary { { "customisationId", Model.CustomisationId.ToString() } }; + var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml index e282d78670..6501684a7f 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml @@ -10,11 +10,11 @@ ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; var editCancelLinkData = new Dictionary { - { "customisationId", Model.CustomisationId.ToString() }, + { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! }, { "promptNumber", Model.PromptNumber.ToString() } }; var addCancelLinkData = new Dictionary { - { "customisationId", Model.CustomisationId.ToString() } + { "customisationId", ViewContext.HttpContext.Request.Query["customisationId"].ToString() } }; } @@ -33,7 +33,6 @@
- { { "customisationId", Model.CustomisationId.ToString() } }; + var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml index 8209489c04..2430c46b8c 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml @@ -11,7 +11,7 @@ ViewData["HeaderPathName"] = "Tracking System"; var confirmError = ViewData.ModelState[nameof(RemoveAdminFieldViewModel.Confirm)]?.Errors?.Count > 0; var confirmFormErrorClass = confirmError ? "nhsuk-form-group--error" : ""; - var cancelLinkRouteData = new Dictionary { { "customisationId", Model.CustomisationId.ToString() } }; + var cancelLinkRouteData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; } @section NavMenuItems { From eac12cd071fee1eff182a47e5a395bba92bfe04d Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 14:28:50 +0100 Subject: [PATCH 09/15] HEEDLS-438 Change bulk post action names and url --- .../CourseSetup/AdminFieldsControllerTests.cs | 4 ++-- .../TrackingSystem/CourseSetup/AdminFieldsController.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index f966b40399..bd1279c69d 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -161,7 +161,7 @@ public void PostEditAdminField_returns_error_with_unexpected_action() } [Test] - public void AdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() + public void EditAdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() { // Given var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1); @@ -447,7 +447,7 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() controller.TempData.Set(initialTempData); // When - var result = controller.AddAdminFieldAnswersBulk(1, inputViewModel); + var result = controller.AddAdminFieldBulkPost(1, inputViewModel); // Then using (new AssertionScope()) diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index 45ab1cc7f5..270b93e4f8 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -127,7 +127,7 @@ public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) } [HttpPost] - [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Bulk")] + [Route("{customisationId:int}/AdminFields/Edit/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] public IActionResult EditAdminFieldBulkPost( @@ -206,7 +206,7 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m [Route("{customisationId:int}/AdminFields/Add/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult AddAdminFieldAnswersBulk(int customisationId) + public IActionResult AddAdminFieldBulkPost(int customisationId) { var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( @@ -221,7 +221,7 @@ public IActionResult AddAdminFieldAnswersBulk(int customisationId) [Route("{customisationId:int}/AdminFields/Add/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult AddAdminFieldAnswersBulk( + public IActionResult AddAdminFieldBulkPost( int customisationId, BulkAdminFieldAnswersViewModel model ) @@ -352,7 +352,7 @@ private IActionResult AddAdminFieldBulk(int customisationId, AddAdminFieldViewMo SetAddAdminFieldTempData(model); return RedirectToAction( - "AddAdminFieldAnswersBulk", + "AddAdminFieldBulkPost", new { customisationId } ); } From 5e02842625449972b92c859db49c1c06270de0d0 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 14:35:23 +0100 Subject: [PATCH 10/15] HEEDLS-439 Fix name change in tests --- .../TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index bd1279c69d..09db05f82c 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -395,7 +395,7 @@ public void AddAdminField_bulk_sets_up_temp_data_and_redirects() using (new AssertionScope()) { AssertAddTempDataIsExpected(model); - result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldAnswersBulk"); + result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldBulkPost"); } } @@ -415,7 +415,7 @@ public void AddAdminField_bulk_redirects_without_admin_field_selected() using (new AssertionScope()) { AssertAddTempDataIsExpected(model); - result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldAnswersBulk"); + result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldBulkPost"); } } From 7aa2b1210f0fe97bc46d90ad92079f0f63f47269 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 16:29:39 +0100 Subject: [PATCH 11/15] HEEDLS-438 Fix Bulk post and split add and edit bulk answers models and views --- .../CourseSetup/AdminFieldsControllerTests.cs | 18 ++++---- .../CourseSetup/AdminFieldsController.cs | 31 +++++++------ .../AddBulkAdminFieldAnswersViewModel.cs | 25 +++++++++++ .../BulkAdminFieldAnswersViewModel.cs | 19 +------- .../AddBulkAdminFieldAnswers.cshtml | 44 +++++++++++++++++++ .../AdminFields/BulkAdminFieldAnswers.cshtml | 18 ++------ 6 files changed, 98 insertions(+), 57 deletions(-) create mode 100644 DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs create mode 100644 DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 09db05f82c..487d511bc1 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -142,7 +142,7 @@ public void PostAdminField_bulk_sets_up_temp_data_and_redirects() using (new AssertionScope()) { AssertEditTempDataIsExpected(model); - result.Should().BeRedirectToActionResult().WithActionName("EditAdminFieldBulk"); + result.Should().BeRedirectToActionResult().WithActionName("EditAdminFieldAnswersBulk"); } } @@ -161,10 +161,10 @@ public void PostEditAdminField_returns_error_with_unexpected_action() } [Test] - public void EditAdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() + public void EditAdminFieldAnswersBulk_updates_temp_data_and_redirects_to_edit() { // Given - var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1); + var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false); var initialEditViewModel = new EditAdminFieldViewModel(1, "Test", "Test"); var expectedViewModel = new EditAdminFieldViewModel(1, "Test", "Test\r\nAnswer"); var initialTempData = new EditAdminFieldData(initialEditViewModel); @@ -175,7 +175,7 @@ public void EditAdminFieldBulkPost_updates_temp_data_and_redirects_to_edit() .Returns(true); // When - var result = controller.EditAdminFieldBulkPost(1, inputViewModel); + var result = controller.EditAdminFieldAnswersBulk(1, 1, inputViewModel); // Then using (new AssertionScope()) @@ -395,7 +395,7 @@ public void AddAdminField_bulk_sets_up_temp_data_and_redirects() using (new AssertionScope()) { AssertAddTempDataIsExpected(model); - result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldBulkPost"); + result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldAnswersBulk"); } } @@ -415,7 +415,7 @@ public void AddAdminField_bulk_redirects_without_admin_field_selected() using (new AssertionScope()) { AssertAddTempDataIsExpected(model); - result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldBulkPost"); + result.Should().BeRedirectToActionResult().WithActionName("AddAdminFieldAnswersBulk"); } } @@ -436,10 +436,10 @@ public void AddAdminField_returns_error_with_unexpected_action() } [Test] - public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() + public void AddAdminFieldAnswersBulk_updates_temp_data_and_redirects_to_add() { // Given - var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false, 1); + var inputViewModel = new AddBulkAdminFieldAnswersViewModel("Test\r\nAnswer", 1); var initialAddViewModel = new AddAdminFieldViewModel(1, "Test"); var expectedViewModel = new AddAdminFieldViewModel(1, "Test\r\nAnswer"); var initialTempData = new AddAdminFieldData(initialAddViewModel); @@ -447,7 +447,7 @@ public void AddAdminFieldBulkPost_updates_temp_data_and_redirects_to_add() controller.TempData.Set(initialTempData); // When - var result = controller.AddAdminFieldBulkPost(1, inputViewModel); + var result = controller.AddAdminFieldAnswersBulk(1, inputViewModel); // Then using (new AssertionScope()) diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index 270b93e4f8..4a234765f3 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -113,25 +113,25 @@ string action [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult EditAdminFieldBulk(int customisationId, int promptNumber) + public IActionResult EditAdminFieldAnswersBulk(int customisationId, int promptNumber) { var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( data.EditModel.OptionsString, - false, - promptNumber + false ); return View("BulkAdminFieldAnswers", model); } [HttpPost] - [Route("{customisationId:int}/AdminFields/Edit/Bulk")] + [Route("{customisationId:int}/AdminFields/{promptNumber:int}/Edit/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult EditAdminFieldBulkPost( + public IActionResult EditAdminFieldAnswersBulk( int customisationId, + int promptNumber, BulkAdminFieldAnswersViewModel model ) { @@ -148,7 +148,7 @@ BulkAdminFieldAnswersViewModel model return RedirectToAction( "EditAdminField", - new { customisationId, promptNumber = model.PromptNumber } + new { customisationId, promptNumber } ); } @@ -206,30 +206,29 @@ public IActionResult AddAdminField(int customisationId, AddAdminFieldViewModel m [Route("{customisationId:int}/AdminFields/Add/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult AddAdminFieldBulkPost(int customisationId) + public IActionResult AddAdminFieldAnswersBulk(int customisationId) { var data = TempData.Peek()!; - var model = new BulkAdminFieldAnswersViewModel( - data.AddModel.OptionsString, - true + var model = new AddBulkAdminFieldAnswersViewModel( + data.AddModel.OptionsString ); - return View("BulkAdminFieldAnswers", model); + return View("AddBulkAdminFieldAnswers", model); } [HttpPost] [Route("{customisationId:int}/AdminFields/Add/Bulk")] [ServiceFilter(typeof(VerifyAdminUserCanAccessCourse))] [ServiceFilter(typeof(RedirectEmptySessionData))] - public IActionResult AddAdminFieldBulkPost( + public IActionResult AddAdminFieldAnswersBulk( int customisationId, - BulkAdminFieldAnswersViewModel model + AddBulkAdminFieldAnswersViewModel model ) { ValidateBulkOptionsString(model.OptionsString); if (!ModelState.IsValid) { - return View("BulkAdminFieldAnswers", model); + return View("AddBulkAdminFieldAnswers", model); } var addData = TempData.Peek()!; @@ -299,7 +298,7 @@ private IActionResult EditAdminFieldBulk(int customisationId, EditAdminFieldView SetEditAdminFieldTempData(model); return RedirectToAction( - "EditAdminFieldBulk", + "EditAdminFieldAnswersBulk", new { customisationId, promptNumber = model.PromptNumber } ); } @@ -352,7 +351,7 @@ private IActionResult AddAdminFieldBulk(int customisationId, AddAdminFieldViewMo SetAddAdminFieldTempData(model); return RedirectToAction( - "AddAdminFieldBulkPost", + "AddAdminFieldAnswersBulk", new { customisationId } ); } diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs new file mode 100644 index 0000000000..5cff048d4b --- /dev/null +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs @@ -0,0 +1,25 @@ +namespace DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup +{ + public class AddBulkAdminFieldAnswersViewModel : BulkAdminFieldAnswersViewModel + { + public AddBulkAdminFieldAnswersViewModel() { } + + public AddBulkAdminFieldAnswersViewModel( + string? optionsString + ) + { + OptionsString = optionsString; + } + + public AddBulkAdminFieldAnswersViewModel( + string? optionsString, + int promptNumber + ) + { + OptionsString = optionsString; + PromptNumber = promptNumber; + } + + private int PromptNumber { get; set; } + } +} diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs index 748f9011f8..3f5120ecc2 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/BulkAdminFieldAnswersViewModel.cs @@ -5,29 +5,12 @@ public class BulkAdminFieldAnswersViewModel public BulkAdminFieldAnswersViewModel() { } public BulkAdminFieldAnswersViewModel( - string? optionsString, - bool isAddPromptJourney, - int promptNumber + string? optionsString ) { OptionsString = optionsString; - IsAddPromptJourney = isAddPromptJourney; - PromptNumber = promptNumber; - } - - public BulkAdminFieldAnswersViewModel( - string? optionsString, - bool isAddPromptJourney - ) - { - OptionsString = optionsString; - IsAddPromptJourney = isAddPromptJourney; } public string? OptionsString { get; set; } - - public bool IsAddPromptJourney { get; set; } - - public int PromptNumber { get; set; } } } diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml new file mode 100644 index 0000000000..062ff34c7e --- /dev/null +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml @@ -0,0 +1,44 @@ +@inject IConfiguration configuration +@using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup +@using Microsoft.Extensions.Configuration +@model AddBulkAdminFieldAnswersViewModel + +@{ + var errorHasOccurred = !ViewData.ModelState.IsValid; + ViewData["Title"] = errorHasOccurred ? "Error: Configure answers in bulk" : "Configure answers in bulk"; + ViewData["Application"] = "Tracking System"; + ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; + ViewData["HeaderPathName"] = "Tracking System"; + var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; +} + +@section NavMenuItems { + +} + +
+
+ @if (errorHasOccurred) { + + } + +

Configure answers in bulk

+ + + + + + + + + + +
+
diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml index 6501684a7f..131939eb8a 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml @@ -9,12 +9,9 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var editCancelLinkData = new Dictionary { + var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! }, - { "promptNumber", Model.PromptNumber.ToString() } - }; - var addCancelLinkData = new Dictionary { - { "customisationId", ViewContext.HttpContext.Request.Query["customisationId"].ToString() } + { "promptNumber", ViewContext.HttpContext.Request.RouteValues["promptNumber"].ToString()! } }; } @@ -30,10 +27,7 @@

Configure answers in bulk

-
- - - + Submit - @if (Model.IsAddPromptJourney) { - - } else { - - } + From 8a588d54ffb0f5a239c31238dfcaaa6e8bfa1cc5 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 16:31:43 +0100 Subject: [PATCH 12/15] HEEDLS-438 Remove isAddPromptJourney from tests --- .../TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs | 2 +- .../TrackingSystem/CourseSetup/AdminFieldsController.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 487d511bc1..53113a96a3 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -164,7 +164,7 @@ public void PostEditAdminField_returns_error_with_unexpected_action() public void EditAdminFieldAnswersBulk_updates_temp_data_and_redirects_to_edit() { // Given - var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer", false); + var inputViewModel = new BulkAdminFieldAnswersViewModel("Test\r\nAnswer"); var initialEditViewModel = new EditAdminFieldViewModel(1, "Test", "Test"); var expectedViewModel = new EditAdminFieldViewModel(1, "Test", "Test\r\nAnswer"); var initialTempData = new EditAdminFieldData(initialEditViewModel); diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index 4a234765f3..d1e7f37f1e 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -118,8 +118,7 @@ public IActionResult EditAdminFieldAnswersBulk(int customisationId, int promptNu var data = TempData.Peek()!; var model = new BulkAdminFieldAnswersViewModel( - data.EditModel.OptionsString, - false + data.EditModel.OptionsString ); return View("BulkAdminFieldAnswers", model); From 94be64437cdd7715d11b59415175523b3fdd5b82 Mon Sep 17 00:00:00 2001 From: OliZor Date: Thu, 30 Sep 2021 16:47:15 +0100 Subject: [PATCH 13/15] HEEDLS-438 Fix typo in DoesCourseExistAtCentre --- DigitalLearningSolutions.Data/DataServices/CourseDataService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs index 2469b2136a..9dae44d545 100644 --- a/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs +++ b/DigitalLearningSolutions.Data/DataServices/CourseDataService.cs @@ -356,7 +356,7 @@ FROM Customisations AS c JOIN Applications AS a on a.ApplicationID = c.ApplicationID WHERE CustomisationID = @customisationId AND c.CentreID = @centreId - AND (a.CourseCategoryID = 0 OR @categoryId IS NULL) + AND (a.CourseCategoryID = @categoryId OR @categoryId IS NULL) ) THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT) END", From b70791eb4d4ac203d26d895de0b3565e230b2303 Mon Sep 17 00:00:00 2001 From: OliZor Date: Fri, 1 Oct 2021 11:26:32 +0100 Subject: [PATCH 14/15] HEEDLS-438 Refactor and create GetRouteValues html extension --- .../Services/CourseService.cs | 8 ++-- ...EditCourseAdminFieldsAccessibilityTests.cs | 2 - .../VerifyAdminUserCanAccessCourseTests.cs | 38 +++++++++---------- .../Extensions/HtmlHelperExtensions.cs | 15 ++++++++ .../AdminFields/AddAdminField.cshtml | 3 +- .../AddBulkAdminFieldAnswers.cshtml | 3 +- .../AdminFields/BulkAdminFieldAnswers.cshtml | 6 +-- .../AdminFields/EditAdminField.cshtml | 3 +- .../CourseSetup/AdminFields/Index.cshtml | 5 ++- .../AdminFields/RemoveAdminField.cshtml | 5 ++- 10 files changed, 54 insertions(+), 34 deletions(-) diff --git a/DigitalLearningSolutions.Data/Services/CourseService.cs b/DigitalLearningSolutions.Data/Services/CourseService.cs index 43680adef0..6a0474bb11 100644 --- a/DigitalLearningSolutions.Data/Services/CourseService.cs +++ b/DigitalLearningSolutions.Data/Services/CourseService.cs @@ -10,7 +10,7 @@ public interface ICourseService public IEnumerable GetTopCourseStatistics(int centreId, int categoryId); public IEnumerable GetCentreSpecificCourseStatistics(int centreId, int categoryId); public IEnumerable GetAllCoursesForDelegate(int delegateId, int centreId); - public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int? categoryId); + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int categoryId); } public class CourseService : ICourseService @@ -54,10 +54,10 @@ public IEnumerable GetAllCoursesForDelegate(int delegateI ); } - public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int? categoryId) + public bool VerifyAdminUserCanAccessCourse(int customisationId, int centreId, int adminCategoryIdClaim) { - var nullableCategoryId = categoryId == 0 ? null : categoryId; - return courseDataService.DoesCourseExistAtCentre(customisationId, centreId, nullableCategoryId); + var categoryIdFilter = adminCategoryIdClaim == 0 ? (int?)null : adminCategoryIdClaim; + return courseDataService.DoesCourseExistAtCentre(customisationId, centreId, categoryIdFilter); } } } diff --git a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs index 4fb6eacd19..d76ea148db 100644 --- a/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs +++ b/DigitalLearningSolutions.Web.AutomatedUiTests/AccessibilityTests/EditCourseAdminFieldsAccessibilityTests.cs @@ -29,11 +29,9 @@ public void EditAdminField_journey_has_no_accessibility_errors() Driver.ClickButtonByText("Submit"); ValidatePageHeading("Edit course admin field"); - var bulkPostResult = new AxeBuilder(Driver).Analyze(); editPageResult.Violations.Should().BeEmpty(); bulkAdditionResult.Violations.Should().BeEmpty(); - bulkPostResult.Violations.Should().BeEmpty(); } } } diff --git a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs index 3637131e0a..a3c8e5803e 100644 --- a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs +++ b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs @@ -18,12 +18,30 @@ public class VerifyAdminUserCanAccessCourseTests { private readonly ICourseService courseService = A.Fake(); + private ActionExecutingContext context = null!; + private HomeController homeController = null!; + + [SetUp] + public void Setup() + { + homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() + .WithMockUser(true, 101); + context = new ActionExecutingContext( + new ActionContext( + new DefaultHttpContext(), + new RouteData(new RouteValueDictionary()), + new ActionDescriptor() + ), + new List(), + new Dictionary(), + homeController + ); + } [Test] public void Returns_NotFound_if_service_returns_false() { // Given - var context = SetupContext(); context.RouteData.Values["customisationId"] = 2; A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(false); @@ -39,7 +57,6 @@ public void Returns_NotFound_if_service_returns_false() public void Does_not_return_NotFound_if_service_returns_true() { // Given - var context = SetupContext(); context.RouteData.Values["customisationId"] = 24286; A.CallTo(() => courseService.VerifyAdminUserCanAccessCourse(A._, A._, A._)) .Returns(true); @@ -50,22 +67,5 @@ public void Does_not_return_NotFound_if_service_returns_true() // Then context.Result.Should().BeNull(); } - - private ActionExecutingContext SetupContext() - { - var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() - .WithMockUser(true, 101); - var context = new ActionExecutingContext( - new ActionContext( - new DefaultHttpContext(), - new RouteData(new RouteValueDictionary()), - new ActionDescriptor() - ), - new List(), - new Dictionary(), - homeController - ); - return context; - } } } diff --git a/DigitalLearningSolutions.Web/Extensions/HtmlHelperExtensions.cs b/DigitalLearningSolutions.Web/Extensions/HtmlHelperExtensions.cs index 9792155412..2cfa3ae855 100644 --- a/DigitalLearningSolutions.Web/Extensions/HtmlHelperExtensions.cs +++ b/DigitalLearningSolutions.Web/Extensions/HtmlHelperExtensions.cs @@ -1,5 +1,9 @@ namespace DigitalLearningSolutions.Web.Extensions { + using System; + using System.Collections.Generic; + using System.Globalization; + using System.Linq; using Microsoft.AspNetCore.Mvc.Rendering; public static class HtmlHelperExtensions @@ -18,5 +22,16 @@ public static string IsSelected( ? selectedCssClass : string.Empty; } + + public static Dictionary GetRouteValues( + this IHtmlHelper htmlHelper + ) + { + var routeValues = htmlHelper.ViewContext.HttpContext.Request.RouteValues; + return routeValues.ToDictionary( + kvp => kvp.Key, + kvp => Convert.ToString(kvp.Value, CultureInfo.InvariantCulture) + ); + } } } diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml index c0380db9f6..24ba73ed8d 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddAdminField.cshtml @@ -1,5 +1,6 @@ @inject IConfiguration configuration @using DigitalLearningSolutions.Web.Controllers.TrackingSystem.CourseSetup +@using DigitalLearningSolutions.Web.Extensions @using DigitalLearningSolutions.Web.ViewComponents @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup @using Microsoft.AspNetCore.Mvc.TagHelpers @@ -14,7 +15,7 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; + var cancelLinkData = Html.GetRouteValues(); } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml index 062ff34c7e..16b6231072 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/AddBulkAdminFieldAnswers.cshtml @@ -1,4 +1,5 @@ @inject IConfiguration configuration +@using DigitalLearningSolutions.Web.Extensions @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup @using Microsoft.Extensions.Configuration @model AddBulkAdminFieldAnswersViewModel @@ -9,7 +10,7 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; + var cancelLinkData = Html.GetRouteValues(); } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml index 131939eb8a..f537942896 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/BulkAdminFieldAnswers.cshtml @@ -1,4 +1,5 @@ @inject IConfiguration configuration +@using DigitalLearningSolutions.Web.Extensions @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup @using Microsoft.Extensions.Configuration @model BulkAdminFieldAnswersViewModel @@ -9,10 +10,7 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var cancelLinkData = new Dictionary { - { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! }, - { "promptNumber", ViewContext.HttpContext.Request.RouteValues["promptNumber"].ToString()! } - }; + var cancelLinkData = Html.GetRouteValues(); } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml index d19554ecbc..ec59f4c40f 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/EditAdminField.cshtml @@ -1,5 +1,6 @@ @inject IConfiguration configuration @using DigitalLearningSolutions.Web.Controllers.TrackingSystem.CourseSetup +@using DigitalLearningSolutions.Web.Extensions @using DigitalLearningSolutions.Web.ViewComponents @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup @using Microsoft.AspNetCore.Mvc.TagHelpers @@ -14,7 +15,7 @@ ViewData["Application"] = "Tracking System"; ViewData["HeaderPath"] = $"{configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; - var cancelLinkData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; + var cancelLinkData = Html.GetRouteValues(); } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml index c8bdb1f0c9..8bd219d610 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/Index.cshtml @@ -1,5 +1,8 @@ @inject IConfiguration Configuration +@using DigitalLearningSolutions.Web.Extensions +@using DigitalLearningSolutions.Web.ViewComponents @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup +@using Microsoft.AspNetCore.Mvc.TagHelpers @using Microsoft.Extensions.Configuration @model AdminFieldsViewModel @@ -11,7 +14,7 @@ ViewData["HeaderPath"] = $"{Configuration["AppRootPath"]}/TrackingSystem/Centre/Dashboard"; ViewData["HeaderPathName"] = "Tracking System"; var canAddNewField = Model.CustomFields.Count < 3; - var backLinkData = new Dictionary { { "customisationId", Model.CustomisationId.ToString() } }; + var backLinkData = Html.GetRouteValues(); } @section NavMenuItems { diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml index 2430c46b8c..4b60bfa532 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/CourseSetup/AdminFields/RemoveAdminField.cshtml @@ -1,5 +1,8 @@ @inject IConfiguration Configuration +@using DigitalLearningSolutions.Web.Extensions +@using DigitalLearningSolutions.Web.ViewComponents @using DigitalLearningSolutions.Web.ViewModels.TrackingSystem.CourseSetup +@using Microsoft.AspNetCore.Mvc.TagHelpers @using Microsoft.Extensions.Configuration @model RemoveAdminFieldViewModel @@ -11,7 +14,7 @@ ViewData["HeaderPathName"] = "Tracking System"; var confirmError = ViewData.ModelState[nameof(RemoveAdminFieldViewModel.Confirm)]?.Errors?.Count > 0; var confirmFormErrorClass = confirmError ? "nhsuk-form-group--error" : ""; - var cancelLinkRouteData = new Dictionary { { "customisationId", ViewContext.HttpContext.Request.RouteValues["customisationId"].ToString()! } }; + var cancelLinkRouteData = Html.GetRouteValues(); } @section NavMenuItems { From 75660ccbb7982fcd668a08967460ef6c959bd329 Mon Sep 17 00:00:00 2001 From: OliZor Date: Fri, 1 Oct 2021 14:11:22 +0100 Subject: [PATCH 15/15] HEEDLS-438 - Remove unnecessary code --- .../TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs | 3 +-- .../ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs | 3 +-- .../TrackingSystem/CourseSetup/AdminFieldsController.cs | 5 +---- .../CourseSetup/AddBulkAdminFieldAnswersViewModel.cs | 2 +- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs index 53113a96a3..ea79b8cd4f 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/CourseSetup/AdminFieldsControllerTests.cs @@ -31,8 +31,7 @@ public void Setup() { controller = new AdminFieldsController( courseAdminFieldsService, - courseAdminFieldsDataService, - courseService + courseAdminFieldsDataService ) .WithDefaultContext() .WithMockUser(true, 101) diff --git a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs index a3c8e5803e..cb567d250d 100644 --- a/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs +++ b/DigitalLearningSolutions.Web.Tests/ServiceFilter/VerifyAdminUserCanAccessCourseTests.cs @@ -19,12 +19,11 @@ public class VerifyAdminUserCanAccessCourseTests { private readonly ICourseService courseService = A.Fake(); private ActionExecutingContext context = null!; - private HomeController homeController = null!; [SetUp] public void Setup() { - homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() + var homeController = new HomeController(A.Fake()).WithDefaultContext().WithMockTempData() .WithMockUser(true, 101); context = new ActionExecutingContext( new ActionContext( diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs index d1e7f37f1e..405edf58db 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/CourseSetup/AdminFieldsController.cs @@ -28,17 +28,14 @@ public class AdminFieldsController : Controller private static readonly DateTimeOffset CookieExpiry = DateTimeOffset.UtcNow.AddDays(7); private readonly ICourseAdminFieldsDataService courseAdminFieldsDataService; private readonly ICourseAdminFieldsService courseAdminFieldsService; - private readonly ICourseService courseService; public AdminFieldsController( ICourseAdminFieldsService courseAdminFieldsService, - ICourseAdminFieldsDataService courseAdminFieldsDataService, - ICourseService courseService + ICourseAdminFieldsDataService courseAdminFieldsDataService ) { this.courseAdminFieldsService = courseAdminFieldsService; this.courseAdminFieldsDataService = courseAdminFieldsDataService; - this.courseService = courseService; } [HttpGet] diff --git a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs index 5cff048d4b..50e1ddaffd 100644 --- a/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs +++ b/DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CourseSetup/AddBulkAdminFieldAnswersViewModel.cs @@ -20,6 +20,6 @@ int promptNumber PromptNumber = promptNumber; } - private int PromptNumber { get; set; } + private int PromptNumber { get; } } }