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);