From 89b909f6de0a45d2043b0dee0f482d93f196b5ed Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 2 Feb 2017 12:33:16 +0800 Subject: [PATCH 1/4] MDL-57769 course: prepare to remove numsections option --- admin/tool/mobile/classes/api.php | 2 +- admin/tool/mobile/tests/externallib_test.php | 2 +- admin/tool/recyclebin/tests/events_test.php | 2 +- blocks/section_links/block_section_links.php | 34 ++--- .../behat/block_section_links_course.feature | 9 +- course/changenumsections.php | 28 +++- course/externallib.php | 4 +- course/format/lib.php | 22 +++ course/format/renderer.php | 68 +++++++-- course/format/upgrade.txt | 5 + course/format/weeks/lang/en/format_weeks.php | 1 + course/lib.php | 80 ++++++++--- .../behat/activities_visibility_icons.feature | 33 ----- course/tests/courselib_test.php | 34 ++--- course/tests/externallib_test.php | 20 ++- enrol/database/lib.php | 3 + enrol/database/tests/sync_test.php | 3 +- lang/en/moodle.php | 2 + lib/classes/event/course_section_created.php | 131 ++++++++++++++++++ lib/testing/generator/data_generator.php | 13 +- lib/testing/tests/generator_test.php | 2 +- 21 files changed, 356 insertions(+), 142 deletions(-) create mode 100644 lib/classes/event/course_section_created.php diff --git a/admin/tool/mobile/classes/api.php b/admin/tool/mobile/classes/api.php index b78de8f3147..c449ae69bf6 100644 --- a/admin/tool/mobile/classes/api.php +++ b/admin/tool/mobile/classes/api.php @@ -185,7 +185,7 @@ class api { $settings->frontpageloggedin = $CFG->frontpageloggedin; $settings->maxcategorydepth = $CFG->maxcategorydepth; $settings->frontpagecourselimit = $CFG->frontpagecourselimit; - $settings->numsections = course_get_format($SITE)->get_course()->numsections; + $settings->numsections = course_get_format($SITE)->get_last_section_number(); $settings->newsitems = $SITE->newsitems; $settings->commentsperpage = $CFG->commentsperpage; diff --git a/admin/tool/mobile/tests/externallib_test.php b/admin/tool/mobile/tests/externallib_test.php index 6cf0f49a141..be10099c031 100644 --- a/admin/tool/mobile/tests/externallib_test.php +++ b/admin/tool/mobile/tests/externallib_test.php @@ -140,7 +140,7 @@ class tool_mobile_external_testcase extends externallib_advanced_testcase { array('name' => 'frontpageloggedin', 'value' => $CFG->frontpageloggedin), array('name' => 'maxcategorydepth', 'value' => $CFG->maxcategorydepth), array('name' => 'frontpagecourselimit', 'value' => $CFG->frontpagecourselimit), - array('name' => 'numsections', 'value' => course_get_format($SITE)->get_course()->numsections), + array('name' => 'numsections', 'value' => course_get_format($SITE)->get_last_section_number()), array('name' => 'newsitems', 'value' => $SITE->newsitems), array('name' => 'commentsperpage', 'value' => $CFG->commentsperpage), array('name' => 'disableuserimages', 'value' => $CFG->disableuserimages), diff --git a/admin/tool/recyclebin/tests/events_test.php b/admin/tool/recyclebin/tests/events_test.php index 05d519b8544..80ab6aa4352 100644 --- a/admin/tool/recyclebin/tests/events_test.php +++ b/admin/tool/recyclebin/tests/events_test.php @@ -121,7 +121,7 @@ class tool_recyclebin_events_testcase extends advanced_testcase { $sink = $this->redirectEvents(); $rb->restore_item($item); $events = $sink->get_events(); - $event = $events[6]; + $event = $events[count($events) - 2]; // Check that the event contains the expected values. $this->assertInstanceOf('\tooL_recyclebin\event\category_bin_item_restored', $event); diff --git a/blocks/section_links/block_section_links.php b/blocks/section_links/block_section_links.php index cc7c88c46a4..7f126833069 100644 --- a/blocks/section_links/block_section_links.php +++ b/blocks/section_links/block_section_links.php @@ -81,35 +81,26 @@ class block_section_links extends block_base { $course = $this->page->course; $courseformat = course_get_format($course); - $courseformatoptions = $courseformat->get_format_options(); + $numsections = $courseformat->get_last_section_number(); $context = context_course::instance($course->id); // Course format options 'numsections' is required to display the block. - if (empty($courseformatoptions['numsections'])) { + if (empty($numsections)) { return $this->content; } - // Prepare the highlight value. - if ($course->format == 'weeks') { - $highlight = ceil((time() - $course->startdate) / 604800); - } else if ($course->format == 'topics') { - $highlight = $course->marker; - } else { - $highlight = 0; - } - // Prepare the increment value. - if (!empty($config->numsections1) and ($courseformatoptions['numsections'] > $config->numsections1)) { + if (!empty($config->numsections1) and ($numsections > $config->numsections1)) { $inc = $config->incby1; - } else if ($courseformatoptions['numsections'] > 22) { + } else if ($numsections > 22) { $inc = 2; } else { $inc = 1; } - if (!empty($config->numsections2) and ($courseformatoptions['numsections'] > $config->numsections2)) { + if (!empty($config->numsections2) and ($numsections > $config->numsections2)) { $inc = $config->incby2; } else { - if ($courseformatoptions['numsections'] > 40) { + if ($numsections > 40) { $inc = 5; } } @@ -119,8 +110,9 @@ class block_section_links extends block_base { $canviewhidden = has_capability('moodle/course:update', $context); $coursesections = $courseformat->get_sections(); $coursesectionscount = count($coursesections); + $sectiontojumpto = false; for ($i = $inc; $i <= $coursesectionscount; $i += $inc) { - if ($i > $courseformatoptions['numsections'] || !isset($coursesections[$i])) { + if ($i > $numsections || !isset($coursesections[$i])) { continue; } $section = $coursesections[$i]; @@ -128,16 +120,16 @@ class block_section_links extends block_base { $sections[$i] = (object)array( 'section' => $section->section, 'visible' => $section->visible, - 'highlight' => ($section->section == $highlight) + 'highlight' => false ); + if ($courseformat->is_section_current($section)) { + $sections[$i]->highlight = true; + $sectiontojumpto = $section->section; + } } } if (!empty($sections)) { - $sectiontojumpto = false; - if ($highlight && isset($sections[$highlight]) && ($sections[$highlight]->visible || $canviewhidden)) { - $sectiontojumpto = $highlight; - } // Render the sections. $renderer = $this->page->get_renderer('block_section_links'); $this->content->text = $renderer->render_section_links($this->page->course, $sections, $sectiontojumpto); diff --git a/blocks/section_links/tests/behat/block_section_links_course.feature b/blocks/section_links/tests/behat/block_section_links_course.feature index f43f6fd9287..d37e5608051 100644 --- a/blocks/section_links/tests/behat/block_section_links_course.feature +++ b/blocks/section_links/tests/behat/block_section_links_course.feature @@ -6,8 +6,8 @@ Feature: The section links block allows users to quickly navigate around a moodl Background: Given the following "courses" exist: - | fullname | shortname | category | - | Course 1 | C1 | 0 | + | fullname | shortname | category | numsections | coursedisplay | + | Course 1 | C1 | 0 | 20 | 1 | And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@example.com | @@ -21,11 +21,6 @@ Feature: The section links block allows users to quickly navigate around a moodl | Assignment name | Test assignment 1 | | Description | Offline text | | assignsubmission_file_enabled | 0 | - And I navigate to "Edit settings" node in "Course administration" - And I set the following fields to these values: - | id_numsections | 20 | - | id_coursedisplay | Show one section per page | - And I press "Save and display" Scenario: Add the section links block to a course. Given I add the "Section links" block diff --git a/course/changenumsections.php b/course/changenumsections.php index 584691f1b40..a7b9495b76b 100644 --- a/course/changenumsections.php +++ b/course/changenumsections.php @@ -29,7 +29,11 @@ require_once(__DIR__.'/../config.php'); require_once($CFG->dirroot.'/course/lib.php'); $courseid = required_param('courseid', PARAM_INT); -$increase = optional_param('increase', true, PARAM_BOOL); +$increase = optional_param('increase', null, PARAM_BOOL); +$insertsection = optional_param('insertsection', null, PARAM_INT); // Insert section at position; 0 means at the end. +$returnurl = optional_param('returnurl', null, PARAM_LOCALURL); // Where to return to after the action. +$sectionreturn = optional_param('sectionreturn', null, PARAM_INT); // Section to return to, ignored if $returnurl is specified. + $course = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST); $courseformatoptions = course_get_format($course)->get_format_options(); @@ -40,10 +44,11 @@ require_login($course); require_capability('moodle/course:update', context_course::instance($course->id)); require_sesskey(); -if (isset($courseformatoptions['numsections'])) { +if (isset($courseformatoptions['numsections']) && $increase !== null) { if ($increase) { // Add an additional section. $courseformatoptions['numsections']++; + course_create_sections_if_missing($course, $courseformatoptions['numsections']); } else { // Remove a section. $courseformatoptions['numsections']--; @@ -55,9 +60,22 @@ if (isset($courseformatoptions['numsections'])) { update_course((object)array('id' => $course->id, 'numsections' => $courseformatoptions['numsections'])); } + if (!$returnurl) { + $returnurl = course_get_url($course); + $returnurl->set_anchor('changenumsections'); + } + +} else if (course_get_format($course)->uses_sections() && $insertsection !== null) { + if ($insertsection) { + // Inserting sections at any position except in the very end requires capability to move sections. + require_capability('moodle/course:movesections', context_course::instance($course->id)); + } + $section = course_create_section($course, $insertsection); + if (!$returnurl) { + $returnurl = course_get_url($course, $section->section, + ($sectionreturn !== null) ? ['sr' => $sectionreturn] : []); + } } -$url = course_get_url($course); -$url->set_anchor('changenumsections'); // Redirect to where we were.. -redirect($url); +redirect($returnurl); diff --git a/course/externallib.php b/course/externallib.php index 73370acc2db..b2ccd3ffa2e 100644 --- a/course/externallib.php +++ b/course/externallib.php @@ -160,7 +160,7 @@ class core_course_external extends external_api { //retrieve sections $modinfo = get_fast_modinfo($course); $sections = $modinfo->get_section_info_all(); - $coursenumsections = course_get_format($course)->get_course()->numsections; + $coursenumsections = course_get_format($course)->get_last_section_number(); //for each sections (first displayed to last displayed) $modinfosections = $modinfo->get_sections(); @@ -468,6 +468,8 @@ class core_course_external extends external_api { // For backward-compartibility $courseinfo['hiddensections'] = $courseformatoptions['hiddensections']; } + // Return numsections for backward-compatibility with clients who expect it. + $courseinfo['numsections'] = course_get_format($course)->get_last_section_number(); $courseinfo['groupmode'] = $course->groupmode; $courseinfo['groupmodeforce'] = $course->groupmodeforce; $courseinfo['defaultgroupingid'] = $course->defaultgroupingid; diff --git a/course/format/lib.php b/course/format/lib.php index 0c7a4f265af..17f28514442 100644 --- a/course/format/lib.php +++ b/course/format/lib.php @@ -265,6 +265,28 @@ abstract class format_base { return $this->course; } + /** + * Method used in the rendered and during backup instead of legacy 'numsections' + * + * Default renderer will treat sections with sectionnumber greater that the value returned by this + * method as "orphaned" and not display them on the course page unless in editing mode. + * Backup will store this value as 'numsections'. + * + * This method ensures that 3rd party course format plugins that still use 'numsections' continue to + * work but at the same time we no longer expect formats to have 'numsections' property. + * + * @return int + */ + public function get_last_section_number() { + $course = $this->get_course(); + if (isset($course->numsections)) { + return $course->numsections; + } + $modinfo = get_fast_modinfo($course); + $sections = $modinfo->get_section_info_all(); + return (int)max(array_keys($sections)); + } + /** * Returns true if the course has a front page. * diff --git a/course/format/renderer.php b/course/format/renderer.php index da25b20b067..eeb126f1016 100644 --- a/course/format/renderer.php +++ b/course/format/renderer.php @@ -303,7 +303,8 @@ abstract class format_section_renderer_base extends plugin_renderer_base { $sectionreturn = $onsectionpage ? $section->section : null; $coursecontext = context_course::instance($course->id); - $isstealth = isset($course->numsections) && ($section->section > $course->numsections); + $numsections = course_get_format($course)->get_last_section_number(); + $isstealth = $section->section > $numsections; $baseurl = course_get_url($course, $sectionreturn); $baseurl->param('sesskey', sesskey()); @@ -369,7 +370,7 @@ abstract class format_section_renderer_base extends plugin_renderer_base { } $url = clone($baseurl); - if ($section->section < $course->numsections) { // Add a arrow to move section down. + if ($section->section < $numsections) { // Add a arrow to move section down. $url->param('section', $section->section); $url->param('move', 1); $strmovedown = get_string('movedown'); @@ -643,7 +644,8 @@ abstract class format_section_renderer_base extends plugin_renderer_base { } $forward = $sectionno + 1; - while ($forward <= $course->numsections and empty($links['next'])) { + $numsections = course_get_format($course)->get_last_section_number(); + while ($forward <= $numsections and empty($links['next'])) { if ($canviewhidden || $sections[$forward]->uservisible) { $params = array(); if (!$sections[$forward]->visible) { @@ -731,7 +733,8 @@ abstract class format_section_renderer_base extends plugin_renderer_base { $sectionmenu[course_get_url($course)->out(false)] = get_string('maincoursepage'); $modinfo = get_fast_modinfo($course); $section = 1; - while ($section <= $course->numsections) { + $numsections = course_get_format($course)->get_last_section_number(); + while ($section <= $numsections) { $thissection = $modinfo->get_section_info($section); $showsection = $thissection->uservisible or !$course->hiddensections; if (($showsection) && ($section != $displaysection) && ($url = course_get_url($course, $section))) { @@ -869,6 +872,7 @@ abstract class format_section_renderer_base extends plugin_renderer_base { // Now the list of sections.. echo $this->start_section_list(); + $numsections = course_get_format($course)->get_last_section_number(); foreach ($modinfo->get_section_info_all() as $section => $thissection) { if ($section == 0) { @@ -881,7 +885,7 @@ abstract class format_section_renderer_base extends plugin_renderer_base { } continue; } - if ($section > $course->numsections) { + if ($section > $numsections) { // activities inside this section are 'orphaned', this section will be printed as 'stealth' below continue; } @@ -917,7 +921,7 @@ abstract class format_section_renderer_base extends plugin_renderer_base { if ($PAGE->user_is_editing() and has_capability('moodle/course:update', $context)) { // Print stealth sections if present. foreach ($modinfo->get_section_info_all() as $section => $thissection) { - if ($section <= $course->numsections or empty($modinfo->sections[$section])) { + if ($section <= $numsections or empty($modinfo->sections[$section])) { // this is not stealth section or it is empty continue; } @@ -928,6 +932,34 @@ abstract class format_section_renderer_base extends plugin_renderer_base { echo $this->end_section_list(); + echo $this->change_number_sections($course, 0); + } else { + echo $this->end_section_list(); + } + + } + + /** + * Returns controls in the bottom of the page to increase/decrease number of sections + * + * @param stdClass $course + * @param int|null $sectionreturn + * @return string + */ + protected function change_number_sections($course, $sectionreturn = null) { + $coursecontext = context_course::instance($course->id); + if (!has_capability('moodle/course:update', $coursecontext)) { + return ''; + } + + $options = course_get_format($course)->get_format_options(); + $supportsnumsections = array_key_exists('numsections', $options); + + if ($supportsnumsections) { + // Current course format has 'numsections' option, which is very confusing and we suggest course format + // developers to get rid of it (see MDL-57769 on how to do it). + // Display "Increase section" / "Decrease section" links. + echo html_writer::start_tag('div', array('id' => 'changenumsections', 'class' => 'mdl-right')); // Increase number of sections. @@ -951,10 +983,28 @@ abstract class format_section_renderer_base extends plugin_renderer_base { } echo html_writer::end_tag('div'); - } else { - echo $this->end_section_list(); - } + } else if (course_get_format($course)->uses_sections()) { + // Current course format does not have 'numsections' option but it has multiple sections suppport. + // Display the "Add section" link that will insert a section in the end. + // Note to course format developers: inserting sections in the other positions should check both + // capabilities 'moodle/course:update' and 'moodle/course:movesections'. + + echo html_writer::start_tag('div', array('id' => 'changenumsections', 'class' => 'mdl-right')); + if (get_string_manager()->string_exists('addsection', 'format_'.$course->format)) { + $straddsection = get_string('addsection', 'format_'.$course->format); + } else { + $straddsection = get_string('addsection'); + } + $url = new moodle_url('/course/changenumsections.php', + ['courseid' => $course->id, 'insertsection' => 0, 'sesskey' => sesskey()]); + if ($sectionreturn !== null) { + $url->param('sectionreturn', $sectionreturn); + } + $icon = $this->output->pix_icon('t/add', $straddsection); + echo html_writer::link($url, $icon . $straddsection, array('class' => 'add-section')); + echo html_writer::end_tag('div'); + } } /** diff --git a/course/format/upgrade.txt b/course/format/upgrade.txt index f5cf7caf072..5beb8964827 100644 --- a/course/format/upgrade.txt +++ b/course/format/upgrade.txt @@ -10,6 +10,11 @@ Overview of this plugin type at http://docs.moodle.org/dev/Course_formats must check $cm->is_visible_on_course_page() when displaying activities list on the course page instead of $cm->uservisible. For all other plugins except course formats the same property $cm->uservisible indicates if the activity contents is actually available to student. +* Option "Number of sections" (numsections) was removed from topics and weeks formats, instead the actual number of records + in the course_sections table is treated as a number of sections (excluding section 0 that should always be present). +* Method create_course() will populate the new course with empty sections if $data->numsections is provided even if + "numsections" is not an option defined by the course format. +* course/changenumsections.php can now be used to insert sections at any positions === 3.2 === * Callback delete_course is deprecated and should be replaced with observer for event \core\event\course_content_deleted diff --git a/course/format/weeks/lang/en/format_weeks.php b/course/format/weeks/lang/en/format_weeks.php index 61626e0454d..5fc3a4be3cd 100644 --- a/course/format/weeks/lang/en/format_weeks.php +++ b/course/format/weeks/lang/en/format_weeks.php @@ -23,6 +23,7 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +$string['addsection'] = 'Add week'; $string['currentsection'] = 'This week'; $string['editsection'] = 'Edit week'; $string['editsectionname'] = 'Edit week name'; diff --git a/course/lib.php b/course/lib.php index 980173f8307..20a1718d47c 100644 --- a/course/lib.php +++ b/course/lib.php @@ -832,6 +832,52 @@ function add_course_module($mod) { return $cmid; } +/** + * Creates a course section and adds it to the specified position + * + * @param int|stdClass $courseorid course id or course object + * @param int $position position to add to, 0 means to the end. If position is greater than + * number of existing secitons, the section is added to the end. This will become sectionnum of the + * new section. All existing sections at this or bigger position will be shifted down. + * @param bool $skipcheck the check has already been made and we know that the section with this position does not exist + * @return stdClass created section object + */ +function course_create_section($courseorid, $position = 0, $skipcheck = false) { + global $DB; + $courseid = is_object($courseorid) ? $courseorid->id : $courseorid; + + // Find the last sectionnum among existing sections. + if ($skipcheck) { + $lastsection = $position - 1; + } else { + $lastsection = (int)$DB->get_field_sql('SELECT max(section) from {course_sections} WHERE course = ?', [$courseid]); + } + + // First add section to the end. + $cw = new stdClass(); + $cw->course = $courseid; + $cw->section = $lastsection + 1; + $cw->summary = ''; + $cw->summaryformat = FORMAT_HTML; + $cw->sequence = ''; + $cw->name = null; + $cw->visible = 1; + $cw->availability = null; + $cw->id = $DB->insert_record("course_sections", $cw); + + // Now move it to the specified position. + if ($position > 0 && $position <= $lastsection) { + $course = is_object($courseorid) ? $courseorid : get_course($courseorid); + move_section_to($course, $cw->section, $position, true); + $cw->section = $position; + } + + core\event\course_section_created::create_from_section($cw)->trigger(); + + rebuild_course_cache($courseid, true); + return $cw; +} + /** * Creates missing course section(s) and rebuilds course cache * @@ -840,31 +886,17 @@ function add_course_module($mod) { * @return bool if there were any sections created */ function course_create_sections_if_missing($courseorid, $sections) { - global $DB; if (!is_array($sections)) { $sections = array($sections); } $existing = array_keys(get_fast_modinfo($courseorid)->get_section_info_all()); - if (is_object($courseorid)) { - $courseorid = $courseorid->id; - } - $coursechanged = false; - foreach ($sections as $sectionnum) { - if (!in_array($sectionnum, $existing)) { - $cw = new stdClass(); - $cw->course = $courseorid; - $cw->section = $sectionnum; - $cw->summary = ''; - $cw->summaryformat = FORMAT_HTML; - $cw->sequence = ''; - $id = $DB->insert_record("course_sections", $cw); - $coursechanged = true; + if ($newsections = array_diff($sections, $existing)) { + foreach ($newsections as $sectionnum) { + course_create_section($courseorid, $sectionnum, true); } + return true; } - if ($coursechanged) { - rebuild_course_cache($courseorid, true); - } - return $coursechanged; + return false; } /** @@ -2398,8 +2430,14 @@ function create_course($data, $editoroptions = NULL) { // Setup the blocks blocks_add_default_course_blocks($course); - // Create a default section. - course_create_sections_if_missing($course, 0); + // Create default section and initial sections if specified (unless they've already been created earlier). + // We do not want to call course_create_sections_if_missing() because to avoid creating course cache. + $numsections = isset($data->numsections) ? $data->numsections : 0; + $existingsections = $DB->get_fieldset_sql('SELECT section from {course_sections} WHERE course = ?', [$newcourseid]); + $newsections = array_diff(range(0, $numsections), $existingsections); + foreach ($newsections as $sectionnum) { + course_create_section($newcourseid, $sectionnum, true); + } // Save any custom role names. save_local_role_names($course->id, (array)$data); diff --git a/course/tests/behat/activities_visibility_icons.feature b/course/tests/behat/activities_visibility_icons.feature index a89880c5a3a..1af07b9e633 100644 --- a/course/tests/behat/activities_visibility_icons.feature +++ b/course/tests/behat/activities_visibility_icons.feature @@ -122,39 +122,6 @@ Feature: Toggle activities visibility from the course page And I should see "(There are no discussion topics yet in this forum)" And I log out - @javascript - Scenario: Activities can be shown and hidden inside an orphaned section - Given the following "users" exist: - | username | firstname | lastname | email | - | teacher1 | Teacher | 1 | teacher1@example.com | - And the following "courses" exist: - | fullname | shortname | format | numsections | - | Course 1 | C1 | topics | 2 | - And the following "course enrolments" exist: - | user | course | role | - | teacher1 | C1 | editingteacher | - And I log in as "teacher1" - And I follow "Course 1" - And I turn editing mode on - And I add a "Forum" to section "2" and I fill the form with: - | Forum name | Test forum name | - | Description | Test forum description | - | Visible | Show | - When I click on ".reduce-sections" "css_element" - Then "Test forum name" activity should be visible - And I open "Test forum name" actions menu - And "Test forum name" actions menu should not have "Show" item - And "Test forum name" actions menu should not have "Make available" item - And "Test forum name" actions menu should not have "Make unavailable" item - And I click on "Hide" "link" in the "Test forum name" activity - And "Test forum name" activity should be hidden - And I open "Test forum name" actions menu - And "Test forum name" actions menu should not have "Hide" item - And "Test forum name" actions menu should not have "Make available" item - And "Test forum name" actions menu should not have "Make unavailable" item - And I click on "Show" "link" in the "Test forum name" activity - And "Test forum name" activity should be visible - @javascript Scenario: Activities can be made available but not visible on a course page Given the following "users" exist: diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index c726278b396..374aaafc64d 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -563,7 +563,6 @@ class core_course_courselib_testcase extends advanced_testcase { $course->summaryformat = FORMAT_PLAIN; $course->format = 'topics'; $course->newsitems = 0; - $course->numsections = 5; $course->category = $defaultcategory; $original = (array) $course; @@ -609,25 +608,26 @@ class core_course_courselib_testcase extends advanced_testcase { global $DB; $this->resetAfterTest(true); + $numsections = 5; $course = $this->getDataGenerator()->create_course( array('shortname' => 'GrowingCourse', 'fullname' => 'Growing Course', - 'numsections' => 5), + 'numsections' => $numsections), array('createsections' => true)); // Ensure all 6 (0-5) sections were created and course content cache works properly $sectionscreated = array_keys(get_fast_modinfo($course)->get_section_info_all()); - $this->assertEquals(range(0, $course->numsections), $sectionscreated); + $this->assertEquals(range(0, $numsections), $sectionscreated); // this will do nothing, section already exists - $this->assertFalse(course_create_sections_if_missing($course, $course->numsections)); + $this->assertFalse(course_create_sections_if_missing($course, $numsections)); // this will create new section - $this->assertTrue(course_create_sections_if_missing($course, $course->numsections + 1)); + $this->assertTrue(course_create_sections_if_missing($course, $numsections + 1)); // Ensure all 7 (0-6) sections were created and modinfo/sectioninfo cache works properly $sectionscreated = array_keys(get_fast_modinfo($course)->get_section_info_all()); - $this->assertEquals(range(0, $course->numsections + 1), $sectionscreated); + $this->assertEquals(range(0, $numsections + 1), $sectionscreated); } public function test_update_course() { @@ -957,31 +957,23 @@ class core_course_courselib_testcase extends advanced_testcase { // Delete last section. $this->assertTrue(course_delete_section($course, 6, true)); $this->assertFalse($DB->record_exists('course_modules', array('id' => $assign6->cmid))); - $this->assertEquals(5, course_get_format($course)->get_course()->numsections); + $this->assertEquals(5, course_get_format($course)->get_last_section_number()); // Delete empty section. $this->assertTrue(course_delete_section($course, 4, false)); - $this->assertEquals(4, course_get_format($course)->get_course()->numsections); + $this->assertEquals(4, course_get_format($course)->get_last_section_number()); // Delete section in the middle (2). $this->assertFalse(course_delete_section($course, 2, false)); $this->assertTrue(course_delete_section($course, 2, true)); $this->assertFalse($DB->record_exists('course_modules', array('id' => $assign21->cmid))); $this->assertFalse($DB->record_exists('course_modules', array('id' => $assign22->cmid))); - $this->assertEquals(3, course_get_format($course)->get_course()->numsections); + $this->assertEquals(3, course_get_format($course)->get_last_section_number()); $this->assertEquals(array(0 => array($assign0->cmid), 1 => array($assign1->cmid), 2 => array($assign3->cmid), 3 => array($assign5->cmid)), get_fast_modinfo($course)->sections); - // Make last section orphaned. - update_course((object)array('id' => $course->id, 'numsections' => 2)); - $this->assertEquals(2, course_get_format($course)->get_course()->numsections); - - // Remove orphaned section. - $this->assertTrue(course_delete_section($course, 3, true)); - $this->assertEquals(2, course_get_format($course)->get_course()->numsections); - // Remove marked section. course_set_marker($course->id, 1); $this->assertTrue(course_get_format($course)->is_section_current(1)); @@ -3549,7 +3541,7 @@ class core_course_courselib_testcase extends advanced_testcase { // Delete empty section. No difference from normal, synchronous behaviour. $this->assertTrue(course_delete_section($course, 4, false, true)); - $this->assertEquals(3, course_get_format($course)->get_course()->numsections); + $this->assertEquals(3, course_get_format($course)->get_last_section_number()); // Delete a module in section 2 (using async). Need to verify this doesn't generate two tasks when we delete // the section in the next step. @@ -3577,7 +3569,7 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEquals(3, $DB->count_records('course_modules', ['section' => $sectionid, 'deletioninprogress' => 1])); // Confirm the section has been deleted. - $this->assertEquals(2, course_get_format($course)->get_course()->numsections); + $this->assertEquals(2, course_get_format($course)->get_last_section_number()); // Check event fired. $events = $sink->get_events(); @@ -3646,7 +3638,7 @@ class core_course_courselib_testcase extends advanced_testcase { // Delete empty section. No difference from normal, synchronous behaviour. $this->assertTrue(course_delete_section($course, 4, false, true)); - $this->assertEquals(3, course_get_format($course)->get_course()->numsections); + $this->assertEquals(3, course_get_format($course)->get_last_section_number()); // Delete section in the middle (2). $section = $DB->get_record('course_sections', ['course' => $course->id, 'section' => '2']); // For event comparison. @@ -3667,7 +3659,7 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEmpty($cmcount); // Confirm the section has been deleted. - $this->assertEquals(2, course_get_format($course)->get_course()->numsections); + $this->assertEquals(2, course_get_format($course)->get_last_section_number()); // Confirm the course_section_deleted event has been generated. $events = $sink->get_events(); diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php index 9616ca2bf15..66950284178 100644 --- a/course/tests/externallib_test.php +++ b/course/tests/externallib_test.php @@ -445,7 +445,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($courseinfo->newsitems, $course2['newsitems']); $this->assertEquals($courseinfo->startdate, $course2['startdate']); $this->assertEquals($courseinfo->enddate, $course2['enddate']); - $this->assertEquals($courseinfo->numsections, $course2['numsections']); + $this->assertEquals(course_get_format($createdcourse['id'])->get_last_section_number(), $course2['numsections']); $this->assertEquals($courseinfo->maxbytes, $course2['maxbytes']); $this->assertEquals($courseinfo->showreports, $course2['showreports']); $this->assertEquals($courseinfo->visible, $course2['visible']); @@ -480,7 +480,8 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($courseinfo->category, $course3['categoryid']); $this->assertEquals($courseinfo->format, $course3['format']); $this->assertEquals($courseinfo->hiddensections, $course3options['hiddensections']); - $this->assertEquals($courseinfo->numsections, $course3options['numsections']); + $this->assertEquals(course_get_format($createdcourse['id'])->get_last_section_number(), + $course3options['numsections']); $this->assertEquals($courseinfo->coursedisplay, $course3options['coursedisplay']); } else { throw moodle_exception('Unexpected shortname'); @@ -612,7 +613,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($course['newsitems'], $dbcourse->newsitems); $this->assertEquals($course['startdate'], $dbcourse->startdate); $this->assertEquals($course['enddate'], $dbcourse->enddate); - $this->assertEquals($course['numsections'], $dbcourse->numsections); + $this->assertEquals($course['numsections'], course_get_format($dbcourse)->get_last_section_number()); $this->assertEquals($course['maxbytes'], $dbcourse->maxbytes); $this->assertEquals($course['showreports'], $dbcourse->showreports); $this->assertEquals($course['visible'], $dbcourse->visible); @@ -626,7 +627,6 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($course['enablecompletion'], $dbcourse->enablecompletion); if ($dbcourse->format === 'topics') { $this->assertEquals($course['courseformatoptions'], array( - array('name' => 'numsections', 'value' => $dbcourse->numsections), array('name' => 'hiddensections', 'value' => $dbcourse->hiddensections), array('name' => 'coursedisplay', 'value' => $dbcourse->coursedisplay), )); @@ -744,7 +744,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { */ private function prepare_get_course_contents_test() { global $DB; - $course = self::getDataGenerator()->create_course(); + $course = self::getDataGenerator()->create_course(['numsections' => 2]); $forumdescription = 'This is the forum description'; $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id, 'intro' => $forumdescription), @@ -908,7 +908,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { // We need to execute the return values cleaning process to simulate the web service server. $sections = external_api::clean_returnvalue(core_course_external::get_course_contents_returns(), $sections); - $this->assertCount(2, $sections); + $this->assertCount(3, $sections); $this->assertCount(1, $sections[0]['modules']); $this->assertEquals($forumcm->id, $sections[0]['modules'][0]["id"]); } @@ -950,7 +950,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { // We need to execute the return values cleaning process to simulate the web service server. $sections = external_api::clean_returnvalue(core_course_external::get_course_contents_returns(), $sections); - $this->assertCount(2, $sections); + $this->assertCount(3, $sections); $this->assertCount(1, $sections[0]['modules']); $this->assertEquals($forumcm->id, $sections[0]['modules'][0]["id"]); } @@ -972,7 +972,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { // We need to execute the return values cleaning process to simulate the web service server. $sections = external_api::clean_returnvalue(core_course_external::get_course_contents_returns(), $sections); - $this->assertCount(2, $sections); + $this->assertCount(3, $sections); $this->assertCount(1, $sections[0]['modules']); $this->assertEquals("page", $sections[0]['modules'][0]["modname"]); $this->assertEquals($pagecm->instance, $sections[0]['modules'][0]["instance"]); @@ -1076,7 +1076,6 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $course2['newsitems'] = 3; $course2['startdate'] = 1420092000; // 01/01/2015. $course2['enddate'] = 1422669600; // 01/31/2015. - $course2['numsections'] = 4; $course2['maxbytes'] = 100000; $course2['showreports'] = 1; $course2['visible'] = 0; @@ -1112,7 +1111,6 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($course2['newsitems'], $courseinfo->newsitems); $this->assertEquals($course2['startdate'], $courseinfo->startdate); $this->assertEquals($course2['enddate'], $courseinfo->enddate); - $this->assertEquals($course2['numsections'], $courseinfo->numsections); $this->assertEquals($course2['maxbytes'], $courseinfo->maxbytes); $this->assertEquals($course2['showreports'], $courseinfo->showreports); $this->assertEquals($course2['visible'], $courseinfo->visible); @@ -1133,7 +1131,7 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals($course1['categoryid'], $courseinfo->category); $this->assertEquals(FORMAT_MOODLE, $courseinfo->summaryformat); $this->assertEquals('topics', $courseinfo->format); - $this->assertEquals(5, $courseinfo->numsections); + $this->assertEquals(5, course_get_format($course['id'])->get_last_section_number()); $this->assertEquals(0, $courseinfo->newsitems); $this->assertEquals(FORMAT_MOODLE, $courseinfo->summaryformat); } else { diff --git a/enrol/database/lib.php b/enrol/database/lib.php index d25e14a91b6..b21578e5fc2 100644 --- a/enrol/database/lib.php +++ b/enrol/database/lib.php @@ -777,6 +777,9 @@ class enrol_database_plugin extends enrol_plugin { if ($templatecourse) { if ($template = $DB->get_record('course', array('shortname'=>$templatecourse))) { $template = fullclone(course_get_format($template)->get_course()); + if (!isset($template->numsections)) { + $template->numsections = course_get_format($template)->get_last_section_number(); + } unset($template->id); unset($template->fullname); unset($template->shortname); diff --git a/enrol/database/tests/sync_test.php b/enrol/database/tests/sync_test.php index 78298331577..03ea36f1e28 100644 --- a/enrol/database/tests/sync_test.php +++ b/enrol/database/tests/sync_test.php @@ -758,8 +758,7 @@ class enrol_database_testcase extends advanced_testcase { $course8['category'] = $defcat->id; $record = $DB->get_record('course', $course8); $this->assertFalse(empty($record)); - $courseformatoptions = course_get_format($record)->get_format_options(); - $this->assertEquals($courseformatoptions['numsections'], 666); + $this->assertEquals(666, course_get_format($record)->get_last_section_number()); // Test invalid category. diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 839580f0cce..85226c77162 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -71,6 +71,7 @@ $string['addresource'] = 'Add a resource...'; $string['addresourceoractivity'] = 'Add an activity or resource'; $string['addresourcetosection'] = 'Add a resource to section \'{$a}\''; $string['address'] = 'Address'; +$string['addsection'] = 'Add section'; $string['addstudent'] = 'Add student'; $string['addsubcategory'] = 'Add a subcategory'; $string['addteacher'] = 'Add teacher'; @@ -740,6 +741,7 @@ $string['eventcourseresetended'] = 'Course reset ended'; $string['eventcourseresetstarted'] = 'Course reset started'; $string['eventcourserestored'] = 'Course restored'; $string['eventcourseupdated'] = 'Course updated'; +$string['eventcoursesectioncreated'] = 'Course section created'; $string['eventcoursesectiondeleted'] = 'Course section deleted'; $string['eventcoursesectionupdated'] = 'Course section updated'; $string['eventcoursemoduleinstancelistviewed'] = 'Course module instance list viewed'; diff --git a/lib/classes/event/course_section_created.php b/lib/classes/event/course_section_created.php new file mode 100644 index 00000000000..9c1e789575d --- /dev/null +++ b/lib/classes/event/course_section_created.php @@ -0,0 +1,131 @@ +. + +/** + * Course section created event. + * + * @package core + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\event; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Course section created event class. + * + * @property-read array $other { + * Extra information about event. + * + * - int sectionnum: section number. + * } + * + * @package core + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class course_section_created extends base { + + /** + * Init method. + * + * @return void + */ + protected function init() { + $this->data['objecttable'] = 'course_sections'; + $this->data['crud'] = 'c'; + $this->data['edulevel'] = self::LEVEL_TEACHING; + } + + /** + * Creates event from the section object + * + * @param \stdClass $section + * @return course_section_created + */ + public static function create_from_section($section) { + $event = self::create([ + 'context' => \context_course::instance($section->course), + 'objectid' => $section->id, + 'other' => ['sectionnum' => $section->section] + ]); + $event->add_record_snapshot('course_sections', $section); + return $event; + } + + /** + * Return localised event name. + * + * @return string + */ + public static function get_name() { + return get_string('eventcoursesectioncreated'); + } + + /** + * Returns non-localised event description with id's for admin use only. + * + * @return string + */ + public function get_description() { + return "The user with id '$this->userid' created section number '{$this->other['sectionnum']}' for the " . + "course with id '$this->courseid'"; + } + + /** + * Get URL related to the action. + * + * @return \moodle_url + */ + public function get_url() { + return new \moodle_url('/course/editsection.php', array('id' => $this->objectid)); + } + + /** + * Custom validation. + * + * @throws \coding_exception + * @return void + */ + protected function validate_data() { + parent::validate_data(); + + if (!isset($this->other['sectionnum'])) { + throw new \coding_exception('The \'sectionnum\' value must be set in other.'); + } + } + + /** + * Mapping for sections object during restore + * + * @return array + */ + public static function get_objectid_mapping() { + return array('db' => 'course_sections', 'restore' => 'course_section'); + } + + /** + * Mapping for other fields during restore + * + * @return bool + */ + public static function get_other_mapping() { + // Sectionnum does not need mapping because it's relative. + return false; + } +} diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index 31f6ce16fbc..5b0bc694482 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -419,15 +419,14 @@ EOD; $record['tags'] = preg_split('/\s*,\s*/', trim($record['tags']), -1, PREG_SPLIT_NO_EMPTY); } + if (!empty($options['createsections']) && empty($record['numsections'])) { + // Since Moodle 3.3 function create_course() automatically creates sections if numsections is specified. + // For BC if 'createsections' is given but 'numsections' is not, assume the default value from config. + $record['numsections'] = get_config('moodlecourse', 'numsections'); + } + $course = create_course((object)$record); context_course::instance($course->id); - if (!empty($options['createsections'])) { - if (isset($course->numsections)) { - course_create_sections_if_missing($course, range(0, $course->numsections)); - } else { - course_create_sections_if_missing($course, 0); - } - } return $course; } diff --git a/lib/testing/tests/generator_test.php b/lib/testing/tests/generator_test.php index ba6e012edc2..2d3e7e970f2 100644 --- a/lib/testing/tests/generator_test.php +++ b/lib/testing/tests/generator_test.php @@ -170,7 +170,7 @@ class core_test_generator_testcase extends advanced_testcase { $this->assertSame('', $course->idnumber); $this->assertSame('topics', $course->format); $this->assertEquals(0, $course->newsitems); - $this->assertEquals(5, $course->numsections); + $this->assertEquals(5, course_get_format($course)->get_last_section_number()); $this->assertRegExp('/^Test course \d/', $course->summary); $this->assertSame(FORMAT_MOODLE, $course->summaryformat); From af0698c007dcfa9e49a4af88a0813e819ea2f9d1 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 2 Feb 2017 12:32:59 +0800 Subject: [PATCH 2/4] MDL-57769 format_topics: remove numsections option --- .../behat/restore_moodle2_courses.feature | 7 +- course/format/topics/db/upgrade.php | 47 +++++++ course/format/topics/db/upgradelib.php | 117 ++++++++++++++++ course/format/topics/format.php | 7 +- .../format/topics/lang/en/format_topics.php | 1 + course/format/topics/lib.php | 80 +++-------- course/format/topics/renderer.php | 3 +- .../tests/behat/edit_delete_sections.feature | 33 +---- .../topics/tests/format_topics_test.php | 26 ---- .../tests/format_topics_upgrade_test.php | 128 ++++++++++++++++++ course/format/topics/version.php | 2 +- 11 files changed, 323 insertions(+), 128 deletions(-) create mode 100644 course/format/topics/db/upgrade.php create mode 100644 course/format/topics/db/upgradelib.php create mode 100644 course/format/topics/tests/format_topics_upgrade_test.php diff --git a/backup/util/ui/tests/behat/restore_moodle2_courses.feature b/backup/util/ui/tests/behat/restore_moodle2_courses.feature index 32c1cd7e7f1..41027904426 100644 --- a/backup/util/ui/tests/behat/restore_moodle2_courses.feature +++ b/backup/util/ui/tests/behat/restore_moodle2_courses.feature @@ -41,11 +41,11 @@ Feature: Restore Moodle 2 course backups Then I should see "Course 1 restored in a new course" And I should see "Community finder" in the "Community finder" "block" And I should see "Test forum name" + And I should see "Topic 15" + And I should not see "Topic 16" And I navigate to "Edit settings" node in "Course administration" And I expand all fieldsets And the field "id_format" matches value "Topics format" - And the field "Number of sections" matches value "15" - And the field "Course layout" matches value "Show one section per page" And I press "Cancel" @javascript @@ -122,11 +122,12 @@ Feature: Restore Moodle 2 course backups And I navigate to "Edit settings" node in "Course administration" And I expand all fieldsets Then the field "id_format" matches value "Topics format" - And the field "Number of sections" matches value "15" And the field "Course layout" matches value "Show one section per page" And I press "Cancel" And section "3" should be hidden And section "7" should be hidden And section "15" should be visible + And I should see "Topic 15" + And I should not see "Topic 16" And I should see "Test URL name" in the "Topic 3" "section" And I should see "Test forum name" in the "Topic 1" "section" diff --git a/course/format/topics/db/upgrade.php b/course/format/topics/db/upgrade.php new file mode 100644 index 00000000000..69bc85e9ca2 --- /dev/null +++ b/course/format/topics/db/upgrade.php @@ -0,0 +1,47 @@ +. + +/** + * Upgrade scripts for course format "Topics" + * + * @package format_topics + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Upgrade script for format_topics + * + * @param int $oldversion the version we are upgrading from + * @return bool result + */ +function xmldb_format_topics_upgrade($oldversion) { + global $CFG, $DB; + + require_once($CFG->dirroot . '/course/format/topics/db/upgradelib.php'); + + if ($oldversion < 2017020200) { + + // Remove 'numsections' option and hide or delete orphaned sections. + format_topics_upgrade_remove_numsections(); + + upgrade_plugin_savepoint(true, 2017020200, 'format', 'topics'); + } + + return true; +} diff --git a/course/format/topics/db/upgradelib.php b/course/format/topics/db/upgradelib.php new file mode 100644 index 00000000000..7182ceadf31 --- /dev/null +++ b/course/format/topics/db/upgradelib.php @@ -0,0 +1,117 @@ +. + +/** + * Upgrade scripts for course format "Topics" + * + * @package format_topics + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * This method finds all courses in 'topics' format that have actual number of sections + * bigger than their 'numsections' course format option. + * For each such course we call {@link format_topics_upgrade_hide_extra_sections()} and + * either delete or hide "orphaned" sections. + */ +function format_topics_upgrade_remove_numsections() { + global $DB; + + $sql1 = "SELECT c.id, max(cs.section) AS sectionsactual + FROM {course} c + JOIN {course_sections} cs ON cs.course = c.id + WHERE c.format = :format1 + GROUP BY c.id"; + + $sql2 = "SELECT c.id, n.value AS numsections + FROM {course} c + JOIN {course_format_options} n ON n.courseid = c.id AND n.format = :format1 AND n.name = :numsections AND n.sectionid = 0 + WHERE c.format = :format2"; + + $params = ['format1' => 'topics', 'format2' => 'topics', 'numsections' => 'numsections']; + + $actual = $DB->get_records_sql_menu($sql1, $params); + $numsections = $DB->get_records_sql_menu($sql2, $params); + $needfixing = []; + + $defaultnumsections = get_config('moodlecourse', 'numsections'); + + foreach ($actual as $courseid => $sectionsactual) { + if (array_key_exists($courseid, $numsections)) { + $n = (int)$numsections[$courseid]; + } else { + $n = $defaultnumsections; + } + if ($sectionsactual > $n) { + $needfixing[$courseid] = $n; + } + } + unset($actual); + unset($numsections); + + foreach ($needfixing as $courseid => $numsections) { + format_topics_upgrade_hide_extra_sections($courseid, $numsections); + } + + $DB->delete_records('course_format_options', ['format' => 'topics', 'sectionid' => 0, 'name' => 'numsections']); +} + +/** + * Find all sections in the course with sectionnum bigger than numsections. + * Either delete these sections or hide them + * + * We will only delete a section if it is completely empty and all sections below + * it are also empty + * + * @param int $courseid + * @param int $numsections + */ +function format_topics_upgrade_hide_extra_sections($courseid, $numsections) { + global $DB; + $sections = $DB->get_records_sql('SELECT id, name, summary, sequence, visible + FROM {course_sections} + WHERE course = ? AND section > ? + ORDER BY section DESC', [$courseid, $numsections]); + $candelete = true; + $tohide = []; + $todelete = []; + foreach ($sections as $section) { + if ($candelete && (!empty($section->summary) || !empty($section->sequence) || !empty($section->name))) { + $candelete = false; + } + if ($candelete) { + $todelete[] = $section->id; + } else if ($section->visible) { + $tohide[] = $section->id; + } + } + if ($todelete) { + // Delete empty sections in the end. + // This is an upgrade script - no events or cache resets are needed. + // We also know that these sections do not have any modules so it is safe to just delete records in the table. + $DB->delete_records_list('course_sections', 'id', $todelete); + } + if ($tohide) { + // Hide other orphaned sections. + // This is different from what set_section_visible() does but we want to preserve actual + // module visibility in this case. + list($sql, $params) = $DB->get_in_or_equal($tohide); + $DB->execute("UPDATE {course_sections} SET visible = 0 WHERE id " . $sql, $params); + } +} diff --git a/course/format/topics/format.php b/course/format/topics/format.php index cf2c0527411..7af285109dc 100644 --- a/course/format/topics/format.php +++ b/course/format/topics/format.php @@ -38,15 +38,16 @@ if ($topic = optional_param('topic', 0, PARAM_INT)) { // End backwards-compatible aliasing.. $context = context_course::instance($course->id); +// Retrieve course format option fields and add them to the $course object. +$course = course_get_format($course)->get_course(); if (($marker >=0) && has_capability('moodle/course:setcurrentsection', $context) && confirm_sesskey()) { $course->marker = $marker; course_set_marker($course->id, $marker); } -// make sure all sections are created -$course = course_get_format($course)->get_course(); -course_create_sections_if_missing($course, range(0, $course->numsections)); +// Make sure section 0 is created. +course_create_sections_if_missing($course, 0); $renderer = $PAGE->get_renderer('format_topics'); diff --git a/course/format/topics/lang/en/format_topics.php b/course/format/topics/lang/en/format_topics.php index 69856d4427e..38991849d52 100644 --- a/course/format/topics/lang/en/format_topics.php +++ b/course/format/topics/lang/en/format_topics.php @@ -23,6 +23,7 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +$string['addsection'] = 'Add topic'; $string['currentsection'] = 'This topic'; $string['editsection'] = 'Edit topic'; $string['editsectionname'] = 'Edit topic name'; diff --git a/course/format/topics/lib.php b/course/format/topics/lib.php index 1bc58ab3e8e..0351be2aacc 100644 --- a/course/format/topics/lib.php +++ b/course/format/topics/lib.php @@ -214,7 +214,6 @@ class format_topics extends format_base { * * Topics format uses the following options: * - coursedisplay - * - numsections * - hiddensections * * @param bool $foreditform @@ -225,10 +224,6 @@ class format_topics extends format_base { if ($courseformatoptions === false) { $courseconfig = get_config('moodlecourse'); $courseformatoptions = array( - 'numsections' => array( - 'default' => $courseconfig->numsections, - 'type' => PARAM_INT, - ), 'hiddensections' => array( 'default' => $courseconfig->hiddensections, 'type' => PARAM_INT, @@ -240,21 +235,7 @@ class format_topics extends format_base { ); } if ($foreditform && !isset($courseformatoptions['coursedisplay']['label'])) { - $courseconfig = get_config('moodlecourse'); - $max = $courseconfig->maxsections; - if (!isset($max) || !is_numeric($max)) { - $max = 52; - } - $sectionmenu = array(); - for ($i = 0; $i <= $max; $i++) { - $sectionmenu[$i] = "$i"; - } $courseformatoptionsedit = array( - 'numsections' => array( - 'label' => new lang_string('numberweeks'), - 'element_type' => 'select', - 'element_attributes' => array($sectionmenu), - ), 'hiddensections' => array( 'label' => new lang_string('hiddensections'), 'help' => 'hiddensections', @@ -295,24 +276,24 @@ class format_topics extends format_base { * @return array array of references to the added form elements. */ public function create_edit_form_elements(&$mform, $forsection = false) { + global $COURSE; $elements = parent::create_edit_form_elements($mform, $forsection); - // Increase the number of sections combo box values if the user has increased the number of sections - // using the icon on the course page beyond course 'maxsections' or course 'maxsections' has been - // reduced below the number of sections already set for the course on the site administration course - // defaults page. This is so that the number of sections is not reduced leaving unintended orphaned - // activities / resources. - if (!$forsection) { - $maxsections = get_config('moodlecourse', 'maxsections'); - $numsections = $mform->getElementValue('numsections'); - $numsections = $numsections[0]; - if ($numsections > $maxsections) { - $element = $mform->getElement('numsections'); - for ($i = $maxsections+1; $i <= $numsections; $i++) { - $element->addOption("$i", $i); - } + if (!$forsection && (empty($COURSE->id) || $COURSE->id == SITEID)) { + // Add "numsections" element to the create course form - it will force new course to be prepopulated + // with empty sections. + // The "Number of sections" option is no longer available when editing course, instead teachers should + // delete and add sections when needed. + $courseconfig = get_config('moodlecourse'); + $max = (int)$courseconfig->maxsections; + $element = $mform->addElement('select', 'numsections', get_string('numberweeks'), range(0, $max ?: 52)); + $mform->setType('numsections', PARAM_INT); + if (is_null($mform->getElementValue('numsections'))) { + $mform->setDefault('numsections', $courseconfig->numsections); } + array_unshift($elements, $element); } + return $elements; } @@ -320,9 +301,7 @@ class format_topics extends format_base { * Updates format options for a course * * In case if course format was changed to 'topics', we try to copy options - * 'coursedisplay', 'numsections' and 'hiddensections' from the previous format. - * If previous course format did not have 'numsections' option, we populate it with the - * current number of sections + * 'coursedisplay' and 'hiddensections' from the previous format. * * @param stdClass|array $data return value from {@link moodleform::get_data()} or array with data * @param stdClass $oldcourse if this function is called from {@link update_course()} @@ -330,7 +309,6 @@ class format_topics extends format_base { * @return bool whether there were any changes to the options values */ public function update_course_format_options($data, $oldcourse = null) { - global $DB; $data = (array)$data; if ($oldcourse !== null) { $oldcourse = (array)$oldcourse; @@ -339,33 +317,11 @@ class format_topics extends format_base { if (!array_key_exists($key, $data)) { if (array_key_exists($key, $oldcourse)) { $data[$key] = $oldcourse[$key]; - } else if ($key === 'numsections') { - // If previous format does not have the field 'numsections' - // and $data['numsections'] is not set, - // we fill it with the maximum section number from the DB - $maxsection = $DB->get_field_sql('SELECT max(section) from {course_sections} - WHERE course = ?', array($this->courseid)); - if ($maxsection) { - // If there are no sections, or just default 0-section, 'numsections' will be set to default - $data['numsections'] = $maxsection; - } } } } } - $changed = $this->update_format_options($data); - if ($changed && array_key_exists('numsections', $data)) { - // If the numsections was decreased, try to completely delete the orphaned sections (unless they are not empty). - $numsections = (int)$data['numsections']; - $maxsection = $DB->get_field_sql('SELECT max(section) from {course_sections} - WHERE course = ?', array($this->courseid)); - for ($sectionnum = $maxsection; $sectionnum > $numsections; $sectionnum--) { - if (!$this->delete_section($sectionnum, false)) { - break; - } - } - } - return $changed; + return $this->update_format_options($data); } /** @@ -420,8 +376,8 @@ class format_topics extends format_base { * @return bool */ public function allow_stealth_module_visibility($cm, $section) { - // Allow the third visibility state inside visible sections or in section 0, not allow in orphaned sections. - return !$section->section || ($section->visible && $section->section <= $this->get_course()->numsections); + // Allow the third visibility state inside visible sections or in section 0. + return !$section->section || $section->visible; } public function section_action($section, $action, $sr) { diff --git a/course/format/topics/renderer.php b/course/format/topics/renderer.php index 963e0666a94..248c73a5cdb 100644 --- a/course/format/topics/renderer.php +++ b/course/format/topics/renderer.php @@ -119,9 +119,8 @@ class format_topics_renderer extends format_section_renderer_base { } $url->param('sesskey', sesskey()); - $isstealth = $section->section > $course->numsections; $controls = array(); - if (!$isstealth && $section->section && has_capability('moodle/course:setcurrentsection', $coursecontext)) { + if ($section->section && has_capability('moodle/course:setcurrentsection', $coursecontext)) { if ($course->marker == $section->section) { // Show the "light globe" on/off. $url->param('marker', 0); $markedthistopic = get_string('markedthistopic'); diff --git a/course/format/topics/tests/behat/edit_delete_sections.feature b/course/format/topics/tests/behat/edit_delete_sections.feature index 1c836478c7b..4c35bafa567 100644 --- a/course/format/topics/tests/behat/edit_delete_sections.feature +++ b/course/format/topics/tests/behat/edit_delete_sections.feature @@ -71,9 +71,7 @@ Feature: Sections can be edited and deleted in topics format Then I should see "Are you absolutely sure you want to completely delete \"Topic 5\" and all the activities it contains?" And I press "Delete" And I should not see "Topic 5" - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" + And I should see "Topic 4" Scenario: Deleting the middle section in topics format When I delete section "4" @@ -81,31 +79,4 @@ Feature: Sections can be edited and deleted in topics format Then I should not see "Topic 5" And I should not see "Test chat name" And I should see "Test choice name" in the "li#section-4" "css_element" - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" - - Scenario: Deleting the orphaned section in topics format - When I follow "Reduce the number of sections" - Then I should see "Orphaned activities (section 5)" in the "li#section-5" "css_element" - And I delete section "5" - And I press "Delete" - And I should not see "Topic 5" - And I should not see "Orphaned activities" - And "li#section-5" "css_element" should not exist - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" - - Scenario: Deleting a section when orphaned section is present in topics format - When I follow "Reduce the number of sections" - Then I should see "Orphaned activities (section 5)" in the "li#section-5" "css_element" - And "li#section-5.orphaned" "css_element" should exist - And "li#section-4.orphaned" "css_element" should not exist - And I delete section "1" - And I press "Delete" - And I should not see "Test book name" - And I should see "Orphaned activities (section 4)" in the "li#section-4" "css_element" - And "li#section-5" "css_element" should not exist - And "li#section-4.orphaned" "css_element" should exist - And "li#section-3.orphaned" "css_element" should not exist + And I should see "Topic 4" diff --git a/course/format/topics/tests/format_topics_test.php b/course/format/topics/tests/format_topics_test.php index 5e477df3e08..527c44e81ae 100644 --- a/course/format/topics/tests/format_topics_test.php +++ b/course/format/topics/tests/format_topics_test.php @@ -36,32 +36,6 @@ require_once($CFG->dirroot . '/course/lib.php'); */ class format_topics_testcase extends advanced_testcase { - public function test_update_course_numsections() { - global $DB; - $this->resetAfterTest(true); - - $generator = $this->getDataGenerator(); - - $course = $generator->create_course(array('numsections' => 10, 'format' => 'topics'), - array('createsections' => true)); - $generator->create_module('assign', array('course' => $course, 'section' => 7)); - - $this->setAdminUser(); - - $this->assertEquals(11, $DB->count_records('course_sections', array('course' => $course->id))); - - // Change the numsections to 8, last two sections did not have any activities, they should be deleted. - update_course((object)array('id' => $course->id, 'numsections' => 8)); - $this->assertEquals(9, $DB->count_records('course_sections', array('course' => $course->id))); - $this->assertEquals(9, count(get_fast_modinfo($course)->get_section_info_all())); - - // Change the numsections to 5, section 8 should be deleted but section 7 should remain as it has activities. - update_course((object)array('id' => $course->id, 'numsections' => 6)); - $this->assertEquals(8, $DB->count_records('course_sections', array('course' => $course->id))); - $this->assertEquals(8, count(get_fast_modinfo($course)->get_section_info_all())); - $this->assertEquals(6, course_get_format($course)->get_course()->numsections); - } - /** * Tests for format_topics::get_section_name method with default section names. */ diff --git a/course/format/topics/tests/format_topics_upgrade_test.php b/course/format/topics/tests/format_topics_upgrade_test.php new file mode 100644 index 00000000000..e32c67dc39d --- /dev/null +++ b/course/format/topics/tests/format_topics_upgrade_test.php @@ -0,0 +1,128 @@ +. + +/** + * format_topics unit tests for upgradelib + * + * @package format_topics + * @copyright 2015 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/course/lib.php'); +require_once($CFG->dirroot . '/course/format/topics/db/upgradelib.php'); + +/** + * format_topics unit tests for upgradelib + * + * @package format_topics + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class format_topics_upgrade_testcase extends advanced_testcase { + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_no_actions() { + global $DB; + + $this->resetAfterTest(true); + + $params = array('format' => 'topics', 'numsections' => 5, 'startdate' => 1445644800); + $course = $this->getDataGenerator()->create_course($params); + // This test is executed after 'numsections' option was already removed, add it manually. + $DB->insert_record('course_format_options', ['courseid' => $course->id, 'format' => 'topics', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '5']); + + // There are 6 sections in the course (0-section and sections 1, ... 5). + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + format_topics_upgrade_remove_numsections(); + + // There are still 6 sections in the course. + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + } + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_delete_empty() { + global $DB; + + $this->resetAfterTest(true); + + // Set default number of sections to 10. + set_config('numsections', 10, 'moodlecourse'); + + $params1 = array('format' => 'topics', 'numsections' => 5, 'startdate' => 1445644800); + $course1 = $this->getDataGenerator()->create_course($params1); + $params2 = array('format' => 'topics', 'numsections' => 20, 'startdate' => 1445644800); + $course2 = $this->getDataGenerator()->create_course($params2); + // This test is executed after 'numsections' option was already removed, add it manually and + // set it to be 2 less than actual number of sections. + $DB->insert_record('course_format_options', ['courseid' => $course1->id, 'format' => 'topics', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '3']); + + // There are 6 sections in the first course (0-section and sections 1, ... 5). + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course1->id])); + // There are 21 sections in the second course. + $this->assertEquals(21, $DB->count_records('course_sections', ['course' => $course2->id])); + + format_topics_upgrade_remove_numsections(); + + // Two sections were deleted in the first course. + $this->assertEquals(4, $DB->count_records('course_sections', ['course' => $course1->id])); + // The second course was reset to 11 sections (default plus 0-section). + $this->assertEquals(11, $DB->count_records('course_sections', ['course' => $course2->id])); + + } + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_hide_non_empty() { + global $DB; + + $this->resetAfterTest(true); + + $params = array('format' => 'topics', 'numsections' => 5, 'startdate' => 1445644800); + $course = $this->getDataGenerator()->create_course($params); + + // Add a module to the second last section. + $cm = $this->getDataGenerator()->create_module('forum', ['course' => $course->id, 'section' => 4]); + + // This test is executed after 'numsections' option was already removed, add it manually and + // set it to be 2 less than actual number of sections. + $DB->insert_record('course_format_options', ['courseid' => $course->id, 'format' => 'topics', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '3']); + + // There are 6 sections. + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + format_topics_upgrade_remove_numsections(); + + // One section was deleted and one hidden. + $this->assertEquals(5, $DB->count_records('course_sections', ['course' => $course->id])); + $this->assertEquals(0, $DB->get_field('course_sections', 'visible', ['course' => $course->id, 'section' => 4])); + // The module is still visible. + $this->assertEquals(1, $DB->get_field('course_modules', 'visible', ['id' => $cm->cmid])); + } +} diff --git a/course/format/topics/version.php b/course/format/topics/version.php index beee812daf6..7946242c962 100644 --- a/course/format/topics/version.php +++ b/course/format/topics/version.php @@ -25,6 +25,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016120500; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2017020200; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2016112900; // Requires this Moodle version. $plugin->component = 'format_topics'; // Full name of the plugin (used for diagnostics). From 98d9af3cdbe1fe2e0986c406aab634761003da01 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 2 Feb 2017 16:21:51 +0800 Subject: [PATCH 3/4] MDL-57769 format_weeks: remove numsections option --- course/format/weeks/db/upgrade.php | 47 +++++++ course/format/weeks/db/upgradelib.php | 117 ++++++++++++++++ course/format/weeks/format.php | 4 +- course/format/weeks/lib.php | 78 +++-------- .../tests/behat/edit_delete_sections.feature | 33 +---- .../format/weeks/tests/format_weeks_test.php | 29 +--- .../weeks/tests/format_weeks_upgrade_test.php | 128 ++++++++++++++++++ course/format/weeks/version.php | 2 +- 8 files changed, 319 insertions(+), 119 deletions(-) create mode 100644 course/format/weeks/db/upgrade.php create mode 100644 course/format/weeks/db/upgradelib.php create mode 100644 course/format/weeks/tests/format_weeks_upgrade_test.php diff --git a/course/format/weeks/db/upgrade.php b/course/format/weeks/db/upgrade.php new file mode 100644 index 00000000000..97516e77cf2 --- /dev/null +++ b/course/format/weeks/db/upgrade.php @@ -0,0 +1,47 @@ +. + +/** + * Upgrade scripts for course format "Weeks" + * + * @package format_weeks + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Upgrade script for format_weeks + * + * @param int $oldversion the version we are upgrading from + * @return bool result + */ +function xmldb_format_weeks_upgrade($oldversion) { + global $CFG, $DB; + + require_once($CFG->dirroot . '/course/format/weeks/db/upgradelib.php'); + + if ($oldversion < 2017020200) { + + // Remove 'numsections' option and hide or delete orphaned sections. + format_weeks_upgrade_remove_numsections(); + + upgrade_plugin_savepoint(true, 2017020200, 'format', 'weeks'); + } + + return true; +} diff --git a/course/format/weeks/db/upgradelib.php b/course/format/weeks/db/upgradelib.php new file mode 100644 index 00000000000..26571f1a56f --- /dev/null +++ b/course/format/weeks/db/upgradelib.php @@ -0,0 +1,117 @@ +. + +/** + * Upgrade scripts for course format "Weeks" + * + * @package format_weeks + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * This method finds all courses in 'weeks' format that have actual number of sections + * bigger than their 'numsections' course format option. + * For each such course we call {@link format_weeks_upgrade_hide_extra_sections()} and + * either delete or hide "orphaned" sections. + */ +function format_weeks_upgrade_remove_numsections() { + global $DB; + + $sql1 = "SELECT c.id, max(cs.section) AS sectionsactual + FROM {course} c + JOIN {course_sections} cs ON cs.course = c.id + WHERE c.format = :format1 + GROUP BY c.id"; + + $sql2 = "SELECT c.id, n.value AS numsections + FROM {course} c + JOIN {course_format_options} n ON n.courseid = c.id AND n.format = :format1 AND n.name = :numsections AND n.sectionid = 0 + WHERE c.format = :format2"; + + $params = ['format1' => 'weeks', 'format2' => 'weeks', 'numsections' => 'numsections']; + + $actual = $DB->get_records_sql_menu($sql1, $params); + $numsections = $DB->get_records_sql_menu($sql2, $params); + $needfixing = []; + + $defaultnumsections = get_config('moodlecourse', 'numsections'); + + foreach ($actual as $courseid => $sectionsactual) { + if (array_key_exists($courseid, $numsections)) { + $n = (int)$numsections[$courseid]; + } else { + $n = $defaultnumsections; + } + if ($sectionsactual > $n) { + $needfixing[$courseid] = $n; + } + } + unset($actual); + unset($numsections); + + foreach ($needfixing as $courseid => $numsections) { + format_weeks_upgrade_hide_extra_sections($courseid, $numsections); + } + + $DB->delete_records('course_format_options', ['format' => 'weeks', 'sectionid' => 0, 'name' => 'numsections']); +} + +/** + * Find all sections in the course with sectionnum bigger than numsections. + * Either delete these sections or hide them + * + * We will only delete a section if it is completely empty and all sections below + * it are also empty + * + * @param int $courseid + * @param int $numsections + */ +function format_weeks_upgrade_hide_extra_sections($courseid, $numsections) { + global $DB; + $sections = $DB->get_records_sql('SELECT id, name, summary, sequence, visible + FROM {course_sections} + WHERE course = ? AND section > ? + ORDER BY section DESC', [$courseid, $numsections]); + $candelete = true; + $tohide = []; + $todelete = []; + foreach ($sections as $section) { + if ($candelete && (!empty($section->summary) || !empty($section->sequence) || !empty($section->name))) { + $candelete = false; + } + if ($candelete) { + $todelete[] = $section->id; + } else if ($section->visible) { + $tohide[] = $section->id; + } + } + if ($todelete) { + // Delete empty sections in the end. + // This is an upgrade script - no events or cache resets are needed. + // We also know that these sections do not have any modules so it is safe to just delete records in the table. + $DB->delete_records_list('course_sections', 'id', $todelete); + } + if ($tohide) { + // Hide other orphaned sections. + // This is different from what set_section_visible() does but we want to preserve actual + // module visibility in this case. + list($sql, $params) = $DB->get_in_or_equal($tohide); + $DB->execute("UPDATE {course_sections} SET visible = 0 WHERE id " . $sql, $params); + } +} diff --git a/course/format/weeks/format.php b/course/format/weeks/format.php index 2f988551ab8..da82d080a72 100644 --- a/course/format/weeks/format.php +++ b/course/format/weeks/format.php @@ -37,9 +37,9 @@ if ($week = optional_param('week', 0, PARAM_INT)) { } // End backwards-compatible aliasing.. -// make sure all sections are created +// Make sure section 0 is created. $course = course_get_format($course)->get_course(); -course_create_sections_if_missing($course, range(0, $course->numsections)); +course_create_sections_if_missing($course, 0); $renderer = $PAGE->get_renderer('format_weeks'); diff --git a/course/format/weeks/lib.php b/course/format/weeks/lib.php index 408b5900ed7..b41a8a06e63 100644 --- a/course/format/weeks/lib.php +++ b/course/format/weeks/lib.php @@ -219,7 +219,6 @@ class format_weeks extends format_base { * * Weeks format uses the following options: * - coursedisplay - * - numsections * - hiddensections * * @param bool $foreditform @@ -230,10 +229,6 @@ class format_weeks extends format_base { if ($courseformatoptions === false) { $courseconfig = get_config('moodlecourse'); $courseformatoptions = array( - 'numsections' => array( - 'default' => $courseconfig->numsections, - 'type' => PARAM_INT, - ), 'hiddensections' => array( 'default' => $courseconfig->hiddensections, 'type' => PARAM_INT, @@ -245,21 +240,7 @@ class format_weeks extends format_base { ); } if ($foreditform && !isset($courseformatoptions['coursedisplay']['label'])) { - $courseconfig = get_config('moodlecourse'); - $sectionmenu = array(); - $max = $courseconfig->maxsections; - if (!isset($max) || !is_numeric($max)) { - $max = 52; - } - for ($i = 0; $i <= $max; $i++) { - $sectionmenu[$i] = "$i"; - } $courseformatoptionsedit = array( - 'numsections' => array( - 'label' => new lang_string('numberweeks'), - 'element_type' => 'select', - 'element_attributes' => array($sectionmenu), - ), 'hiddensections' => array( 'label' => new lang_string('hiddensections'), 'help' => 'hiddensections', @@ -300,24 +281,24 @@ class format_weeks extends format_base { * @return array array of references to the added form elements. */ public function create_edit_form_elements(&$mform, $forsection = false) { + global $COURSE; $elements = parent::create_edit_form_elements($mform, $forsection); - // Increase the number of sections combo box values if the user has increased the number of sections - // using the icon on the course page beyond course 'maxsections' or course 'maxsections' has been - // reduced below the number of sections already set for the course on the site administration course - // defaults page. This is so that the number of sections is not reduced leaving unintended orphaned - // activities / resources. - if (!$forsection) { - $maxsections = get_config('moodlecourse', 'maxsections'); - $numsections = $mform->getElementValue('numsections'); - $numsections = $numsections[0]; - if ($numsections > $maxsections) { - $element = $mform->getElement('numsections'); - for ($i = $maxsections+1; $i <= $numsections; $i++) { - $element->addOption("$i", $i); - } + if (!$forsection && (empty($COURSE->id) || $COURSE->id == SITEID)) { + // Add "numsections" element to the create course form - it will force new course to be prepopulated + // with empty sections. + // The "Number of sections" option is no longer available when editing course, instead teachers should + // delete and add sections when needed. + $courseconfig = get_config('moodlecourse'); + $max = (int)$courseconfig->maxsections; + $element = $mform->addElement('select', 'numsections', get_string('numberweeks'), range(0, $max ?: 52)); + $mform->setType('numsections', PARAM_INT); + if (is_null($mform->getElementValue('numsections'))) { + $mform->setDefault('numsections', $courseconfig->numsections); } + array_unshift($elements, $element); } + return $elements; } @@ -344,33 +325,11 @@ class format_weeks extends format_base { if (!array_key_exists($key, $data)) { if (array_key_exists($key, $oldcourse)) { $data[$key] = $oldcourse[$key]; - } else if ($key === 'numsections') { - // If previous format does not have the field 'numsections' - // and $data['numsections'] is not set, - // we fill it with the maximum section number from the DB - $maxsection = $DB->get_field_sql('SELECT max(section) from {course_sections} - WHERE course = ?', array($this->courseid)); - if ($maxsection) { - // If there are no sections, or just default 0-section, 'numsections' will be set to default - $data['numsections'] = $maxsection; - } } } } } - $changed = $this->update_format_options($data); - if ($changed && array_key_exists('numsections', $data)) { - // If the numsections was decreased, try to completely delete the orphaned sections (unless they are not empty). - $numsections = (int)$data['numsections']; - $maxsection = $DB->get_field_sql('SELECT max(section) from {course_sections} - WHERE course = ?', array($this->courseid)); - for ($sectionnum = $maxsection; $sectionnum > $numsections; $sectionnum--) { - if (!$this->delete_section($sectionnum, false)) { - break; - } - } - } - return $changed; + return $this->update_format_options($data); } /** @@ -479,6 +438,9 @@ class format_weeks extends format_base { if ($mform->elementExists($fieldnames['numsections'])) { $numsections = $mform->getElementValue($fieldnames['numsections']); $numsections = $mform->getElement($fieldnames['numsections'])->exportValue($numsections); + } else if ($this->get_courseid()) { + // For existing courses get the number of sections. + $numsections = $this->get_last_section_number(); } else { // Fallback to the default value for new courses. $numsections = get_config('moodlecourse', $fieldnames['numsections']); @@ -507,8 +469,8 @@ class format_weeks extends format_base { * @return bool */ public function allow_stealth_module_visibility($cm, $section) { - // Allow the third visibility state inside visible sections or in section 0, not allow in orphaned sections. - return !$section->section || ($section->visible && $section->section <= $this->get_course()->numsections); + // Allow the third visibility state inside visible sections or in section 0. + return !$section->section || $section->visible; } public function section_action($section, $action, $sr) { diff --git a/course/format/weeks/tests/behat/edit_delete_sections.feature b/course/format/weeks/tests/behat/edit_delete_sections.feature index 6269da09f5a..db90bbb1fc4 100644 --- a/course/format/weeks/tests/behat/edit_delete_sections.feature +++ b/course/format/weeks/tests/behat/edit_delete_sections.feature @@ -77,9 +77,7 @@ Feature: Sections can be edited and deleted in weeks format Then I should see "Are you absolutely sure you want to completely delete \"29 May - 4 June\" and all the activities it contains?" And I press "Delete" And I should not see "29 May - 4 June" - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" + And I should see "22 May - 28 May" Scenario: Deleting the middle section in weeks format Given I should see "29 May - 4 June" in the "li#section-5" "css_element" @@ -88,31 +86,4 @@ Feature: Sections can be edited and deleted in weeks format Then I should not see "29 May - 4 June" And I should not see "Test chat name" And I should see "Test choice name" in the "li#section-4" "css_element" - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" - - Scenario: Deleting the orphaned section in weeks format - When I follow "Reduce the number of sections" - Then I should see "Orphaned activities (section 5)" in the "li#section-5" "css_element" - And I delete section "5" - And I press "Delete" - And I should not see "29 May - 4 June" - And I should not see "Orphaned activities" - And "li#section-5" "css_element" should not exist - And I navigate to "Edit settings" node in "Course administration" - And I expand all fieldsets - And the field "Number of sections" matches value "4" - - Scenario: Deleting a section when orphaned section is present in weeks format - When I follow "Reduce the number of sections" - Then I should see "Orphaned activities (section 5)" in the "li#section-5" "css_element" - And "li#section-5.orphaned" "css_element" should exist - And "li#section-4.orphaned" "css_element" should not exist - And I delete section "1" - And I press "Delete" - And I should not see "Test book name" - And I should see "Orphaned activities (section 4)" in the "li#section-4" "css_element" - And "li#section-5" "css_element" should not exist - And "li#section-4.orphaned" "css_element" should exist - And "li#section-3.orphaned" "css_element" should not exist + And I should see "22 May - 28 May" diff --git a/course/format/weeks/tests/format_weeks_test.php b/course/format/weeks/tests/format_weeks_test.php index 481caa771db..32a25f6106b 100644 --- a/course/format/weeks/tests/format_weeks_test.php +++ b/course/format/weeks/tests/format_weeks_test.php @@ -36,32 +36,6 @@ require_once($CFG->dirroot . '/course/lib.php'); */ class format_weeks_testcase extends advanced_testcase { - public function test_update_course_numsections() { - global $DB; - $this->resetAfterTest(true); - - $generator = $this->getDataGenerator(); - - $course = $generator->create_course(array('numsections' => 10, 'format' => 'weeks'), - array('createsections' => true)); - $generator->create_module('assign', array('course' => $course, 'section' => 7)); - - $this->setAdminUser(); - - $this->assertEquals(11, $DB->count_records('course_sections', array('course' => $course->id))); - - // Change the numsections to 8, last two sections did not have any activities, they should be deleted. - update_course((object)array('id' => $course->id, 'numsections' => 8)); - $this->assertEquals(9, $DB->count_records('course_sections', array('course' => $course->id))); - $this->assertEquals(9, count(get_fast_modinfo($course)->get_section_info_all())); - - // Change the numsections to 5, section 8 should be deleted but section 7 should remain as it has activities. - update_course((object)array('id' => $course->id, 'numsections' => 6)); - $this->assertEquals(8, $DB->count_records('course_sections', array('course' => $course->id))); - $this->assertEquals(8, count(get_fast_modinfo($course)->get_section_info_all())); - $this->assertEquals(6, course_get_format($course)->get_course()->numsections); - } - /** * Tests for format_weeks::get_section_name method with default section names. */ @@ -224,7 +198,7 @@ class format_weeks_testcase extends advanced_testcase { * @return void */ public function test_default_course_enddate() { - global $CFG, $DB; + global $CFG, $DB, $PAGE; $this->resetAfterTest(true); @@ -247,6 +221,7 @@ class format_weeks_testcase extends advanced_testcase { 'returnurl' => new moodle_url('/'), ]; + $PAGE->set_course($course); $courseform = new testable_course_edit_form(null, $args); $courseform->definition_after_data(); diff --git a/course/format/weeks/tests/format_weeks_upgrade_test.php b/course/format/weeks/tests/format_weeks_upgrade_test.php new file mode 100644 index 00000000000..e44cc5a204f --- /dev/null +++ b/course/format/weeks/tests/format_weeks_upgrade_test.php @@ -0,0 +1,128 @@ +. + +/** + * format_weeks unit tests for upgradelib + * + * @package format_weeks + * @copyright 2015 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/course/lib.php'); +require_once($CFG->dirroot . '/course/format/weeks/db/upgradelib.php'); + +/** + * format_weeks unit tests for upgradelib + * + * @package format_weeks + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class format_weeks_upgrade_testcase extends advanced_testcase { + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_no_actions() { + global $DB; + + $this->resetAfterTest(true); + + $params = array('format' => 'weeks', 'numsections' => 5, 'startdate' => 1445644800); + $course = $this->getDataGenerator()->create_course($params); + // This test is executed after 'numsections' option was already removed, add it manually. + $DB->insert_record('course_format_options', ['courseid' => $course->id, 'format' => 'weeks', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '5']); + + // There are 6 sections in the course (0-section and sections 1, ... 5). + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + format_weeks_upgrade_remove_numsections(); + + // There are still 6 sections in the course. + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + } + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_delete_empty() { + global $DB; + + $this->resetAfterTest(true); + + // Set default number of sections to 10. + set_config('numsections', 10, 'moodlecourse'); + + $params1 = array('format' => 'weeks', 'numsections' => 5, 'startdate' => 1445644800); + $course1 = $this->getDataGenerator()->create_course($params1); + $params2 = array('format' => 'weeks', 'numsections' => 20, 'startdate' => 1445644800); + $course2 = $this->getDataGenerator()->create_course($params2); + // This test is executed after 'numsections' option was already removed, add it manually and + // set it to be 2 less than actual number of sections. + $DB->insert_record('course_format_options', ['courseid' => $course1->id, 'format' => 'weeks', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '3']); + + // There are 6 sections in the first course (0-section and sections 1, ... 5). + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course1->id])); + // There are 21 sections in the second course. + $this->assertEquals(21, $DB->count_records('course_sections', ['course' => $course2->id])); + + format_weeks_upgrade_remove_numsections(); + + // Two sections were deleted in the first course. + $this->assertEquals(4, $DB->count_records('course_sections', ['course' => $course1->id])); + // The second course was reset to 11 sections (default plus 0-section). + $this->assertEquals(11, $DB->count_records('course_sections', ['course' => $course2->id])); + + } + + /** + * Test upgrade step to remove orphaned sections. + */ + public function test_numsections_hide_non_empty() { + global $DB; + + $this->resetAfterTest(true); + + $params = array('format' => 'weeks', 'numsections' => 5, 'startdate' => 1445644800); + $course = $this->getDataGenerator()->create_course($params); + + // Add a module to the second last section. + $cm = $this->getDataGenerator()->create_module('forum', ['course' => $course->id, 'section' => 4]); + + // This test is executed after 'numsections' option was already removed, add it manually and + // set it to be 2 less than actual number of sections. + $DB->insert_record('course_format_options', ['courseid' => $course->id, 'format' => 'weeks', + 'sectionid' => 0, 'name' => 'numsections', 'value' => '3']); + + // There are 6 sections. + $this->assertEquals(6, $DB->count_records('course_sections', ['course' => $course->id])); + + format_weeks_upgrade_remove_numsections(); + + // One section was deleted and one hidden. + $this->assertEquals(5, $DB->count_records('course_sections', ['course' => $course->id])); + $this->assertEquals(0, $DB->get_field('course_sections', 'visible', ['course' => $course->id, 'section' => 4])); + // The module is still visible. + $this->assertEquals(1, $DB->get_field('course_modules', 'visible', ['id' => $cm->cmid])); + } +} diff --git a/course/format/weeks/version.php b/course/format/weeks/version.php index 7c6b84336ab..9defc109a2d 100644 --- a/course/format/weeks/version.php +++ b/course/format/weeks/version.php @@ -25,6 +25,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016120500; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2017020200; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2016112900; // Requires this Moodle version. $plugin->component = 'format_weeks'; // Full name of the plugin (used for diagnostics). From 1091687ac2821383e006a70b4247905063536541 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 23 Mar 2017 11:32:46 +0800 Subject: [PATCH 4/4] MDL-57769 backup: support removed 'numsections' in backup/restore --- backup/moodle2/backup_stepslib.php | 7 + .../behat/restore_moodle2_courses.feature | 118 +++++++++++++++++ .../restore_format_topics_plugin.class.php | 121 ++++++++++++++++++ .../restore_format_weeks_plugin.class.php | 121 ++++++++++++++++++ 4 files changed, 367 insertions(+) create mode 100644 course/format/topics/backup/moodle2/restore_format_topics_plugin.class.php create mode 100644 course/format/weeks/backup/moodle2/restore_format_weeks_plugin.class.php diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index f81e3bea8bd..83300f1c9a0 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -436,6 +436,13 @@ class backup_course_structure_step extends backup_structure_step { $courserec->$key = $value; } + // Add 'numsections' in order to be able to restore in previous versions of Moodle. + // Even though Moodle does not officially support restore into older verions of Moodle from the + // version where backup was made, without 'numsections' restoring will go very wrong. + if (!property_exists($courserec, 'numsections') && course_get_format($courserec)->uses_sections()) { + $courserec->numsections = course_get_format($courserec)->get_last_section_number(); + } + $course->set_source_array(array($courserec)); $categoryrec = $DB->get_record('course_categories', array('id' => $courserec->category)); diff --git a/backup/util/ui/tests/behat/restore_moodle2_courses.feature b/backup/util/ui/tests/behat/restore_moodle2_courses.feature index 41027904426..bac494ecd03 100644 --- a/backup/util/ui/tests/behat/restore_moodle2_courses.feature +++ b/backup/util/ui/tests/behat/restore_moodle2_courses.feature @@ -10,6 +10,7 @@ Feature: Restore Moodle 2 course backups | Course 1 | C1 | 0 | topics | 15 | 1 | | Course 2 | C2 | 0 | topics | 5 | 0 | | Course 3 | C3 | 0 | topics | 2 | 0 | + | Course 4 | C4 | 0 | topics | 20 | 0 | And the following "activities" exist: | activity | course | idnumber | name | intro | section | | assign | C3 | assign1 | Test assign name | Assign description | 1 | @@ -123,6 +124,123 @@ Feature: Restore Moodle 2 course backups And I expand all fieldsets Then the field "id_format" matches value "Topics format" And the field "Course layout" matches value "Show one section per page" + And the field "Course short name" matches value "C1_1" + And I press "Cancel" + And section "3" should be visible + And section "7" should be hidden + And section "15" should be visible + And I should see "Topic 15" + And I should not see "Topic 16" + And I should see "Test URL name" in the "Topic 3" "section" + And I should see "Test forum name" in the "Topic 1" "section" + + @javascript + Scenario: Restore a backup in an existing course keeping the target course settings + Given I add a "URL" to section "3" and I fill the form with: + | Name | Test URL name | + | Description | Test URL description | + | id_externalurl | http://www.moodle.org | + And I hide section "3" + And I hide section "7" + When I backup "Course 1" course using this options: + | Confirmation | Filename | test_backup.mbz | + And I restore "test_backup.mbz" backup into "Course 2" course using this options: + | Schema | Overwrite course configuration | No | + And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets + Then the field "id_format" matches value "Topics format" + And the field "Course short name" matches value "C2" + And the field "Course layout" matches value "Show all sections on one page" + And I press "Cancel" + And section "3" should be visible + And section "7" should be hidden + And section "15" should be visible + And I should see "Topic 15" + And I should not see "Topic 16" + And I should see "Test URL name" in the "Topic 3" "section" + And I should see "Test forum name" in the "Topic 1" "section" + + @javascript + Scenario: Restore a backup in an existing course deleting contents and retaining the backup course settings + Given I add a "URL" to section "3" and I fill the form with: + | Name | Test URL name | + | Description | Test URL description | + | id_externalurl | http://www.moodle.org | + And I hide section "3" + And I hide section "7" + When I backup "Course 1" course using this options: + | Initial | Include enrolled users | 0 | + | Confirmation | Filename | test_backup.mbz | + And I am on site homepage + And I follow "Course 2" + And I navigate to "Restore" node in "Course administration" + And I merge "test_backup.mbz" backup into the current course after deleting it's contents using this options: + | Schema | Overwrite course configuration | Yes | + And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets + Then the field "id_format" matches value "Topics format" + And the field "Course layout" matches value "Show one section per page" + And the field "Course short name" matches value "C1_1" + And I press "Cancel" + And section "3" should be hidden + And section "7" should be hidden + And section "15" should be visible + And I should see "Topic 15" + And I should not see "Topic 16" + And I should see "Test URL name" in the "Topic 3" "section" + And I should see "Test forum name" in the "Topic 1" "section" + + @javascript + Scenario: Restore a backup in an existing course deleting contents and keeping the current course settings + Given I add a "URL" to section "3" and I fill the form with: + | Name | Test URL name | + | Description | Test URL description | + | id_externalurl | http://www.moodle.org | + And I hide section "3" + And I hide section "7" + When I backup "Course 1" course using this options: + | Initial | Include enrolled users | 0 | + | Confirmation | Filename | test_backup.mbz | + And I am on site homepage + And I follow "Course 2" + And I navigate to "Restore" node in "Course administration" + And I merge "test_backup.mbz" backup into the current course after deleting it's contents using this options: + | Schema | Overwrite course configuration | No | + And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets + Then the field "id_format" matches value "Topics format" + And the field "Course short name" matches value "C2" + And the field "Course layout" matches value "Show all sections on one page" + And I press "Cancel" + And section "3" should be hidden + And section "7" should be hidden + And section "15" should be visible + And I should see "Topic 15" + And I should not see "Topic 16" + And I should see "Test URL name" in the "Topic 3" "section" + And I should see "Test forum name" in the "Topic 1" "section" + + @javascript + Scenario: Restore a backup in an existing course deleting contents decreasing the number of sections + Given I add a "URL" to section "3" and I fill the form with: + | Name | Test URL name | + | Description | Test URL description | + | id_externalurl | http://www.moodle.org | + And I hide section "3" + And I hide section "7" + When I backup "Course 1" course using this options: + | Initial | Include enrolled users | 0 | + | Confirmation | Filename | test_backup.mbz | + And I am on site homepage + And I follow "Course 4" + And I navigate to "Restore" node in "Course administration" + And I merge "test_backup.mbz" backup into the current course after deleting it's contents using this options: + | Schema | Overwrite course configuration | No | + And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets + Then the field "id_format" matches value "Topics format" + And the field "Course short name" matches value "C4" + And the field "Course layout" matches value "Show all sections on one page" And I press "Cancel" And section "3" should be hidden And section "7" should be hidden diff --git a/course/format/topics/backup/moodle2/restore_format_topics_plugin.class.php b/course/format/topics/backup/moodle2/restore_format_topics_plugin.class.php new file mode 100644 index 00000000000..0e918c94c2a --- /dev/null +++ b/course/format/topics/backup/moodle2/restore_format_topics_plugin.class.php @@ -0,0 +1,121 @@ +. + +/** + * Specialised restore for format_topics + * + * @package format_topics + * @category backup + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Specialised restore for format_topics + * + * Processes 'numsections' from the old backup files and hides sections that used to be "orphaned" + * + * @package format_topics + * @category backup + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class restore_format_topics_plugin extends restore_format_plugin { + + /** @var int */ + protected $originalnumsections = 0; + + /** + * Checks if backup file was made on Moodle before 3.3 and we should respect the 'numsections' + * and potential "orphaned" sections in the end of the course. + * + * @return bool + */ + protected function need_restore_numsections() { + $backupinfo = $this->step->get_task()->get_info(); + $backuprelease = $backupinfo->backup_release; + return version_compare($backuprelease, '3.3', 'lt'); + } + + /** + * Creates a dummy path element in order to be able to execute code after restore + * + * @return restore_path_element[] + */ + public function define_course_plugin_structure() { + global $DB; + + // Since this method is executed before the restore we can do some pre-checks here. + // In case of merging backup into existing course find the current number of sections. + $target = $this->step->get_task()->get_target(); + if (($target == backup::TARGET_CURRENT_ADDING || $target == backup::TARGET_EXISTING_ADDING) && + $this->need_restore_numsections()) { + $maxsection = $DB->get_field_sql( + 'SELECT max(section) FROM {course_sections} WHERE course = ?', + [$this->step->get_task()->get_courseid()]); + $this->originalnumsections = (int)$maxsection; + } + + // Dummy path element is needed in order for after_restore_course() to be called. + return [new restore_path_element('dummy_course', $this->get_pathfor('/dummycourse'))]; + } + + /** + * Dummy process method + */ + public function process_dummy_course() { + + } + + /** + * Executed after course restore is complete + * + * This method is only executed if course configuration was overridden + */ + public function after_restore_course() { + global $DB; + + if (!$this->need_restore_numsections()) { + // Backup file was made in Moodle 3.3 or later, we don't need to process 'numsecitons'. + return; + } + + $data = $this->connectionpoint->get_data(); + $backupinfo = $this->step->get_task()->get_info(); + if ($backupinfo->original_course_format !== 'topics' || !isset($data['tags']['numsections'])) { + // Backup from another course format or backup file does not even have 'numsections'. + return; + } + + $numsections = (int)$data['tags']['numsections']; + foreach ($backupinfo->sections as $key => $section) { + // For each section from the backup file check if it was restored and if was "orphaned" in the original + // course and mark it as hidden. This will leave all activities in it visible and available just as it was + // in the original course. + // Exception is when we restore with merging and the course already had a section with this section number, + // in this case we don't modify the visibility. + if ($this->step->get_task()->get_setting_value($key . '_included')) { + $sectionnum = (int)$section->title; + if ($sectionnum > $numsections && $sectionnum > $this->originalnumsections) { + $DB->execute("UPDATE {course_sections} SET visible = 0 WHERE course = ? AND section = ?", + [$this->step->get_task()->get_courseid(), $sectionnum]); + } + } + } + } +} diff --git a/course/format/weeks/backup/moodle2/restore_format_weeks_plugin.class.php b/course/format/weeks/backup/moodle2/restore_format_weeks_plugin.class.php new file mode 100644 index 00000000000..e6109e81bd8 --- /dev/null +++ b/course/format/weeks/backup/moodle2/restore_format_weeks_plugin.class.php @@ -0,0 +1,121 @@ +. + +/** + * Specialised restore for format_weeks + * + * @package format_weeks + * @category backup + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Specialised restore for format_weeks + * + * Processes 'numsections' from the old backup files and hides sections that used to be "orphaned" + * + * @package format_weeks + * @category backup + * @copyright 2017 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class restore_format_weeks_plugin extends restore_format_plugin { + + /** @var int */ + protected $originalnumsections = 0; + + /** + * Checks if backup file was made on Moodle before 3.3 and we should respect the 'numsections' + * and potential "orphaned" sections in the end of the course. + * + * @return bool + */ + protected function need_restore_numsections() { + $backupinfo = $this->step->get_task()->get_info(); + $backuprelease = $backupinfo->backup_release; + return version_compare($backuprelease, '3.3', 'lt'); + } + + /** + * Creates a dummy path element in order to be able to execute code after restore + * + * @return restore_path_element[] + */ + public function define_course_plugin_structure() { + global $DB; + + // Since this method is executed before the restore we can do some pre-checks here. + // In case of merging backup into existing course find the current number of sections. + $target = $this->step->get_task()->get_target(); + if (($target == backup::TARGET_CURRENT_ADDING || $target == backup::TARGET_EXISTING_ADDING) && + $this->need_restore_numsections()) { + $maxsection = $DB->get_field_sql( + 'SELECT max(section) FROM {course_sections} WHERE course = ?', + [$this->step->get_task()->get_courseid()]); + $this->originalnumsections = (int)$maxsection; + } + + // Dummy path element is needed in order for after_restore_course() to be called. + return [new restore_path_element('dummy_course', $this->get_pathfor('/dummycourse'))]; + } + + /** + * Dummy process method + */ + public function process_dummy_course() { + + } + + /** + * Executed after course restore is complete + * + * This method is only executed if course configuration was overridden + */ + public function after_restore_course() { + global $DB; + + if (!$this->need_restore_numsections()) { + // Backup file was made in Moodle 3.3 or later, we don't need to process 'numsecitons'. + return; + } + + $data = $this->connectionpoint->get_data(); + $backupinfo = $this->step->get_task()->get_info(); + if ($backupinfo->original_course_format !== 'weeks' || !isset($data['tags']['numsections'])) { + // Backup from another course format or backup file does not even have 'numsections'. + return; + } + + $numsections = (int)$data['tags']['numsections']; + foreach ($backupinfo->sections as $key => $section) { + // For each section from the backup file check if it was restored and if was "orphaned" in the original + // course and mark it as hidden. This will leave all activities in it visible and available just as it was + // in the original course. + // Exception is when we restore with merging and the course already had a section with this section number, + // in this case we don't modify the visibility. + if ($this->step->get_task()->get_setting_value($key . '_included')) { + $sectionnum = (int)$section->title; + if ($sectionnum > $numsections && $sectionnum > $this->originalnumsections) { + $DB->execute("UPDATE {course_sections} SET visible = 0 WHERE course = ? AND section = ?", + [$this->step->get_task()->get_courseid(), $sectionnum]); + } + } + } + } +}