Closed danelowe closed 9 years ago
@jsmestad Apologies, I wasn't aware of Responses::Stable
Responses::Beta
. Just found them on the branch feature/jsonapi-beta-parser
, but was working off develop
, which does not seem to have those two modules, just Responses
.
What is your suggestion?
Ah good call. I should have merged that into develop. I guess I did not push that commit up. - To your question, yes push it onto that jsonapi-beta-parser branch and then ill merge both into develop
Hey @jsmestad. Looking at what you've got there, I expect it already covers off what I'd done on the parsing side of things. I was not aware of the previously. The other two things that would be changing are:
The question is, do you envisage there being multiple versions of those two things also?
If so, there's not much that can be re-used here (there wasn't much changed anyway), and it might be better if one were to start again?
What are your thoughts on how to proceed? I'm happy to re-do changes onto separate serializer/query objects for the version if you wished.
@danelowe The plan was to make the parser and serializer pluggable. We will only ship a default (removing the "Stable" parser before we release) for the JSONAPI standard. - The pluggable approach resolves issues where users are using jsonapi-consumer with an API that supports custom JSONAPI extensions that must be parsed.
Hey @jsmestad I've started breaking out the query and serializer into separate objects per version in order to have the 'data' key present in create/update requests (which I need in order for it to work with my resource server).
https://github.com/danelowe/jsonapi-consumer/commit/5aff3ca8d13dcea8d4b25d4e357162fcbb275ebf
Are you interested in looking at a PR for this approach?
Also, thinking on it further, it makes sense (to me) if the logic in the below method were moved from the query to the serializer
def build_params(args)
@params = {klass.json_key => args}
end
The reason being that the serializer would eventually have to deal with adding the 'included'
node, which is on the same level as the 'data'
node.
This would also mean there is probably no reason to have separate query objects (profiles) per JSONAPI version.
A few ideas to take this further:
'data'
and 'included'
nodes, and could call the serializers on individual included items.What are your thoughts?
@danelowe so it looks like you switched the
Responses::Stable
spec to use thedata
attribute, if I read that right? - I think you meant to look atResponses::Beta
for the new payload schema