getgrav / grav-plugin-form

Grav Form Plugin
http://getgrav.org
MIT License
53 stars 79 forks source link

Implement Post/Redirect/Get pattern to avoid form resubmission #465

Open NicoHood opened 3 years ago

NicoHood commented 3 years ago

See: https://stackoverflow.com/q/6320113/14348748 And: https://en.wikipedia.org/wiki/Post/Redirect/Get

It would be nice to have an option to redirect to the same page using a statuscode of 303. The form plugin has a redirect option available, and even the process event supports a redirect_code. But it is not used yet. Also the processing of the parameters does not include the page variable. The function always uses the redirect code 302.

Suggested fix: https://github.com/getgrav/grav-plugin-form/pull/466

NicoHood commented 3 years ago

Maybe it is a bit offtopic, but I've seen undocumented code about remember_state , remember_redirect and uniqueid. What can this be used for? Basically I want to avoid resubmitting the same form again, but currently it seems this is still possible with pressing F5 after submitting. Even with the fix above, if you go back in history that is still possible. Is there already an option to avoid that?

mahagr commented 3 years ago

302 Found This is the most popular redirect code, but also an example of industrial practice contradicting the standard. HTTP/1.0 specification (RFC 1945 ) required the client to perform a temporary redirect (the original describing phrase was “Moved Temporarily”), but popular browsers implemented it as a 303 See Other. Therefore, HTTP/1.1 added status codes 303 and 307 to disambiguate between the two behaviors. However, the majority of Web applications and frameworks still use the 302 status code as if it were the 303. https://www.pmg.com/blog/301-302-303-307-many-redirects/

This said I think we should default on 303 as the intended method is a GET for the redirect. @rhukster ?

If you press F5, the form will reset and the browser may or may not pre-fill the data with your input. So basically submitting the form again will be a different form than the one you submitted before.

NicoHood commented 3 years ago

When speaking of defaults, I'd also default to page.url here, as it will simply implement the Post/Redirect/Get pattern. That would make sense ;-)

I've updated the PR: https://github.com/getgrav/grav-plugin-form/pull/466/commits/7d1c158aeb3c2ff50aaea41fad853c23570a91fe

mahagr commented 3 years ago

It does make sense, though because of how the page.url works, you may actually end up going to a different page as pagination and any other grav/query parameter would be lost.

NicoHood commented 3 years ago

I am not sure if I understand correct.

The current behavior:

The new behavior:

The patch only extends the options and is fully backward compatible.

If someone wants something special, he must use a different twig template. I've done that in my code already, as I need to redirect and include a specific anchor: route: '{{ page.url ~ "#modal-rating-close" }}'

NicoHood commented 3 years ago

I just noticed, that page.url returns the url of the page, that contains the form definition, not the page that is currently loaded. That is a pity, but still no regression. I will try to fix that though. If you have any hints, let me know.

NicoHood commented 3 years ago

Update: The issue is a separate one, so this PR is not directly affected. I've tracked the issue here: https://github.com/getgrav/grav-plugin-form/issues/478