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

Difficult to use PUT for creating records #3047

Closed
jamesarosen opened this issue May 6, 2015 · 11 comments
Closed

Difficult to use PUT for creating records #3047

jamesarosen opened this issue May 6, 2015 · 11 comments

Comments

@jamesarosen
Copy link

It's common to use POST for creates because the server generates the IDs and thus the URLs. But if the client knows the full URL, a PUT is more standard.

For example, POST /pages might create a resource whose location is /pages/39391.

But for things like a has-one relationship, PUT /user/2395/credit_card is perfectly sensible.

RestAdapter#createRecord doesn't make it easy to override this behavior. My adapter has to override the whole createRecord, with no way of delegating to _super.

@pangratz
Copy link
Member

pangratz commented May 6, 2015

I'm 👍 for a better way to override. I think you should add a comment in emberjs/rfcs#4 to add the possibility of specifying the method as well...

@knownasilya
Copy link
Contributor

Cross-posting from that rfc:

@jamesarosen see emberjs/rfcs#32 and davewasmer/ember-data-actions#1

Would love your feedback on that last issue.

@amiel
Copy link
Contributor

amiel commented May 7, 2015

#2898 added a requestType to buildURL. I'm thinking maybe you could add a buildVerb or whatever like this:

  createRecord: function(store, type, snapshot) {
    var data = {};
    var serializer = store.serializerFor(type.typeKey);
    var url = this.buildURL(type.typeKey, null, snapshot, 'createRecord');
    var verb = this.buildVerb('createRecord');

    serializer.serializeIntoHash(data, type, snapshot, { includeId: true });

    return this.ajax(url, verb, { data: data });
  },

and the default implementation of buildVerb would use a simple object map:

{
  "find": "POST",
  "createRecord": "POST",
  "updateRecord": "PUT",
  // etc
}

@jamesarosen
Copy link
Author

I might recommend adding the type and snapshot to buildVerb. Most notably, I think my ApplicationAdapter's implementation might look like

buildVerb: function(typeKey, snapshot, action) {
  if (snapshot.id && action === 'createRecord') { return 'PUT'; }
  return this._super.apply(this, arguments);
}

@knownasilya
Copy link
Contributor

@amiel I like that idea, and yes @jamesarosen adding those parameters would be very helpful.

@amiel
Copy link
Contributor

amiel commented May 7, 2015

@jamesarosen good idea

@igorT
Copy link
Member

igorT commented May 7, 2015

Maybe the correct thing to do would be to refactor so the adapter methods return the options hash and a different method actually sends the request. This would make it much easier to override, right?

@pangratz
Copy link
Member

@jamesarosen I refactored getting the various AJAX options in #3099. Can you check if this would address your needs sufficiently? Cheers!

@jamesarosen
Copy link
Author

@pangratz that looks good! I'd probably write my override as

export default ApplicationAdapter.extend({
  methodForRequest(params) {
    if (params.requestType === 'createRecord') { return 'PUT'; }
    return this._super(params);
  }
});

@joelalejandro
Copy link
Contributor

I had the same issue attempting to use PATCH in ember-data. Since it needed a bit more of logic than simply overriding the HTTP verb, I ended up building an addon that enables a .patch() method on any model.

@pangratz
Copy link
Member

#3099 has been merged and is available in canary behind the ds-improved-ajax feature flag.

Having this flag enabled, you can do this in your adapter to make a PUT instead of PATCH:

// app/adapters/application.js
import RESTAdapter from "ember-data/adapters/rest";

export default RESTAdapter.extend({
  methodForRequest({ requestType }) {
    if (requestType === "createRecord") {
      return "PUT";
    }

    return this._super(...arguments);
  }
});

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

6 participants