Moodle

allow activity modules to add sub menus of settings pages to admin menu

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9.1
  • Component/s: Administration, Quiz
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

We want to allow quiz reports to add settings pages to the tree of admin settings pages.

  • We think the pages should be sub pages of module / activity / quiz page.
  • Unfortunately the current settings.php module hook does not allow us to do more than add fields to one page for a module.
  • Don't want to interfere with how api currently works. So we introduce a new hook : settingstree.php and check for that before settings.php

Thoroughly tested :

  • Links on module activity page to module settings still work.
  • If module is disabled then the sub menu and general settigngs are all made invisible.

I attach :

  • a patch to plugins.php to enable the new hook.
  • and an example settingstree.php that scans quiz/report/ sub directories for settings.php files to add sub pages to menu for.
  • and a screen shot of the menu with a sub menu for quiz report settings pages. The quiz module settings page is 'General Settings' under the main Quiz folder.
  1. pluginspatch.txt
    01/May/08 8:31 PM
    2 kB
    Jamie Pratt
  2. settingstree.php
    01/May/08 8:32 PM
    2 kB
    Jamie Pratt
  1. ScreenShot004.gif
    32 kB
    01/May/08 7:23 PM

Issue Links

Activity

Hide
Jamie Pratt added a comment -

Added Tim, Petr and Martin D here as I want your opinions on whether this seems a sensible way of allowing activities to add more setting pages to the site admin menu.

Show
Jamie Pratt added a comment - Added Tim, Petr and Martin D here as I want your opinions on whether this seems a sensible way of allowing activities to add more setting pages to the site admin menu.
Hide
Luke Hudson added a comment -

// add a top level category for us to add stuff to
$ADMIN->add('root', new admin_category('racp', get_string('racp','local')));

$ADMIN->add('racp', new admin_externalpage('curriculum', get_string('editcurriculum', 'local'),
"$CFG->wwwroot/local/admin/editcurriculum.php",
'moodle/local:editcurriculum'));

$ADMIN->add('racp', new admin_externalpage('modsettinglplan', get_string('adminnotifications', 'lplan'),
"$CFG->wwwroot/$CFG->admin/settings.php?section=modsettinglplan",
'moodle/site:config'));

$ADMIN->add('racp', new admin_externalpage('authsettingdb', get_string('configauthdb', 'local'),
"$CFG->wwwroot/$CFG->admin/auth_config.php?auth=db",
'moodle/site:config'));

Show
Luke Hudson added a comment - // add a top level category for us to add stuff to $ADMIN->add('root', new admin_category('racp', get_string('racp','local'))); $ADMIN->add('racp', new admin_externalpage('curriculum', get_string('editcurriculum', 'local'), "$CFG->wwwroot/local/admin/editcurriculum.php", 'moodle/local:editcurriculum')); $ADMIN->add('racp', new admin_externalpage('modsettinglplan', get_string('adminnotifications', 'lplan'), "$CFG->wwwroot/$CFG->admin/settings.php?section=modsettinglplan", 'moodle/site:config')); $ADMIN->add('racp', new admin_externalpage('authsettingdb', get_string('configauthdb', 'local'), "$CFG->wwwroot/$CFG->admin/auth_config.php?auth=db", 'moodle/site:config'));
Hide
Jamie Pratt added a comment -

I understand how to add categories to the admin tree. The problem is the code to construct the tree is only looking in settings.php for form fields to add to the module settings page. See the code fragments below :

in the class file for admin_root :

/**

  • full tree flag - true means all settings required, false onlypages required
    */
    var $fulltree;

and in admin/settings/plugins.php we have :

if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); }

which means that we can only use settings.php to add stuff to one settings page for each module.

Show
Jamie Pratt added a comment - I understand how to add categories to the admin tree. The problem is the code to construct the tree is only looking in settings.php for form fields to add to the module settings page. See the code fragments below : in the class file for admin_root : /**
  • full tree flag - true means all settings required, false onlypages required */ var $fulltree;
and in admin/settings/plugins.php we have : if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); } which means that we can only use settings.php to add stuff to one settings page for each module.
Hide
Jamie Pratt added a comment -

Also I should add that the quiz module is not using settings.php but config.html. The settings form for the quiz module could not be laid out in a user friendly way using the admin menu methods to construct a form.

So I can see that it might be suggested that each module uses only one form and plugins could add settings to that form. But this would only be possible if the plug-in was using settings.php to add form fields to the form. Having to use settings.php instead of config.html would be a big set back in terms of usability of the form.

Show
Jamie Pratt added a comment - Also I should add that the quiz module is not using settings.php but config.html. The settings form for the quiz module could not be laid out in a user friendly way using the admin menu methods to construct a form. So I can see that it might be suggested that each module uses only one form and plugins could add settings to that form. But this would only be possible if the plug-in was using settings.php to add form fields to the form. Having to use settings.php instead of config.html would be a big set back in terms of usability of the form.
Hide
Martin Dougiamas added a comment -

Um, perhaps I'm missing something but why don't you just put some code in mod/quiz/settings.php to cycle through the reports and include settings.php files that might be found there?

