Skip to content

Fixed: ModelState validation failed when [Range] does not include property default value #1253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2023

Conversation

bkoelman
Copy link
Member

@bkoelman bkoelman commented Feb 4, 2023

Before this change, the JADNC ModelState validation filter would only kick in for required properties, which is incorrect. We didn't have a test for an optional property containing a range validation, where the property default value is not in the required range. In that case, using such a resource as non-toplevel (for example: update relationship) would incorrectly produce a validation error.

For example:

[Attr]
[Range(10, 100)]
public int Value { get; set; }

QUALITY CHECKLIST

… would only kick in for required attributes, which is incorrect. We didn't have a test for an optional property containing a range validation, where the property default value is not in the required range. In that case, using such a resource as non-toplevel (for example: update relationship) would incorrectly produce a validation error.
@bkoelman bkoelman changed the title Fixed: ModelState validation incorrectly fails when [Range] does not contain property default value Fixed: ModelState validation failed when [Range] does not include property default value Feb 4, 2023
@codecov
Copy link

codecov bot commented Feb 4, 2023

Codecov Report

Merging #1253 (7d08a55) into master (c835ff2) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1253      +/-   ##
==========================================
- Coverage   92.62%   92.62%   -0.01%     
==========================================
  Files         243      243              
  Lines        7796     7795       -1     
==========================================
- Hits         7221     7220       -1     
  Misses        575      575              
Impacted Files Coverage Δ
...Core/Configuration/JsonApiModelMetadataProvider.cs 66.66% <100.00%> (-3.34%) ⬇️
...otNetCore/Configuration/JsonApiValidationFilter.cs 95.65% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bkoelman bkoelman marked this pull request as ready for review February 4, 2023 15:27
@bkoelman bkoelman requested a review from maurei February 4, 2023 15:27
@bkoelman bkoelman mentioned this pull request Feb 4, 2023
4 tasks
@maurei maurei merged commit 2324b69 into master Feb 5, 2023
@maurei maurei deleted the fix-modelstate-validation-non-required branch February 5, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants