Use POST request for deleting items #2090
Conversation
|
@jonasdekeukelaere, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tijsverkoyen, @WouterSioen and @jessedobbelaere to be potential reviewers. |
jonasdekeukelaere
force-pushed
the
2086-post-deleting
branch
from
1b847ee
to
94eafc4
Compare
jonasdekeukelaere
force-pushed
the
2086-post-deleting
branch
from
94eafc4
to
1fab62f
Compare
| 'Categories', | ||
| null, | ||
| null, | ||
| ['error' => 'delete-category-not-allowed', 'var' => $this->record['title']] |
There was a problem hiding this comment.
will this handle the rawurlencode ?
There was a problem hiding this comment.
yes, http_query_build doet url-encoding https://github.com/forkcms/forkcms/blob/5.0.0-dev/src/Backend/Core/Engine/Model.php#L99
There was a problem hiding this comment.
that also is the reason your tests are failing, spaces are now translated to + instead of %20
There was a problem hiding this comment.
Hmm, haven't looked at that yet, still working on the general delete form type. Do I change the tests to +? Or do I change the http_query_build enc_type to PHP_QUERY_RFC3986 so spaces are translated back to %20?
There was a problem hiding this comment.
@forkcms/core-contributors what do you think?
There was a problem hiding this comment.
I think that %20 has the benefit that you always know it should be a space and that no information is lost, but I'd like to hear the input from the others as well.
There was a problem hiding this comment.
%20 or + doesn't make a difference, it is just another way of encoding it
There was a problem hiding this comment.
changed the tests 90a8e1a
| @@ -152,7 +151,11 @@ private function validateForm(): void | |||
|
|
|||
| private function loadDeleteForm(): void | |||
| { | |||
| $deleteForm = $this->createForm(FaqDeleteType::class, ['id' => $this->record['id']]); | |||
| $deleteForm = $this->createForm( | |||
| DeleteType::class, | |||
There was a problem hiding this comment.
@jonasdekeukelaere it is hidden as outdated but I think it is still in there
There was a problem hiding this comment.
it is there, because I tested it -> https://github.com/jonasdekeukelaere/forkcms/blob/90a8e1a9a0cd8eec0ad9f543edaac4d8cd8c0aa0/src/Backend/Modules/Faq/Actions/Edit.php#L17
There was a problem hiding this comment.
because it has been added in the previous commit (for faq feedback) ;)
|
@carakas I think all feedback has been handled |
Type
Resolves the following issues
fixes #2086
Pull request description
Use post request for deleting items instead of get request.