Show
Martin Dougiamas added a comment - Um, perhaps I'm missing something but why don't you just put some code in mod/quiz/settings.php to cycle through the reports and include settings.php files that might be found there?
Hide
Jamie Pratt added a comment -

The problem is the if condition I mention above :

if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); }

It precludes having anything in settings.php or included from settings.php but form fields within one page.

Show
Jamie Pratt added a comment - The problem is the if condition I mention above : if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); } It precludes having anything in settings.php or included from settings.php but form fields within one page.
Hide
Jamie Pratt added a comment -

an alternative is to move the :

if ($ADMIN->fulltree) {

condition into settings.php I suppose. But this might break some custom modules. Better not too change api but to add to it??

Also moving the :

if ($ADMIN->fulltree) {

into settings.php would not work for the quiz module that is still using config.html as I mention above.

Show
Jamie Pratt added a comment - an alternative is to move the : if ($ADMIN->fulltree) { condition into settings.php I suppose. But this might break some custom modules. Better not too change api but to add to it?? Also moving the : if ($ADMIN->fulltree) { into settings.php would not work for the quiz module that is still using config.html as I mention above.
Hide
Martin Dougiamas added a comment -

OK, I understand the problem better now. I think the approach you took is basically sound but I'll leave it to Petr for a final review.

Show
Martin Dougiamas added a comment - OK, I understand the problem better now. I think the approach you took is basically sound but I'll leave it to Petr for a final review.
Hide
Jamie Pratt added a comment -

Sorry I just thought we can still use an admin_externalpage from settings.php and use config.html. So the quiz module could use settings.php if we were to decide to move the if ($ADMIN->fulltree) { into settings.php.

Look forward to Petr's comments.

Show
Jamie Pratt added a comment - Sorry I just thought we can still use an admin_externalpage from settings.php and use config.html. So the quiz module could use settings.php if we were to decide to move the if ($ADMIN->fulltree) { into settings.php. Look forward to Petr's comments.
Hide
Petr Škoda (skodak) added a comment -

hmm, I personally did not like what I saw in quiz/config.htm - I hoped somebody will convert it to settings.php before 1.9 goes gold and I still think that it should be done.

I propose:
1/ convert finally config.html in quiz
2/ add settingspage.php.php as settings.php - config as the last and legacy only option
2/ remove support for config.html from HEAD

Show
Petr Škoda (skodak) added a comment - hmm, I personally did not like what I saw in quiz/config.htm - I hoped somebody will convert it to settings.php before 1.9 goes gold and I still think that it should be done. I propose: 1/ convert finally config.html in quiz 2/ add settingspage.php.php as settings.php - config as the last and legacy only option 2/ remove support for config.html from HEAD
Hide
Jamie Pratt added a comment -

I asked Petr for clarification of what he meant in his last comment and he said :

"let's have both settings.php and settingstree.php - and legacy option in 1.9 - but always only one of them"

He is in favour of removing config.html altogether before 2.0.

Show
Jamie Pratt added a comment - I asked Petr for clarification of what he meant in his last comment and he said : "let's have both settings.php and settingstree.php - and legacy option in 1.9 - but always only one of them" He is in favour of removing config.html altogether before 2.0.
Hide
Petr Škoda (skodak) added a comment -

some more clarification - I like the screenshot!
and I do not like config.html

Show
Petr Škoda (skodak) added a comment - some more clarification - I like the screenshot! and I do not like config.html
Hide
Dan Poltawski added a comment -

Just a note that this reminded me of.. last time I checked we hadn't produced any documentation on (or even a mention of) the settings.php structure for module authors.

Show
Dan Poltawski added a comment - Just a note that this reminded me of.. last time I checked we hadn't produced any documentation on (or even a mention of) the settings.php structure for module authors.
Hide
Tim Hunt added a comment -

Jamie and Petr seem to have reached a consensus. And I think it is a good one.

The only quesiton is who has time to change quiz/config.html to settingstree.php. That seems to be MDL-7010. I don't really have time to work on it, but the buck stops with me on the quiz, so if there are no other volunteers, assign that to me.

Show
Tim Hunt added a comment - Jamie and Petr seem to have reached a consensus. And I think it is a good one. The only quesiton is who has time to change quiz/config.html to settingstree.php. That seems to be MDL-7010. I don't really have time to work on it, but the buck stops with me on the quiz, so if there are no other volunteers, assign that to me.
Hide
Jamie Pratt added a comment -

Fixed in HEAD and 1.9 branch. I added a TODO comment to delete the if branch that allows legacy config.html which will be deleted as soon as someone migrates quiz/config.html

Show
Jamie Pratt added a comment - Fixed in HEAD and 1.9 branch. I added a TODO comment to delete the if branch that allows legacy config.html which will be deleted as soon as someone migrates quiz/config.html
Hide
Petr Škoda (skodak) added a comment -

thanks, closing

Show
Petr Škoda (skodak) added a comment - thanks, closing

Dates

  • Created:
    Updated:
    Resolved: