Skip to content

Conversation

@agarzola
Copy link
Member

@agarzola agarzola commented Sep 2, 2018

This PR implements the nested relationship fetching described in #11. The related config option now accepts an object within which a property of the Model in question may be used as a key, and its value is another object whose key/value pairs are type: 'property' format. This informs which nested relationships should be requested using dot notation in the request to the API, like so:

/posts/1?include=author.roles

To-Do:

  • Limit depth of nested relationships.
    A strategy must be implemented to ensure we don’t ever fall into an infinite loop. One such strategy is described in the issue comments, but I’m open to ideas.
  • Add comprehensive tests.
    I implemented this with a single happy-path test. Additional tests (and, probably, work) is needed to ensure this feature can handle mixed relationship definitions (e.g. an array of objects and strings at the root of the related config key), supplying a Boolean or an Array of properties as the value for the nested type key (e.g. { related: { author: { person: true } } } and { related: { author: { person: [ 'roles', 'birthplace' ] } } }), and thoroughly testing the nested relationship limit described in the to-do item above.
  • Switch base back to master.
    Since this branch is based on the branch for Fetch relationships in parallel #13, I’ve set the base for this PR to that branch. Once that PR is merged, this should be switched back to master.

@agarzola agarzola added the work in progress This is still a work in progress and should not be merged. label Sep 2, 2018
@agarzola agarzola changed the base branch from master to feature/fetch-rels-in-parallel September 2, 2018 16:23
@agarzola agarzola force-pushed the feature/fetch-nested-relationships branch from 0d27003 to 38e4093 Compare September 12, 2018 17:44
@agarzola agarzola force-pushed the feature/fetch-rels-in-parallel branch from 254ebe0 to 6e02914 Compare September 12, 2018 17:44
@agarzola agarzola changed the base branch from feature/fetch-rels-in-parallel to master November 9, 2018 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

work in progress This is still a work in progress and should not be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants