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

Use PATCH instead of PUT; Fixes issue #348 #425

Closed
wants to merge 1 commit into from
Closed

Conversation

dlee
Copy link
Contributor

@dlee dlee commented May 6, 2011

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.

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.
@fxn
Copy link
Member

fxn commented May 6, 2011

Excellent dlee :).

I'll check it out when 3.1 is branched.

@smartinez87
Copy link
Contributor

Now this is a perfectly complete pull request: code, tests and docs. Love it. Bravo!!

@dlee
Copy link
Contributor Author

dlee commented May 7, 2011

Oops, that wasn't supposed to go there.

@josevalim
Copy link
Contributor

Closed in favor of #431.

@josevalim josevalim closed this May 7, 2011
matthewd pushed a commit that referenced this pull request Apr 24, 2018
Fix warnings from test_to_sql test
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 this pull request may close these issues.

None yet

5 participants