dchester / epilogue

Create flexible REST endpoints and controllers from Sequelize models in your Express app
846 stars 116 forks source link

associations vs include #107

Closed philotas closed 8 years ago

philotas commented 8 years ago

I figured out that the auto-associations branch is already included in the master.

When using it and having the include and associations option turned on, I get this error when using list on the resource:

`"SQLITE_ERROR: ambiguous column name: Users.id"

Removing the include option makes it work. So it seems to me include and associations do not work together.

I do not understand why it has to be this way, because I thought associations will make the the routes for the associations available and will provide the association function for the resource.

Why should it make include useless? Maybe I want the associations to work, but still control what is returned vial list?

My main problem is, that via associations I cannot control the attributes that are returned like I can in include

Am I just using it wrong?

mbroadst commented 8 years ago

@philotas it shouldn't make it useless as the routes generated for the auto association should not conflict with the existing resource routes. Have you confirmed that your code works without enabling auto-associations? Can you provide a code sample expressing this issue?

philotas commented 8 years ago

@mbroadst Yes, without associations enabled it is working.

My code:

module.exports = function(sequelize, DataTypes) {
  var Channel = sequelize.define('Channel', {
    name: DataTypes.STRING
  }, 
  {
  classMethods: {
    associate: function(models) {
      Channel.hasMany(models.AdSlot,{ onDelete: 'cascade', hooks: true });
    }
  }
  });

  return Channel;
};
module.exports = function(sequelize, DataTypes) {
  var AdSlot = sequelize.define('AdSlot', {},
  {
    timestamps: false,
    classMethods: {
      associate: function(models) {
        AdSlot.belongsTo(models.Channel);
      }
    }
  });

  return AdSlot;
};

With the above, this does not work on /api/channels/1 and /api/channels:

var channelResource = epilogue.resource({
  model: db.Channel,
  associations: true,
  attributes: ['id','name'],
  endpoints: ['/api/channels', '/api/channels/:id'],
  include: [{
    model: db.AdSlot
  }]
});

The error is:

{
  "message": "internal error",
  "errors": [
    "SQLITE_ERROR: ambiguous column name: AdSlots.id"
  ]
}

The association /api/channels/1/adslots/ and /api/channels/1/adslots/1 are working.

This does work however:

var channelResource = epilogue.resource({
  model: db.Channel,
  //associations: true,
  attributes: ['id','name'],
  endpoints: ['/api/channels', '/api/channels/:id'],
  include: [{
    model: db.AdSlot
  }]
});

and this does work as well:

var channelResource = epilogue.resource({
  model: db.Channel,
  associations: true,
  attributes: ['id','name'],
  endpoints: ['/api/channels', '/api/channels/:id']
 // include: [{
 //   model: db.AdSlot
 // }]
});
mbroadst commented 8 years ago

@philotas hi, I've added a test case here to track these behaviors. It turns out this is actually expected behavior, let me explain..

When you specify that the resource should "auto associate" it iterates through all the existing associations in sequelize, and before even creating sub resources it will add the model to the resources include. So, in fact, your adding include: [{ model: db.AdSlot }] is adding a duplicate include to the resource, leading to your ambiguity issue.

mark-lester commented 8 years ago

I was about to post an issue about unexpected joins on what I thought should be a record fetch, e.g. /api/Stops/1 produces an array as Stops owns multiple StopTimes. I hacked out this line https://github.com/dchester/epilogue/blob/master/lib/Controllers/read.js#L42 to get what I want, but do I need to just set a switch ?.

mbroadst commented 8 years ago

@mark-lester can you please open a new issue with more details on your problem?