Skip to content
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
Merged

Add support for scope in has-one association #5113

merged 3 commits into from Dec 30, 2015

Conversation

Verdier
Copy link
Contributor

@Verdier 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
Copy link
Contributor

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

@Verdier
Copy link
Contributor Author

Verdier commented Dec 29, 2015

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

@mickhansen
Copy link
Contributor

Needs code and test for associationGetter aswell.

@Verdier
Copy link
Contributor Author

Verdier commented Dec 29, 2015

Ok I'll do that tomorrow

@mickhansen
Copy link
Contributor

Great, will merge then :)

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

Verdier commented Dec 29, 2015

@mickhansen done. Ready to be merged.

mickhansen added a commit that referenced this pull request Dec 30, 2015
Add support for scope in has-one association
@mickhansen mickhansen merged commit e4b9567 into sequelize:master Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants