Skip to content

Fix for "delete" action not working with Sf2.2 #1184

Merged
merged 1 commit into from Mar 27, 2013

3 participants

@mweimerskirch

This is an alternative to #1157 and #1176

@rande rande commented on an outdated diff Mar 1, 2013
Controller/CRUDController.php
@@ -211,7 +211,8 @@ public function deleteAction($id)
throw new AccessDeniedException();
}
- if ($this->getRequest()->getMethod() == 'DELETE') {
+ $request = $this->getRequest();
+ if ($request->getMethod() == 'DELETE' || $request->getHttpMethodParameterOverride() == false && $request->getMethod() == 'POST') {
@rande
Sonata Project member
rande added a note Mar 1, 2013

Can you create a method getHttpMethod() which return the correct REST Verb ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rande
Sonata Project member
rande commented Mar 6, 2013
@mweimerskirch

@rande: Sorry, had a project launch this morning. I'll work on the patch this afternoon.

@mweimerskirch

@rande: Here you go. Is this the implementation you meant?

@rande rande commented on an outdated diff Mar 6, 2013
Controller/CRUDController.php
@@ -62,6 +62,23 @@ public function isXmlHttpRequest()
}
/**
+ * Returns the correct RESTful verb, given either by the request itself or
+ * via the "_method" parameter.
+ *
+ * @return string HTTP method, either
+ */
+ protected function getHttpMethod() {
+ $request = $this->getRequest();
+ if($request->getHttpMethodParameterOverride()) {
@rande
Sonata Project member
rande added a note Mar 6, 2013

it is a static method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rande rande commented on an outdated diff Mar 6, 2013
Controller/CRUDController.php
@@ -62,6 +62,23 @@ public function isXmlHttpRequest()
}
/**
+ * Returns the correct RESTful verb, given either by the request itself or
+ * via the "_method" parameter.
+ *
+ * @return string HTTP method, either
+ */
+ protected function getHttpMethod() {
+ $request = $this->getRequest();
+ if($request->getHttpMethodParameterOverride()) {
+ return $request->getMethod();
+ } elseif($request->request->has('_method')) {
+ return $request->request->get('_method');
+ } else {
@rande
Sonata Project member
rande added a note Mar 6, 2013

this else is useless

@rande
Sonata Project member
rande added a note Mar 6, 2013

Side note: This should be part of the symfony Request object .... @fabpot

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

I removed the else and collapsed it into the first if + changed the method call to static

@rande rande and 1 other commented on an outdated diff Mar 6, 2013
Controller/CRUDController.php
@@ -62,6 +62,20 @@ public function isXmlHttpRequest()
}
/**
+ * Returns the correct RESTful verb, given either by the request itself or
+ * via the "_method" parameter.
+ *
+ * @return string HTTP method, either
+ */
+ protected function getHttpMethod() {
@rande
Sonata Project member
rande added a note Mar 6, 2013

Can you rename it getRestMethod() ? sorry I didn't spot it at first

@rande
Sonata Project member
rande added a note Mar 6, 2013

and put a new line before the {

@mweimerskirch
mweimerskirch added a note Mar 6, 2013

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rande
Sonata Project member
rande commented Mar 6, 2013

@mweimerskirch are you sure about the PUT/POST action too ? do need to change the code ?

@mweimerskirch

No, I'm not sure :-(
But on my setup (that is in production since a few days) I haven't run into any other issues with the AdminBundle running on sf 2.2 so far.

@rande
Sonata Project member
rande commented Mar 6, 2013

good to know ;) There is no issue with form ?

@mweimerskirch

I'm probably not using all the field types and I also applied this for my project: sonata-project/SonataDoctrineORMAdminBundle#167 (only the first commit).
Also, there are a few deprecation notices though. I'll see when I get the time to patch those as well.
But apart from that the form works fine.

@mweimerskirch

Related patch: symfony/symfony#7202 (unfortunately pulled only into master and not into 2.2)

@sstok
sstok commented Mar 10, 2013

How about checking for both DELETE and POST?

Btw. for using POST I would recommend adding an CSRF token (configurable) for better security.

@rande rande merged commit bf3a584 into sonata-project:master Mar 27, 2013

1 check failed

Details default Scrutinizer: 6 Comments, 0 Changed Files — Travis: Errored
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.