Skip to content

Commit

Permalink
[ODS-6253] Improve error message for RequiredWithNonDefault values (#…
Browse files Browse the repository at this point in the history 
…1035)
  • Loading branch information
@simpat-jesus
simpat-jesus authored May 2, 2024
1 parent acb3e80 commit a8a4eb3
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 16 deletions.
9 changes: 8 additions & 1 deletion Application/EdFi.Ods.Common/Attributes/RequiredWithNonDefaultAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.");
}
Expand All @@ -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()))
Expand Down
14 changes: 8 additions & 6 deletions ...man Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.\", () => {",
Expand Down Expand Up @@ -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.\"",
" ]",
" });",
"});",
""
],
Expand Down
89 changes: 80 additions & 9 deletions Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@
"name": "AcademicWeeks",
"item": [
{
"name": "WeekIdentifier is required validation",
"name": "WeekIdentifier(empty) is required validation",
"event": [
{
"listen": "prerequest",
Expand Down Expand Up @@ -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": [
Expand Down Expand Up @@ -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.\"",
" ]",
" });",
"});"
Expand Down Expand Up @@ -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.\"",
" ]",
" });",
"});"
Expand Down

0 comments on commit a8a4eb3

Please sign in to comment.