Use PATCH instead of PUT; Fixes issue #348

[dlee]

PATCH is the correct HTML verb to map to the #update action. The semantics for
PATCH allows for partial updates, whereas PUT requires a complete replacement.

Changes:

• adds the #patch verb to routes to detect PATCH requests
• adds #patch? to Request
• adds the PATCH -> update mapping in the #resource(s) routes.
• changes default form helpers to prefer :patch instead of :put for updates
• changes documentation and comments to indicate the preference for PATCH

This change tries to maintain complete backwards compatibility by keeping the
original PUT -> update mapping. Users using the #resource(s) routes should not
notice a change in behavior since both PUT and PATCH requests get mapped to
update.

UPDATE: Added note in guides; fixed typo in comments.

[josevalim]

There is no need to send a new pull request. Github updates the previous one once you push to the branch. Should I close the previous one?

[dlee]

Yeah, sending a pull request from master gave me a hard time sending in multiple pull requests. Let's use this pull request instead.

[NZKoz]

I think this is a case where a religious adherence to the spec isn't warranted. There are already a huge number of applications, documents, books, conference talks, etc. All these refer to PUT.

I think we could add an option for users to switch to PATCH if they'd prefer that, (or potentially suppport both) but changing the default will simply break far too much to be worth it.

[NZKoz]

I should be clearer too, I don't mean 'break' in the strictly programming sense of the word where an app doesn't work after upgrade, you've addressed that well. But more changing expectations, documentation, etc.

[dlee]

@NZKoz, thanks for expressing some of your concerns. I understand what you're saying, but I do not think they override the need to correctly implement Rails and encourage users to use it according to HTTP specs.

First, adherence to the HTTP spec isn't "religious" but practical and common sense. Letting Rails be a proper citizen of the HTTP ecosystem will help the platform inter-operate better with caches, proxies, browsers, and other HTTP clients. Refraining from using proper HTTP semantics might cause other problems, such as security issues, that we can't foresee as of now.

As for saying that changing defaults "breaks far too much to be worth it", if we follow your line of argument, we would never be able to move forward. I'm of the opinion that fixing the way 90% of apps use HTTP is worth breaking expectations and documentation, especially since the change is backwards compatible and allows users to override the defaults (just as you suggested, except in reverse).

Most likely, the change would not be a problem since it would come with a clear warning from Rails release notes and documentation.

Finally, Rails is a good instructor and scaffold of how to build proper web apps, and in filling this role, it should itself use proper HTTP semantics. That's why we have "RESTful" routes in the first place! :)

I hope this addresses your concerns. With strides in Rails in the proper direction, hopefully we'll have a huge number of applications, documents, and books that refer to proper HTTP semantics.

[josevalim]

From what I have noticed, this patch is completely backwards compatible. All in all, it only adds a "patch :update" route for all resources. In any case, I think an option that allows me to choose to have only PATCH, PUT or both for update would be welcome.

[dlee]

The other--and more disruptive--thing the patch does is make update forms use PATCH instead of PUT by default (which is overrideable). I am assuming it is having this default that is NZkoz's main point of contention.

...if not...
@NZKoz, are you thinking that resource routes would not accept PUT requests? If so, then rest assured that the change retains the PUT => update mapping.

[josevalim]

Right, I missed this form part. Hrm, a configuration option is definitely required. It can be added in a later pull request though.

[NZKoz]

@dlee no, I followed the changes you've made. It's the changes to the generated forms that I disagree with, and having it as the default.

The routing changes, supporting a patch? method etc, is all good. Just remove the changes to form_helper. People who want their web requests to generate PATCH requests can use :html=>{:method=>:patch} and all is well.

[dlee]

@NZKoz, Can you explain why Rails should keep PUT as the default for update forms?

For your reference, there's an extended discussion on issue #348 where we discuss this.

[josevalim]

@dlee for me it would be backwards compatibility. we can eventually replace on 4.0, but not 3.2.

[NZKoz]

For the backwards compatibility reasons I outlined above. Users who
want to change it can, the rest of the world don't get their apps
broken.

Remember, those forms also post to routes which aren't generated by
the resources helper, so changing the verb will break applications.

So adding the option means that users like yourself can change it, and
we can flip the default in 4.0.

On Tue, May 10, 2011 at 8:41 AM, dlee
reply@reply.github.com
wrote:

@NZKoz, Can you explain why Rails should keep PUT as the default for update forms?

For your reference, there's an extended discussion on issue #348 where we discuss this.

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

Cheers

Koz

[dlee]

@NZKoz: I doubt the rest of the world would have their apps broken with this change. Also, form helpers should only use PATCH when using routes generated by the resources helper.

That said, I don't mind waiting to fix the default in 4.0 meanwhile adding the config option for changing the default in 3.2.

[josevalim]

@dlee You are really underestimating the amount of Rails apps out there. A simple example that will break if we apply your commit as is: "do_something if request.put? || request.delete?"

[josevalim]

And this "form helpers should only use PATCH when using routes generated by the resources helper" is the most common case, e.g. form_for(@post) would use PATCH after your patch if the @post is persisted.

[dlee]

@josevalim: Good point. The request.put? check is a valid breakage that will cause problems. We could have request.put? return true for more robust backwards compatibility, but I don't think it's worth it.

"form helpers should only use PATCH..." was not to say that PATCH usage will be uncommon, but that it would be limited to routes in the app. This was a response to "Remember, those forms also post to routes which aren't generated by the resources helper, so changing the verb will break applications."

[NZKoz]

@dlee it's just not worth it given all the potential issues and the frankly tiny benefit. Please feel free to resubmit this without any form helper changes, for now we've made the call that changing those helpers sits on the wrong side of the cost-benefit equation and won't take a pull request which includes those changes.