From 9c3710854bdca479faac1fc7bd74bc12e67c389c Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 26 May 2016 17:18:13 +0800 Subject: [PATCH] MDL-53598 block_glossary_random: do not fail if glossary was deleted This commit refactors how associated glossary is searched for and removes unnecessary DB queries. Also prevents from situations when the global glossary or course have been deleted --- ...store_glossary_random_block_task.class.php | 26 +-- .../glossary_random/block_glossary_random.php | 152 ++++++++++-------- .../behat/glossary_random_global.feature | 84 ++++++++++ 3 files changed, 183 insertions(+), 79 deletions(-) create mode 100644 blocks/glossary_random/tests/behat/glossary_random_global.feature diff --git a/blocks/glossary_random/backup/moodle2/restore_glossary_random_block_task.class.php b/blocks/glossary_random/backup/moodle2/restore_glossary_random_block_task.class.php index cfc5bc14f9701..236351cf33b96 100644 --- a/blocks/glossary_random/backup/moodle2/restore_glossary_random_block_task.class.php +++ b/blocks/glossary_random/backup/moodle2/restore_glossary_random_block_task.class.php @@ -60,19 +60,27 @@ public function after_restore() { if ($configdata = $DB->get_field('block_instances', 'configdata', array('id' => $blockid))) { $config = unserialize(base64_decode($configdata)); if (!empty($config->glossary)) { - // Get glossary mapping and replace it in config if ($glossarymap = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'glossary', $config->glossary)) { - $mappedglossary = $DB->get_record('glossary', array('id' => $glossarymap->newitemid), - 'id,course,globalglossary', MUST_EXIST); - $config->glossary = $mappedglossary->id; - $config->courseid = $mappedglossary->course; - $config->globalglossary = $mappedglossary->globalglossary; - $configdata = base64_encode(serialize($config)); - $DB->set_field('block_instances', 'configdata', $configdata, array('id' => $blockid)); + // Get glossary mapping and replace it in config + $config->glossary = $glossarymap->newitemid; + } else if ($this->is_samesite()) { + // We are restoring on the same site, check if glossary can be used in the block in this course. + $glossaryid = $DB->get_field_sql("SELECT id FROM {glossary} " . + "WHERE id = ? AND (course = ? OR globalglossary = 1)", + [$config->glossary, $this->get_courseid()]); + if (!$glossaryid) { + unset($config->glossary); + } } else { // The block refers to a glossary not present in the backup file. - $DB->set_field('block_instances', 'configdata', '', array('id' => $blockid)); + unset($config->glossary); } + // Unset config variables that are no longer used. + unset($config->globalglossary); + unset($config->courseid); + // Save updated config. + $configdata = base64_encode(serialize($config)); + $DB->set_field('block_instances', 'configdata', $configdata, array('id' => $blockid)); } } } diff --git a/blocks/glossary_random/block_glossary_random.php b/blocks/glossary_random/block_glossary_random.php index 6ca3bb57a513e..bf9f6a7dcea49 100644 --- a/blocks/glossary_random/block_glossary_random.php +++ b/blocks/glossary_random/block_glossary_random.php @@ -29,6 +29,12 @@ class block_glossary_random extends block_base { + /** + * @var cm_info|stdClass has properties 'id' (course module id) and 'uservisible' + * (whether the glossary is visible to the current user) + */ + protected $glossarycm = null; + function init() { $this->title = get_string('pluginname','block_glossary_random'); } @@ -58,6 +64,11 @@ function specialization() { //check if it's time to put a new entry in cache if (time() > $this->config->nexttime) { + if (!($cm = $this->get_glossary_cm()) || !$cm->uservisible) { + // Skip generating of the cache if we can't display anything to the current user. + return false; + } + // place glossary concept and definition in $pref->cache if (!$numberofentries = $DB->count_records('glossary_entries', array('glossaryid'=>$this->config->glossary, 'approved'=>1))) { @@ -65,20 +76,6 @@ function specialization() { $this->instance_config_commit(); } - // Get glossary instance, if not found then return without error, as this will be handled in get_content. - if (!$glossary = $DB->get_record('glossary', array('id' => $this->config->glossary))) { - return false; - } - - $this->config->globalglossary = $glossary->globalglossary; - - // Save course id in config, so we can get correct course module. - $this->config->courseid = $glossary->course; - - // Get module and context, to be able to rewrite urls - if (! $cm = get_coursemodule_from_instance('glossary', $glossary->id, $this->config->courseid)) { - return false; - } $glossaryctx = context_module::instance($cm->id); $limitfrom = 0; @@ -156,88 +153,103 @@ function specialization() { } } - function instance_allow_multiple() { - // Are you going to allow multiple instances of each block? - // If yes, then it is assumed that the block WILL USE per-instance configuration - return true; + /** + * Replace the instance's configuration data with those currently in $this->config; + */ + function instance_config_commit($nolongerused = false) { + // Unset config variables that are no longer used. + unset($this->config->globalglossary); + unset($this->config->courseid); + parent::instance_config_commit($nolongerused); } - function get_content() { - global $USER, $CFG, $DB; - + /** + * Checks if glossary is available - it should be either located in the same course or be global + * + * @return null|cm_info|stdClass object with properties 'id' (course module id) and 'uservisible' + */ + protected function get_glossary_cm() { + global $DB; if (empty($this->config->glossary)) { - $this->content = new stdClass(); - if ($this->user_can_edit()) { - $this->content->text = get_string('notyetconfigured','block_glossary_random'); - } else { - $this->content->text = ''; - } - $this->content->footer = ''; - return $this->content; + // No glossary is configured. + return null; } - require_once($CFG->dirroot.'/course/lib.php'); + if (!empty($this->glossarycm)) { + return $this->glossarycm; + } - // If $this->config->globalglossary is not set then get glossary info from db. - if (!isset($this->config->globalglossary)) { - if (!$glossary = $DB->get_record('glossary', array('id' => $this->config->glossary))) { - return ''; - } else { - $this->config->courseid = $glossary->course; - $this->config->globalglossary = $glossary->globalglossary; - $this->instance_config_commit(); + if (!empty($this->page->course->id)) { + // First check if glossary belongs to the current course (we don't need to make any DB queries to find it). + $modinfo = get_fast_modinfo($this->page->course); + if (isset($modinfo->instances['glossary'][$this->config->glossary])) { + $this->glossarycm = $modinfo->instances['glossary'][$this->config->glossary]; + if ($this->glossarycm->uservisible) { + // The glossary is in the same course and is already visible to the current user, + // no need to check if it is global, save on DB query. + return $this->glossarycm; + } } } - $modinfo = get_fast_modinfo($this->config->courseid); - // If deleted glossary or non-global glossary on different course page, then reset. - if (!isset($modinfo->instances['glossary'][$this->config->glossary]) - || ((empty($this->config->globalglossary) && ($this->config->courseid != $this->page->course->id)))) { + // Find course module id for the given glossary, only if it is global. + $cm = $DB->get_record_sql("SELECT cm.id, cm.visible AS uservisible + FROM {course_modules} cm + JOIN {modules} md ON md.id = cm.module + JOIN {glossary} g ON g.id = cm.instance + WHERE g.id = :instance AND md.name = :modulename AND g.globalglossary = 1", + ['instance' => $this->config->glossary, 'modulename' => 'glossary']); + + if ($cm) { + // This is a global glossary, create an object with properties 'id' and 'uservisible'. We don't need any + // other information so why bother retrieving it. Full access check is skipped for global glossaries for + // performance reasons. + $this->glossarycm = $cm; + } else if (empty($this->glossarycm)) { + // Glossary does not exist. Remove it in the config so we don't repeat this check again later. $this->config->glossary = 0; - $this->config->cache = ''; $this->instance_config_commit(); + } - $this->content = new stdClass(); - if ($this->user_can_edit()) { - $this->content->text = get_string('notyetconfigured','block_glossary_random'); - } else { - $this->content->text = ''; - } - $this->content->footer = ''; + return $this->glossarycm; + } + + function instance_allow_multiple() { + // Are you going to allow multiple instances of each block? + // If yes, then it is assumed that the block WILL USE per-instance configuration + return true; + } + + function get_content() { + if ($this->content !== null) { return $this->content; } + $this->content = (object)['text' => '', 'footer' => '']; - $cm = $modinfo->instances['glossary'][$this->config->glossary]; - if (!has_capability('mod/glossary:view', context_module::instance($cm->id))) { - return ''; + if (!$cm = $this->get_glossary_cm()) { + if ($this->user_can_edit()) { + $this->content->text = get_string('notyetconfigured', 'block_glossary_random'); + } + return $this->content; } if (empty($this->config->cache)) { $this->config->cache = ''; } - if ($this->content !== NULL) { - return $this->content; - } - - $this->content = new stdClass(); - - // Show glossary if visible and place links in footer. - if ($cm->visible) { + if ($cm->uservisible) { + // Show glossary if visible and place links in footer. $this->content->text = $this->config->cache; if (has_capability('mod/glossary:write', context_module::instance($cm->id))) { - $this->content->footer = ''.$this->config->addentry.'
'; - } else { - $this->content->footer = ''; + $this->content->footer = html_writer::link(new moodle_url('/mod/glossary/edit.php', ['cmid' => $cm->id]), + format_string($this->config->addentry)) . '
'; } - $this->content->footer .= ''.$this->config->viewglossary.''; - - // Otherwise just place some text, no link. + $this->content->footer .= html_writer::link(new moodle_url('/mod/glossary/view.php', ['id' => $cm->id]), + format_string($this->config->viewglossary)); } else { - $this->content->footer = $this->config->invisible; + // Otherwise just place some text, no link. + $this->content->footer = format_string($this->config->invisible); } return $this->content; diff --git a/blocks/glossary_random/tests/behat/glossary_random_global.feature b/blocks/glossary_random/tests/behat/glossary_random_global.feature new file mode 100644 index 0000000000000..2eea426a972c4 --- /dev/null +++ b/blocks/glossary_random/tests/behat/glossary_random_global.feature @@ -0,0 +1,84 @@ +@block @block_glossary_random +Feature: Random glossary entry block linking to global glossary + In order to show the entries from glossary + As a teacher + I can add the random glossary entry to a course page + + Background: + Given the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + | Course 2 | C2 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | globalglossary | defaultapproval | + | glossary | Tips and Tricks | Frontpage glossary description | C2 | glossary0 | 1 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | student1 | Sam1 | Student1 | student1@example.com | + | teacher1 | Terry1 | Teacher1 | teacher1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | student1 | C1 | student | + | teacher1 | C1 | editingteacher | + + Scenario: View random (last) entry in the global glossary + When I log in as "admin" + And I am on site homepage + And I follow "Course 2" + And I follow "Tips and Tricks" + And I press "Add a new entry" + And I set the following fields to these values: + | Concept | Never come late | + | Definition | Come in time for your classes | + And I press "Save changes" + And I log out + # As a teacher add a block to the course page linking to the global glossary. + And I log in as "teacher1" + And I follow "Course 1" + And I turn editing mode on + And I add the "Random glossary entry" block + And I configure the "block_glossary_random" block + And I set the following fields to these values: + | Title | Tip of the day | + | Take entries from this glossary | Tips and Tricks | + | How a new entry is chosen | Last modified entry | + And I press "Save changes" + Then I should see "Never come late" in the "Tip of the day" "block" + And I should not see "Add a new entry" in the "Tip of the day" "block" + And I should see "View all entries" in the "Tip of the day" "block" + And I log out + # Student who can't see the module is still able to view entries in this block (because the glossary was marked as global) + And I log in as "student1" + And I follow "Course 1" + And I should see "Never come late" in the "Tip of the day" "block" + And I should not see "Add a new entry" in the "Tip of the day" "block" + And I should see "View all entries" in the "Tip of the day" "block" + And I log out + + Scenario: Removing the global glossary that is used in random glossary block + And I log in as "teacher1" + And I follow "Course 1" + And I turn editing mode on + And I add the "Random glossary entry" block + And I configure the "block_glossary_random" block + And I set the following fields to these values: + | Title | Tip of the day | + | Take entries from this glossary | Tips and Tricks | + | How a new entry is chosen | Last modified entry | + And I press "Save changes" + And I log out + And I log in as "admin" + And I am on site homepage + And I follow "Course 2" + And I follow "Tips and Tricks" + And I follow "Edit settings" + And I set the field "globalglossary" to "0" + And I press "Save and return to course" + And I am on site homepage + And I follow "Course 1" + Then I should see "Please configure this block using the edit icon." in the "Tip of the day" "block" + And I log out + And I log in as "student1" + And I follow "Course 1" + And "Tip of the day" "block" should not exist + And I log out