From c7b738546c48c2423029920da65b7a02aa2fd811 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Fri, 20 Mar 2015 17:52:35 +0000 Subject: [PATCH 1/2] MDL-49434 availability: Use renderables for multiple messages Co-Authored-By: Andrew Nicols --- availability/classes/info.php | 14 +++- availability/classes/multiple_messages.php | 70 +++++++++++++++++++ availability/classes/tree.php | 6 +- .../tests/behat/conditional_bug.feature | 60 ++++++++++++++++ availability/renderer.php | 21 +++--- theme/upgrade.txt | 6 ++ 6 files changed, 162 insertions(+), 15 deletions(-) create mode 100644 availability/classes/multiple_messages.php create mode 100644 availability/condition/completion/tests/behat/conditional_bug.feature diff --git a/availability/classes/info.php b/availability/classes/info.php index afd1d40b013..05d82fa7a5d 100644 --- a/availability/classes/info.php +++ b/availability/classes/info.php @@ -713,11 +713,21 @@ abstract class info { * that we can guarantee does not happen from within building the modinfo * object. * - * @param string $info Info string + * @param \renderable|string $inforenderable Info string or renderable * @param int|\stdClass $courseorid * @return string Correctly formatted info string */ - public static function format_info($info, $courseorid) { + public static function format_info($inforenderable, $courseorid) { + global $PAGE; + + // Use renderer if required. + if (is_string($inforenderable)) { + $info = $inforenderable; + } else { + $renderer = $PAGE->get_renderer('core', 'availability'); + $info = $renderer->render($inforenderable); + } + // Don't waste time if there are no special tags. if (strpos($info, '. + +/** + * Represents multiple availability messages. + * + * These are messages like 'Not available until '. This class includes + * multiple messages so that they can be rendered into a combined display, e.g. + * using bulleted lists. + * + * The tree structure of this object matches that of the availability + * restrictions. + * + * @package core_availability + * @copyright 2015 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +/** + * Represents multiple availability messages. + * + * These are messages like 'Not available until '. This class includes + * multiple messages so that they can be rendered into a combined display, e.g. + * using bulleted lists. + * + * The tree structure of this object matches that of the availability + * restrictions. + * + * @package core_availability + * @copyright 2015 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_availability_multiple_messages implements renderable { + /** @var bool True if this object represents the root of the tree */ + public $root; + /** @var bool True if items use the AND operator (false = OR) */ + public $andoperator; + /** @var bool True if this part of the tree is marked 'hide entirely' for non-matching users */ + public $treehidden; + /** @var array Array of child items (may be string or this type) */ + public $items; + + /** + * Constructor. + * + * @param bool $root True if this object represents the root of the tree + * @param bool $andoperator True if items use the AND operator (false = OR) + * @param bool $treehidden True if this part of the tree is marked 'hide entirely' for non-matching users + * @param array $items Array of items (may be string or this type) + */ + public function __construct($root, $andoperator, $treehidden, array $items) { + $this->root = $root; + $this->andoperator = $andoperator; + $this->treehidden = $treehidden; + $this->items = $items; + } +} diff --git a/availability/classes/tree.php b/availability/classes/tree.php index 88493207a48..ef15c585ee4 100644 --- a/availability/classes/tree.php +++ b/availability/classes/tree.php @@ -471,11 +471,10 @@ class tree extends tree_node { * @param result $result Result object if this is a student display, else null * @param bool $root True if this is the root item * @param bool $hidden Staff display; true if this tree has show=false (from parent) + * @return string|renderable Information to render */ protected function get_full_information_recursive( $not, info $info, result $result = null, $root, $hidden = false) { - global $PAGE; - // Get list of children - either full list, or those which are shown. $children = $this->children; $staff = true; @@ -550,8 +549,7 @@ class tree extends tree_node { } // Format output for display. - $renderer = $PAGE->get_renderer('core', 'availability'); - return $renderer->multiple_messages($root, $andoperator, $treehidden, $items); + return new \core_availability_multiple_messages($root, $andoperator, $treehidden, $items); } /** diff --git a/availability/condition/completion/tests/behat/conditional_bug.feature b/availability/condition/completion/tests/behat/conditional_bug.feature new file mode 100644 index 00000000000..b7f6fec1135 --- /dev/null +++ b/availability/condition/completion/tests/behat/conditional_bug.feature @@ -0,0 +1,60 @@ +@availability @availability_completion +Feature: Confirm that conditions on completion no longer cause a bug + In order to use completion conditions + As a teacher + I need it to not break when I set up certain conditions on some modules + + Background: + Given the following "courses" exist: + | fullname | shortname | format | + | Course 1 | C1 | topics | + And the following "users" exist: + | username | + | teacher1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following config values are set as admin: + | enableavailability | 1 | + | enablecompletion | 1 | + + @javascript + Scenario: Multiple completion conditions on glossary + # Set up course. + Given I log in as "teacher1" + And I follow "Course 1" + And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets + And I set the field "Enable completion tracking" to "Yes" + And I press "Save and display" + And I turn editing mode on + + # Add a couple of Pages with manual completion. + And I add a "Page" to section "1" and I fill the form with: + | Name | Page1 | + | Page content | x | + And I add a "Page" to section "1" and I fill the form with: + | Name | Page2 | + | Page content | x | + + # Add a Glossary. + When I add a "Glossary" to section "1" + And I set the following fields to these values: + | Name | TestGlossary | + And I expand all fieldsets + + # Add restrictions to the previous Pages being complete. + And I press "Add restriction..." + And I click on "Activity completion" "button" in the "Add restriction..." "dialogue" + And I set the field "Activity or resource" to "Page1" + And I press "Add restriction..." + And I click on "Activity completion" "button" in the "Add restriction..." "dialogue" + And I set the field with xpath "//div[@class='availability-item'][preceding-sibling::div]//select[@name='cm']" to "Page2" + And I press "Save and return to course" + And I should see "Not available unless:" in the ".activity.glossary" "css_element" + And I should see "The activity Page1 is marked complete" in the ".activity.glossary" "css_element" + And I should see "The activity Page2 is marked complete" in the ".activity.glossary" "css_element" + And I follow "TestGlossary" + + # Behat will automatically check there is no error on this page. + Then I should see "TestGlossary" diff --git a/availability/renderer.php b/availability/renderer.php index b098e811347..20fff45341f 100644 --- a/availability/renderer.php +++ b/availability/renderer.php @@ -40,22 +40,25 @@ class core_availability_renderer extends plugin_renderer_base { * * This function will not be called unless there are at least two messages. * - * @param bool $root True if this is a root-level list for an activity - * @param bool $andoperator True if the messages are being combined as AND - * @param bool $roothidden True if the root level should use 'hidden' message - * @param array $messages Messages to render + * @param core_availability_multiple_messages $renderable Multiple messages * @return string Combined HTML */ - public function multiple_messages($root, $andoperator, $roothidden, array $messages) { + public function render_core_availability_multiple_messages( + core_availability_multiple_messages $renderable) { // Get initial message. - $out = get_string('list_' . ($root ? 'root_' : '') . - ($andoperator ? 'and' : 'or') . ($roothidden ? '_hidden' : ''), + $out = get_string('list_' . ($renderable->root ? 'root_' : '') . + ($renderable->andoperator ? 'and' : 'or') . ($renderable->treehidden ? '_hidden' : ''), 'availability'); // Make the list. $out .= html_writer::start_tag('ul'); - foreach ($messages as $message) { - $out .= html_writer::tag('li', $message); + foreach ($renderable->items as $item) { + if (is_string($item)) { + $str = $item; + } else { + $str = $this->render($item); + } + $out .= html_writer::tag('li', $str); } $out .= html_writer::end_tag('ul'); return $out; diff --git a/theme/upgrade.txt b/theme/upgrade.txt index b503392b0bd..ba34087c357 100644 --- a/theme/upgrade.txt +++ b/theme/upgrade.txt @@ -1,6 +1,12 @@ This files describes API changes in /theme/* themes, information provided here is intended especially for theme designer. +=== 3.0 === + +* The renderer function core_availability_renderer::multiple_messages was changed to + render_core_availability_multiple_messages, requiring a parameter of the new + \core_availability\multiple_messages type. + === 2.9 === * Themes Bootstrapbase, Clean and More have undergone some changes for RTL layouts see - MDL-48160. From aad5b7f1d020dd63845b8a8d00a389406ef20d55 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 24 Jun 2015 11:09:27 +0100 Subject: [PATCH 2/2] MDL-49434 availability: Revert MDL-49418 (obsoleted by this change) This reverts commit 51fef1de359439c79fa98bd9ebec19077e66cb57. --- lib/moodlelib.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index c33fc8f351c..574225b2703 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -2807,15 +2807,6 @@ function require_login($courseorid = null, $autologinguest = true, $cm = null, $ } } - // Set the global $COURSE. - // TODO MDL-49434: setting current course/cm should be after the check $cm->uservisible . - if ($cm) { - $PAGE->set_cm($cm, $course); - $PAGE->set_pagelayout('incourse'); - } else if (!empty($courseorid)) { - $PAGE->set_course($course); - } - // Check visibility of activity to current user; includes visible flag, conditional availability, etc. if ($cm && !$cm->uservisible) { if ($preventredirect) { @@ -2829,6 +2820,14 @@ function require_login($courseorid = null, $autologinguest = true, $cm = null, $ redirect($url, get_string('activityiscurrentlyhidden')); } + // Set the global $COURSE. + if ($cm) { + $PAGE->set_cm($cm, $course); + $PAGE->set_pagelayout('incourse'); + } else if (!empty($courseorid)) { + $PAGE->set_course($course); + } + // Finally access granted, update lastaccess times. user_accesstime_log($course->id); }