Pr support client id in included

[BryanCrotaz]

support server supplying __id__ in included objects support server using links on relationships add documentation and tests

[BryanCrotaz]

fixes #11 and #13

[frank06]

Thanks for your contributions! I am pretty busy at the moment, i need some time to have a thorough look at the PR

[BryanCrotaz]

we're using it live and it's working well

[BryanCrotaz]

start with the readme and the new tests to see what we've done

[tylergets]

@BryanCrotaz I am having trouble with this PR creating duplicate records, one with an id of null and the other has the proper id.

[ashrafhasson]

any chance this PR is considered for merging and release soon, please?

[BryanCrotaz]

We've been using it from this PR branch for 12 months and it works fine

[ashrafhasson]

thanks @BryanCrotaz I've been testing this PR with Mirage and it seems that Mirage is returning the internal __id__ key as --id-- in the included object, possibly because it's camelizing (or maybe somewhere dasherinzing) the key as part of its JSON API format routines? although as per JSONAPI specs, attributes member names must not start or end with U+002D HYPHEN-MINUS, “-“. While this might be possible to circumvent with a custom per-model adapter (I think), does it make sense to make this addon/PR more Mirage friendly by supporting --id-- too? Granted, it would make total sense to avoid U+002D HYPHEN-MINUS, “-“ and U+005F LOW LINE, “_” as first and/or last chars or a key member name, maybe an alternative needs to be devised to maintain JSONAPI compliance? I'm not too sure what's the best route to follow, but since this addon/PR is serving as a gap filler, can we just add more to it until the specs are a bit more dry? :)

Thanks,

[BryanCrotaz]

@ashrafhasson sounds like Mirage is not standards compliant then. If the spec says the name must not start with a hyphen then it's Mirage we should fix, not this PR.

[ashrafhasson]

@BryanCrotaz Mirage has a JSONAPISerializer but the problem seems to be the same issue ember-data-save-relationships is trying to cover; the relationship serialization is not standardized so I'm having to implement a route handling function to deal with the serialized relationship and since ember-data-save-relationships is using __id__, I'm passing it transparently to create the server-side models before handing it off to Mirage again to return the parent model with the newly created related models which the JSONAPISerializer then dasherizes before sending it back. Mirage itself doesn't care what you put in your mock/test data. The ORM doesn't inspect the serializer before creating a model.

While Mirage can be a bit smarter and not dasherize everything from the model blindly, neither should ember-data-save-relationships use __id__ in the attributes object, If ember-data-save-relationships maybe uses something more reflective of the fact that this is a transient or internal id like transient_id this wouldn't become an issue?

My point is, if/when it's fixed/restricted in Mirage, it may no longer be possible to resolve the returned data if __id__ is removed or maybe even create Mirage model records for any data serialized with ember-data-save-relationships in the first place.

An issue has also been created to raise the compliance concern in Mirage

[ashrafhasson]

Returning the #internal-model with __id__ key in included seems to update the store for the models with those ids set to null with the newly returned ids as expected but also duplicates the records!

[mhluska]

@ashrafhasson @tylergets @frank06 I'm also seeing the bug with duplicate records. Was this merged prematurely?