Strip ++api++ from in-browser location changes

[JeffersonBledsoe]

Solves the Volto part of https://github.com/plone/volto/issues/4800. We're stripping the ++api++ if it is in a location change as we probably don't want to ever navigate directly to a page with ++api++ in it in the browser. Change the action causes the redux store to be correct, but the URL updating occurs earlier in the react-router stack, and so we also manually update the URL to not have ++api++ in it. History is still preserved correctly (++api++ pages aren't in the history stack but we can still go back and forwards normally).

The logic is currently mixed in with the blacklistRoutes middleware while I get some feedback, but this logic should probably be its own middleware for readability.

Note that the <Redirect> at https://github.com/plone/volto/blob/87ff1f561788c40809934edc12e2c0c683736522/src/components/theme/View/View.jsx#L217 is causing SSR'd pages to send a 302 rather than forwarding on the status code sent by the REST API. Applying the changes mentioned in the react-router-dom docs for StaticRouting to pass the status along to the <StaticRouter> should resolve this and can be done in a follow-on PR.

[netlify[bot]]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	5b17e9c3e44ec1204541192f3875dfcca5082e67
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/64e4cab01b989a000994dcab

[JeffersonBledsoe]

Hi @mamico I've incorporated your changes from https://github.com/plone/volto/pull/4821 into this PR and fixed some of the issues mentioned by @davisagli as well as resolved some other issues I had with client-side and SSR'd content while trialing your PR. I don't think either PR makes sense without the other really which is why I've merged them into 1.

[davisagli]

@JeffersonBledsoe This is the new one you opened today. Did you mean to close it?

[JeffersonBledsoe]

@davisagli Not sure what happened there, I didn't close it. I had a GitHub bug where i ended up with two branches of the same name which is why i created a second copy of this PR. Mind reviewing this one?

[mamico]

Hi @mamico I've incorporated your changes from #4821 ...

Thanks. I have a busy few days, but on Monday I can help and review everything. Although I think you have a complete view on the whole problem, having experienced it myself, I hope I can make my own small contribution. Talking about legacy traversing, (those without ++api++ to be clear) I guess we can consider them unsupported here, right?

[cypress[bot]]

Passing run #6945 ↗︎

0

553

20

0

0

Details:

Merge branch 'master' into handle-api-redirect
Project: Volto	Commit: 5b17e9c3e4
Status: Passed	Duration: 14:30 💡
Started: Aug 22, 2023 2:51 PM	Ended: Aug 22, 2023 3:05 PM

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[mamico]

@JeffersonBledsoe @djay @davisagli there are any news on this? thx.

[JeffersonBledsoe]

@mamico We’ve been running this in production for a while now, and while it’s mostly been okay, we’ve noticed some weird URLs appearing. I need to verify that this PR isn’t the cause before I’d feel comfortable merging it

[mamico]

@JeffersonBledsoe @djay @davisagli @sneridagh are there any updates on this? thx

[djay]

@mamico what we are seeing is that google search console is ending up indexing urls like

https://digitalnsw.pretagov.com.au/components/cards?expand=actions,breadcrumbs,navigation&expand.navigation.depth=2

(where there was an old url of https://digitalnsw.pretagov.com.au/demo-pages/cards)

it might not be change to blame though. We just haven't had the time yet to investigate where these are coming from.

[djay]

ok I looked into it.

This code needs a change to strip off expand or volto related parts of the api call when a redirect happens and only keep the arguments that were part of the original call. I think we can safely say that we don't allow p.a.redirect to turn a url without arguments into one with arguments right @davisagli ?

so

• [ ] get the original request (when in node/ssr) and get the original querystring
• [ ] discard any querystring if an api call resulted in a redirect and replace it with original querystring (if there was one)

[mamico]

ok I looked into it.

Great.

This code needs a change to strip off expand or volto related parts of the api call when a redirect happens and only keep the arguments that were part of the original call. I think we can safely say that we don't allow p.a.redirect to turn a url without arguments into one with arguments right @davisagli ?

so

* [ ]  get the original request (when in node/ssr) and get the original querystring

* [ ]  discard any querystring if an api call resulted in a redirect and replace it with original querystring (if there was one)

Looking at the code, the original querystring already seems to be added, probably we just need to remove the query string returned from the backend?

I'll give it a try.

https://github.com/plone/volto/blob/5b17e9c3e44ec1204541192f3875dfcca5082e67/src/components/theme/View/View.jsx#L217

[netlify[bot]]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	8fdab9f331e66fb6a58b3fb8221246574d66d11a
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/6642b8f321d45a000833df3f

[netlify[bot]]

✅ Deploy Preview for plone-components canceled.

Name	Link
🔨 Latest commit	8fdab9f331e66fb6a58b3fb8221246574d66d11a
🔍 Latest deploy log	https://app.netlify.com/sites/plone-components/deploys/6642b8f35313df0008499a71

[JeffersonBledsoe]

Thanks for finishing this!