From 9d3bbe6c02faf2c6137f602efb15d9309523ffaa Mon Sep 17 00:00:00 2001
From: Ferran Recio <ferran@moodle.com>
Date: Fri, 2 Feb 2024 10:26:47 +0100
Subject: [PATCH] MDL-80715 core_courseformat: new section update action

---
 .../format/classes/local/sectionactions.php   |  92 +++++
 .../tests/local/sectionactions_test.php       | 321 ++++++++++++++++++
 course/lib.php                                |  61 +---
 course/tests/courselib_test.php               |  22 +-
 4 files changed, 434 insertions(+), 62 deletions(-)

diff --git a/course/format/classes/local/sectionactions.php b/course/format/classes/local/sectionactions.php
index 23ad84ef003..3a73bc03b15 100644
--- a/course/format/classes/local/sectionactions.php
+++ b/course/format/classes/local/sectionactions.php
@@ -18,6 +18,7 @@ namespace core_courseformat\local;
 
 use section_info;
 use stdClass;
+use core\event\course_module_updated;
 use core\event\course_section_deleted;
 
 /**
@@ -336,4 +337,95 @@ class sectionactions extends baseactions {
         $sectioninfo = $this->get_section_info($sectioninfo->id);
         return $this->delete_format_data($sectioninfo, $forcedeleteifnotempty, $event);
     }
+
+    /**
+     * Update a course section.
+     *
+     * @param section_info $sectioninfo the section info or database record to update.
+     * @param array|stdClass $fields the fields to update.
+     * @return bool whether section was updated
+     */
+    public function update(section_info $sectioninfo, array|stdClass $fields): bool {
+        global $DB;
+
+        $courseid = $this->course->id;
+
+        // Some fields can not be updated using this method.
+        $fields = array_diff_key((array) $fields, array_flip(['id', 'course', 'section', 'sequence']));
+        if (array_key_exists('name', $fields) && \core_text::strlen($fields['name']) > 255) {
+            throw new \moodle_exception('maximumchars', 'moodle', '', 255);
+        }
+
+        if (empty($fields)) {
+            return false;
+        }
+
+        $fields['id'] = $sectioninfo->id;
+        $fields['timemodified'] = time();
+        $DB->update_record('course_sections', $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);
+
+        course_get_format($courseid)->update_section_format_options($fields);
+
+        $event = \core\event\course_section_updated::create(
+            [
+                'objectid' => $sectioninfo->id,
+                'courseid' => $courseid,
+                'context' => \context_course::instance($courseid),
+                'other' => ['sectionnum' => $sectioninfo->section],
+            ]
+        );
+        $event->trigger();
+
+        if (isset($fields['visible'])) {
+            $this->transfer_visibility_to_cms($sectioninfo, (bool) $fields['visible']);
+        }
+        return true;
+    }
+
+    /**
+     * Transfer the visibility of the section to the course modules.
+     *
+     * @param section_info $sectioninfo the section info or database record to update.
+     * @param bool $visibility the new visibility of the section.
+     */
+    protected function transfer_visibility_to_cms(section_info $sectioninfo, bool $visibility): void {
+        global $DB;
+
+        if (empty($sectioninfo->sequence) || $visibility == (bool) $sectioninfo->visible) {
+            return;
+        }
+
+        $modules = explode(',', $sectioninfo->sequence);
+        $cmids = [];
+        foreach ($modules as $moduleid) {
+            $cm = get_coursemodule_from_id(null, $moduleid, $this->course->id);
+            if (!$cm) {
+                continue;
+            }
+
+            $modupdated = false;
+            if ($visibility) {
+                // As we unhide the section, we use the previously saved visibility stored in visibleold.
+                $modupdated = set_coursemodule_visible($moduleid, $cm->visibleold, $cm->visibleoncoursepage, false);
+            } else {
+                // We hide the section, so we hide the module but we store the original state in visibleold.
+                $modupdated = set_coursemodule_visible($moduleid, 0, $cm->visibleoncoursepage, false);
+                if ($modupdated) {
+                    $DB->set_field('course_modules', 'visibleold', $cm->visible, ['id' => $moduleid]);
+                }
+            }
+
+            if ($modupdated) {
+                $cmids[] = $cm->id;
+                course_module_updated::create_from_cm($cm)->trigger();
+            }
+        }
+
+        \course_modinfo::purge_course_modules_cache($this->course->id, $cmids);
+        rebuild_course_cache($this->course->id, false, true);
+    }
 }
diff --git a/course/format/tests/local/sectionactions_test.php b/course/format/tests/local/sectionactions_test.php
index adac5fc0762..df61302767b 100644
--- a/course/format/tests/local/sectionactions_test.php
+++ b/course/format/tests/local/sectionactions_test.php
@@ -541,4 +541,325 @@ class sectionactions_test extends \advanced_testcase {
         }
         $this->assertEquals(3, $count);
     }
