Attempt to reload current page in newly-selected version

[andrewandante]

Inspired by silverstripe/api.silverstripe.org#106

Most of the time when I toggle the version switcher, I'm hoping to see the page I am currently on, but in a different version.

This PR attempts to do that, rather then defaulting to the homepage of the newly-selected version.

[williamdes]

That's a good idea !
What does it do when it does not exist on the target version ?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.92%. Comparing base (2ca5517) to head (0447854).
Report is 23 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main      #60      +/-   ##
============================================
- Coverage     64.31%   63.92%   -0.39%     
- Complexity     1241     1254      +13     
============================================
  Files            53       53              
  Lines          3584     3615      +31     
============================================
+ Hits           2305     2311       +6     
- Misses         1279     1304      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andrewandante]

That's a good idea ! What does it do when it does not exist on the target version ?

It just 404s. Which is not-great, I think, but can be useful as "oh this class doesn't exist in version 4" or whatever, so I'm somewhat ok with it as a concept.

[williamdes]

That's a good idea ! What does it do when it does not exist on the target version ?

It just 404s. Which is not-great, I think, but can be useful as "oh this class doesn't exist in version 4" or whatever, so I'm somewhat ok with it as a concept.

Okay, well maybe we can find some middle ground with this
Maybe asking the search bar to load the page that we where on before
Like some return_to argument

[andrewandante]

We could sort of naively check for a 200-399 response before re-routing? Something like (pseudocode):

var headReq= new XMLHttpRequest();
headReq.open('HEAD', url, false);
headReq.send();
if (headReq.status < 200 || headReq.status > 399)
  return newVersionIndex;
else
  return url;

Or something like that. Essentially, the old behaviour unless the page exists

[williamdes]

I like this idea !
Not all servers support HEAD, but we can agree to say that they should and ignore this corner case.
Or just try using GET if the first one fails

[andrewandante]

agree to say that they should

Yeah I think that's the approach - if it's unsupported it should return a 405 (I think) and the existing behaviour will resurface anyway. I've pushed an update to support this HEAD request

[williamdes]

Thank you for your work and patience, this is a great feature !

[andrewandante]

Thanks @williamdes - though I've just had a cursory glance at the code I wrote and think I might have the conditionals around the wrong way?

[williamdes]

=200 && < 400

@andrewandante I think that's what you wanted?

[andrewandante]

@williamdes I think I just have line 88 and line 91 around the wrong way.

If the status is lower than 200 or higher than 399, that means the page "doesn't exist" and so we should default.

The pseudo-code I wrote has it that way around, not sure how I managed to mix it up 😅