From a8a4eb3b10b1ad03d73291c2852fcae28922aff3 Mon Sep 17 00:00:00 2001 From: Jesus Flores <43448209+simpat-jesus@users.noreply.github.com> Date: Wed, 1 May 2024 23:45:14 -0600 Subject: [PATCH] [ODS-6253] Improve error message for RequiredWithNonDefault values (#1035) --- .../RequiredWithNonDefaultAttribute.cs | 9 +- ...uite ResponseTests.postman_collection.json | 14 +-- ...gration Test Suite.postman_collection.json | 89 +++++++++++++++++-- 3 files changed, 96 insertions(+), 16 deletions(-) diff --git a/Application/EdFi.Ods.Common/Attributes/RequiredWithNonDefaultAttribute.cs b/Application/EdFi.Ods.Common/Attributes/RequiredWithNonDefaultAttribute.cs index bd4b90f9cf..4c5191bac2 100644 --- a/Application/EdFi.Ods.Common/Attributes/RequiredWithNonDefaultAttribute.cs +++ b/Application/EdFi.Ods.Common/Attributes/RequiredWithNonDefaultAttribute.cs @@ -3,6 +3,7 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +using System; using System.ComponentModel.DataAnnotations; using EdFi.Ods.Common.Extensions; @@ -19,7 +20,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali { if (value is string s) { - if (string.IsNullOrEmpty(s)) + if (s.Equals(string.Empty)) { return BuildValidationResult($"{validationContext.DisplayName} is required and should not be left empty."); } @@ -36,6 +37,12 @@ protected override ValidationResult IsValid(object value, ValidationContext vali // In case of decimal types, accept default values return ValidationResult.Success; } + else if (value is DateTime dt && dt.Equals(value.GetType().GetDefaultValue())) + { + // DateTime default value is very specific (1/1/0001 12:00:00 AM), so it can be confusing for users + // if they didn't used that value; no need to inform more than "the field is required" + return BuildValidationResult($"{validationContext.DisplayName} is required."); + } else if (value != null) { if (value.Equals(value.GetType().GetDefaultValue())) diff --git a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json index 8fd953aaff..a412cb9ca7 100644 --- a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json +++ b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json @@ -3969,7 +3969,7 @@ "script": { "exec": [ "pm.test(\"Status code is 400\", () => {", - " pm.expect(pm.response.code).to.equal(400);", + " pm.expect(pm.response.code).to.equal(400);", "});", "", "pm.test(\"Should return a message indicating that Validation of 'Student' failed.LastSurname is required.\", () => {", @@ -6202,16 +6202,18 @@ " pm.expect(pm.response.code).to.equal(400);", "});", "", - "const problemDetails = pm.response.json();", + "pm.test(\"Should return a message indicating that Validation of 'Student' failed.LastSurname is required.\", () => {", + " const problemDetails = pm.response.json();", "", - "pm.test(\"Should return a problem details result\", () => {", " pm.expect(pm.response.code).equal(problemDetails.status);", " pm.expect(problemDetails.type).to.equal(\"urn:ed-fi:api:bad-request:data\");", " pm.expect(problemDetails.detail).to.equal(\"Data validation failed. See 'validationErrors' for details.\");", - "});", "", - "pm.test(\"Should return a message indicating that Validation of 'Student' failed.LastSurname is required.\", () => {", - " pm.expect(problemDetails.validationErrors[\"$.lastSurname\"]).to.contain(\"LastSurname is required and should not be left empty.\");", + " pm.expect(problemDetails.validationErrors).to.deep.equal({", + " \"$.lastSurname\": [", + " \"LastSurname is required and should not be left empty.\"", + " ]", + " });", "});", "" ], diff --git a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json index c5ffd6325c..52e5cdbdfc 100644 --- a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json +++ b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json @@ -960,7 +960,7 @@ "name": "AcademicWeeks", "item": [ { - "name": "WeekIdentifier is required validation", + "name": "WeekIdentifier(empty) is required validation", "event": [ { "listen": "prerequest", @@ -1032,6 +1032,77 @@ }, "response": [] }, + { + "name": "WeekIdentifier(null) is required validation", + "event": [ + { + "listen": "prerequest", + "script": { + "exec": [ + "" + ], + "type": "text/javascript" + } + }, + { + "listen": "test", + "script": { + "exec": [ + "pm.test(\"Status code is 400\", () => {", + " pm.expect(pm.response.code).to.equal(400);", + "});", + "", + "const problemDetails = pm.response.json();", + "", + "pm.test(\"Should return a problem details result\", () => {", + " pm.expect(pm.response.code).equal(problemDetails.status);", + " pm.expect(problemDetails.type).to.equal(\"urn:ed-fi:api:bad-request:data\");", + " pm.expect(problemDetails.detail).to.equal(\"Data validation failed. See 'validationErrors' for details.\");", + "});", + "", + "pm.test(\"Should return the item with WeekIdentifier is required string\", () => {", + " pm.expect(problemDetails.validationErrors).to.deep.equals({", + " \"$.weekIdentifier\": [", + " \"WeekIdentifier is required.\"", + " ]", + " });", + "});", + "" + ], + "type": "text/javascript" + } + } + ], + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "name": "Content-Type", + "type": "text", + "value": "application/json" + } + ], + "body": { + "mode": "raw", + "raw": "{\r\n \"schoolReference\": {\r\n \"schoolId\": \"1234567\"\r\n },\r\n \"beginDate\": \"2020-02-20\",\r\n \"endDate\": \"2020-02-27\",\r\n \"totalInstructionalDays\": \"45\"\r\n}" + }, + "url": { + "raw": "{{ApiBaseUrl}}/data/v3/ed-fi/academicWeeks", + "host": [ + "{{ApiBaseUrl}}" + ], + "path": [ + "data", + "v3", + "ed-fi", + "academicWeeks" + ] + }, + "description": "This api post method adds new academicWeeks for particular school .\nThis test method will throw WeekIdentifier is required error when WeekIdentifier is not passed" + }, + "response": [] + }, { "name": "SchoolId is required validation", "event": [ @@ -1401,18 +1472,18 @@ " pm.expect(problemDetails.detail).to.equal(\"Data validation failed. See 'validationErrors' for details.\");", "});", "", - "pm.test(\"Should return the item with BeginDate must be different than default value\", () => {", + "pm.test(\"Should return the item with BeginDate is required\", () => {", " pm.expect(problemDetails.validationErrors).to.deep.include({", " \"$.beginDate\": [", - " \"BeginDate value must be different than 1/1/0001 12:00:00 AM.\"", + " \"BeginDate is required.\"", " ]", " });", "});", "", - "pm.test(\"Should return the item with EndDate must be different than default value\", () => {", + "pm.test(\"Should return the item with EndDate is required\", () => {", " pm.expect(problemDetails.validationErrors).to.deep.include({", " \"$.endDate\": [", - " \"EndDate value must be different than 1/1/0001 12:00:00 AM.\"", + " \"EndDate is required.\"", " ]", " });", "});" @@ -1967,18 +2038,18 @@ " pm.expect(problemDetails.detail).to.equal(\"Data validation failed. See 'validationErrors' for details.\");", "});", "", - "pm.test(\"Should return the item with BeginDate must be different than default value\", () => {", + "pm.test(\"Should return the item with BeginDate is required\", () => {", " pm.expect(problemDetails.validationErrors).to.deep.include({", " \"$.beginDate\": [", - " \"BeginDate value must be different than 1/1/0001 12:00:00 AM.\"", + " \"BeginDate is required.\"", " ]", " });", "});", "", - "pm.test(\"Should return the item with EndDate must be different than default value\", () => {", + "pm.test(\"Should return the item with EndDate is required\", () => {", " pm.expect(problemDetails.validationErrors).to.deep.include({", " \"$.endDate\": [", - " \"EndDate value must be different than 1/1/0001 12:00:00 AM.\"", + " \"EndDate is required.\"", " ]", " });", "});"