Deserializer plug #222
Deserializer plug #222
Conversation
There was a problem hiding this comment.
Looks good to me @snewcomer! Do we want to give @jeregrine or @jherdman a chance to look this over before we merge and release?
There was a problem hiding this comment.
Apologies, my head is a little out of the game on this project these days, so I may not be the best person to ask. That said, how does this differ from the UnderscoreParameters plug? This is a bit unclear from the docs included with the new code.
|
Welp you are right. My too quick of a take before I started this PR was it was for query params but I didn't take it to it's logical conclusion. I'll close this and see if I can help with some docs if need be! |
|
Reopening to explore the reverse of serializing... |
…; else dasherized
| "id" => "1", | ||
| "type" => "user" | ||
| "foo-bar" => true, | ||
| "baz-id" => "2" |
There was a problem hiding this comment.
What happens with polymorphic records? A totally contrived example...
"relationships" => %{
"authors" => %{
"data" => [
%{"type" => "staff", "id" => "123"},
%{"type" => "contributor", "id" => "456"}
]
}
}There was a problem hiding this comment.
Great idea! Added a test. I'm not sure though there is a way to process other than this...
738c2b9#diff-ced812e7235b283a4598ca0ae957ecd0R91
Otherwise, down in this file, I have tests for includes with many types.
README.md
Outdated
| 3. Deserializing *incoming* body params requires you add the | ||
| `JSONAPI.Deserializer` Plug to your API's pipeline. Note that the deserializer | ||
| expects the same casing for your *outgoing* params as your *incoming* params. |
There was a problem hiding this comment.
I find this description a tad confusing. The way I understand this plug it sort of "flattens" the parts of a resource object to make it more convenient to use, especially when creating or updating records. The description, as is, has some overlap with the language used by the UnderscoreParmeters section, and has overlap with the sample code used by the UnderscoreParamters pipeline.
How about this:
JSONAPI.Deserializeris a plug designed to make a JSON:API resource object more convenient to work with when creating or updating resources. This plug works by taking the resource object format and flattening it into an easier to manipulate Map.Your pipeline in a Phoenix app might look something like this:
pipeline :api do plug JSONAPI.EnsureSpec # not required, but recommended for Deserializer plug JSONAPI.Deserializer end
There was a problem hiding this comment.
Yes reading again and you are right! Great suggestion!
lib/jsonapi/plugs/deserializer.ex
Outdated
| @@ -0,0 +1,66 @@ | |||
| defmodule JSONAPI.Deserializer do | |||
| @moduledoc """ | |||
| This plug "deserializes" params for ease of use when casting to changesets. | |||
There was a problem hiding this comment.
Assuming my above stated understanding is correct, it may be prudent to use the word "flatten" instead of "deserialize".
| end | ||
| end | ||
|
|
||
| @ct "application/vnd.api+json" |
There was a problem hiding this comment.
You may just want to reference, or alias, JSONAPI.mime_type so that we don't spread this string throughout the code.
| assert result.params["bazId"] == "2" | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
This is really good, but you may want to add a has-many relationship example too.
There was a problem hiding this comment.
I have a bunch of examples in data_to_params_test.exs. Do you think that is sufficient?
Co-Authored-By: James Herdman <james.herdman@gmail.com>
test/utils/data_to_params_test.exs
Outdated
| "id" => "1", | ||
| "type" => "user", | ||
| "foo-bar" => true, | ||
| "baz-id" => ["2", "3"] |
There was a problem hiding this comment.
Doesn't this mean you have a bug? Imagine a system like this... I have a super class called "Car", and I want to create two records, a "Ford" and a "Subaru". The way I'm reading this you've lost the typing information that the user provided — thus in my scenario I wouldn't know that a user was creating a "Subaru" and a "Ford".
close #210