+
+    /**
+     * Test section update method.
+     *
+     * @covers ::update
+     * @dataProvider update_provider
+     * @param string $fieldname the name of the field to update
+     * @param int|string $value the value to set
+     * @param int|string $expected the expected value after the update ('=' to specify the same value as original field)
+     * @param bool $expectexception if the method should throw an exception
+     */
+    public function test_update(
+        string $fieldname,
+        int|string $value,
+        int|string $expected,
+        bool $expectexception
+    ): void {
+        global $DB;
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course(
+            ['format' => 'topics', 'numsections' => 1],
+            ['createsections' => true]
+        );
+        $section = get_fast_modinfo($course)->get_section_info(1);
+
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertNotEquals($value, $sectionrecord->$fieldname);
+        $this->assertNotEquals($value, $section->$fieldname);
+
+        if ($expectexception) {
+            $this->expectException(\moodle_exception::class);
+        }
+
+        if ($expected === '=') {
+            $expected = $section->$fieldname;
+        }
+
+        $sectionactions = new sectionactions($course);
+        $sectionactions->update($section, [$fieldname => $value]);
+
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertEquals($expected, $sectionrecord->$fieldname);
+
+        $section = get_fast_modinfo($course)->get_section_info(1);
+        $this->assertEquals($expected, $section->$fieldname);
+    }
+
+    /**
+     * Data provider for test_update.
+     * @return array
+     */
+    public static function update_provider(): array {
+        return [
+            'Id will not be updated' => [
+                'fieldname' => 'id',
+                'value' => -1,
+                'expected' => '=',
+                'expectexception' => false,
+            ],
+            'Course will not be updated' => [
+                'fieldname' => 'course',
+                'value' => -1,
+                'expected' => '=',
+                'expectexception' => false,
+            ],
+            'Section number will not be updated' => [
+                'fieldname' => 'section',
+                'value' => -1,
+                'expected' => '=',
+                'expectexception' => false,
+            ],
+            'Sequence will be updated' => [
+                'fieldname' => 'name',
+                'value' => 'new name',
+                'expected' => 'new name',
+                'expectexception' => false,
+            ],
+            'Summary can be updated' => [
+                'fieldname' => 'summary',
+                'value' => 'new summary',
+                'expected' => 'new summary',
+                'expectexception' => false,
+            ],
+            'Visible can be updated' => [
+                'fieldname' => 'visible',
+                'value' => 0,
+                'expected' => 0,
+                'expectexception' => false,
+            ],
+            'component can be updated' => [
+                'fieldname' => 'component',
+                'value' => 'mod_assign',
+                'expected' => 'mod_assign',
+                'expectexception' => false,
+            ],
+            'itemid can be updated' => [
+                'fieldname' => 'itemid',
+                'value' => 1,
+                'expected' => 1,
+                'expectexception' => false,
+            ],
+            'Long names throws and exception' => [
+                'fieldname' => 'name',
+                'value' => str_repeat('a', 256),
+                'expected' => '=',
+                'expectexception' => true,
+            ],
+        ];
+    }
+
+    /**
+     * Test section update method updating several values at once.
+     *
+     * @covers ::update
+     */
+    public function test_update_multiple_fields(): void {
+        global $DB;
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course(
+            ['format' => 'topics', 'numsections' => 1],
+            ['createsections' => true]
+        );
+        $section = get_fast_modinfo($course)->get_section_info(1);
+
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertEquals(1, $sectionrecord->visible);
+        $this->assertNull($section->name);
+
+        $sectionactions = new sectionactions($course);
+        $sectionactions->update($section, ['name' => 'New name', 'visible' => 0]);
+
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertEquals('New name', $sectionrecord->name);
+        $this->assertEquals(0, $sectionrecord->visible);
+
+        $section = get_fast_modinfo($course)->get_section_info(1);
+        $this->assertEquals('New name', $section->name);
+        $this->assertEquals(0, $section->visible);
+    }
+
+    /**
+     * Test updating a section trigger a course section update log event.
+     *
+     * @covers ::update
+     */
+    public function test_course_section_updated_event(): void {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course(
+            ['format' => 'topics', 'numsections' => 1],
+            ['createsections' => true]
+        );
+        $section = get_fast_modinfo($course)->get_section_info(1);
+
+        $sink = $this->redirectEvents();
+
+        $sectionactions = new sectionactions($course);
+        $sectionactions->update($section, ['name' => 'New name', 'visible' => 0]);
+
+        $events = $sink->get_events();
+        $event = reset($events);
+
+        // Check that the event data is valid.
+        $this->assertInstanceOf('\core\event\course_section_updated', $event);
+        $data = $event->get_data();
+        $this->assertEquals(\context_course::instance($course->id), $event->get_context());
+        $this->assertEquals($section->id, $data['objectid']);
+    }
+
+    /**
+     * Test section update change the modified date.
+     *
+     * @covers ::update
+     */
+    public function test_update_time_modified(): void {
+        global $DB;
+        $this->resetAfterTest();
+
+        // Create the course with sections.
+        $course = $this->getDataGenerator()->create_course(
+            ['format' => 'topics', 'numsections' => 1],
+            ['createsections' => true]
+        );
+        $section = get_fast_modinfo($course)->get_section_info(1);
+
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $oldtimemodified = $sectionrecord->timemodified;
+
+        $sectionactions = new sectionactions($course);
+
+        // Ensuring that the section update occurs at a different timestamp.
+        $this->waitForSecond();
+
+        // The timemodified should only be updated if the section is actually updated.
+        $result = $sectionactions->update($section, []);
+        $this->assertFalse($result);
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertEquals($oldtimemodified, $sectionrecord->timemodified);
+
+        // Now update something to prove timemodified changes.
+        $result = $sectionactions->update($section, ['name' => 'New name']);
+        $this->assertTrue($result);
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertGreaterThan($oldtimemodified, $sectionrecord->timemodified);
+    }
+
+    /**
+     * Test section updating visibility will hide or show section activities.
+     *
+     * @covers ::update
+     */
+    public function test_update_hide_section_activities(): void {
+        global $DB;
+        $this->resetAfterTest();
+
+        // Create 4 activities (visible, visible, hidden, hidden).
+        $course = $this->getDataGenerator()->create_course(
+            ['format' => 'topics', 'numsections' => 1],
+            ['createsections' => true]
+        );
+        $activity1 = $this->getDataGenerator()->create_module(
+            'assign',
+            ['course' => $course->id, 'section' => 1]
+        );
+        $activity2 = $this->getDataGenerator()->create_module(
+            'assign',
+            ['course' => $course->id, 'section' => 1]
+        );
+        $activity3 = $this->getDataGenerator()->create_module(
+            'assign',
+            ['course' => $course->id, 'section' => 1, 'visible' => 0]
+        );
+        $activity4 = $this->getDataGenerator()->create_module(
+            'assign',
+            ['course' => $course->id, 'section' => 1, 'visible' => 0]
+        );
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(1, $cm1->visible);
+        $this->assertEquals(1, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(0, $cm4->visible);
+
+        $sectionactions = new sectionactions($course);
+
+        // Validate hidding section hides all activities.
+        $section = $modinfo->get_section_info(1);
+        $sectionactions->update($section, ['visible' => 0]);
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(0, $cm1->visible);
+        $this->assertEquals(0, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(0, $cm4->visible);
+
+        // Validate showing the section restores the previous visibility.
+        $section = $modinfo->get_section_info(1);
+        $sectionactions->update($section, ['visible' => 1]);
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(1, $cm1->visible);
+        $this->assertEquals(1, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(0, $cm4->visible);
+
+        // Swap two activities visibility to alter visible values.
+        set_coursemodule_visible($cm2->id, 0, 0, true);
+        set_coursemodule_visible($cm4->id, 1, 1, true);
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(1, $cm1->visible);
+        $this->assertEquals(0, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(1, $cm4->visible);
+
+        // Validate hidding the section again.
+        $section = $modinfo->get_section_info(1);
+        $sectionactions->update($section, ['visible' => 0]);
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(0, $cm1->visible);
+        $this->assertEquals(0, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(0, $cm4->visible);
+
+        // Validate showing the section once more to check previous state is restored.
+        $section = $modinfo->get_section_info(1);
+        $sectionactions->update($section, ['visible' => 1]);
+
+        $modinfo = get_fast_modinfo($course);
+        $cm1 = $modinfo->get_cm($activity1->cmid);
+        $cm2 = $modinfo->get_cm($activity2->cmid);
+        $cm3 = $modinfo->get_cm($activity3->cmid);
+        $cm4 = $modinfo->get_cm($activity4->cmid);
+        $this->assertEquals(1, $cm1->visible);
+        $this->assertEquals(0, $cm2->visible);
+        $this->assertEquals(0, $cm3->visible);
+        $this->assertEquals(1, $cm4->visible);
+    }
 }
diff --git a/course/lib.php b/course/lib.php
index a809df14c61..b5f5d56b6cd 100644
--- a/course/lib.php
+++ b/course/lib.php
@@ -1277,70 +1277,21 @@ function course_delete_section_async($section, $forcedeleteifnotempty = true) {
  *
  * This function does not check permissions or clean values - this has to be done prior to calling it.
  *
- * @param int|stdClass $course
+ * @param int|stdClass $courseorid
  * @param stdClass $section record from course_sections table - it will be updated with the new values
  * @param array|stdClass $data
  */
-function course_update_section($course, $section, $data) {
-    global $DB;
+function course_update_section($courseorid, $section, $data) {
+    $sectioninfo = get_fast_modinfo($courseorid)->get_section_info_by_id($section->id);
+    formatactions::section($courseorid)->update($sectioninfo, $data);
 
-    $courseid = (is_object($course)) ? $course->id : (int)$course;
-
-    // Some fields can not be updated using this method.
-    $data = array_diff_key((array)$data, array('id', 'course', 'section', 'sequence'));
-    $changevisibility = (array_key_exists('visible', $data) && (bool)$data['visible'] != (bool)$section->visible);
-    if (array_key_exists('name', $data) && \core_text::strlen($data['name']) > 255) {
-        throw new moodle_exception('maximumchars', 'moodle', '', 255);
-    }
-
-    // Update record in the DB and course format options.
-    $data['id'] = $section->id;
-    $data['timemodified'] = time();
-    $DB->update_record('course_sections', $data);
-    // Invalidate the section cache by given section id.
-    course_modinfo::purge_course_section_cache_by_id($courseid, $section->id);
-    rebuild_course_cache($courseid, false, true);
-    course_get_format($courseid)->update_section_format_options($data);
-
-    // Update fields of the $section object.
+    // Update $section object fields (for legacy compatibility).
+    $data = array_diff_key((array) $data, array_flip(['id', 'course', 'section', 'sequence']));
     foreach ($data as $key => $value) {
         if (property_exists($section, $key)) {
             $section->$key = $value;
         }
     }
-
-    // Trigger an event for course section update.
-    $event = \core\event\course_section_updated::create(
-        array(
-            'objectid' => $section->id,
-            'courseid' => $courseid,
-            'context' => context_course::instance($courseid),
-            'other' => array('sectionnum' => $section->section)
-        )
-    );
-    $event->trigger();
-
-    // If section visibility was changed, hide the modules in this section too.
-    if ($changevisibility && !empty($section->sequence)) {
-        $modules = explode(',', $section->sequence);
-        $cmids = [];
-        foreach ($modules as $moduleid) {
-            if ($cm = get_coursemodule_from_id(null, $moduleid, $courseid)) {
-                $cmids[] = $cm->id;
-                if ($data['visible']) {
-                    // As we unhide the section, we use the previously saved visibility stored in visibleold.
-                    set_coursemodule_visible($moduleid, $cm->visibleold, $cm->visibleoncoursepage, false);
-                } else {
-                    // We hide the section, so we hide the module but we store the original state in visibleold.
-                    set_coursemodule_visible($moduleid, 0, $cm->visibleoncoursepage, false);
-                    $DB->set_field('course_modules', 'visibleold', $cm->visible, ['id' => $moduleid]);
-                }
-                \core\event\course_module_updated::create_from_cm($cm)->trigger();
-            }
-        }
-        \course_modinfo::purge_course_modules_cache($courseid, $cmids);
-        rebuild_course_cache($courseid, false, true);
-    }
 }
 
 /**
diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php
index 4dcc420907a..0fa63e9af94 100644
--- a/course/tests/courselib_test.php
+++ b/course/tests/courselib_test.php
@@ -782,19 +782,27 @@ class courselib_test extends advanced_testcase {
         $this->resetAfterTest();
 
         // Create the course with sections.
-        $course = $this->getDataGenerator()->create_course(array('numsections' => 10), array('createsections' => true));
-        $sections = $DB->get_records('course_sections', array('course' => $course->id));
+        $course = $this->getDataGenerator()->create_course(
+            ['numsections' => 10],
+            ['createsections' => true]
+        );
+        $sections = $DB->get_records('course_sections', ['course' => $course->id]);
 
         // Get the last section's time modified value.
         $section = array_pop($sections);
         $oldtimemodified = $section->timemodified;
 
-        // Update the section.
-        $this->waitForSecond(); // Ensuring that the section update occurs at a different timestamp.
-        course_update_section($course, $section, array());
+        // Ensuring that the section update occurs at a different timestamp.
+        $this->waitForSecond();
 
-        // Check that the time has changed.
-        $section = $DB->get_record('course_sections', array('id' => $section->id));
+        // The timemodified should only be updated if the section is actually updated.
+        course_update_section($course, $section, []);
+        $sectionrecord = $DB->get_record('course_sections', ['id' => $section->id]);
+        $this->assertEquals($oldtimemodified, $sectionrecord->timemodified);
+
+        // Now update something to prove timemodified changes.
+        course_update_section($course, $section, ['name' => 'New name']);
+        $section = $DB->get_record('course_sections', ['id' => $section->id]);
         $newtimemodified = $section->timemodified;
         $this->assertGreaterThan($oldtimemodified, $newtimemodified);
     }