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

Add support for scope in has-one association #5113

Merged
merged 3 commits into from Dec 30, 2015

Conversation

2 participants
@Verdier
Contributor

Verdier commented Dec 29, 2015

This PR fix two issues:

  • the association's scope was not injected in where clause when eager loaded. This is extremely critical because any raw that match without the where restriction is eager loaded, that could result in sending private data to the client (that's what append to us...),
  • the association's scope was not used is the creator helper method (ie. createInstance)
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 29, 2015

hasOne never had scope support, soooo ;p
The two bugs you mention are actually just the fact that the implementation does not exist.

@Verdier

This comment has been minimized.

Contributor

Verdier commented Dec 29, 2015

Haha @mickhansen that's fun... the implementation is here. Could you merge?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 29, 2015

Needs code and test for associationGetter aswell.

@Verdier

This comment has been minimized.

Contributor

Verdier commented Dec 29, 2015

Ok I'll do that tomorrow

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 29, 2015

Great, will merge then :)

@Verdier Verdier changed the title from Fix has-one scope not injected in where clause (critical) and not used in creator to Add support for scope in has-one association Dec 29, 2015

@Verdier

This comment has been minimized.

Contributor

Verdier commented Dec 29, 2015

@mickhansen done. Ready to be merged.

mickhansen added a commit that referenced this pull request Dec 30, 2015

Merge pull request #5113 from Verdier/fix-scope
Add support for scope in has-one association

@mickhansen mickhansen merged commit e4b9567 into sequelize:master Dec 30, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment