From 43a152f8e9a578f57b1876c00821d9b9951839b1 Mon Sep 17 00:00:00 2001 From: Daniel Clarke Date: Thu, 1 Aug 2024 11:24:58 +0100 Subject: [PATCH] fix for the additional requirements checks not matching up questions to answers properly --- .../QualificationDetailsController.cs | 68 ++++++++++++------- .../QualificationDetailsControllerTests.cs | 15 +++- 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index e6ed0a93..1f165d2d 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -94,35 +94,51 @@ public async Task Index(string qualificationId) return View(model); } - + private (bool isValid, IActionResult? actionResult) ValidateAdditionalQuestions(Qualification qualification, QualificationDetailsModel model) { - // If the qualification has no additional requirements then skip this check - if (qualification.AdditionalRequirementQuestions != null && - qualification.AdditionalRequirementQuestions.Count != 0) + // If the qualification has no additional requirements then skip all checks and return. + if (qualification.AdditionalRequirementQuestions == null || + qualification.AdditionalRequirementQuestions.Count == 0) return (true, null); + + var additionalRequirementsAnswers = userJourneyCookieService.GetAdditionalQuestionsAnswers(); + + // If there is a mismatch between the questions answered, then clear the answers and navigate back to the additional requirements check page + if (additionalRequirementsAnswers == null || + qualification.AdditionalRequirementQuestions.Count != additionalRequirementsAnswers.Count) { - var additionalRequirementsAnswers = userJourneyCookieService.GetAdditionalQuestionsAnswers(); + return (false, + RedirectToAction("Index", "CheckAdditionalRequirements", + new { qualification.QualificationId })); + } - // If there is a mismatch between the questions answered, then clear the answers and navigate back to the additional requirements check page - if (additionalRequirementsAnswers == null || - qualification.AdditionalRequirementQuestions.Count != additionalRequirementsAnswers.Count) - { - return (false, RedirectToAction("Index", "CheckAdditionalRequirements", new { qualification.QualificationId })); - } + // If there are not any answers to the questions that are not full and relevant we can continue back to check the ratios. + if (!CheckForAnyNonFAndRAnswers(qualification.AdditionalRequirementQuestions, + additionalRequirementsAnswers)) return (true, null); + + // At this point, there will be at least one question answered in a non full and relevant way. + // we mark the ratios as not full and relevant and return. + model.RatioRequirements = MarkAsNotFullAndRelevant(model.RatioRequirements); + return (false, View(model)); - if ((from question in qualification.AdditionalRequirementQuestions + } + + /// + /// A function to take in the additional requirement questions and answers, match them up and check to see if the + /// user has answered any in a non full and relevant way. + /// + /// This should come from the qualification model + /// This should come from the user's selected answers + /// True if we find any question answered in a non full and relevant way, false if none are found + private static bool CheckForAnyNonFAndRAnswers(List additionalRequirementQuestions, + Dictionary additionalRequirementsAnswers) + { + return (from question in additionalRequirementQuestions from answer in additionalRequirementsAnswers - where (question.AnswerToBeFullAndRelevant && answer.Value == "no") || - (!question.AnswerToBeFullAndRelevant && answer.Value == "yes") - select question).Any()) - { - model.RatioRequirements = MarkAsNotFullAndRelevant(model.RatioRequirements); - return (false, View(model)); - } - } - - return (true, null); + where question.Question == answer.Key && ((question.AnswerToBeFullAndRelevant && answer.Value == "no") || + (!question.AnswerToBeFullAndRelevant && answer.Value == "yes")) + select question).Any(); } private void CheckRatioRequirements(int startDateYear, Qualification qualification, QualificationDetailsModel model) @@ -133,13 +149,13 @@ private void CheckRatioRequirements(int startDateYear, Qualification qualificati model.RatioRequirements.ApprovedForLevel2 = CheckRatio(propertyToCheck, RatioRequirements.Level2RatioRequirementName, qualification); - + model.RatioRequirements.ApprovedForLevel3 = CheckRatio(propertyToCheck, RatioRequirements.Level3RatioRequirementName, qualification); - + model.RatioRequirements.ApprovedForLevel6 = CheckRatio(propertyToCheck, RatioRequirements.Level6RatioRequirementName, qualification); - + model.RatioRequirements.ApprovedForUnqualified = true; } @@ -154,7 +170,7 @@ private bool CheckRatio(string propertyToCheck, string ratioName, Qualification catch { logger.LogError("Could not find property: {PropertyToCheck} within {RatioName} for qualification: {QualificationId}", - propertyToCheck, ratioName, qualification.QualificationId); + propertyToCheck, ratioName, qualification.QualificationId); throw; } } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs index 95d6bd5c..616b018c 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs @@ -212,8 +212,11 @@ public async Task Index_QualificationHasAdditionalQuestionsButNoneAnswered_Redir } [TestMethod] + [DataRow("no", "no")] + [DataRow("yes", "yes")] + [DataRow("no", "yes")] public async Task - Index_Index_QualificationHasAdditionalQuestionsButAnswersAreNotCorrect_MarkAsNotRelevantAndReturn() + Index_Index_QualificationHasAdditionalQuestionsButAnswersAreNotCorrect_MarkAsNotRelevantAndReturn(string answer1, string answer2) { var mockLogger = new Mock>(); var mockContentService = new Mock(); @@ -221,7 +224,7 @@ public async Task var mockInsetTextRenderer = new Mock(); var mockHtmlRenderer = new Mock(); var mockUserJourneyCookieService = new Mock(); - + const string qualificationId = "eyq-145"; var listOfAdditionalReqs = new List @@ -230,13 +233,19 @@ public async Task { Question = "Some Question", AnswerToBeFullAndRelevant = true + }, + new() + { + Question = "Another Question", + AnswerToBeFullAndRelevant = false } }; // Question has been answered, but the answer is not what we want for the qualification to be full and relevant var listOfAdditionalReqsAnswered = new Dictionary() { - { "Some Question", "no" } + { "Some Question", answer1 }, + { "Another Question", answer2 } }; var qualificationResult = new Qualification(qualificationId, "Qualification Name", "NCFE", 2, "2014", "2019",