Handle nested JSON and JSON arrays in params properly #1338
Conversation
a-alhusaini
changed the title
| @@ -6,7 +6,7 @@ module Amber::Validators | |||
| class BaseRule | |||
| getter predicate : (String -> Bool) | |||
| getter field : String | |||
| getter value : String? | |||
| getter value : String | Array(JSON::Any) | JSON::Any | Nil | |||
There was a problem hiding this comment.
JSON::Any::Type includes Array(JSON::Any), so is it redundant to include both, or is there something I'm missing?
There was a problem hiding this comment.
That was my thought as well, so I don't think we need the extra Array(JSON::Any)
| @@ -1,4 +1,9 @@ | |||
| module ValidationsHelper | |||
| def json_params_builder(body = "") | |||
| request = HTTP::Request.new("POST", "/", HTTP::Headers{"Content-Type" => "application/json"}, body) | |||
There was a problem hiding this comment.
This is hard coded as a POST request. Does that mean that it can only be used with POST requests, or can it be used with PUT and PATCH requests too?
There was a problem hiding this comment.
Well this is just a test, but we should be testing POST PATCH AND PUT
| required("y") | ||
| end | ||
|
|
||
| validator.validate!["x"].should be_a Array(JSON::Any) |
There was a problem hiding this comment.
Maybe returning a JSON::PullParser would be better here, so people could use JSON::Serializable to decode these parameters. 🤔
There was a problem hiding this comment.
When was that introduced in the language? I don't recall seeing this at the time I wrote this PR.
There was a problem hiding this comment.
@a-alhusaini it's been part of the std library for a few years
Before:
After