Validations Send source.pointer #371
Conversation
| @@ -6,7 +6,7 @@ namespace JsonApiDotNetCore.Extensions | |||
| { | |||
| public static class ModelStateExtensions | |||
| { | |||
| public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState) | |||
| public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDictionary modelState, IContextGraph contextGraph) | |||
There was a problem hiding this comment.
So, that we don't have to release this in a major version, let's leave the original implementation and deprecate it using the [Obsolete("Use the generic overload instead")]. This prevents us from breaking anyone that is calling this API directly.
Also, I've recently been leaning towards the idea of depending on the static disregard, that's going to be too big of an issue to bite off right now (i.e. there will be broken tests)ContextGraph.Instance rather than injecting everywhere. I haven't found many reasons the ContextGraph should change after app start. Thoughts?
There was a problem hiding this comment.
🤔 for ContextGraph.Instance I wonder if that's safe given possible different versions of the API running at once with different context graphs?
If ContextGraph.Instance is used other places that could still be an issue there, but IDK if we'd want to tie this down too?
| @@ -16,10 +16,19 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary | |||
|
|
|||
| foreach (var modelError in entry.Value.Errors) | |||
| { | |||
| var attrName =contextGraph.GetPublicAttributeName<T>(entry.Key); | |||
There was a problem hiding this comment.
let's do this lookup outside this loop since entry.Key doesn't change...results in unnecessary enumerations of the graph
No description provided.