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

Allow reference by DaoFactory in foreign key definition #761

Merged
merged 8 commits into from Jul 12, 2013

Conversation

3 participants
@sdepold
Member

sdepold commented Jul 11, 2013

This PR extends @optilude great work and makes it possible to pass a DaoFactory to the references option of the foreign key definition:

  var Author = this.sequelize.define('author', { firstName: Sequelize.STRING })
  var Post   = this.sequelize.define('post', {
    title:    Sequelize.STRING,
    authorId: {
      type:          Sequelize.INTEGER,
      references:    Author,
      referencesKey: "id"
    }
  })

This will automatically use the Author's tableName.

@optilude

This comment has been minimized.

Contributor

optilude commented Jul 11, 2013

Does this mean that it's no longer necessary to add a hasMany(), hasOne() or belongsTo() declaration for the relationship? The work I did applied to relationship definitions, not the column definitions, but this approach looks nice. I've always been in two minds about defining the models in one place and the relationships somewhere else.

@sdepold

This comment has been minimized.

Member

sdepold commented Jul 12, 2013

uhm this PR basically just reads the tableName from the model instead of expecting a string with the tablename

@sdepold

This comment has been minimized.

Member

sdepold commented Jul 12, 2013

It does not change anything about the way associations are done.

@sdepold

This comment has been minimized.

Member

sdepold commented Jul 12, 2013

Also we could try to automate the referencesKey by using the first primary key of the passed model. The problem with this approach is, that primary keys are creepy in sequelize ^^

durango added a commit that referenced this pull request Jul 12, 2013

Merge pull request #761 from sequelize/features/reference_by_model
Allow reference by DaoFactory in foreign key definition

@durango durango merged commit d3ee3bf into master Jul 12, 2013

1 check passed

default The Travis CI build passed
Details
@optilude

This comment has been minimized.

Contributor

optilude commented Jul 12, 2013

I'm confused. This is how I set up relationships in my app now:

User
        .hasMany(Portfolio,  {as: 'portfolios', foreignKey: 'userId', onDelete: 'cascade'});

The example above seems to do this on definition of an attribute in the definition of the main model. That is a nice (possibly nicer) option, but it isn't what I did in my previous Pull Request. ;-)

@janmeier janmeier deleted the features/reference_by_model branch Jul 18, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment