Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-23754 Performance improvements META
  3. MDL-27123

Module security admin config performance improvement

    XMLWordPrintable

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.1
    • Component/s: Administration
    • Labels:

      Description

      I had need to create a module multi selection admin page so looked to the Site administration > Security > Module security page as a starting point.

      Because I was looking, it seems that the database hit on line 95 of admin/settings/security.php could be avoided for most pages if the lazy loading feature of the base class admin_setting_configselect was used.

      I suggest that the admin_setting_multiconfigselect class is extended to create a new admin_setting_multiconfigselect_modules class where the load_choices function may be overwritten with the database hit. This will improve performance for many pages as it is one less database hit, and it would also provides me with a class that I may re-use.

      Suggested code:
      lib/adminlib.php:
      /**

      • Multiselect for current modules
        *
      • @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
        */
        class admin_setting_configmultiselect_modules extends admin_setting_configmultiselect {
        /**
      • Calls parent::__construct - note array $choices is not required
        */
        public function __construct($name, $visiblename, $description) { parent::__construct($name, $visiblename, $description, array(), null); }

      /**

      • Loads an array of current module choices
        *
      • @return bool always return true
        */
        public function load_choices() {
        if (is_array($this->choices)) { return true; }

        $this->choices = array();

      global $CFG, $DB;
      $records = $DB->get_records('modules', array('visible'=>1), 'name');
      foreach ($records as $record) {
      if (file_exists("$CFG->dirroot/mod/$record->name/lib.php"))

      { $this->choices[$record->id] = $record->name; }

      }
      return true;
      }
      }

      admin/settings/security.php:
      remove lines 95 to 101 and change line 102 to:
      $temp->add(new admin_setting_configmultiselect('defaultallowedmodules', get_string('defaultallowedmodules', 'admin'), get_string('configdefaultallowedmodules', 'admin')));

      Please let me know if this sounds reasonable, or if there are any objections.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                timhunt Tim Hunt
                Reporter:
                jb23347 John Beedell
                Participants:
                Component watchers:
                Andrew Nicols, Mathew May, Michael Hawkins, Shamim Rezaie, Simey Lameze
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Jul/11