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

Enhancements to "with" #65

Open
techniq opened this issue Feb 16, 2016 · 3 comments
Open

Enhancements to "with" #65

techniq opened this issue Feb 16, 2016 · 3 comments

Comments

@techniq
Copy link
Member

techniq commented Feb 16, 2016

Just logging some notes to think about at this point

Retrieve a subset of properties

var users = yield Event.findAll({...}, {
  with: [
    // load all comments, what we currently support
    'comments',

    // load only comments with category_id = 1 (shorthand for '==')
    {comments: {category_id: 1}},

    // load only comments with category_id > 1
    {comments: {category_id: {'>': 1}}}
  ]
});

Support for adding calculated/aggregate properties

var events = yield Event.findAll({...}, {
  with: [
    // add `review_count` property to each event
    {review_count: {'review.rating': 'count'}},

    // add `review_average` property to each event
    {review_average: {'review.rating': 'average'}},
  ]
});
@jmdobry
Copy link
Member

jmdobry commented Feb 16, 2016

I'm thinking rather than {comments: {category_id: 1}} we should do:

{
  relation: 'comments', // "relation" is either the "localField" or name of the related resource
  query: { category_id: 1 } // "query" follows JSData's query syntax
}

@techniq
Copy link
Member Author

techniq commented Feb 16, 2016

hmm, it's more verbose (which might not be a bad thing). I had all intentions of support all of JSData's query syntax with {comments: /* query syntax */}

What are you thoughts on the calculated/aggregate properties?

@jmdobry
Copy link
Member

jmdobry commented Feb 16, 2016

The issue with {comments: /* query syntax */} is it has to be parsed using something like Object.keys, whereas { relation: 'comments' ... gives instance information about which relation we're dealing with.

Maybe with isn't the right place to specify aggregations that should be included in the result? with is an array, seems like we need an object here, e.g.

var events = yield Event.findAll({...}, {
  // This object is essentially "merged" into the result
  // This seems fully expressive
  compute: {
    // add `review_count` property to each event
    review_count: {
      relation: 'review',
      query: {}, // optional,
      op: 'count'
    },
    // add `review_average` property to each event
    review_average: {
      relation: 'review',
      query: {}, // optional selection query
      op: 'average',
      field: 'rating'
    }
  }
});

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

No branches or pull requests

2 participants