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

Feature: Association scopes #2268

Merged
merged 30 commits into from Sep 16, 2014

Conversation

4 participants
@mickhansen
Contributor

mickhansen commented Sep 11, 2014

// 1:M/N:M scope on target
Post.hasMany(Comment, {
  scope: {
    commentable: 'post'
  }
}

// N:M scope on through model
Post.hasMany(Tag, {
  through: {
    model: ItemTag,
    scope: {
      taggable: 'post'
    }
  }
}

Any association getters, setters or adders for post's comment association will then apply {commentable: 'post'} to the values or the where.

feat(associations): add a scope property that is used for default val…
…ues in createAssociation/setAssociations/addAssociation

@mickhansen mickhansen changed the title from [WIP] Feature: Association scopes to Feature: Association scopes Sep 11, 2014

@mickhansen mickhansen changed the title from Feature: Association scopes to [WIP] Feature: Association scopes Sep 11, 2014

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 12, 2014

@janmeier scopes for N:M, we need them on the through model for poly, but does it make sense for it to be the default for a hasMany scope?

I can think of a case where you might have different levels of nested tags and might want to define that you only want the tags with parent_id: 0 or similar.

We could do something like:

.hasMany(Model, {through: Model, scope: {}}); // Scope applies to target model
.hasMany(Model, {through: {model: Model, scope: {}}}); // Scope applies to through model

Or:

.hasMany(Model, {through: Model, scope: {}}); // Scope applies to through model

I'm leaning towards the first solution since it's the more generic one, and can handle more cases than just poly.

@janmeier

This comment has been minimized.

Member

janmeier commented Sep 12, 2014

First solution, definitely. That will also make it more in line with find calls, if you want to add where to a nested model, you nest the condition

mickhansen and others added some commits Sep 12, 2014

Merge pull request #2271 from seth-admittedly/feature/paranoid-after-…
…destroy

Save deletedAt without hooks but maintain hooks option for afterDestroy
feat(associations): add a scope property that is used for default val…
…ues in createAssociation/setAssociations/addAssociation
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 15, 2014

God i hate rebasing ;p

@fakewaffle

This comment has been minimized.

fakewaffle commented Sep 15, 2014

Love this.

}
return attributes;
});
}.bind(this));

This comment has been minimized.

@thanpolas

thanpolas Sep 16, 2014

Contributor

V8's .bind() implementation is unfortunately slow. It's preferable as a practice in highly repeatable routines to use the self closure variable...

This comment has been minimized.

@mickhansen

mickhansen Sep 16, 2014

Contributor

Not sure this part qualifies as a highly repeatable routine - But we might as well be prudent i suppose :)

mickhansen added a commit that referenced this pull request Sep 16, 2014

Merge pull request #2268 from sequelize/feat-association-scope
[WIP] Feature: Association scopes

@mickhansen mickhansen merged commit c58bc41 into master Sep 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@mickhansen mickhansen changed the title from [WIP] Feature: Association scopes to Feature: Association scopes Sep 27, 2014

@janmeier janmeier deleted the feat-association-scope branch Nov 19, 2014

@janmeier janmeier referenced this pull request Apr 26, 2016

Closed

Polymorphic BelongsTo #5787

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