Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Bug fix: hasMany loaded as null is throwing exception #848

Merged
merged 1 commit into from

4 participants

@bradfol

If a hasMany relationship is loaded as null, don't throw Adapter is either null or does not implement 'findHasMany' method. I have written some failing tests for this situation. I also committed a proposed fix, but not sure if it is the right approach.

Change introduced at this commit 81e6fde for these files:
system/relationships/has_many.js:20
system/store.js:679

@bradfol

Improved fix to use not equal null, because undefined == null. Better than using strict not equal against undefined, based on this info.

@tomdale
Owner

I think this is a good fix, but can we bring this up-to-date with the recent reference changes?

@bradfol

@tomdale thanks for taking a look. I have updated my commits to reflect the recent changes.

@sly7-7
Collaborator

@bradfol It seems like the same as #936, with the patch :). Perhaps it would be usefull to add a test against undefined
@tomdale If this one is merged, then you could close the other one as well ;)

@tomdale
Owner

Why are we checking for false?

@bradfol

@tomdale I have removed the check for false. (Was patched to deal with an API that returned false for empty relationships, but I see that is not the correct API behavior.)

@sly7-7 Also added a test for undefined which is already passing.

@sly7-7
Collaborator

@bradfol Thank you, I will close my PR. I think @tomdale will merge this one if you squash the commits.

@bradfol

I have squashed the commits, thanks @sly7-7 @tomdale

@bradfol

@stefanpenner could we get this merged in?

@stefanpenner stefanpenner merged commit ca7ba71 into emberjs:master
@stefanpenner

@bradfol boom!

@bradfol

@stefanpenner Thank you!

@bradfol bradfol deleted the bradfol:hasmany branch
@sly7-7
Collaborator

great, thanks @bradfol @stefanpenner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 14, 2013
  1. @bradfol
This page is out of date. Refresh to see the latest.
View
2  packages/ember-data/lib/system/store.js
@@ -673,7 +673,7 @@ DS.Store = Ember.Object.extend(DS._Mappable, {
if (adapter && adapter.findHasMany) {
adapter.findHasMany(this, record, relationship, idsOrReferencesOrOpaque);
- } else if (idsOrReferencesOrOpaque !== undefined) {
+ } else if (!isNone(idsOrReferencesOrOpaque)) {
Ember.assert("You tried to load many records but you have no adapter (for " + type + ")", adapter);
Ember.assert("You tried to load many records but your adapter does not implement `findHasMany`", adapter.findHasMany);
}
View
16 packages/ember-data/tests/integration/relationships/one_to_many_relationships_test.js
@@ -48,6 +48,22 @@ test("Referencing a null belongsTo relationship returns null", function(){
equal(comment.get('post'), null, "null belongsTo relationship returns null");
});
+test("Referencing an undefined hasMany relationship returns empty array", function() {
+ store.load(App.Post, {id: 1, title: "parent with undefined children"});
+ var post = store.find(App.Post, 1);
+ ok(post.get('comments'), "able to get the post's comments");
+ equal(post.get('comments.length'), 0, "undefined hasMany relationship returns empty array");
+ deepEqual(post.get('comments').toArray(), [], "comments is empty array");
+});
+
+test("Referencing a null hasMany relationship returns empty array", function(){
+ store.load(App.Post, { id: 1, comments: null, title: "parent with children set to null" });
+ var post = store.find(App.Post, 1);
+ ok(post.get('comments'), "able to get the post's comments");
+ equal(post.get('comments.length'), 0, "null masMany relationship returns empty array");
+ deepEqual(post.get('comments').toArray(), [], "comments is an empty array");
+});
+
test("When setting a record's belongsTo relationship to another record, that record should be added to the inverse hasMany array", function() {
store.load(App.Post, { id: 1, title: "parent" });
store.load(App.Comment, { id: 2, body: "child" });
Something went wrong with that request. Please try again.