feathersjs-ecosystem / feathers-sequelize

A Feathers service adapter for the Sequelize ORM. Supporting MySQL, MariaDB, Postgres, SQLite, and SQL Server
MIT License
208 stars 75 forks source link

How do you retrieve associations in the response? #12

Closed lionelrudaz closed 8 years ago

lionelrudaz commented 8 years ago

Hi guys,

I can't find that in any documentation.

I have a conversation model that has many messages.

I'd like to add messageIds to the response of the REST calls to conversation.

How can I do that? Is it with a hook?

I'd like to follow the recommandations from here: https://guides.emberjs.com/v1.10.0/models/the-rest-adapter/

Let me know if you need more information.

Thanks in advance,

Lionel

ekryski commented 8 years ago

@lionelrudaz It's late here so I'm just off to bed but you should be able to just use an after hook and make a call to your other service.

// after hook
function(options) {
  return function(hook) {
    return hook.app.service('/messages').find({ conversationId: hook.result.conversation.id}).then(result => {
       return hook.result.conversation.messages = result;
    });
  }
}

I think that should be the idea, or at least be close.

ekryski commented 8 years ago

You can check out hook docs right here: http://docs.feathersjs.com/hooks/readme.html

ekryski commented 8 years ago

@lionelrudaz since you are using Postgres and therefore using Sequelize you could also utilize the include directive. http://docs.sequelizejs.com/en/latest/docs/associations/

Then just normalize the data however you want it to look in an after hook.

lionelrudaz commented 8 years ago

Hi Erik,

Thanks for your quick answer.

I tried the first solution and got two issues:

1) The service retrieves all the messages. It's like the params aren't taken into account

2) I've got the error: Error: after hook for 'get' method returned invalid hook object

With Sequelize, the thing is that include seems to be available only on an instance, and examples show only that it works with creation. So I found that we can query for associated objects here http://docs.sequelizejs.com/en/latest/docs/associations/#associating-objects. But in that way, that means that I have to create a conversation instance first, right?

Sorry if my questions sound newbie. That's what I am :-)

Thanks again

daffl commented 8 years ago

Easy to miss, on the server, find parameters have to be in a query property:

hook.app.service('/messages').find({
  query: {
    conversationId: hook.result.conversation.id
  }
})

Don't the service methods return Sequelize model instances? I'm just familiar enough with Sequelize to get the adapter to work so I'm not sure. How do you retrieve a model and it's associations?

lionelrudaz commented 8 years ago

Now the request works, thanks.

But I still get the following error:

Error: after hook for 'get' method returned invalid hook object

Here's the function:

var showMessages = function() {
  return function(hook) {

    return hook.app.service('/api/v1/messages').find({
      query: {
        conversationId: hook.result.conversation.id
      }
    }).then(result => {
       return hook.result.conversation.messages = result;
    });
  }
}

You can view the code here: https://github.com/lionelrudaz/wellnow-node

The services are in app.js for the moment and the models are in the models directory.

I set the model to the service like that:

app.use('/api/v1/conversations', service({
  Model: models.conversation
}));

Any thoughts?

daffl commented 8 years ago

Hooks expect you to return hook, a Promise that returns hook or nothing. Yours would look like:

var showMessages = function() {
  return function(hook) {

    hook.app.service('/api/v1/messages').find({
      query: {
        conversationId: hook.result.conversation.id
      }
    }).then(result => {
       hook.result.conversation.messages = result;
    });
  }
}
lionelrudaz commented 8 years ago

That's weird. The response isn't changed.

I tried this:

var showMessages = function() {
  return function(hook) {
    hook.result.conversation.test = "Test";
  }
}

The response remain also unchanged.

{
conversation: {
id: 1,
title: "Test conversation",
createdAt: "2016-01-27T22:33:37.936Z",
updatedAt: "2016-01-27T22:33:37.936Z"
}
}

When I test without conversation:

var showMessages = function() {
  return function(hook) {
    hook.result.test = "Test";
  }
}

It shows me the test value:

{
conversation: {
id: 1,
title: "Test conversation",
createdAt: "2016-01-27T22:33:37.936Z",
updatedAt: "2016-01-27T22:33:37.936Z"
},
test: "Test"
}

I wonder if it's because I have two after hooks for the conversation service. One to add the results in conversation, one to add the association.

Any clue?

daffl commented 8 years ago

What does console.log(hook.result) give you? One reason may be that it actually returns a model instance and doesn't allow adding properties to it (similar to Mongoose models). Try

var showMessages = function() {
  return function(hook) {
    var conversations = hook.result.conversations.get({ plain: true });
    conversations.test = 'Test';

    hook.result.conversations = conversations;
  }
}
lionelrudaz commented 8 years ago

All good, here's the final function:

var showMessages = function() {
  return function(hook) {
    var conversation = hook.result.conversation.get({ plain: true });

    return hook.app.service('/api/v1/messages').find({
      query: {
        conversationId: hook.result.conversation.id
      }
    }).then(result => {
       conversation.messages = result.messages.map(function(message) {
         return message.id;
       });
       hook.result.conversation = conversation;
    });
  }
}

Now I'm struggling to do the same for the find method where I have to do that for each conversation. I think my limited competencies in JavaScript don't help there.

Any recommendation?

daffl commented 8 years ago

I'm not too familiar with Sequelize but isn't there a way to tell it to retrieve associated models? Making a query for each item might become a little inefficient this way.

lionelrudaz commented 8 years ago

Yes, there is. http://docs.sequelizejs.com/en/latest/docs/querying/#relations-associations

But I don't know how to fit that in my code.

daffl commented 8 years ago

This may require a change in the plugin, adding a params.sequelize to https://github.com/feathersjs/feathers-sequelize/blob/master/src/index.js#L31 like this:

    let query = Object.assign({
      where, order,
      limit: filters.$limit,
      offset: filters.$skip,
      attributes: filters.$select || null
    }, params.sequelize);

That way you could then pass those options to find (e.g. in a hook):

app.service('conversations').find({
  sequelize: {
    include: [{
      model: Task,
      where: { state: Sequelize.col('project.state') }
    }]
  }
});

If you make a pull request I can get it out as a new release pretty quick.

lionelrudaz commented 8 years ago

Should I close the issue?

ekryski commented 8 years ago

Ya I think we can close this now.

ekryski commented 8 years ago

Thanks again for the PR @lionelrudaz!