-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Do not use a Symfony Form to delete an article #427
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{{ include('blog/_delete_post_confirmation.html.twig') }} | ||
<form action="{{ url('admin_post_delete', { id: post.id }) }}" method="post" data-confirmation="true"> | ||
<input type="hidden" name="token" value="{{ csrf_token('delete') }}" /> | ||
<input type="submit" value="{{ 'action.delete_post'|trans }}" class="btn btn-lg btn-block btn-danger" /> | ||
</form> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,11 +126,8 @@ public function showAction(Post $post) | |
throw $this->createAccessDeniedException('Posts can only be shown to their authors.'); | ||
} | ||
|
||
$deleteForm = $this->createDeleteForm($post); | ||
|
||
return $this->render('admin/blog/show.html.twig', [ | ||
'post' => $post, | ||
'delete_form' => $deleteForm->createView(), | ||
]); | ||
} | ||
|
||
|
@@ -148,12 +145,11 @@ public function editAction(Post $post, Request $request) | |
|
||
$entityManager = $this->getDoctrine()->getManager(); | ||
|
||
$editForm = $this->createForm(PostType::class, $post); | ||
$deleteForm = $this->createDeleteForm($post); | ||
$form = $this->createForm(PostType::class, $post); | ||
|
||
$editForm->handleRequest($request); | ||
$form->handleRequest($request); | ||
|
||
if ($editForm->isSubmitted() && $editForm->isValid()) { | ||
if ($form->isSubmitted() && $form->isValid()) { | ||
$post->setSlug($this->get('slugger')->slugify($post->getTitle())); | ||
$entityManager->flush(); | ||
|
||
|
@@ -164,16 +160,15 @@ public function editAction(Post $post, Request $request) | |
|
||
return $this->render('admin/blog/edit.html.twig', [ | ||
'post' => $post, | ||
'edit_form' => $editForm->createView(), | ||
'delete_form' => $deleteForm->createView(), | ||
'form' => $form->createView(), | ||
]); | ||
} | ||
|
||
/** | ||
* Deletes a Post entity. | ||
* | ||
* @Route("/{id}", name="admin_post_delete") | ||
* @Method("DELETE") | ||
* @Route("/{id}/delete", name="admin_post_delete") | ||
* @Method("POST") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you change it to POST (which is something I do all the time to avoid having to enable the HttpMethodOverride which is disabled by default), I suggest changing the URL to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you don't use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stof good catch, I will change the URL There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @javiereguiluz enabling it make it easier to create CSRF attacks (well, not for this action as it has a CSRF token). This is why it is disabled by default in Symfony. and it brings no value (the browser is not doing a DELETE request) and can become a nightmare when dealing with cache proxies (because it is a POST request, and you cannot add a Vary on this override to use the right cache. |
||
* @Security("post.isAuthor(user)") | ||
* | ||
* The Security annotation value is an expression (if it evaluates to false, | ||
|
@@ -182,40 +177,17 @@ public function editAction(Post $post, Request $request) | |
*/ | ||
public function deleteAction(Request $request, Post $post) | ||
{ | ||
$form = $this->createDeleteForm($post); | ||
$form->handleRequest($request); | ||
if (!$this->isCsrfTokenValid('delete', $request->request->get('token'))) { | ||
return $this->redirectToRoute('admin_post_index'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The request could come from "edit page" or "show page", is right to redirect to index page? would be valid redirect to In the other hand, could you add a flash messages (error) when this happens? Similar to:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I keep the same behavior than before my changes... |
||
} | ||
|
||
if ($form->isSubmitted() && $form->isValid()) { | ||
$entityManager = $this->getDoctrine()->getManager(); | ||
$entityManager = $this->getDoctrine()->getManager(); | ||
|
||
$entityManager->remove($post); | ||
$entityManager->flush(); | ||
$entityManager->remove($post); | ||
$entityManager->flush(); | ||
|
||
$this->addFlash('success', 'post.deleted_successfully'); | ||
} | ||
$this->addFlash('success', 'post.deleted_successfully'); | ||
|
||
return $this->redirectToRoute('admin_post_index'); | ||
} | ||
|
||
/** | ||
* Creates a form to delete a Post entity by id. | ||
* | ||
* This is necessary because browsers don't support HTTP methods different | ||
* from GET and POST. Since the controller that removes the blog posts expects | ||
* a DELETE method, the trick is to create a simple form that *fakes* the | ||
* HTTP DELETE method. | ||
* See http://symfony.com/doc/current/cookbook/routing/method_parameters.html. | ||
* | ||
* @param Post $post The post object | ||
* | ||
* @return \Symfony\Component\Form\Form The form | ||
*/ | ||
private function createDeleteForm(Post $post) | ||
{ | ||
return $this->createFormBuilder() | ||
->setAction($this->generateUrl('admin_post_delete', ['id' => $post->getId()])) | ||
->setMethod('DELETE') | ||
->getForm() | ||
; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyrixx, you could use
_method
field in the form instead of changing the required HTTP method to POST. http://symfony.com/doc/current/form/action_method.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but I always found this totally useless ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you like the
DELETE
method? Is because browsers don't support it and we must simulate it with the_method
trick? Thanks!