grav-plugin-form icon indicating copy to clipboard operation
grav-plugin-form copied to clipboard

Implement Post/Redirect/Get pattern to avoid form resubmission

Open NicoHood opened this issue 3 years ago • 7 comments

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 avatar Dec 27 '20 11:12 NicoHood

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?

NicoHood avatar Dec 27 '20 11:12 NicoHood

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.

mahagr avatar Jan 08 '21 09:01 mahagr

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

NicoHood avatar Jan 08 '21 13:01 NicoHood

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.

mahagr avatar Jan 11 '21 11:01 mahagr

I am not sure if I understand correct.

The current behavior:

  • Redirect to a static page
  • Or specify a twig template to create a dynamic url (but only from is available as variable)

The new behavior:

  • Everything as above
  • Add page as additional variable to twig processing
  • Add the option to specify a redirect code
  • Default redirect code to 303
  • Default redirect url to page.url

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 avatar Jan 11 '21 11:01 NicoHood

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 avatar Jan 11 '21 12:01 NicoHood

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

NicoHood avatar Jan 11 '21 17:01 NicoHood