Request: No access to request method used #2353

Open
assertchris opened this Issue Apr 20, 2012 · 8 comments

Projects

None yet

4 participants

@assertchris
assertchris commented Apr 20, 2012 edited

It is currently possible to determine which request method was specified in initialization, but not which request method was used in the last request. Not sure what the best solution to this is, but I imagine it involves passing the request method as an argument to the events, or setting it on the response object...

In the meantime, I am adding the header "X-Requested-Method" via the server, and reading that value in the event callbacks.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/55895-request-no-access-to-request-method-used?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github).
@DimitarChristoff
Member

wouldn't that only matter if you don't use .send() and go .get / .post etc anyway? otherwise, this.options.method will be true. you can just rubberstamp it here by re-implementing these methods: https://github.com/mootools/mootools-core/blob/master/Source/Request/Request.js#L234-243. why is it important to know the method AFTER the response? anyway, it's so trivial - http://jsfiddle.net/dimitar/yVbYQ/3025/

@assertchris

I have one Request instance which I am reusing for 6 different method/URL combinations:

GET/add.html, POST/add.html
GET/edit.html, POST/edit.html
GET/delete.html, POST/delete.html

The problem is that the success callback needs to differentiate between GET and POST calls and it cannot currently do that without the extra header I added. Interestingly enough, I have changed all calls to .get/.post to be .send calls but the options are still not updated when I supply them to the .send method.

TL;DR: this.options isn't updated for .get/.post calls or .send parameters.

@oelmekki

TL;DR: this.options isn't updated for .get/.post calls or .send parameters.

Not sure it should, i mean : it's a default for the request. You can then specify an other method, but it shouldn't change the default.

My take on this is that, if you need two different callbacks, you probably should have two different requests objects, here.

@assertchris

I'm not sure changing the base options is the right approach either. The issue is just that there is no way to tell what request method was used in the current success context. Almost all of my success callback's functionality remains the same for any of the URL's and request methods, so it doesn't really make sense for me to duplicate this functionality 6 times...

@DimitarChristoff
Member

then refactor .send to leave a reference to the last local options.method - to be honest, I don't think this will be a bad change, although I can't see many uses besides RESTful model updates with a common event handler (for example).

i would definitely prefer the options object passed to send not to be overriding the default options options permanently by merging into this.options - if you want to do this, just .setOptions(newobj).send()

@oelmekki

I thought of sub classing too, but Request.send is a way too big method (in LOC) to simply override. It should at least be split into several methods to be easily overridable.

@arian
Member
arian commented Aug 8, 2012

@oelmekki true, subclassing Request isn't very nice, however that's more difficult than it looks... without a complete rewrite.. pull request are always welcome though ;)

@chrispitt @DimitarChristoff any news on this.. I think setting properties of the method, url and data isn't a bad idea.

@arian
Member
arian commented Aug 8, 2012

#2344 has the same problem, but for the url instead of method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment