From d95c5e1076172aa347a6193cb0122dabe25d5fe8 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 9 Jul 2024 17:25:48 +0100 Subject: [PATCH 1/6] Very minor ReSharper and formatting suggestions. --- .../QualificationDetailsController.cs | 8 ++++- .../Controllers/QuestionsController.cs | 21 ++++++------ .../QualificationDetailsControllerTests.cs | 33 ++++++++++++------- .../Services/UserJourneyCookieServiceTests.cs | 30 ++++++++--------- 4 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index 6e4bc97f..2dff25cc 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -34,10 +34,16 @@ public async Task Get() return View(model); } - + [HttpPost] public IActionResult Refine(string refineSearch) { + if (!ModelState.IsValid) + { + logger.LogError("Invalid model state"); + return RedirectToAction("Index", "Error"); + } + userJourneyCookieService.SetQualificationNameSearchCriteria(refineSearch); return RedirectToAction("Get"); diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs index 0ce6956f..f69e7716 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs @@ -36,7 +36,7 @@ public async Task WhereWasTheQualificationAwarded(RadioQuestionMo if (!ModelState.IsValid) { var questionPage = await contentService.GetRadioQuestionPage(QuestionPages.WhereWasTheQualificationAwarded); - + // ReSharper disable once InvertIf if (questionPage is not null) { @@ -173,7 +173,7 @@ public async Task WhatIsTheAwardingOrganisation(DropdownQuestionM private bool WithinDateRange() { - (int? startDateMonth, int? startDateYear) = userJourneyCookieService.GetWhenWasQualificationAwarded(); + var (startDateMonth, startDateYear) = userJourneyCookieService.GetWhenWasQualificationAwarded(); if (startDateMonth is not null && startDateYear is not null) { var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); @@ -182,11 +182,11 @@ private bool WithinDateRange() return false; } - + private async Task> GetFilteredQualifications() { - int? level = userJourneyCookieService.GetLevelOfQualification(); - (int? startDateMonth, int? startDateYear) = userJourneyCookieService.GetWhenWasQualificationAwarded(); + var level = userJourneyCookieService.GetLevelOfQualification(); + var (startDateMonth, startDateYear) = userJourneyCookieService.GetWhenWasQualificationAwarded(); return await contentFilterService.GetFilteredQualifications(level, startDateMonth, startDateYear, null, null); } @@ -241,11 +241,12 @@ private static DropdownQuestionModel MapDropdownModel(DropdownQuestionModel mode { var awardingOrganisationExclusions = new[] { AwardingOrganisations.AllHigherEducation, AwardingOrganisations.Various }; - var uniqueAwardingOrganisations = qualifications.Select(x => x.AwardingOrganisationTitle) - .Distinct() - .Where(x => !awardingOrganisationExclusions.Any(x.Contains)) - .Order() - .ToList(); + + var uniqueAwardingOrganisations + = qualifications.Select(x => x.AwardingOrganisationTitle) + .Distinct() + .Where(x => !Array.Exists(awardingOrganisationExclusions, x.Contains)) + .Order(); model.ActionName = actionName; model.ControllerName = controllerName; diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs index 9f4799ee..27805265 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs @@ -28,7 +28,8 @@ public async Task Index_PassInNullQualificationId_ReturnsBadRequest() var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -46,7 +47,6 @@ public async Task Index_PassInNullQualificationId_ReturnsBadRequest() resultType!.StatusCode.Should().Be(400); } - [TestMethod] public async Task Index_ContentServiceReturnsNullDetailsPage_RedirectsToHomeError() { @@ -58,7 +58,8 @@ public async Task Index_ContentServiceReturnsNullDetailsPage_RedirectsToHomeErro var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -93,7 +94,8 @@ public async Task Index_ContentServiceReturnsNoQualification_RedirectsToErrorPag var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -128,7 +130,8 @@ public async Task Index_ContentServiceReturnsQualification_ReturnsQualificationD var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -173,13 +176,19 @@ public async Task Get_ReturnsView() var mockUserJourneyCookieService = new Mock(); mockContentFilterService - .Setup(x => x.GetFilteredQualifications(It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())).ReturnsAsync(new List()); + .Setup(x => + x.GetFilteredQualifications(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync([]); mockUserJourneyCookieService.Setup(x => x.GetUserJourneyModelFromCookie()).Returns(new UserJourneyModel()); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -198,7 +207,7 @@ public async Task Get_ReturnsView() }, Header = "TEST" }); - + var result = await controller.Get(); result.Should().NotBeNull(); @@ -216,7 +225,8 @@ public async Task Get_NoContent_LogsAndRedirectsToError() var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext @@ -250,7 +260,8 @@ public void Refine_SaveQualificationName_RedirectsToGet() var mockUserJourneyCookieService = new Mock(); var controller = - new QualificationDetailsController(mockLogger.Object, mockContentService.Object, mockContentFilterService.Object, mockInsetTextRenderer.Object, + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) { ControllerContext = new ControllerContext diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs index 48f63b97..0201912d 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs @@ -178,7 +178,7 @@ public void ResetUserJourneyCookie_FullModelAsCookieExists_AddsBaseModelAsCookie var serialisedModelToCheck = JsonSerializer.Serialize(new UserJourneyModel()); CheckSerializedModelWasSet(mockHttpContextAccessor, serialisedModelToCheck); } - + [TestMethod] public void GetWhereWasQualificationAwarded_CookieValueIsEmpty_ReturnsNull() { @@ -195,7 +195,7 @@ public void GetWhereWasQualificationAwarded_CookieValueIsEmpty_ReturnsNull() model.Should().BeNull(); } - + [TestMethod] public void GetWhereWasQualificationAwarded_CookieHasValue_ReturnsValueWithUpperCaseFirstLetter() { @@ -212,7 +212,7 @@ public void GetWhereWasQualificationAwarded_CookieHasValue_ReturnsValueWithUpper model.Should().Be("England"); } - + [TestMethod] public void GetAwardingOrganisation_CookieValueIsEmpty_ReturnsNull() { @@ -229,7 +229,7 @@ public void GetAwardingOrganisation_CookieValueIsEmpty_ReturnsNull() model.Should().BeNull(); } - + [TestMethod] public void GetAwardingOrganisation_CookieHasValue_ReturnsValue() { @@ -246,7 +246,7 @@ public void GetAwardingOrganisation_CookieHasValue_ReturnsValue() model.Should().Be("NCFE"); } - + [TestMethod] public void GetLevelOfQualification_CookieValueIsEmpty_ReturnsNull() { @@ -263,7 +263,7 @@ public void GetLevelOfQualification_CookieValueIsEmpty_ReturnsNull() model.Should().BeNull(); } - + [TestMethod] public void GetLevelOfQualification_CookieHasValue_ReturnsValue() { @@ -280,7 +280,7 @@ public void GetLevelOfQualification_CookieHasValue_ReturnsValue() model.Should().Be(4); } - + [TestMethod] public void GetWhenWasQualificationAwarded_CookieValueIsEmpty_ReturnsNull() { @@ -293,12 +293,12 @@ public void GetWhenWasQualificationAwarded_CookieValueIsEmpty_ReturnsNull() var service = new UserJourneyCookieService(mockHttpContextAccessor.Object, mockLogger.Object); - (int? startMonth, int? startYear) = service.GetWhenWasQualificationAwarded(); + var (startMonth, startYear) = service.GetWhenWasQualificationAwarded(); startMonth.Should().BeNull(); startYear.Should().BeNull(); } - + [TestMethod] public void GetWhenWasQualificationAwarded_CookieHasInvalidValue_ReturnsNull() { @@ -311,12 +311,12 @@ public void GetWhenWasQualificationAwarded_CookieHasInvalidValue_ReturnsNull() var service = new UserJourneyCookieService(mockHttpContextAccessor.Object, mockLogger.Object); - (int? startMonth, int? startYear) = service.GetWhenWasQualificationAwarded(); + var (startMonth, startYear) = service.GetWhenWasQualificationAwarded(); startMonth.Should().BeNull(); startYear.Should().BeNull(); } - + [TestMethod] public void GetWhenWasQualificationAwarded_CookieHasValidValue_ReturnsValue() { @@ -329,12 +329,12 @@ public void GetWhenWasQualificationAwarded_CookieHasValidValue_ReturnsValue() var service = new UserJourneyCookieService(mockHttpContextAccessor.Object, mockLogger.Object); - (int? startMonth, int? startYear) = service.GetWhenWasQualificationAwarded(); + var (startMonth, startYear) = service.GetWhenWasQualificationAwarded(); startMonth.Should().Be(4); startYear.Should().Be(2015); } - + [TestMethod] public void SetNameSearchCriteria_StringProvided_SetsCookieCorrectly() { @@ -354,7 +354,7 @@ public void SetNameSearchCriteria_StringProvided_SetsCookieCorrectly() CheckSerializedModelWasSet(mockHttpContextAccessor, serialisedModelToCheck); } - + [TestMethod] public void GetSearchCriteria_CookieHasInvalidValue_ReturnsNull() { @@ -371,7 +371,7 @@ public void GetSearchCriteria_CookieHasInvalidValue_ReturnsNull() searchCriteria.Should().BeNull(); } - + [TestMethod] public void GetSearchCriteria_CookieHasValidValue_ReturnsValue() { From 614f757d247600ad815f05bd4fc4d2c91259e5cf Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 17 Jul 2024 15:41:42 +0100 Subject: [PATCH 2/6] Don't redirect on invalid model state, just log. Allow for .NET treatment of POSTed empty string. --- .../QualificationDetailsController.cs | 7 +-- .../QualificationDetailsControllerTests.cs | 59 +++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index 2dff25cc..b6e176ba 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -36,15 +36,14 @@ public async Task Get() } [HttpPost] - public IActionResult Refine(string refineSearch) + public IActionResult Refine(string? refineSearch) { if (!ModelState.IsValid) { - logger.LogError("Invalid model state"); - return RedirectToAction("Index", "Error"); + logger.LogError($"Invalid model state in {nameof(QualificationDetailsController)} POST"); } - userJourneyCookieService.SetQualificationNameSearchCriteria(refineSearch); + userJourneyCookieService.SetQualificationNameSearchCriteria(refineSearch ?? string.Empty); return RedirectToAction("Get"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs index 27805265..f6889be5 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs @@ -279,4 +279,63 @@ public void Refine_SaveQualificationName_RedirectsToGet() actionResult.ActionName.Should().Be("Get"); mockUserJourneyCookieService.Verify(x => x.SetQualificationNameSearchCriteria("Test"), Times.Once); } + + [TestMethod] + public void Refine_NullParam_RedirectsToGet() + { + var mockLogger = new Mock>(); + var mockContentService = new Mock(); + var mockContentFilterService = new Mock(); + var mockInsetTextRenderer = new Mock(); + var mockHtmlRenderer = new Mock(); + var mockUserJourneyCookieService = new Mock(); + + var controller = + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, + mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) + { + ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext() + } + }; + + var result = controller.Refine(null); + + result.Should().BeOfType(); + + var actionResult = (RedirectToActionResult)result; + + actionResult.ActionName.Should().Be("Get"); + mockUserJourneyCookieService.Verify(x => x.SetQualificationNameSearchCriteria(string.Empty), Times.Once); + } + + [TestMethod] + public void Refine_InvalidModel_LogsError() + { + var mockLogger = new Mock>(); + var mockContentService = new Mock(); + var mockContentFilterService = new Mock(); + var mockInsetTextRenderer = new Mock(); + var mockHtmlRenderer = new Mock(); + var mockUserJourneyCookieService = new Mock(); + + var controller = + new QualificationDetailsController(mockLogger.Object, mockContentService.Object, + mockContentFilterService.Object, mockInsetTextRenderer.Object, + mockHtmlRenderer.Object, mockUserJourneyCookieService.Object) + { + ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext() + } + }; + + controller.ModelState.AddModelError("Key", "Error message"); + + controller.Refine(null); + + mockLogger.VerifyError($"Invalid model state in {nameof(QualificationDetailsController)} POST"); + } } \ No newline at end of file From 5821953f5c5158f917c6f809df725c9c14f9ae4b Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 18 Jul 2024 10:56:31 +0100 Subject: [PATCH 3/6] More issues addressed. --- .../Program.cs | 98 +++++++++++-------- .../ContentfulContentFilterService.cs | 48 ++++----- .../QualificationDetailsController.cs | 4 +- .../Controllers/QuestionsController.cs | 15 ++- .../UserJourneyCookieService.cs | 2 + .../DateQuestionModelValidatorTests.cs | 12 +-- .../Services/UserJourneyCookieServiceTests.cs | 2 +- 7 files changed, 104 insertions(+), 77 deletions(-) diff --git a/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs b/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs index f742ffb0..a5e371ea 100644 --- a/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs +++ b/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs @@ -15,6 +15,8 @@ public static class Program private const string SpaceId = ""; private const string ManagementApiKey = ""; private const string CsvFile = "./csv/ey-quals-full-2024-with-new-reference-fields.csv"; + private const string QualificationContentTypeId = "Qualification"; + private const string FieldTypeSymbol = "Symbol"; // ReSharper disable once UnusedParameter.Global // ...args standard for Program.Main() @@ -38,7 +40,7 @@ private static async Task PopulateContentfulWithQualifications(ContentfulManagem // Check existing entries and identify those to update, and those to create. var currentEntries = await - client.GetEntriesForLocale(new QueryBuilder>().ContentTypeIs("Qualification").Limit(1000), + client.GetEntriesForLocale(new QueryBuilder>().ContentTypeIs(QualificationContentTypeId).Limit(1000), Locale); var qualificationsToAddOrUpdate = GetQualificationsToAddOrUpdate(); @@ -56,7 +58,8 @@ private static async Task PopulateContentfulWithQualifications(ContentfulManagem } // Create new entry - var entryToPublish = await client.CreateOrUpdateEntry(entryToAddOrUpdate, contentTypeId: "Qualification", + var entryToPublish = await client.CreateOrUpdateEntry(entryToAddOrUpdate, + contentTypeId: QualificationContentTypeId, version: entryToAddOrUpdate.SystemProperties.Version); await client.PublishEntry(entryToPublish.SystemProperties.Id, @@ -67,7 +70,7 @@ await client.PublishEntry(entryToPublish.SystemProperties.Id, private static async Task SetUpContentModels(ContentfulManagementClient client) { // Check current version of model - var contentTypeModel = await client.GetContentType("Qualification"); + var contentTypeModel = await client.GetContentType(QualificationContentTypeId); var version = contentTypeModel?.SystemProperties.Version ?? 1; @@ -75,9 +78,9 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { SystemProperties = new SystemProperties { - Id = "Qualification" + Id = QualificationContentTypeId }, - Name = "Qualification", + Name = QualificationContentTypeId, Description = "Model for storing all the early years qualifications", DisplayField = "qualificationName", Fields = @@ -86,7 +89,7 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { Name = "Qualification ID", Id = "qualificationId", - Type = "Symbol", + Type = FieldTypeSymbol, Required = true, Validations = [new UniqueValidator()] }, @@ -94,7 +97,7 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { Name = "Qualification Name", Id = "qualificationName", - Type = "Symbol", + Type = FieldTypeSymbol, Required = true }, new Field @@ -108,26 +111,26 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { Name = "Awarding Organisation Title", Id = "awardingOrganisationTitle", - Type = "Symbol", + Type = FieldTypeSymbol, Required = true }, new Field { Name = "From Which Year", Id = "fromWhichYear", - Type = "Symbol" + Type = FieldTypeSymbol }, new Field { Name = "To Which Year", Id = "toWhichYear", - Type = "Symbol" + Type = FieldTypeSymbol }, new Field { Name = "Qualification Number", Id = "qualificationNumber", - Type = "Symbol" + Type = FieldTypeSymbol }, new Field { @@ -144,10 +147,14 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { Type = "Link", LinkType = "Entry", - Validations = [new LinkContentTypeValidator - { - ContentTypeIds = ["additionalRequirementQuestion"] - }] + Validations = + [ + new LinkContentTypeValidator + { + ContentTypeIds = + ["additionalRequirementQuestion"] + } + ] } }, new Field @@ -159,25 +166,28 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { Type = "Link", LinkType = "Entry", - Validations = [new LinkContentTypeValidator - { - ContentTypeIds = ["ratioRequirement"] - }] + Validations = + [ + new LinkContentTypeValidator + { + ContentTypeIds = ["ratioRequirement"] + } + ] } } ] }; var typeToActivate = await client.CreateOrUpdateContentType(contentType, version: version); - await client.ActivateContentType("Qualification", typeToActivate.SystemProperties.Version!.Value); - + await client.ActivateContentType(QualificationContentTypeId, typeToActivate.SystemProperties.Version!.Value); + Thread.Sleep(2000); // Allows the API time to activate the content type await SetHelpText(client); } private static async Task SetHelpText(ContentfulManagementClient client) { - var editorInterface = await client.GetEditorInterface("Qualification"); + var editorInterface = await client.GetEditorInterface(QualificationContentTypeId); SetHelpTextForField(editorInterface, "qualificationId", "The unique identifier used to reference the qualification"); SetHelpTextForField(editorInterface, "qualificationName", "The name of the qualification"); @@ -191,10 +201,13 @@ private static async Task SetHelpText(ContentfulManagementClient client) SetHelpTextForField(editorInterface, "qualificationNumber", "The number of the qualification"); SetHelpTextForField(editorInterface, "additionalRequirements", "The additional requirements for the qualification", SystemWidgetIds.MultipleLine); - SetHelpTextForField(editorInterface, "additionalRequirementQuestions", "The optional additional requirements questions", SystemWidgetIds.EntryMultipleLinksEditor); - SetHelpTextForField(editorInterface, "ratioRequirements", "The optional ratio requirements", SystemWidgetIds.EntryMultipleLinksEditor); - - await client.UpdateEditorInterface(editorInterface, "Qualification", editorInterface.SystemProperties.Version!.Value); + SetHelpTextForField(editorInterface, "additionalRequirementQuestions", + "The optional additional requirements questions", SystemWidgetIds.EntryMultipleLinksEditor); + SetHelpTextForField(editorInterface, "ratioRequirements", "The optional ratio requirements", + SystemWidgetIds.EntryMultipleLinksEditor); + + await client.UpdateEditorInterface(editorInterface, QualificationContentTypeId, + editorInterface.SystemProperties.Version!.Value); } private static void SetHelpTextForField(EditorInterface editorInterface, string fieldId, string helpText, @@ -207,25 +220,26 @@ private static void SetHelpTextForField(EditorInterface editorInterface, string private static Entry BuildEntryFromQualification(QualificationUpload qualification) { - var addtionalRequirementQuestions = new List(); + var additionalRequirementQuestions = new List(); if (qualification.AdditionalRequirementQuestions is not null) { + // ReSharper disable once LoopCanBeConvertedToQuery foreach (var questionId in qualification.AdditionalRequirementQuestions) { - addtionalRequirementQuestions.Add(new Reference(SystemLinkTypes.Entry, questionId)); + additionalRequirementQuestions.Add(new Reference(SystemLinkTypes.Entry, questionId)); } } - + var ratioRequirements = new List(); if (qualification.RatioRequirements is not null) { + // ReSharper disable once LoopCanBeConvertedToQuery foreach (var ratioId in qualification.RatioRequirements) { ratioRequirements.Add(new Reference(SystemLinkTypes.Entry, ratioId)); } } - - + var entry = new Entry { SystemProperties = new SystemProperties @@ -275,16 +289,16 @@ private static Entry BuildEntryFromQualification(QualificationUpload qu { { Locale, qualification.AdditionalRequirements ?? "" } }, - - additionalRequirementQuestions = new Dictionary>() + + additionalRequirementQuestions = new Dictionary> { - { Locale, addtionalRequirementQuestions } + { Locale, additionalRequirementQuestions } }, - - ratioRequirements = new Dictionary>() - { - { Locale, ratioRequirements } - } + + ratioRequirements = new Dictionary> + { + { Locale, ratioRequirements } + } } }; @@ -309,13 +323,13 @@ private static List GetQualificationsToAddOrUpdate() var additionalRequirements = t[7]; var additionalRequirementQuestionString = t[8]; var ratioRequirementsString = t[9]; - + string[] additionalRequirementQuestionsArray = []; if (!string.IsNullOrEmpty(additionalRequirementQuestionString)) { additionalRequirementQuestionsArray = additionalRequirementQuestionString.Split(':'); } - + string[] ratioRequirementsArray = []; if (!string.IsNullOrEmpty(ratioRequirementsString)) { @@ -343,7 +357,7 @@ private static List ReadCsvFile(string filePath) { var result = new List(); using var csvParser = new TextFieldParser(filePath); - csvParser.SetDelimiters([","]); + csvParser.SetDelimiters(","); csvParser.HasFieldsEnclosedInQuotes = true; // Skip the row with the column names diff --git a/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs b/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs index 99026cd3..5e1ef7c5 100644 --- a/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs +++ b/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs @@ -96,27 +96,29 @@ private static List IncludeLinkedOrganisations(string awardingOrganisati { var result = new List(); - if (awardingOrganisation is AwardingOrganisations.Edexcel or AwardingOrganisations.Pearson) + switch (awardingOrganisation) { - result.AddRange(new List { AwardingOrganisations.Edexcel, AwardingOrganisations.Pearson }); - } - else if (awardingOrganisation is AwardingOrganisations.Ncfe or AwardingOrganisations.Cache - && startDateMonth.HasValue && startDateYear.HasValue) - { - var cutOffDate = new DateOnly(2014, 9, 1); - var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); - if (date >= cutOffDate) + case AwardingOrganisations.Edexcel or AwardingOrganisations.Pearson: + result.AddRange(new List { AwardingOrganisations.Edexcel, AwardingOrganisations.Pearson }); + break; + case AwardingOrganisations.Ncfe or AwardingOrganisations.Cache when startDateMonth.HasValue && startDateYear.HasValue: { - result.AddRange(new List { AwardingOrganisations.Ncfe, AwardingOrganisations.Cache }); + var cutOffDate = new DateOnly(2014, 9, 1); + var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); + if (date >= cutOffDate) + { + result.AddRange(new List { AwardingOrganisations.Ncfe, AwardingOrganisations.Cache }); + } + else + { + result.Add(awardingOrganisation); + } + + break; } - else - { + default: result.Add(awardingOrganisation); - } - } - else - { - result.Add(awardingOrganisation); + break; } return result; @@ -230,14 +232,14 @@ private List FilterQualificationsByDate(int? startDateMonth, int? return (false, 0, 0); } - if (!Months.TryGetValue(abbreviatedMonth, out var month)) + if (Months.TryGetValue(abbreviatedMonth, out var month)) { - logger.LogError("Qualification date {QualificationDate} contains unexpected month value", - qualificationDate); - - return (false, 0, 0); + return (true, month, yearPart); } - return (true, month, yearPart); + logger.LogError("Qualification date {QualificationDate} contains unexpected month value", + qualificationDate); + + return (false, 0, 0); } } \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index eb5ac3bb..502e3de7 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -131,7 +131,7 @@ private FilterModel GetFilterModel(QualificationListPage content) } var level = userJourneyCookieService.GetLevelOfQualification(); - if (level is not null && level > 0) + if (level > 0) { filterModel.Level = $"Level {level}"; } @@ -148,6 +148,8 @@ private FilterModel GetFilterModel(QualificationListPage content) private static List GetBasicQualificationsModels(List? qualifications) { var basicQualificationsModels = new List(); + + // ReSharper disable once InvertIf if (qualifications is not null && qualifications.Count > 0) { foreach (var qualification in qualifications) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs index 15a00b63..2c4f474c 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs @@ -89,6 +89,8 @@ public async Task WhenWasTheQualificationStarted(DateQuestionMode if (!ModelState.IsValid || !questionModelValidator.IsValid(model)) { var questionPage = await contentService.GetDateQuestionPage(QuestionPages.WhenWasTheQualificationStarted); + + // ReSharper disable once InvertIf if (questionPage is not null) { model = MapDateModel(model, questionPage, nameof(this.WhenWasTheQualificationStarted), Questions); @@ -117,6 +119,8 @@ public async Task WhatLevelIsTheQualification(RadioQuestionModel if (!ModelState.IsValid) { var questionPage = await contentService.GetRadioQuestionPage(QuestionPages.WhatLevelIsTheQualification); + + // ReSharper disable once InvertIf if (questionPage is not null) { model = await MapRadioModel(model, questionPage, nameof(this.WhatLevelIsTheQualification), Questions); @@ -162,6 +166,8 @@ public async Task WhatIsTheAwardingOrganisation(DropdownQuestionM { var questionPage = await contentService.GetDropdownQuestionPage(QuestionPages.WhatIsTheAwardingOrganisation); + + // ReSharper disable once InvertIf if (questionPage is not null) { var qualifications = await GetFilteredQualifications(); @@ -183,13 +189,14 @@ public async Task WhatIsTheAwardingOrganisation(DropdownQuestionM private bool WasAwardedBetweenSeptember2014AndAugust2019() { var (startDateMonth, startDateYear) = userJourneyCookieService.GetWhenWasQualificationAwarded(); - if (startDateMonth is not null && startDateYear is not null) + + if (startDateMonth is null || startDateYear is null) { - var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); - return date >= new DateOnly(2014, 09, 01) && date <= new DateOnly(2019, 08, 31); + return false; } - return false; + var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); + return date >= new DateOnly(2014, 09, 01) && date <= new DateOnly(2019, 08, 31); } private async Task> GetFilteredQualifications() diff --git a/src/Dfe.EarlyYearsQualification.Web/Services/UserJourneyCookieService/UserJourneyCookieService.cs b/src/Dfe.EarlyYearsQualification.Web/Services/UserJourneyCookieService/UserJourneyCookieService.cs index cee77ac0..3a44392d 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Services/UserJourneyCookieService/UserJourneyCookieService.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Services/UserJourneyCookieService/UserJourneyCookieService.cs @@ -106,6 +106,8 @@ public void ResetUserJourneyCookie() int? startDateMonth = null; int? startDateYear = null; var qualificationAwardedDateSplit = cookie.WhenWasQualificationAwarded.Split('/'); + + // ReSharper disable once InvertIf if (qualificationAwardedDateSplit.Length == 2 && int.TryParse(qualificationAwardedDateSplit[0], out var parsedStartMonth) && int.TryParse(qualificationAwardedDateSplit[1], out var parsedStartYear)) diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/DateQuestionModelValidatorTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/DateQuestionModelValidatorTests.cs index 72282b27..cbaa6f9d 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/DateQuestionModelValidatorTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/DateQuestionModelValidatorTests.cs @@ -25,8 +25,8 @@ public void DateQuestionModelValidator_GivenDateInRecentPast_ValidatesTrue() [TestMethod] public void DateQuestionModelValidator_GivenDateEarlierThisMonth_ValidatesTrue() { - var thisYear = 2024; - var thisMonth = 7; + const int thisYear = 2024; + const int thisMonth = 7; var dateTimeAdapter = new Mock(); dateTimeAdapter.Setup(d => d.Now()) @@ -42,8 +42,8 @@ public void DateQuestionModelValidator_GivenDateEarlierThisMonth_ValidatesTrue() [TestMethod] public void DateQuestionModelValidator_GivenDateThisMonthOnFirstOfMonth_ValidatesTrue() { - var thisYear = 2024; - var thisMonth = 7; + const int thisYear = 2024; + const int thisMonth = 7; var dateTimeAdapter = new Mock(); dateTimeAdapter.Setup(d => d.Now()) @@ -59,8 +59,8 @@ public void DateQuestionModelValidator_GivenDateThisMonthOnFirstOfMonth_Validate [TestMethod] public void DateQuestionModelValidator_GivenDateLaterThisYear_ValidatesFalse() { - var thisYear = 2024; - var thisMonth = 7; + const int thisYear = 2024; + const int thisMonth = 7; var dateTimeAdapter = new Mock(); dateTimeAdapter.Setup(d => d.Now()) diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs index 0201912d..99a11936 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Services/UserJourneyCookieServiceTests.cs @@ -344,7 +344,7 @@ public void SetNameSearchCriteria_StringProvided_SetsCookieCorrectly() var service = new UserJourneyCookieService(mockHttpContextAccessor.Object, mockLogger.Object); - var searchCriteria = "This is a test"; + const string searchCriteria = "This is a test"; service.SetQualificationNameSearchCriteria(searchCriteria); var serialisedModelToCheck = JsonSerializer.Serialize(new UserJourneyModel From 72eca3f5ff3ec8aae4df635b6b7ebf1985c42e9e Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 18 Jul 2024 11:15:28 +0100 Subject: [PATCH 4/6] Format blocks. --- .../Services/ContentfulContentFilterService.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs b/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs index 5e1ef7c5..8f538fd9 100644 --- a/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs +++ b/src/Dfe.EarlyYearsQualification.Content/Services/ContentfulContentFilterService.cs @@ -99,9 +99,12 @@ private static List IncludeLinkedOrganisations(string awardingOrganisati switch (awardingOrganisation) { case AwardingOrganisations.Edexcel or AwardingOrganisations.Pearson: + { result.AddRange(new List { AwardingOrganisations.Edexcel, AwardingOrganisations.Pearson }); break; - case AwardingOrganisations.Ncfe or AwardingOrganisations.Cache when startDateMonth.HasValue && startDateYear.HasValue: + } + case AwardingOrganisations.Ncfe or AwardingOrganisations.Cache + when startDateMonth.HasValue && startDateYear.HasValue: { var cutOffDate = new DateOnly(2014, 9, 1); var date = new DateOnly(startDateYear.Value, startDateMonth.Value, 1); @@ -117,8 +120,10 @@ private static List IncludeLinkedOrganisations(string awardingOrganisati break; } default: + { result.Add(awardingOrganisation); break; + } } return result; From 8363a0950a9db10517cf67ed69608c36aa712587 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 18 Jul 2024 11:41:00 +0100 Subject: [PATCH 5/6] Use already-existing Qualification constant. --- .../Program.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs b/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs index a5e371ea..bc70095e 100644 --- a/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs +++ b/content/Dfe.EarlyYearsQualification.ContentUpload/Program.cs @@ -4,6 +4,7 @@ using Contentful.Core.Models; using Contentful.Core.Models.Management; using Contentful.Core.Search; +using Dfe.EarlyYearsQualification.Content.Constants; using Microsoft.VisualBasic.FileIO; namespace Dfe.EarlyYearsQualification.ContentUpload; @@ -15,7 +16,6 @@ public static class Program private const string SpaceId = ""; private const string ManagementApiKey = ""; private const string CsvFile = "./csv/ey-quals-full-2024-with-new-reference-fields.csv"; - private const string QualificationContentTypeId = "Qualification"; private const string FieldTypeSymbol = "Symbol"; // ReSharper disable once UnusedParameter.Global @@ -38,10 +38,12 @@ public static async Task Main(string[] args) private static async Task PopulateContentfulWithQualifications(ContentfulManagementClient client) { // Check existing entries and identify those to update, and those to create. + var queryBuilder = new QueryBuilder>() + .ContentTypeIs(ContentTypes.Qualification) + .Limit(1000); + var currentEntries = - await - client.GetEntriesForLocale(new QueryBuilder>().ContentTypeIs(QualificationContentTypeId).Limit(1000), - Locale); + await client.GetEntriesForLocale(queryBuilder, Locale); var qualificationsToAddOrUpdate = GetQualificationsToAddOrUpdate(); @@ -59,7 +61,7 @@ private static async Task PopulateContentfulWithQualifications(ContentfulManagem // Create new entry var entryToPublish = await client.CreateOrUpdateEntry(entryToAddOrUpdate, - contentTypeId: QualificationContentTypeId, + contentTypeId: ContentTypes.Qualification, version: entryToAddOrUpdate.SystemProperties.Version); await client.PublishEntry(entryToPublish.SystemProperties.Id, @@ -70,7 +72,7 @@ await client.PublishEntry(entryToPublish.SystemProperties.Id, private static async Task SetUpContentModels(ContentfulManagementClient client) { // Check current version of model - var contentTypeModel = await client.GetContentType(QualificationContentTypeId); + var contentTypeModel = await client.GetContentType(ContentTypes.Qualification); var version = contentTypeModel?.SystemProperties.Version ?? 1; @@ -78,9 +80,9 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) { SystemProperties = new SystemProperties { - Id = QualificationContentTypeId + Id = ContentTypes.Qualification }, - Name = QualificationContentTypeId, + Name = ContentTypes.Qualification, Description = "Model for storing all the early years qualifications", DisplayField = "qualificationName", Fields = @@ -179,7 +181,7 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) }; var typeToActivate = await client.CreateOrUpdateContentType(contentType, version: version); - await client.ActivateContentType(QualificationContentTypeId, typeToActivate.SystemProperties.Version!.Value); + await client.ActivateContentType(ContentTypes.Qualification, typeToActivate.SystemProperties.Version!.Value); Thread.Sleep(2000); // Allows the API time to activate the content type await SetHelpText(client); @@ -187,7 +189,7 @@ private static async Task SetUpContentModels(ContentfulManagementClient client) private static async Task SetHelpText(ContentfulManagementClient client) { - var editorInterface = await client.GetEditorInterface(QualificationContentTypeId); + var editorInterface = await client.GetEditorInterface(ContentTypes.Qualification); SetHelpTextForField(editorInterface, "qualificationId", "The unique identifier used to reference the qualification"); SetHelpTextForField(editorInterface, "qualificationName", "The name of the qualification"); @@ -206,7 +208,7 @@ private static async Task SetHelpText(ContentfulManagementClient client) SetHelpTextForField(editorInterface, "ratioRequirements", "The optional ratio requirements", SystemWidgetIds.EntryMultipleLinksEditor); - await client.UpdateEditorInterface(editorInterface, QualificationContentTypeId, + await client.UpdateEditorInterface(editorInterface, ContentTypes.Qualification, editorInterface.SystemProperties.Version!.Value); } From 8a71e7762e445bcdead45143a70db05f258842e8 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 18 Jul 2024 11:45:12 +0100 Subject: [PATCH 6/6] This is an error, but there's no operational effect. --- .../Controllers/QualificationDetailsController.cs | 2 +- .../Controllers/QualificationDetailsControllerTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index 502e3de7..f70f2bfb 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -40,7 +40,7 @@ public IActionResult Refine(string? refineSearch) { if (!ModelState.IsValid) { - logger.LogError($"Invalid model state in {nameof(QualificationDetailsController)} POST"); + logger.LogWarning($"Invalid model state in {nameof(QualificationDetailsController)} POST"); } userJourneyCookieService.SetQualificationNameSearchCriteria(refineSearch ?? string.Empty); diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs index f6889be5..375fe880 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs @@ -312,7 +312,7 @@ public void Refine_NullParam_RedirectsToGet() } [TestMethod] - public void Refine_InvalidModel_LogsError() + public void Refine_InvalidModel_LogsWarning() { var mockLogger = new Mock>(); var mockContentService = new Mock(); @@ -336,6 +336,6 @@ public void Refine_InvalidModel_LogsError() controller.Refine(null); - mockLogger.VerifyError($"Invalid model state in {nameof(QualificationDetailsController)} POST"); + mockLogger.VerifyWarning($"Invalid model state in {nameof(QualificationDetailsController)} POST"); } } \ No newline at end of file