Migration path to replace removed jquery versions

Good idea to have a fallback if the variable doesn't match available settings.

I wrote an update hook in the early patches in #3166985: [Proposal] provide supported / recommended jQuery versions for Security coverage which might help as a starting point.

Technically we could say this is a new branch and there's no smooth upgrade path, but I think a few simple measures to avoid the most obvious problems that sites will hit would be worthwhile.

1. +++ b/jquery_update.install
   @@ -178,3 +197,34 @@ function jquery_update_update_7001() {
   +  $jquery_version = variable_get('jquery_update_jquery_version', JQUERY_UPDATE_DEFAULT_JQUERY_VERSION);
   

   Do we need to make sure we've included the .module file here too (as that's where the constant is defined)?

2. +++ b/jquery_update.module
   @@ -285,6 +285,14 @@ function jquery_update_jquery_replace(&$javascript, $cdn, $path, $min, $version)
   +    module_load_install('jquery_update');
   +    $version = _jquery_update_map_version_to_secure_default($version);
   

   If this might happen repeatedly on an upgraded site, would we be better to put the helper function in the .module file and include it when we need to from .install rather than the other way around?

In general, I wonder about silently remapping a variable that no longer matches the module's available options to either 1.12 or 2.2 on every request.

Perhaps that'd be okay if we also show a warning in the admin UI / Status report to show that's what's going on?

Or we could store the remapped value, but I'm not certain we should do that without human supervision.

Oh also, any sites that have already moved to 7.x-4.x and are happily using - say - a custom jQuery 3.6.1...

We'd want to avoid remapping them to jQuery 2.4 obviously! I'm guessing that would not have happened because it'd have been in supported options?

However, I've actually just removed the jquery_update_get_supported_versions() function as we're no longer thinking in terms of supported / unsupported versions.

So back to NW I think (largely my fault for changing things around in the last few days).

This is partly a reroll and partly a refactor based on changes made in the module.

I don't think it addresses every point we've discussed in the last few comments, but hopefully we can start doing so from here.

Oops.. back to NR for tests (which pass locally including the new one).

+++ b/jquery_update.module
@@ -283,6 +283,14 @@ function jquery_update_jquery_replace(&$javascript, $cdn, $path, $min, $version)
+    _jquery_update_convert_to_supported_version();
+    $version = _jquery_update_map_to_supported_version($version);

I was a bit puzzled by this at first; why do we need to call the mapping function again when it was called by the first function?

It's because the variable / theme setting will have been updated if they needed to be, but we don't know whether jquery_update_jquery_replace() has been called with the main jQuery version or a specific one for a theme.

Perhaps we can add a comment to explain to the next person who wonders what's going on :)

Overall I think this looks pretty good. Great to have the tests too, thanks!

I was considering a slightly different approach where we allow the outdated version to be remapped on every request, but we raise a warning in the admin UI / status report to suggest that it gets fixed properly (building on some of what we've worked on recently in #3312151: improve jQuery Update's latest version checks).

Either approach ought to work, and there's an argument to say just fixing it once (and logging the change) is better.

In terms of having a runtime fallback and an update hook that do the same thing... I think that's okay. The British phrase is "belt and braces".

I might make a couple of tweaks to comments, but I think I'm going to commit this shortly.

Changes are almost all the comments. I've also renamed one function.

I'll do a little bit of manual testing upgrading a site, but I think this is ready to commit if tests still pass.

I tested installing the currently supported / recommended 7.x-2.7 release and enabling the defaults on a vanilla D7 site.

I then swapped out the jquery_update module for 7.x-4.x with the latest changes from this issue applied.

Without running db updates, refreshing the front page worked fine.

The script tags in the page went from e.g.:

<script type="text/javascript" src="http://d7.example.com/sites/all/modules/contrib/jquery_update/replace/jquery/1.10/jquery.min.js?v=1.10.2"></script>
<script type="text/javascript" src="http://d7.example.com/misc/jquery-extend-3.4.0.js?v=1.10.2"></script>

..to:

<script type="text/javascript" src="http://d7.example.com/sites/all/modules/contrib/jquery_update/replace/jquery/1.12/jquery.min.js?v=1.12.4"></script>
<script type="text/javascript" src="http://d7.example.com/misc/jquery-extend-3.4.0.js?v=1.12.4"></script>

I got a log message:

$ drush ws
 ID  Date           Type             Severity  Message                                                           
 55  29/Sep 16:59  jquery_update  warning   jquery_update_jquery_version updated from 1.10 to 1.12            

Running the database updates after this went without error.

The variable has been updated:

$ drush vget jquery_update_jquery_version
jquery_update_jquery_version: '1.12'

So I'd call that a success.

I'm sure there are plenty of other scenarios we could test for, but I think this covers the basic upgrade from the currently recommended release.

Thanks @poker10!