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

Fixing put action for proper PUT method handling. #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lewik
Copy link

@Lewik Lewik commented Oct 23, 2014

According to https://github.com/FriendsOfSymfony/FOSRestBundle/blob/master/Resources/doc/5-automatic-route-generation_single-restful-controller.md

PUT - this action accepts PUT requests to the url /resources/{id} and updates a single resource for this type
PATCH - This action also accepts PATCH requests to the url /resources/{id} and is supposed to partially modify the resource or made custom updates.

So PUT is intended to update all fields of the entity at once, while PATCH could be used for updating subset of all fields.

Consider following scenario: we have a form with 3 fields - name, email, phone. All fields are required. If field is empty JS-frameworks often do not send it. User fill only first 2 fields (name, email) by mistake and send a PUT request to out REST API. We expect form validation to fail (since user omitted phone field). But if generated code switch request method from PUT to PATCH - request is passed but with undesired result - we now have name and email updated in our entity. Symphony code for handling form request ($form->handleRequest) detects PATCH method. For non-PATCH methods missing fields are cleared, for PATCH - they are filled with values from database-entity.

Commit where this functionality was added https://github.com/symfony/symfony/pull/7849/files

If you want, we could implement parameter for controlling this behavior (either switching PUT to PATCH or not).

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.

1 participant