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

Issue when using handleUpdate #60

Closed
pedrokiefer opened this issue Jan 30, 2015 · 7 comments
Closed

Issue when using handleUpdate #60

pedrokiefer opened this issue Jan 30, 2015 · 7 comments

Comments

@pedrokiefer
Copy link
Contributor

I'm writing some tests using handleUpdate, but it keeps failing. I think it's because factory-guy returns an empty set on PUT requests.

The error message is the following:
"An adapter cannot assign a new id to a record that already has an id. <my-app@model:mymodel::ember882:1> had id: 1 and you tried to update it with null. This likely happened because your server returned data in response to a find or update that had a different id than the one you sent."

In this project I'm using ember-data-tastypie-adapter, ember 1.9.1 and ember-data beta.14. Maybe returning the id would solve, or serializing the hole object, not sure though. Maybe this specific for this adapter, or related to beta.14.

@danielspaniel
Copy link
Collaborator

True, @pedrokiefer about the return of empty for PUT's. I put a notice on the README saying I was having problems with the beta.14.1 release and was going to wait for beta.15 to come out to see if that was better ( whacky issues that seemed ember data related ) .. so my question is, can you revert to ember-data beta.12 for now ( that is what I am doing ) .. and it should be fine.

@pedrokiefer
Copy link
Contributor Author

@danielspaniel sure I can. It's just hard to keep up with ember folks, there is always a new release and lots of new features. I haven't read the changelog for what is going on on beta.15, so I can't comment if this issue will go away.

My idea to solve this issues was to serialize the object and return it. The problem is, we need to serialize it just before returning the data and not on handleUpdate registration, or else the update will return the old object data.

@danielspaniel
Copy link
Collaborator

Exactly @pedrokiefer ... not so simple .. and in the ember beta.12 release returning empty is the standard when nothing changes except the updates you made, so that strategy was pretty good.
For the next beta release I will have to adjust and as you say .. I may have to do something tricky to get that to work .. not sure yet though, but in the next few weeks when that is released I will try and fix it.

@pedrokiefer
Copy link
Contributor Author

I was reading mockjax docs, and it accepts a function for response and calls it before returning data. So we can have something like:

function create_response_callback(modelName, record, mapping) {
    return function (settings) {
        var json = record.toJSON({includeId: true});
        var responseJson = mapping(modelName, json);
        this.responseText = responseJson;
    }
}

Where modelName is the model name, record is the record being updated and mapping is FactoryGuyTestMixin.mapFind function.

stubEndpointForHttpRequest would need to accept a the result of this function and set mockjax options to use this function.

What do you think about that? I can try and make a PR if this works as expected.

@danielspaniel
Copy link
Collaborator

It's true about the function .. and it just might work to do something like this @pedrokiefer.
Take a look at the handleCreate method. It's using a function to check the data.
I think you could do same kind of thing to accomplish this for handleUpdate ( check request data and set response data ).
If you do make a pull request, make sure to add a test or two that fail without this new functionality you are adding ( if you know what I mean )

@danielspaniel
Copy link
Collaborator

Closing this one since releasing new version 0.9.10 with you new code I just pulled in.

@danielspaniel
Copy link
Collaborator

@pedrokiefer, I am reworking the way factory guy handles buiding json, to return properly formatted json that is ready to be send to adapter ( see issue #113 )
This means that the mapFind method might be out of the loop when building ( unless I can figure out how to bring it back in ).

This means that now the handleFind method will no longer have to make the build and then call mapFind as it does now, since the json returned will already be setup with modelName preset:

  handleFind: function () {
    var args = Array.prototype.slice.call(arguments);
    var modelName = args.shift();
    var json = FactoryGuy.build.apply(FactoryGuy, arguments);
    var id;

    if (FactoryGuy.useJSONAPI()) {
      id = json.data.id
    } else {
      id = json.id;
      json = this.mapFind(modelName, json);
    }

    var url = this.buildURL(modelName, id);
    new MockGetRequest(url, modelName, json);
    return id;
  },

What will happen now is that the build funciton will produce this:

  var json = build('user', 'with_hats');

  // json ->
  {
    user: {
      id: 1,
      name: 'User1',
      hats: [{type: 'big_hat', id:1},{type: 'big_hat', id:2}],
    },
    'big_hats': [
      {
        id: 1,
        type: "BigHat"
      },
      {
        id: 2,
        type: "BigHat"
      }
    ]
  }

So mapFind will not be used anymore since the json is already mapped.

Maybe there is a way to use a method like afterBuild or afterFind or convertJsonAfterFind ? to convert the json to something you want? rather than mapFind ??

any thoughts? questions comments ? before I change this?

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

No branches or pull requests

2 participants