From c87e52f5778cf4a7ab9c911e4eeb0ebf5814aca0 Mon Sep 17 00:00:00 2001 From: ferranrecio Date: Fri, 19 Jul 2024 06:25:18 +0200 Subject: [PATCH 1/3] MDL-82260 course: add behat section settings page url --- course/tests/behat/behat_course.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/course/tests/behat/behat_course.php b/course/tests/behat/behat_course.php index 28a0ca7eb1e..5caedc4b850 100644 --- a/course/tests/behat/behat_course.php +++ b/course/tests/behat/behat_course.php @@ -178,7 +178,11 @@ class behat_course extends behat_base { */ protected function resolve_page_instance_url(string $type, string $identifier): moodle_url { $type = strtolower($type); + + $sectionpage = 'section.php'; switch ($type) { + case 'section settings': + $sectionpage = 'editsection.php'; case 'section': $identifiers = explode('>', $identifier); $identifiers = array_map('trim', $identifiers); @@ -196,7 +200,7 @@ class behat_course extends behat_base { if (!$section) { throw new Exception("The specified section $identifier does not exist."); } - return new moodle_url('/course/section.php', ['id' => $section->id]); + return new moodle_url('/course/' . $sectionpage, ['id' => $section->id]); } throw new Exception('Unrecognised core page type "' . $type . '."'); } From 72b154e33fa263cd041d068083e87dc23bc3caec Mon Sep 17 00:00:00 2001 From: ferranrecio Date: Fri, 19 Jul 2024 06:25:41 +0200 Subject: [PATCH 2/3] MDL-82260 course: add hidden course field to edit section form A course id hidden field is needed to edit availability conditions directly from behat. This fields is not relevant to normal execution but it is not harmfull as section update will ignore it. --- course/editsection_form.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/course/editsection_form.php b/course/editsection_form.php index 2e4f5babafd..4ea413b5f3e 100644 --- a/course/editsection_form.php +++ b/course/editsection_form.php @@ -47,6 +47,9 @@ class editsection_form extends moodleform { $mform->addElement('hidden', 'id'); $mform->setType('id', PARAM_INT); + $mform->addElement('hidden', 'course', 0); + $mform->setType('course', PARAM_INT); + // additional fields that course format has defined $courseformat = course_get_format($course); $formatoptions = $courseformat->section_format_options(true); From 9065e509da9eec4ef4e8133c1582ac0d95d079a4 Mon Sep 17 00:00:00 2001 From: ferranrecio Date: Thu, 18 Jul 2024 11:03:16 +0200 Subject: [PATCH 3/3] MDL-82260 core_courseformat: sync subsections access restrictions --- .../format/classes/local/sectionactions.php | 4 +- course/format/classes/sectiondelegate.php | 15 +++++ .../format/classes/sectiondelegatemodule.php | 16 +++++ course/modlib.php | 10 ++- mod/subsection/lib.php | 24 ++++++- .../subsection_access_restrictions.feature | 66 +++++++++++++++++++ 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 mod/subsection/tests/behat/subsection_access_restrictions.feature diff --git a/course/format/classes/local/sectionactions.php b/course/format/classes/local/sectionactions.php index f0e5657a9e1..e7ca6c5f032 100644 --- a/course/format/classes/local/sectionactions.php +++ b/course/format/classes/local/sectionactions.php @@ -54,7 +54,7 @@ class sectionactions extends baseactions { 'sequence' => '', 'name' => $fields->name ?? null, 'visible' => $fields->visible ?? 1, - 'availability' => null, + 'availability' => $fields->availability ?? null, 'component' => $fields->component ?? null, 'itemid' => $fields->itemid ?? null, 'timemodified' => time(), @@ -367,6 +367,8 @@ class sectionactions extends baseactions { $fields['timemodified'] = time(); $DB->update_record('course_sections', $fields); + $sectioninfo->get_component_instance()?->section_updated((object) $fields); + // We need to update the section cache before the format options are updated. \course_modinfo::purge_course_section_cache_by_id($courseid, $sectioninfo->id); rebuild_course_cache($courseid, false, true); diff --git a/course/format/classes/sectiondelegate.php b/course/format/classes/sectiondelegate.php index 14af95ff4db..d5ab54608a1 100644 --- a/course/format/classes/sectiondelegate.php +++ b/course/format/classes/sectiondelegate.php @@ -22,6 +22,7 @@ use section_info; use core_courseformat\stateupdates; use core_courseformat\output\local\content\section\controlmenu; use core_courseformat\base as course_format; +use stdClass; /** * Section delegate base class. @@ -133,4 +134,18 @@ abstract class sectiondelegate { public function get_parent_section(): ?section_info { return null; } + + /** + * Handler executed when a section has been updated. + * + * This method uses a record instead of a section_info object because + * section updates can be done in batch and the course_info may not be yet updated. + * + * This method does not need to recalculate the section_info object. + * + * @param stdClass $sectionrecord the new section data + */ + public function section_updated(stdClass $sectionrecord): void { + // By default, do nothing. + } } diff --git a/course/format/classes/sectiondelegatemodule.php b/course/format/classes/sectiondelegatemodule.php index c91ba739812..7a38c326bb8 100644 --- a/course/format/classes/sectiondelegatemodule.php +++ b/course/format/classes/sectiondelegatemodule.php @@ -168,4 +168,20 @@ abstract class sectiondelegatemodule extends sectiondelegate { $cm = get_coursemodule_from_instance($this->get_module_name(), $section->itemid); $updates->add_cm_put($cm->id); } + + public function section_updated(stdClass $sectionrecord): void { + global $DB; + + $cmrecord = []; + if (isset($sectionrecord->availability) && $sectionrecord->availability !== $this->cm->availability) { + $cmrecord['availability'] = $sectionrecord->availability; + } + + if (empty($cmrecord)) { + return; + } + + $cmrecord['id'] = $this->cm->id; + $DB->update_record('course_modules', (object)$cmrecord); + } } diff --git a/course/modlib.php b/course/modlib.php index e71132f1616..480d0873ade 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -620,14 +620,18 @@ function update_moduleinfo($cm, $moduleinfo, $course, $mform = null) { // This code is used both when submitting the form, which uses a long // name to avoid clashes, and by unit test code which uses the real // name in the table. + $newavailability = $cm->availability; if (property_exists($moduleinfo, 'availabilityconditionsjson')) { if ($moduleinfo->availabilityconditionsjson !== '') { - $cm->availability = $moduleinfo->availabilityconditionsjson; + $newavailability = $moduleinfo->availabilityconditionsjson; } else { - $cm->availability = null; + $newavailability = null; } } else if (property_exists($moduleinfo, 'availability')) { - $cm->availability = $moduleinfo->availability; + $newavailability = $moduleinfo->availability; + } + if ($cm->availability != $newavailability) { + $cm->availability = $newavailability; } // If there is any availability data, verify it. if ($cm->availability) { diff --git a/mod/subsection/lib.php b/mod/subsection/lib.php index b45271cea3b..8c1fa4d7dbf 100644 --- a/mod/subsection/lib.php +++ b/mod/subsection/lib.php @@ -65,12 +65,22 @@ function subsection_add_instance($moduleinstance, $mform = null) { $id = $DB->insert_record('subsection', $moduleinstance); + // Due to name collision, when the object came from the form, the availability conditions are called + // availabilityconditionsjson instead of availability. + $cmavailability = $moduleinstance->availabilityconditionsjson ?? $moduleinstance->availability ?? null; + // Availability could be an empty string but we need to force null. + if (empty($cmavailability)) { + $cmavailability = null; + } + formatactions::section($moduleinstance->course)->create_delegated( manager::PLUGINNAME, $id, (object)[ 'name' => $moduleinstance->name, - ]); + 'availability' => $cmavailability, + ] + ); return $id; } @@ -91,6 +101,18 @@ function subsection_update_instance($moduleinstance, $mform = null) { $moduleinstance->timemodified = time(); $moduleinstance->id = $moduleinstance->instance; + // Due to name collision, when the object came from the form, the availability conditions are called + // availabilityconditionsjson instead of availability. + $cmavailability = $moduleinstance->availabilityconditionsjson ?? $moduleinstance->availability ?? null; + if (!empty($cmavailability)) { + $DB->set_field( + 'course_sections', + 'availability', + $cmavailability, + ['component' => manager::PLUGINNAME, 'itemid' => $moduleinstance->id] + ); + } + return $DB->update_record('subsection', $moduleinstance); } diff --git a/mod/subsection/tests/behat/subsection_access_restrictions.feature b/mod/subsection/tests/behat/subsection_access_restrictions.feature new file mode 100644 index 00000000000..408f5c13672 --- /dev/null +++ b/mod/subsection/tests/behat/subsection_access_restrictions.feature @@ -0,0 +1,66 @@ +@mod @mod_subsection +Feature: Testing subsection_access_restrictions in mod_subsection + In order restrict the access to subsections based on conditions + As a teacher + I need to set subsection conditions which prevent student access + + Background: + Given I enable "subsection" "mod" plugin + And the following "users" exist: + | username | firstname | lastname | + | teacher1 | Teacher | 1 | + | student1 | Student | 1 | + | student2 | Student | 2 | + And the following "courses" exist: + | fullname | shortname | category | numsections | initsections | + | Course 1 | C1 | 0 | 2 | 1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "groups" exist: + | course | name | idnumber | + | C1 | G1 | GI1 | + And the following "group members" exist: + | user | group | + | student1 | GI1 | + And the following "groupings" exist: + | name | course | idnumber | + | GX1 | C1 | GXI1 | + And the following "grouping groups" exist: + | grouping | group | + | GXI1 | GI1 | + And I log in as "teacher1" + + Scenario: Teacher can set access restrictions to an existing subsection + Given the following "activities" exist: + | activity | name | course | idnumber | section | + | subsection | Subsection1 | C1 | Subsection1 | 1 | + | data | Subactivity | C1 | data1 | 3 | + When I am on the "C1 > Subsection1" "course > section settings" page + And I set the following fields to these values: + | Access restrictions | Grouping: GX1 | + And I press "Save changes" + Then I should see "Not available unless: You belong to a group in GX1" + And I log out + And I am on the "Course 1" "course" page logged in as "student1" + And I should see "Subsection1" in the "region-main" "region" + And I should see "Subactivity" in the "region-main" "region" + And I log out + And I am on the "Course 1" "course" page logged in as "student2" + And I should see "Not available unless: You belong to a group in GX1" + And I should not see "Subactivity" + + Scenario: Teacher sets access restrictions to a new subsection + When I add a subsection activity to course "Course 1" section "1" and I fill the form with: + | Name | Subsection2 | + | Access restrictions | Grouping: GX1 | + Then I should see "Not available unless: You belong to a group in GX1" + And I log out + And I am on the "Course 1" "course" page logged in as "student1" + And I should see "Subsection2" in the "region-main" "region" + And I log out + And I am on the "Course 1" "course" page logged in as "student2" + And I should see "Subsection2" in the "region-main" "region" + And I should see "Not available unless: You belong to a group in GX1"