201-created / ember-data-hal-9000

An ember-data compatible ember-cli addon that provides a HAL adapter
https://github.com/201-created/ember-data-hal-9000
MIT License
38 stars 23 forks source link

Possible incorrect format being sent on save #31

Open scolyer opened 9 years ago

scolyer commented 9 years ago

I've been tinkering around with the ember data 1.13.8, the ember-data-hal-9000 and ember-data.model-fragments plugins, and I was a bit surprised to see the following data in a PATCH when I saved my model:

{
    "data":{
        "_id":"55b21c520fabf0ba57bf0631",
        "attributes":{
            "email":"user@site.com",
            "user_type": "power",
            "profile":{
                  "data":{
                       "attributes":{
                           "nickname":"SuperJoe",
                           "bio":null,
                        },
                        "type":"profiles"
                   }
             },
            "_updated":"Fri, 24 Jul 2015 11:06:58 GMT",
            "_created":"Fri, 24 Jul 2015 11:06:58 GMT",
            "_etag":"f6687527bd6a10205d7c1462109e6193b58c8ae9"
        },
        "type":"accounts"
    }
}

I didn't expect the addition of the attributes, data, and type keys... Is there a way to turn these off and send the data back in the same format it was received? e.g:

{
    "_id":"55b21c520fabf0ba57bf0631",
    "email":"user@site.com",
    "user_type": "power",
     "profile":{
         "nickname":"SuperJoe",
          "bio":null,
     },
    "_updated":"Fri, 24 Jul 2015 11:06:58 GMT",
    "_created":"Fri, 24 Jul 2015 11:06:58 GMT",
    "_etag":"f6687527bd6a10205d7c1462109e6193b58c8ae9"
}
makepanic commented 9 years ago

You're right, this addon isn't overwriting the serialize method as of now. This means that the snapshot will be serialized as a json api document.

Also https://github.com/201-created/ember-data-hal-9000/blob/master/tests/unit/models/transformers-test.js#L28 should fail because it's checking the data field which shouldn't exist on in the hal document. :no_good:

makepanic commented 9 years ago

@scolyer could you try to use the serializer from https://github.com/201-created/ember-data-hal-9000/pull/33 ? I'd love to get feedback or cases where the serialize method is incomplete/wrong.

bantic commented 9 years ago

@scolyer Any chance you can try using the serializer from #33, as @makepanic suggested? I'm curious if that covers your use case, as well.

scolyer commented 9 years ago

Hi @makepanic and @bantic , Unfortunately, no – this didn't work for me. I've been playing around with using python-eve as my backend. To make it work, I had to implement the following (CoffeeScript):

# serializers/application.coffee
`import DS from 'ember-data'`
`import HalSerializer from 'ember-data-hal-9000/serializer'`

ApplicationSerializer = HalSerializer.extend(
    primaryKey: '_id',

    serialize: (snapshot, options) ->
      json = snapshot.record.toJSON()
      delete json._etag if json._etag?
      delete json._created if json._created?
      return json
)

Which I am not happy with as it feels like a giant hack, but it does work for my limited use case :-| The thing I did notice about #33 when I tried to use it was that my request ended up being wrapped in a data attribute, which was my original problem.

makepanic commented 9 years ago

@scolyer Are you sure you used the https://github.com/makepanic/ember-data-hal-9000/tree/31-serialize branch?

I added a test case for your initial payload and it worked: https://github.com/makepanic/ember-data-hal-9000/commit/8f901271da2a4caf3446f6bb8736e68ade4ce452#diff-ad99947263e865c76a16145b28a419c0R86

scolyer commented 9 years ago

I may have done it incorrectly - how do I install an addon directly from a repo instead of from ember-cli?

On Mon, Sep 7, 2015 at 10:34 PM, Christian notifications@github.com wrote:

Are you sure you used the https://github.com/makepanic/ember-data-hal-9000/tree/31-serialize branch? I added a test case for your initial payload and it worked:

https://github.com/makepanic/ember-data-hal-9000/commit/8f901271da2a4caf3446f6bb8736e68ade4ce452#diff-ad99947263e865c76a16145b28a419c0R86

Reply to this email directly or view it on GitHub: https://github.com/201-created/ember-data-hal-9000/issues/31#issuecomment-138292418

bantic commented 9 years ago

@scolyer If you:

That should do it. You can check by looking in your node_modules/ember-data-hal-9000 directory and checking that addon/serializer.js has the extra serialize method that you can see in the pr.

bantic commented 9 years ago

to be clear, that's change the version field for the ember-data-hal-9000 devDependency, not the top-level version key.

scolyer commented 9 years ago

Ah, cool thanks @bantic . When I installed it I couldn't get it to work with the ember-data.model-fragments properly. Here's the error message:

Error while processing route: index.dashboard Assertion Failed: The `JSONAPISerializer` is not suitable for model fragments, please use `JSONSerializer` Error: Assertion Failed: The `JSONAPISerializer` is not suitable for model fragments, please use `JSONSerializer`
    at new Error (native)
    ...
Ramblurr commented 8 years ago

Is there a way as of Nov 2016 to model.save() with this adapter and have it generate a HAL payload?

kdomagal commented 7 years ago

I wrote my custom updateRecord method in HalAdapter to make it working:

`

 updateRecord(store, type, snapshot) {

    let data = this.serialize(snapshot, { includeId: true });
     let url = this.buildURL(type.modelName, snapshot.id, snapshot, 'updateRecord');
    return this.ajax(url, 'PATCH', {data: data.data.attributes});
}`

generally I'm sending model only which is in data.data.attributes.

rainerfrey commented 5 years ago

I know this issue has been stale for a long time, but FWIW here's my view:

HAL is meant as a read format and does not specify anything about how to write data. But for a single resource, it also only adds a few special properties to a "natural" JSON representation of an object. What I would expect as the write format for a HAL API (and what we actually use at my company) is that natural JSON serialization of the object properties. Nowadays this (or something very close to this) seems to be implemented by ember-data's JSONSerializer.

IMO if serialization could be delegated to this JSONSerializer it would be much less surprising than the serialization that is currently done by the JSONAPISerializer, and it could be a way to achieve a useful serialization behavior without burdening this add-on with a lot of code to maintain.

I am not familiar enough with ember-data internals as to how to implement that delegation to submit a PR for this right now though.