From 58fe8e5fc5bd750d71520c78e268ef834f71e5e2 Mon Sep 17 00:00:00 2001 From: Daniel Bloxham Date: Wed, 1 Sep 2021 10:01:59 +0100 Subject: [PATCH 1/3] HEEDLS-519 - Change system notification cookie management --- .../SystemNotificationControllerTests.cs | 65 +++++++++++++++---- .../Centre/SystemNotificationsController.cs | 18 +++-- 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs index 405bc336aa..e341d4b199 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs @@ -1,8 +1,11 @@ namespace DigitalLearningSolutions.Web.Tests.Controllers.TrackingSystem.Centre { using System; + using System.Collections.Generic; using DigitalLearningSolutions.Data.DataServices; + using DigitalLearningSolutions.Data.Models; using DigitalLearningSolutions.Data.Services; + using DigitalLearningSolutions.Data.Tests.TestHelpers; using DigitalLearningSolutions.Web.Controllers.TrackingSystem.Centre; using DigitalLearningSolutions.Web.Helpers; using DigitalLearningSolutions.Web.Tests.ControllerHelpers; @@ -15,10 +18,10 @@ public class SystemNotificationControllerTests { private readonly IClockService clockService = A.Fake(); + private SystemNotificationsController controller = null!; private HttpRequest httpRequest = null!; private HttpResponse httpResponse = null!; private ISystemNotificationsDataService systemNotificationsDataService = null!; - private SystemNotificationsController controller = null!; [SetUp] public void Setup() @@ -30,7 +33,35 @@ public void Setup() new SystemNotificationsController(systemNotificationsDataService, clockService) .WithMockHttpContext(httpRequest, response: httpResponse) .WithMockUser(true) - .WithMockServices(); + .WithMockServices() + .WithMockTempData(); + } + + [Test] + public void Index_sets_cookie_when_unacknowledged_notifications_exist() + { + // Given + var testDate = new DateTime(2021, 8, 23); + A.CallTo(() => clockService.UtcNow).Returns(testDate); + var expectedExpiry = testDate.AddHours(24); + A.CallTo(() => systemNotificationsDataService.GetUnacknowledgedSystemNotifications(A._)) + .Returns(new List { SystemNotificationTestHelper.GetDefaultSystemNotification() }); + + // When + var result = controller.SkipNotifications(); + + // Then + using (new AssertionScope()) + { + result.Should().BeRedirectToActionResult().WithControllerName("Dashboard").WithActionName("Index"); + A.CallTo( + () => httpResponse.Cookies.Append( + SystemNotificationCookieHelper.CookieName, + "7", + A.That.Matches(co => co.Expires == expectedExpiry) + ) + ).MustHaveHappened(); + } } [Test] @@ -69,7 +100,8 @@ public void SkipNotifications_does_not_acknowledge_notifications() controller.SkipNotifications(); // Then - A.CallTo(() => systemNotificationsDataService.AcknowledgeNotification(A._, A._)).MustNotHaveHappened(); + A.CallTo(() => systemNotificationsDataService.AcknowledgeNotification(A._, A._)) + .MustNotHaveHappened(); } [Test] @@ -82,53 +114,62 @@ public void Post_acknowledges_notification_and_redirects() using (new AssertionScope()) { A.CallTo(() => systemNotificationsDataService.AcknowledgeNotification(1, 7)).MustHaveHappened(); - result.Should().BeRedirectToActionResult().WithControllerName("SystemNotifications").WithActionName("Index"); + result.Should().BeRedirectToActionResult().WithControllerName("SystemNotifications") + .WithActionName("Index"); } } [Test] - public void Post_deletes_cookie_if_one_exists_for_user() + public void Index_deletes_cookie_if_one_exists_for_user_and_no_unacknowledged_notifications_exist() { // Given A.CallTo(() => httpRequest.Cookies).Returns( ControllerContextHelper.SetUpFakeRequestCookieCollection(SystemNotificationCookieHelper.CookieName, "7") ); + A.CallTo(() => systemNotificationsDataService.GetUnacknowledgedSystemNotifications(A._)) + .Returns(new List()); // When - controller.AcknowledgeNotification(1, 1); + controller.Index(); // Then A.CallTo(() => httpResponse.Cookies.Delete(SystemNotificationCookieHelper.CookieName)).MustHaveHappened(); } [Test] - public void Post_does_not_delete_cookie_if_one_exists_for_someone_other_than_current_user() + public void Index_does_not_delete_cookie_if_one_exists_for_someone_other_than_current_user() { // Given A.CallTo(() => httpRequest.Cookies).Returns( ControllerContextHelper.SetUpFakeRequestCookieCollection(SystemNotificationCookieHelper.CookieName, "8") ); + A.CallTo(() => systemNotificationsDataService.GetUnacknowledgedSystemNotifications(A._)) + .Returns(new List()); // When - controller.AcknowledgeNotification(1, 1); + controller.Index(); // Then - A.CallTo(() => httpResponse.Cookies.Delete(SystemNotificationCookieHelper.CookieName)).MustNotHaveHappened(); + A.CallTo(() => httpResponse.Cookies.Delete(SystemNotificationCookieHelper.CookieName)) + .MustNotHaveHappened(); } [Test] - public void Post_does_not_delete_cookie_if_one_does_not_exist() + public void Index_does_not_delete_cookie_if_one_does_not_exist() { // Given A.CallTo(() => httpRequest.Cookies).Returns( A.Fake() ); + A.CallTo(() => systemNotificationsDataService.GetUnacknowledgedSystemNotifications(A._)) + .Returns(new List()); // When - controller.AcknowledgeNotification(1, 1); + controller.Index(); // Then - A.CallTo(() => httpResponse.Cookies.Delete(SystemNotificationCookieHelper.CookieName)).MustNotHaveHappened(); + A.CallTo(() => httpResponse.Cookies.Delete(SystemNotificationCookieHelper.CookieName)) + .MustNotHaveHappened(); } } } diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs index bcb40cbf64..5df493f2d7 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs @@ -33,6 +33,19 @@ public IActionResult Index(int page = 1) var adminId = User.GetAdminId()!.Value; var unacknowledgedNotifications = systemNotificationsDataService.GetUnacknowledgedSystemNotifications(adminId).ToList(); + + if (unacknowledgedNotifications.Count == 0) + { + if (Request.Cookies.HasSkippedNotificationsCookie(adminId)) + { + Response.Cookies.DeleteSkipSystemNotificationCookie(); + } + } + else + { + Response.Cookies.SetSkipSystemNotificationCookie(adminId, clockService.UtcNow); + } + var model = new SystemNotificationsViewModel(unacknowledgedNotifications, page); return View(model); } @@ -53,11 +66,6 @@ public IActionResult AcknowledgeNotification(int systemNotificationId, int page) var adminId = User.GetAdminId()!.Value; systemNotificationsDataService.AcknowledgeNotification(systemNotificationId, adminId); - if (Request.Cookies.HasSkippedNotificationsCookie(adminId)) - { - Response.Cookies.DeleteSkipSystemNotificationCookie(); - } - return RedirectToAction("Index", "SystemNotifications", new { page }); } } From f1b507629677b692d9c738509ac280171734e6f3 Mon Sep 17 00:00:00 2001 From: Daniel Bloxham Date: Wed, 1 Sep 2021 16:09:16 +0100 Subject: [PATCH 2/3] HEEDLS-519 - remove redundant SkipNotificationsAction --- .../SystemNotificationControllerTests.cs | 44 +------------------ .../Centre/SystemNotificationsController.cs | 9 ---- .../Centre/SystemNotifications/Index.cshtml | 4 +- 3 files changed, 4 insertions(+), 53 deletions(-) diff --git a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs index e341d4b199..b5763f4982 100644 --- a/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs +++ b/DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/SystemNotificationControllerTests.cs @@ -48,12 +48,12 @@ public void Index_sets_cookie_when_unacknowledged_notifications_exist() .Returns(new List { SystemNotificationTestHelper.GetDefaultSystemNotification() }); // When - var result = controller.SkipNotifications(); + var result = controller.Index(); // Then using (new AssertionScope()) { - result.Should().BeRedirectToActionResult().WithControllerName("Dashboard").WithActionName("Index"); + result.Should().BeViewResult().WithDefaultViewName(); A.CallTo( () => httpResponse.Cookies.Append( SystemNotificationCookieHelper.CookieName, @@ -64,46 +64,6 @@ public void Index_sets_cookie_when_unacknowledged_notifications_exist() } } - [Test] - public void SkipNotifications_sets_cookie_and_redirects_to_dashboard() - { - // Given - var testDate = new DateTime(2021, 8, 23); - A.CallTo(() => clockService.UtcNow).Returns(testDate); - var expectedExpiry = testDate.AddHours(24); - - // When - var result = controller.SkipNotifications(); - - // Then - using (new AssertionScope()) - { - result.Should().BeRedirectToActionResult().WithControllerName("Dashboard").WithActionName("Index"); - A.CallTo( - () => httpResponse.Cookies.Append( - SystemNotificationCookieHelper.CookieName, - "7", - A.That.Matches(co => co.Expires == expectedExpiry) - ) - ).MustHaveHappened(); - } - } - - [Test] - public void SkipNotifications_does_not_acknowledge_notifications() - { - // Given - var testDate = new DateTime(2021, 8, 23); - A.CallTo(() => clockService.UtcNow).Returns(testDate); - - // When - controller.SkipNotifications(); - - // Then - A.CallTo(() => systemNotificationsDataService.AcknowledgeNotification(A._, A._)) - .MustNotHaveHappened(); - } - [Test] public void Post_acknowledges_notification_and_redirects() { diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs index 5df493f2d7..e9f58c5b7f 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs @@ -50,15 +50,6 @@ public IActionResult Index(int page = 1) return View(model); } - [HttpGet] - [Route("SkipNotifications")] - public IActionResult SkipNotifications() - { - var adminId = User.GetAdminId()!.Value; - Response.Cookies.SetSkipSystemNotificationCookie(adminId, clockService.UtcNow); - return RedirectToAction("Index", "Dashboard"); - } - [HttpPost] [Route("{page:int}")] public IActionResult AcknowledgeNotification(int systemNotificationId, int page) diff --git a/DigitalLearningSolutions.Web/Views/TrackingSystem/Centre/SystemNotifications/Index.cshtml b/DigitalLearningSolutions.Web/Views/TrackingSystem/Centre/SystemNotifications/Index.cshtml index ab518ab10a..123c8bd7b8 100644 --- a/DigitalLearningSolutions.Web/Views/TrackingSystem/Centre/SystemNotifications/Index.cshtml +++ b/DigitalLearningSolutions.Web/Views/TrackingSystem/Centre/SystemNotifications/Index.cshtml @@ -20,8 +20,8 @@

New system notifications

- @if (Model.UnacknowledgedNotification == null) { From 3798b262d4531bc093da1077ce050df29963d8ac Mon Sep 17 00:00:00 2001 From: Daniel Bloxham Date: Mon, 6 Sep 2021 16:13:00 +0100 Subject: [PATCH 3/3] HEEDLS-519 - neaten up system notification cookie management --- .../Centre/SystemNotificationsController.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs index e9f58c5b7f..138217040a 100644 --- a/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs +++ b/DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/SystemNotificationsController.cs @@ -34,16 +34,13 @@ public IActionResult Index(int page = 1) var unacknowledgedNotifications = systemNotificationsDataService.GetUnacknowledgedSystemNotifications(adminId).ToList(); - if (unacknowledgedNotifications.Count == 0) + if (unacknowledgedNotifications.Count > 0) { - if (Request.Cookies.HasSkippedNotificationsCookie(adminId)) - { - Response.Cookies.DeleteSkipSystemNotificationCookie(); - } + Response.Cookies.SetSkipSystemNotificationCookie(adminId, clockService.UtcNow); } - else + else if (Request.Cookies.HasSkippedNotificationsCookie(adminId)) { - Response.Cookies.SetSkipSystemNotificationCookie(adminId, clockService.UtcNow); + Response.Cookies.DeleteSkipSystemNotificationCookie(); } var model = new SystemNotificationsViewModel(unacknowledgedNotifications, page);