From 9065e509da9eec4ef4e8133c1582ac0d95d079a4 Mon Sep 17 00:00:00 2001 From: ferranrecio Date: Thu, 18 Jul 2024 11:03:16 +0200 Subject: [PATCH] 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"