scoutforpets / jsonapi-mapper

JSON API-Compliant Serialization for your Node ORM
The Unlicense
42 stars 24 forks source link

Missing related object in included when foreign key in the last row is null #77

Closed npatmaja closed 8 years ago

npatmaja commented 8 years ago

Hi, I have a problem when mapping a collection that has a non mandatory belongsTo relationship. So when the last row's foreign key of the fetched data is null, then mapper wont include all that related object into included even though there are some other rows that have that foreign key filled. To better picture the case consider the following table:

table: foo

id name bar_id
1 name1 1
2 name2 <null>
3 name3 <null>
4 name4 2

Bookshelf model:

const Foo = bookshelf.Model.extend({
  tablename: 'foo',

  bar() {
    return this.belongsTo('Bar', 'bar_id');
  },
});

When I fetched the first 3 rows, the Bar object won't be included in the resulting object after being mapped. However, when I fetched all the rows, the last row's bar_id is present, the Bar object is correctly included.

Please let me know if I'm missing something in my implementation.

I'm using bookshelf@0.9.5 and jsonapi-mapper@1.0.0-beta.1

ShadowManu commented 8 years ago

This has probably being fixed in some of the newer betas (there was a bug regarding sampling which was fixed recently). Consider using the last release at the moment beta.5

chamini2 commented 8 years ago

This was fixed in 1.0.0-beta.3

npatmaja commented 8 years ago

@ShadowManu @chamini2 thanks, will try it :)

npatmaja commented 8 years ago

@ShadowManu @chamini2 I just updated to beta.6, sadly, the problem is still there :(

chamini2 commented 8 years ago

Hey @npatmaja, I just pushed a test case to master with your scenario and it runs correctly. I don't know what may be the problem with your current usage... Maybe you forgot to load('bar') before calling the mapper on the case where your relations don't appear?

Let me know if the test doesn't represent your case.

npatmaja commented 8 years ago

@chamini2 thanks for replying, I will look at my case again maybe I'm missing something. I will update what I found here.

peterviergutz commented 8 years ago

Hi this is maybe related, I am using 1.0.0-beta.6 and also facing empty includes of an object when the first element of a collection has a null belongsTo reference. I tracked it down to: If the first of two Items in a collection

It seems that template in Bookshelf.prototype.map is not initialized correctly (i.e. with all needed attributes on the null object), when the first element has a null reference, because sample(data) in utils.js obviously can't extract attributes from an empty object. This causes that.opts in serializer.js to have insufficient mappings for all rows.

In order to be most resilient against these issues, maybe it helps to generate / inject the template to json-serializer individually for each row?

chamini2 commented 8 years ago

I had misunderstood the error, I'm trying to replicate it in a spec. Maybe you could help me by writing your cases so the spec fails, as you are experiencing?

peterviergutz commented 8 years ago

I can confirm this is fixed in 1.0.0-beta.7. Thanks for the rapid response :)

npatmaja commented 8 years ago

@peterviergutz @chamini2 thank you for the massive help guys :)

jamesdixon commented 8 years ago

Well done, everyone!