Skip to content
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

Extract IncludeDirectives. #679

Closed
wants to merge 1 commit into from

Conversation

beauby
Copy link
Contributor

@beauby beauby commented Apr 16, 2016

Extract IncludeDirectives into the jsonapi_parser gem, in an attempt to share work between ActiveModelSerializers and jsonapi-resources (c.f. rails-api/active_model_serializers#1685).

c.f. https://github.com/beauby/jsonapi_parser/blob/master/lib/json/api/include_directive.rb

@beauby
Copy link
Contributor Author

beauby commented Apr 16, 2016

Note that Travis is failing because of dependency resolution failing for rails 4.1 (jsonapi_parser currently requires activesupport ~> 4.2, but this is not a strong requirement at all, I'll change it if there is interest in this PR).

@beauby
Copy link
Contributor Author

beauby commented Apr 17, 2016

A bit of context here:

  • The proposed IncludeDirective implementation has an underlying structure of a Hash. example:
{
  posts: {
    author: {},
    comments: {
      author: {}
    }
  }
}

whereas the current implementation has a somewhat similar structure, with the addition of an included flag for each relationship (which in practice is always true), and the sub-relationships are stored under an :include_related key.

  • The proposed implementation has the additional benefit of handling wildcards (* for "all relationships", ** for "all relationships and their relationships, and so on recursively"). Those are not part of the spec, and are therefore optional (using the :allow_wildcard option to the IncludeDirective constructor – not enabled in this PR). There currently are discussions whether the spec should allow some kind of wildcard, and whichever choice is made would be easily added in the proposed implementation.

@beauby beauby closed this Apr 18, 2016
@beauby beauby reopened this Apr 18, 2016
@lgebhardt
Copy link
Member

@beauby Good idea. I'm looking at the parsing logic for requests again (in anticipation of support for Operations in the spec), so this is good timing. We should coordinate on the features.

The included flag is a remnant from before the spec was finalized, and can probably be removed (I doubt anyone is using it).

@beauby beauby force-pushed the extract-include-directives branch from 0db26e3 to d59a077 Compare May 24, 2016 16:13
@beauby
Copy link
Contributor Author

beauby commented May 24, 2016

Rebased.

@beauby beauby closed this Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants