search

streamingltd / MEDIAL-Moodle-Activity

The MEDIAL activity plugin for Moodle
GNU General Public License v3.0
3 stars 8 forks source link

$PAGE->url reference in settings.php throws notice #5

Closed thepurpleblob closed 5 years ago

thepurpleblob commented 5 years ago

This bit of code in settings.php

if ($PAGE->url->get_param("section")=="modsettinghelixmedia") {
    require_once($CFG->dirroot.'/mod/helixmedia/locallib.php');
    $settings->add(new admin_setting_heading('helixmedia/version_check', get_string("version_check_title", "mod_helixmedia"),
        helixmedia_version_check()));
}

throws the following notice with Developer debugging enabled...

This page did not call $PAGE->set_url(...). Using http://pc164-239.cent.gla.ac.uk/moodle37/admin/search.php

It appears that the expected url has not been set prior to settings.php being called.

thepurpleblob commented 5 years ago

Further... I can't see any reason to have a version check on this page at all. Moodle already has a facility to show new versions of plugins.

tim1mw commented 5 years ago

Sorry for the slow reply, our normal bug report process doesn't use github so this isn't monitored.

@thepurpleblob the version check is testing the version of Medial you have is compatible with the plugin version that you have installed, it isn't checking for upgrades to the Moodle plugin itself, this still happens as part of the normal Moodle plugin update code.

Re the notice, the issue is that in some contexts Moodle is setting the page URL prior to calling this code, in others it doesn't hence the error. It's not the page URL I'm after here, I just want to suppress the version check (which involves an API call to MEDIAL so is quite slow) from being performed unless you're actually on the MEDIAL settings page, so I'll re-write this to use $PAGE->pagetype which is automatically set so should always be present and will tell me what I need to know in a cleaner way (qualified_me would require the code to parse the URL to work that out). That will be in the next plugin release when it goes through QA, the code change will look like this if you want to manually patch your version in the interim:

if ($PAGE->pagetype=="admin-setting-modsettinghelixmedia") { require_once($CFG->dirroot.'/mod/helixmedia/locallib.php'); $settings->add(new admin_setting_heading('helixmedia/version_check', get_string("version_check_title", "mod_helixmedia"), helixmedia_version_check())); }

I'll trim out the superfluous global $CFG at the same time, as you say that's not actually needed.