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

Possible incorrect format being sent on save #31

Open
scolyer opened this issue Aug 27, 2015 · 12 comments · May be fixed by #33
Open

Possible incorrect format being sent on save #31

scolyer opened this issue Aug 27, 2015 · 12 comments · May be fixed by #33

Comments

@scolyer
Copy link

scolyer commented Aug 27, 2015

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
Copy link
Contributor

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. 🙅

makepanic added a commit to makepanic/ember-data-hal-9000 that referenced this issue Aug 31, 2015
@makepanic makepanic linked a pull request Aug 31, 2015 that will close this issue
@makepanic
Copy link
Contributor

@scolyer could you try to use the serializer from #33 ?
I'd love to get feedback or cases where the serialize method is incomplete/wrong.

@bantic
Copy link
Member

bantic commented Sep 6, 2015

@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
Copy link
Author

scolyer commented Sep 7, 2015

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
Copy link
Contributor

@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:
makepanic@8f90127#diff-ad99947263e865c76a16145b28a419c0R86

@scolyer
Copy link
Author

scolyer commented Sep 8, 2015

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:

makepanic@8f90127#diff-ad99947263e865c76a16145b28a419c0R86

Reply to this email directly or view it on GitHub:
#31 (comment)

@bantic
Copy link
Member

bantic commented Sep 8, 2015

@scolyer If you:

  • rm your existing ember-data-hal-9000 directory in node_modules
  • change the version field in your package.json to "makepanic/ember-data-hal-9000#31-serialize"
  • npm install again

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
Copy link
Member

bantic commented Sep 8, 2015

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

@scolyer
Copy link
Author

scolyer commented Sep 10, 2015

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
Copy link

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

@kdomagal
Copy link

kdomagal commented Aug 8, 2017

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
Copy link

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.

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 a pull request may close this issue.

6 participants