Moodle
  1. Moodle
  2. MDL-31911

Error creating a settings page for an admin tool

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.1, 2.3
    • Fix Version/s: 2.2.3
    • Component/s: Administration
    • Labels:
      None
    • Testing Instructions:
      Hide

      You will need a new admin tool with a settings.php file that creates an admin_settingpage for the tool - the one attached to this issue will be OK unless you wish to create a new one.

      Install your new admin tool and visit the Manage Admin Tools screen admin/tools.php. Before this fix a PHP error appears at the top of the screen. After, the PHP error does not appear.

      Show
      You will need a new admin tool with a settings.php file that creates an admin_settingpage for the tool - the one attached to this issue will be OK unless you wish to create a new one. Install your new admin tool and visit the Manage Admin Tools screen admin/tools.php. Before this fix a PHP error appears at the top of the screen. After, the PHP error does not appear.
    • Workaround:
      Hide

      My developer has suggested...

      if I look in /admin/settings/plugins.php

      I see this aroung line 480

      // Now add various admin tools
      foreach (get_plugin_list('tool') as $plugin => $plugindir) {
      $settings_path = "$plugindir/settings.php";
      if (file_exists($settings_path))

      { include($settings_path); }

      }

      /// Add all admin tools
      if ($hassiteconfig) {
      $ADMIN->add('modules', new admin_category('tools', get_string('tools', 'admin')));
      $ADMIN->add('tools', new admin_externalpage('managetools', get_string('toolsmanage', 'admin'),
      $CFG->wwwroot . '/' . $CFG->admin . '/tools.php'));
      }

      it tries to add the settings before it has created the admin category for them, if I change to

      /// Add all admin tools
      if ($hassiteconfig) {
      $ADMIN->add('modules', new admin_category('tools', get_string('tools', 'admin')));
      $ADMIN->add('tools', new admin_externalpage('managetools', get_string('toolsmanage', 'admin'),
      $CFG->wwwroot . '/' . $CFG->admin . '/tools.php'));
      }

      // Now add various admin tools
      foreach (get_plugin_list('tool') as $plugin => $plugindir) {
      $settings_path = "$plugindir/settings.php";
      if (file_exists($settings_path)) {
      include($settings_path);
      }
      }

      Show
      My developer has suggested... if I look in /admin/settings/plugins.php I see this aroung line 480 // Now add various admin tools foreach (get_plugin_list('tool') as $plugin => $plugindir) { $settings_path = "$plugindir/settings.php"; if (file_exists($settings_path)) { include($settings_path); } } /// Add all admin tools if ($hassiteconfig) { $ADMIN->add('modules', new admin_category('tools', get_string('tools', 'admin'))); $ADMIN->add('tools', new admin_externalpage('managetools', get_string('toolsmanage', 'admin'), $CFG->wwwroot . '/' . $CFG->admin . '/tools.php')); } it tries to add the settings before it has created the admin category for them, if I change to /// Add all admin tools if ($hassiteconfig) { $ADMIN->add('modules', new admin_category('tools', get_string('tools', 'admin'))); $ADMIN->add('tools', new admin_externalpage('managetools', get_string('toolsmanage', 'admin'), $CFG->wwwroot . '/' . $CFG->admin . '/tools.php')); } // Now add various admin tools foreach (get_plugin_list('tool') as $plugin => $plugindir) { $settings_path = "$plugindir/settings.php"; if (file_exists($settings_path)) { include($settings_path); } }
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Rank:
      38560

      Description

      On display of the admin tools manage screen I get

      parent does not exist!

      line 908 of /lib/adminlib.php: call to debugging()
      line 34 of /admin/tool/manageocw/settings.php: call to admin_category->add()
      line 484 of /admin/settings/plugins.php: call to include()
      line 5929 of /lib/adminlib.php: call to require()
      line 5814 of /lib/adminlib.php: call to admin_get_root()
      line 34 of /admin/tools.php: call to admin_externalpage_setup()

      With the attached settings file. Am I doing something wrong?

        Activity

        Hide
        Jenny Gray added a comment -

        Confirmed following conversation with Petr in hq dev chat.

        [11:56] skodak: you are 100% right - it is a bug - the line 490 has to be moved before 480.

        I will submit fix for integration shortly.

        Show
        Jenny Gray added a comment - Confirmed following conversation with Petr in hq dev chat. [11:56] skodak: you are 100% right - it is a bug - the line 490 has to be moved before 480. I will submit fix for integration shortly.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Some hours ago...

        the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Some notes:

        • the git repo was wrong
        • the branch names too
        • the commit message did not include the MDL-xxxx
        • The 22_STABLE was 100% wrong. I've committed proper one.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks! Some notes: the git repo was wrong the branch names too the commit message did not include the MDL-xxxx The 22_STABLE was 100% wrong. I've committed proper one. Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Tested with 22_STABLE and master. The patch fixes the message. Passing.

        Show
        Eloy Lafuente (stronk7) added a comment - Tested with 22_STABLE and master. The patch fixes the message. Passing.
        Hide
        Jenny Gray added a comment -

        About the notes - yikes, Eloy I'm so sorry to have made things more difficult for you. I'll try to do better next time!

        Show
        Jenny Gray added a comment - About the notes - yikes, Eloy I'm so sorry to have made things more difficult for you. I'll try to do better next time!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        FCT (fixed, closing, thanks). Ciao

        "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
        ~ Benjamin Disraeli

        Show
        Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: