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

[question] Associations, criterias and milestones API #85

Closed
f1nnix opened this issue Aug 13, 2015 · 14 comments
Closed

[question] Associations, criterias and milestones API #85

f1nnix opened this issue Aug 13, 2015 · 14 comments

Comments

@f1nnix
Copy link

f1nnix commented Aug 13, 2015

Hi, guys,

thank you for aweseome lib! I've recently started to play with epilogue, got some questions.

I've got two models: User and Folder. User.hasMany(Folder). So:

...
// Create REST resource
var userResource = epilogue.resource({
  model       : db.User,
  endpoints   : ['/users', '/users/:id']
});

1. Auto-associations?

Let's test:

curl http://192.168.59.103:8089/users/5/

{
  "id": 5,
  "email": "john@doe.com",
  "hash": "...",
  ...
}

curl http://192.168.59.103:8089/users/5/folders/

Cannot GET /users/5/folders/

How to make epilogue handle related associations, but not to include them while ordinary request? I mean, I've tried

{
  accociations: [db.Folder]
}

and

{
  include: [db.Folder]
}

but epilogue includes [Folders] even in request to User node (/users/5). Instead if this I'd like epilogue to handle /users/5/folders, /users/5/folders/:id

2. Incorrect where attributes

My API provides aliases for id. /users/me/ === /users/5, if current user's ID is 5. So, I've tried such approach:

users.read.fetch.before(function(req, res, context) {
  var userId = req.params.user_id

  if (userId === "me") {
    context.criteria.id = req.user.id
  }
  return context.continue;
})

Not working. Any ideas?

3. Milestones API?

It seems midlestones API is a bit messy right now. Some snippets points for

users.read.fetch.BEFORE({...})

Some — without before node:

users.read.auth({...})

Which is the proper way to use it? Which hooks are invoked for which controller? Any actual docs or the best option is to check the source?

BTW, I would be happy to help with docs as soon as I get this to work as expected myself :) Very helpful lib, just what I needed.

@mbroadst
Copy link
Collaborator

Howdy, hopefully these answer your questions a bit:

  1. auto-association is not included in master, however we have an auto-association branch with work I started that was finished up by @Fridus. You can give that a shot to see if it does what you need, I'm waiting for the final okay from @Fridus as to whether we should merge it in for a point release

  2. this should work, can you please open a new issue so we can discuss this if there is an actual problem here.

  3. I wouldn't say they are exactly messy, but maybe a little inconsistent. Have you taken a look at this yet? It seems to answer most of your milestone questions, specifically "which hooks are invoked for which controller" (https://github.com/dchester/epilogue/blob/master/docs/Milestones.md#default-milestones-for-actions)

Any help with the documentation effort is most welcome!

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

@mbroadst Hi, Matt, thanks for fast response. I'll check it out and come back with the feedback later today,

@mbroadst
Copy link
Collaborator

It might be worth noting that for milestones: if a milestone isn't set (e.g. resource.fetch hasn't been declared by default in the controller, or by you), then the before and after hooks are not run for it, which makes total sense to me but may be initially confusing

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

@mbroadst About second question

root@6636e419acf7:/src/node_modules/epilogue# git branch
* auto-associations
  master
// user.js
User('...', {
  classMethods: {
    associate: function(models) {
      User.hasMany(models.Folder, {
        onDelete: 'cascade',
        hooks: true
      });
      ...
// api.js
var users = epilogue.resource({
  model       : db.User,
  associations: [db.Folder],
  endpoints   : ['/api/v0/users', '/api/v0/users/:id']
});

users.use(require('./users/users.js'))

Trying:

curl http://192.168.59.103:8089/api/v0/users/5/folders

{"message":"Not Found","error":{"status":404},"stack":"Error: Not Found\n...

I suppose, I'd doing something wrong. Could you point me the right direction?

Thanks you for your help.

@mbroadst
Copy link
Collaborator

@f1nnix we don't have docs written up for auto-association yet, but you can take a look at the tests for how we've been using it. You would specify:

// api.js
var users = epilogue.resource({
  model       : db.User,
  associations: true,
  endpoints   : ['/api/v0/users', '/api/v0/users/:id']
});

users.use(require('./users/users.js'))

and it should automatically associate everything for you (that's already defined as an association through Sequelize definitions)

@Fridus
Copy link
Contributor

Fridus commented Aug 13, 2015

@f1nnix Try associations: true

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

@mbroadst @Fridus No luck :(

var users = epilogue.resource({
  model       : db.User,
  associations: true,
  endpoints   : ['/api/v0/users', '/api/v0/users/:id']
});
GET /api/v0/users/5/folders HTTP/1.1
Host: 192.168.59.103:8089
Cache-Control: no-cache
{
  "message": "Not Found",
  "error": {
    "status": 404
  },
  ...
}

To ensure, Folder is associated to User, let's try to include it into root node request:

var users = epilogue.resource({
  model       : db.User,
  include     : [db.Folder],
  associations: true,
  endpoints   : ['/api/v0/users', '/api/v0/users/:id']
});
GET /api/v0/users/5 HTTP/1.1
Host: 192.168.59.103:8089
Cache-Control: no-cache
{
  "id": 5,
  "email": "john@doe.com",
  "hash": "...",
  "Folders": [
    {
      "id": 5,
      "title": "untitled",
      "UserId": 5,
      ...
    }
  ]
}

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

Looked through the tests, hmm. Code looks the same at first look. Still digging...

Thanks, guys. I'll come back soon, if I find the problem.

@mbroadst
Copy link
Collaborator

@f1nnix try not specifying the end points (these will be overridden anyway with auto associations). you can still specify a base URL for all of them (/api/v0 in this case).

Also there are very likely issues with pluralization, though I tried to make some safe guesses, so mess around with "folder" vs "folders" etc.

Sorry for the trouble, this is still an incomplete feature :)

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

@mbroadst I've finally made it to work. Not sure what was the problem, but after rebuilding all the dev environment and specifying

"epilogue": "git+ssh://git@github.com/dchester/epilogue.git#auto-associations",

in package.json, and then npm install, it just works. Hope, this tip will be helpful for someone else.

My settings (as you suggested):

epilogue.initialize({
  app: app,
  sequelize: db.sequelize,
  base: '/api/v0'
});

var users = epilogue.resource({
  model: db.User,
  associations: true
});

Now:

curl http://192.168.59.103:8089/api/v0/users/2/folders

[
  {
    "id": 1,
    "title": "untitled",
    ...
  }
]

However, I've noticed, that for request to root note (User), Folder is also populated:

curl http://192.168.59.103:8089/api/v0/users/

[
  {
    "id": 2,
    "email": "...",
    "hash": null,
    ...
    "Folders": [
      {
        "id": 1,
        "title": "untitled",
        ...
        "UserId": 2
      }
    ]
  }
]

Is it supposed behavour?

IMHO, it's not quite correct to auto-populate associations. In fact /users/ — is a request to User model. If I want to get user.folders, I can explicitly request /users/1/folders. I may be wrong, but master branch had a separate option include: [], that allowed to explicitly specify which models should be populated in root note request. Is there a way to disable auto-population for root note, preserving auto-generated association endpoints?

Anyway, it's working now. Thank you for your work!

@f1nnix
Copy link
Author

f1nnix commented Aug 13, 2015

BTW,

GET /api/v0/users/2/folders/1 200 17.621 ms - 44
POST /api/v0/users/2/folders 404 69.192 ms - 2264

Are all CRUD actions created for associated models? Can I create folder for user, performing POST to /users/me/folders?

According to tests, only read and list actions are supported at this moment. If I'm right, do you have any plans to implement others?

Thanks.

@mbroadst
Copy link
Collaborator

@f1nnix you'll probably want to do a quick read over this issue: #34, where this feature was discussed at length. You'll see that @Fridus only continued my work to finish off support for read (and list on a few) operations on associated routes. In fact, the add/update case is non-trivially more complicated and it seems few people have the time needed to implement it :)

As for include that's actually still available everywhere even in the auto-associations branch (in fact auto-associations are implemented USING include). I tend to agree with you that we should allow the user to retrieve as little or as much information as possible, however as it stands today it looks like all the info is populated. Again, help here would be more than welcome, I'm very willing to walk you through a PR related to this if you'd like.

@sdebionne
Copy link
Contributor

@mbroadst I'm considering a PR to add an option (or change the behavior) of resource with associations. I also don't think it is correct to return a resource and its associations when the primary resource endpoints are called without specifying an association. In a typical use case where we have users and projects, /users or /users/:id should not return associated projects. Only /users/:id/projects should. Are you still looking for help on this?

@mbroadst
Copy link
Collaborator

@sdebionne yes certainly still interested in help with the module, please open up a new issue with your proposed changes so there can be some discussion about it. The root of this problem is that many different frameworks do this differently, and we find ourselves with people using this module from all of them - there needs to be compromise at some point 😄

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

4 participants