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

Allow autoStoreData to work on the root page #259

Open
mikemonteith opened this issue Sep 20, 2023 · 6 comments
Open

Allow autoStoreData to work on the root page #259

mikemonteith opened this issue Sep 20, 2023 · 6 comments
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)

Comments

@mikemonteith
Copy link
Contributor

mikemonteith commented Sep 20, 2023

The regex here is enabling autoStoreData functionality on all pages except the home page /.
This should be changed to all pages including homepage.

app.post(/^\/([^.]+)$/, (req, res) => {


Edit:
I didn't really word this properly. I should have said:
The regex here is handling the POST > GET redirect which is necessary for the autoStoreData functionality on all pages except the home page /.

@joelanman
Copy link

is it not working for you?

that line is about POST > GET redirect. The autostoredata line is here, and works on all paths:

https://github.com/nhsuk/nhsuk-prototype-kit/blob/main/app.js#L96

Even the line you highlighted for redirects looks like it works on /, it might not work on root without the slash, is that what you're trying to do?

@mikemonteith
Copy link
Contributor Author

mikemonteith commented Sep 22, 2023

The regex doesn't match /. It only matches / followed by one or more characters.

> "/".match(/^\/([^.]+)$/)
null
>
> "/home/".match(/^\/([^.]+)$/)
[ '/home/', 'home/', index: 0, input: '/home/', groups: undefined ]

@mikemonteith
Copy link
Contributor Author

mikemonteith commented Sep 22, 2023

In fact, if you want to just redirect all POST to GET while keeping the URL the same, there's no need for any regex:

app.post("*", (req, res) => {
  res.redirect(req.originalUrl);
});

I'll try this out later and submit a PR

@joelanman
Copy link

oh yeh sorry you're right, at least one char:
https://regexper.com/#%5E%5C%2F%28%5B%5E.%5D%2B%29%24

the original seems to not want to match a . but I don't know why (I wrote it)

@joelanman
Copy link

If you feel like it you can do a PR to the GOV.UK kit:
https://github.com/alphagov/govuk-prototype-kit/blob/main/server.js#L165

@joelanman
Copy link

and I see we have a fix that isnt in the NHS kit - it retains query strings

@frankieroberto frankieroberto added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Development

No branches or pull requests

3 participants