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

Small fixes #5

Merged
merged 3 commits into from Sep 15, 2016
Merged

Small fixes #5

merged 3 commits into from Sep 15, 2016

Conversation

janmeier
Copy link
Collaborator

This fixes some small bugs I found during my benchmarks.

  1. Belongs to many worked with string through, but not model. The check for paired was wrong (we checked on the prototype, not the instance). We were too quick to remove options.association, so it wasn't available the second time the loader runs
  2. Handling on findById(null) - otherwise dataloader complains about trying to load a null key
  3. We check before trying to shim again. This means we can call dataloaderSequelize(model) in the graphql-sequelize resolver and not worry about shimming more than once

@@ -12,149 +12,162 @@ describe('belongsToMany', function () {
this.sandbox.restore();
});

describe('simple association', function () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No code changes, just indentation and wrapping to test for both string and model through

@mickhansen mickhansen merged commit 09e1fe3 into master Sep 15, 2016
@mickhansen mickhansen deleted the smallFixes branch September 15, 2016 11:32
@mickhansen
Copy link
Owner

1.2.1

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