turnitin / moodle-mod_turnitintooltwo

Turnitin Direct plugin (version 2) for Moodle
http://www.turnitin.com
31 stars 64 forks source link

Convert Javascript to AMD Modules #375

Open tbigby-kristin opened 6 years ago

tbigby-kristin commented 6 years ago

Hi there,

Moodle now recommends the use of AMD Modules for Javascript in Plugins: https://docs.moodle.org/dev/jQuery - 'How to Use jQuery'.

turnitintooltwo still uses the old method of $PAGE->requires->jquery_plugin().

This causes a caching issue on upgrades to turnitintooltwo where the browser does not get the new version of the Javascript, which causes other odd errors. The best way to resolve this appears to be to upgrade the Javascript to AMD modules.

$PAGE->requires->jquery_plugin() does not include the $jsrev variable in the URL, which is used to indicate that a new version of Javascript code is available and that the browser needs to download it. The $PAGE->requires->jquery_plugin() method assumes that a jQuery plugin must include a version number in the Javascript file name, like how jQuery itself is named: jquery-1.8.2.min.js . If a plugin updates a jQuery plugin it should result in a new file name. Reference: Andrew Nicols' comment on MDL-43986: https://tracker.moodle.org/browse/MDL-43986?focusedCommentId=269621&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-269621 .

The specific caching problem we encountered was:

The Javascript turnitintooltwo.min.js was loaded through the URL: https://sitename/theme/jquery.php/mod_turnitintooltwo/turnitintooltwo.min.js .

When loaded this way, jquery.php returns the caching headers: Etag: "a7f493cdc4cd2fe16fc051d844663e87f6278587" Last-Modified: Tue, 26 Sep 2017 22:15:51 GMT Expires: Sun, 31 Dec 2017 05:24:44 GMT Cache-Control: public, max_age=7776000 Date: Mon, 2 Oct 2017 05:24:44 GMT

Because the 'expires' header is so far in the future, when the turnitintooltwo plugin is upgraded on the server, browsers won't pick up the new Javascript code as they think the old cached version is still valid. The browsers are now starting to honour these values more strictly, even a reload of the page in Chrome still uses the cache now: https://blog.chromium.org/2017/01/reload-reloaded-faster-and-leaner-page_26.html .

We upgraded our dev server from v2017031301 to v2017080901, and the most noticable effect was on the Teacher view submissions list - it was displaying Student Last Name only and the _raw values in general. I worked out this was because the columns to be displayed are different between the versions and the old Javascript was displaying the wrong columns now.

The fix is to load the 'turnitintooltwo.min.js' (and _settings and _extra) via a URL that includes the $jsrev variable, so that when the site is upgraded the browsers can be notified that new Javascript is required.

In the meantime, as a workaround, I modified turnitintooltwo_view.class.php to not call $PAGE->requires->jquery_plugin() but instead to call $PAGE->requires->js(). Eg:

#        $PAGE->requires->jquery_plugin('turnitintooltwo-turnitintooltwo', 'mod_turnitintooltwo');
        $PAGE->requires->js('/mod/turnitintooltwo/jquery/turnitintooltwo.min.js');
#        $PAGE->requires->jquery_plugin('turnitintooltwo-turnitintooltwo_extra', 'mod_turnitintooltwo');
        $PAGE->requires->js('/mod/turnitintooltwo/jquery/turnitintooltwo_extra.min.js');
#        $PAGE->requires->jquery_plugin('turnitintooltwo-turnitintooltwo_settings', 'mod_turnitintooltwo');
        $PAGE->requires->js('/mod/turnitintooltwo/jquery/turnitintooltwo_settings.min.js');

This returns URLs in the format: https://sitename/lib/javascript.php/1507082538/mod/turnitintooltwo/jquery/turnitintooltwo.min.js

Rather than switch over to this js() method it would be more appropriate to follow the Moodle recommendations and convert to including all Javascript as AMD modules.

Thanks very much, Tony

dwinn commented 6 years ago

Hi @tbigby-kristin,

Thank you for the detailed information on this. We are aware of this caching issue with our JS files following the change we implemented to the student names in the inbox and we do plan to change the way we include JS files.

While I like your proposed AMD modules solution it would seem this was only possible in Moodle 2.9 onwards, and as the plugin supports versions 2.7 onwards we may have to look into a different solution.

danmarsden commented 6 years ago

Hi David,

Moodle dropped support for 2.7 in May this year, any sites still using 2.7 are no longer supported by Moodle and many 3rd party plugins have also dropped maintenance and support.

You might have a few key clients still running 2.7 (hopefully not many and they should have a plan for upgrading soon) and you may want to support them for a bit longer but I'd suggest you develop a policy for ending support for unsupported Moodle releases which will help you to make these sorts of decisions.

Also - most plugins these days don't maintain a single branch for all supported moodle releases - it's worked for you so far but you may continually increase the technical debt in your plugin and make it difficult to maintain in the longer term by continuing to do it on a single branch.

danmarsden commented 6 years ago

FYI 2.8, 2.9, 3.0 are also now out of support. (2.7 was an LTS so was supported a bit longer than other normal releases)

alex-rowe commented 4 years ago

Has this been looked at in the last few years? The plugin is still using the jquery_plugin function and as a result is causing caching issues.

Also the versioning of the file is still 2018102601 but that isn't the current version number either.