Skip to content
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

Preserve query string when redirecting POSTs to GETs #1120

Merged

Conversation

edwardhorsford
Copy link
Contributor

When the kit redirects POSTs to GETs it doesn't preserve any query string in the url. This updates to preserve that.

This means if a query string is included in the form action, when the user is redirected to the resulting GET url, that query string is as it was.

This is useful when:

  • You’re using the query string to set flash messages or return paths
  • You want to rely on explicitly what’s in the query string for a specific page, rather than saved data.

I added support for this in our service in September last year and it's worked well - you can now use query strings seamlessly and they're preserved regardless of whether you're doing a GET or a POST.

@edwardhorsford edwardhorsford force-pushed the preserve-query-when-redirecting-posts branch 3 times, most recently from 313a33f to 5f34720 Compare October 22, 2021 10:20
@joelanman joelanman added awaiting triage 🕔 Days A few unknowns, but we roughly know what’s involved. and removed awaiting triage labels Nov 2, 2021
@joelanman
Copy link
Contributor

looks good thanks, just waiting on a review with the rest of the team

@trang-erskine trang-erskine added 🕔 Hours A well understood issue which we expect to take less than a day to resolve. and removed 🕔 Days A few unknowns, but we roughly know what’s involved. labels Nov 9, 2021
@domoscargin domoscargin self-assigned this Nov 16, 2021
@domoscargin
Copy link
Contributor

This looks good to me, thanks @edwardhorsford.

Do you want to add a feature changelog entry? Happy to do it myself.

@edwardhorsford
Copy link
Contributor Author

@domoscargin added a changelog entry. Hope the formatting is ok. Do you allow commits to be squashed on merge, or does that need to happen beforehand?

@domoscargin
Copy link
Contributor

@domoscargin added a changelog entry. Hope the formatting is ok. Do you allow commits to be squashed on merge, or does that need to happen beforehand?

We generally squash beforehand.

@edwardhorsford
Copy link
Contributor Author

@domoscargin well I borked that. Rather than squashing, I seem to have brought in all the intermediary commits on the kit. Can you help?

@domoscargin domoscargin force-pushed the preserve-query-when-redirecting-posts branch from 4f213f9 to 4005a81 Compare November 16, 2021 15:17
@domoscargin
Copy link
Contributor

@edwardhorsford Should be sorted.

We're just going to write some documentation for this.

@domoscargin domoscargin merged commit fedcefb into alphagov:main Nov 24, 2021
@edwardhorsford edwardhorsford deleted the preserve-query-when-redirecting-posts branch November 25, 2021 11:03
@edwardhorsford
Copy link
Contributor Author

Thanks both!

@domoscargin domoscargin added this to the v12.0 milestone Dec 2, 2021
@domoscargin domoscargin mentioned this pull request Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕔 Hours A well understood issue which we expect to take less than a day to resolve.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants