Merge branch 'MDL-87572-main' of https://github.com/sarjona/moodle

This commit is contained in:
cescobedo
2026-02-25 14:23:13 +01:00
2 changed files with 357 additions and 48 deletions
@@ -18,6 +18,7 @@ namespace core_course\route\controller;
use core\router\route;
use core\router\require_login;
use core\url;
use core_course\cm_info;
use core_course\modinfo;
use core_course\section_info;
@@ -60,18 +61,26 @@ class course_navigation {
// The pathinfo module returns a stdClass and not a cm_info, so we need to
// get the cm_info instance from the course modinfo.
$cm = cm_info::create($cm);
$allcms = $this->get_all_cms($cm->get_modinfo());
// Note here: we don't check if the cmindex is false because the the path_module parameter will return a 404
// if the cm is not found in the course, so we can assume that it is always found.
$cmindex = array_search($cm, $allcms, true);
$cmcount = count($allcms);
$modinfo = $cm->get_modinfo();
$section = $this->get_section($cm);
$allsectioncms = $this->get_all_section_cms($modinfo, $section);
$cmindex = array_search($cm, $allsectioncms, true);
// Last element in the section should redirect to next section page
// so student can see the next section title and description.
if ($cmindex + 1 >= count($allsectioncms)) {
return $this->redirect_to_next_section($response, $modinfo, $section);
}
// Search for the next module.
$cmcount = count($allsectioncms);
for ($cmindex++; $cmindex < $cmcount; $cmindex++) {
$nextcm = $allcms[$cmindex];
$nextcm = $allsectioncms[$cmindex];
if ($this->is_valid_cm($nextcm)) {
return $this->redirect($response, $nextcm->get_url());
}
}
return $this->page_not_found($request, $response);
}
@@ -101,11 +110,22 @@ class course_navigation {
// The pathinfo module returns a stdClass and not a cm_info, so we need to
// get the cm_info instance from the course modinfo.
$cm = cm_info::create($cm);
$allcms = $this->get_all_cms($cm->get_modinfo());
$cmindex = array_search($cm, $allcms, true);
$modinfo = $cm->get_modinfo();
$section = $this->get_section($cm);
$allsectioncms = $this->get_all_section_cms($modinfo, $section);
$cmindex = array_search($cm, $allsectioncms, true);
// First element in the section should redirect to previous section page
// so student can see the previous section title and description.
if ($cmindex === 0) {
if ($result = $this->redirect_to_previous_section($response, $modinfo, $section)) {
return $result;
}
}
// Search for the previous module.
for ($cmindex--; $cmindex >= 0; $cmindex--) {
$prevcm = $allcms[$cmindex];
$prevcm = $allsectioncms[$cmindex];
if ($this->is_valid_cm($prevcm)) {
return $this->redirect($response, $prevcm->get_url());
}
@@ -125,24 +145,19 @@ class course_navigation {
}
/**
* Get all course modules in the course in order, including also activities inside sub-sections.
* Get the section of a course module; if the section is delegated, get the parent section.
*
* @param \core_course\modinfo $modinfo
* @return cm_info[]
* @param cm_info $cm
* @return \section_info
*/
private function get_all_cms(modinfo $modinfo): array {
$cms = [];
$sections = $modinfo->get_section_info_all();
foreach ($sections as $section) {
if ($section->is_delegated()) {
continue;
}
$cms = array_merge(
$cms,
$this->get_all_section_cms($modinfo, $section),
);
private function get_section(cm_info $cm): section_info {
$section = $cm->get_section_info();
if (!$section->is_delegated()) {
return $section;
}
return $cms;
// If the section is delegated, we need to get the parent section.
return $section->get_component_instance()->get_parent_section();
}
/**
@@ -167,4 +182,88 @@ class course_navigation {
}
return $sectioncms;
}
/**
* Redirect to the next/previous section view page.
*
* @param ResponseInterface $response
* @param modinfo $modinfo
* @param section_info $currentsection
* @param string $direction 'next' or 'previous' to indicate the direction of the redirection.
* @return ResponseInterface
*/
private function redirect_to_section(
ResponseInterface $response,
modinfo $modinfo,
section_info $currentsection,
string $direction = 'next',
): ?ResponseInterface {
if ($direction === 'previous') {
$section = $modinfo->get_section_info($currentsection->sectionnum);
} else {
$section = $modinfo->get_section_info($currentsection->sectionnum + 1);
}
if ($section === null) {
// No more sections.
return $this->redirect_to_course($response, $modinfo->get_course()->id);
}
if (!$section->uservisible || $section->is_delegated()) {
return $this->redirect_to_section($response, $modinfo, $section, $direction);
}
return $this->redirect(
$response,
new url('/course/section.php', ['id' => $section->id]),
);
}
/**
* Redirect to the next view page.
*
* @param ResponseInterface $response
* @param modinfo $modinfo
* @param section_info $currentsection
* @return ResponseInterface|null
*/
private function redirect_to_next_section(
ResponseInterface $response,
modinfo $modinfo,
section_info $currentsection,
): ResponseInterface {
return $this->redirect_to_section($response, $modinfo, $currentsection, 'next');
}
/**
* Redirect to the previous view page.
*
* @param ResponseInterface $response
* @param modinfo $modinfo
* @param section_info $currentsection
* @return ResponseInterface|null
*/
private function redirect_to_previous_section(
ResponseInterface $response,
modinfo $modinfo,
section_info $currentsection,
): ResponseInterface {
return $this->redirect_to_section($response, $modinfo, $currentsection, 'previous');
}
/**
* Redirect to the course view page.
*
* @param ResponseInterface $response
* @param int $courseid The course ID to redirect to.
* @return ResponseInterface
*/
private function redirect_to_course(
ResponseInterface $response,
int $courseid,
): ResponseInterface {
return $this->redirect(
$response,
new url('/course/view.php', ['id' => $courseid]),
);
}
}
@@ -39,6 +39,7 @@ final class course_navigation_test extends route_testcase {
* @param string $current
* @param array $expected
* @param string $role
* @param array $hiddensections
*/
#[DataProvider('cm_next_provider')]
public function test_cm_next(
@@ -46,6 +47,7 @@ final class course_navigation_test extends route_testcase {
string $current,
array $expected,
string $role = 'student',
array $hiddensections = [],
): void {
$this->execute_cm_navigation_test(
cmsdef: $cmsdef,
@@ -53,6 +55,7 @@ final class course_navigation_test extends route_testcase {
expected: $expected,
role: $role,
direction: 'next',
hiddensections: $hiddensections,
);
}
@@ -111,8 +114,8 @@ final class course_navigation_test extends route_testcase {
],
'current' => 'cm2',
'expected' => [
'type' => 'error',
'statuscode' => 404,
'type' => 'section',
'id' => '1',
],
];
yield 'With next module being a subsection (student)' => [
@@ -145,8 +148,7 @@ final class course_navigation_test extends route_testcase {
],
'current' => 'cm1',
'expected' => [
'type' => 'error',
'statuscode' => 404,
'type' => 'course',
],
];
yield 'With module that does not exist (student)' => [
@@ -160,6 +162,104 @@ final class course_navigation_test extends route_testcase {
'statuscode' => 404,
],
];
yield 'Sections - Simple case (teacher)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm1',
'expected' => [
'type' => 'section',
'id' => '2',
],
'role' => 'teacher',
];
yield 'Sections - Simple case (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm1',
'expected' => [
'type' => 'section',
'id' => '2',
],
];
yield 'Sections - Hidden section (student)' => [
'cmsdef' => [
['name' => 'cm0', 'options' => ['section' => 0]],
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm0',
'expected' => [
'type' => 'section',
'id' => '2', // Students cannot see the hidden section, so the next one should be the one after.
],
'hiddensections' => [1],
];
yield 'Sections - Hidden section (teacher)' => [
'cmsdef' => [
['name' => 'cm0', 'options' => ['section' => 0]],
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm0',
'expected' => [
'type' => 'section',
'id' => '2', // Non-editing teachers cannot see the hidden section, so the next one should be the one after.
],
'role' => 'teacher',
'hiddensections' => [1],
];
yield 'Sections - Hidden section (editingteacher)' => [
'cmsdef' => [
['name' => 'cm0', 'options' => ['section' => 0]],
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm0',
'expected' => [
'type' => 'section',
'id' => '1', // Teachers can see the hidden section.
],
'role' => 'editingteacher',
'hiddensections' => [1],
];
yield 'Sections - With last module in a hidden section (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm1',
'expected' => [
'type' => 'course', // As the next section is hidden, we should redirect to course page.
],
'hiddensections' => [2],
];
yield 'Sections - With last module in a hidden section (editingteacher)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm1',
'expected' => [
'type' => 'section', // Editing teachers can see the hidden section.
'id' => '2',
],
'role' => 'editingteacher',
'hiddensections' => [2],
];
yield 'Sections - Empty section (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
],
'current' => 'cm1',
'expected' => [
'type' => 'section',
'id' => '2',
],
];
}
/**
@@ -169,6 +269,7 @@ final class course_navigation_test extends route_testcase {
* @param string $current
* @param array $expected
* @param string $role
* @param array $hiddensections
*/
#[DataProvider('cm_previous_provider')]
public function test_cm_previous(
@@ -176,6 +277,7 @@ final class course_navigation_test extends route_testcase {
string $current,
array $expected,
string $role = 'student',
array $hiddensections = [],
): void {
$this->execute_cm_navigation_test(
cmsdef: $cmsdef,
@@ -183,6 +285,7 @@ final class course_navigation_test extends route_testcase {
expected: $expected,
role: $role,
direction: 'previous',
hiddensections: $hiddensections,
);
}
@@ -241,8 +344,8 @@ final class course_navigation_test extends route_testcase {
],
'current' => 'cm1',
'expected' => [
'type' => 'error',
'statuscode' => 404,
'type' => 'section',
'id' => '0',
],
];
yield 'With previous module being a subsection (student)' => [
@@ -256,6 +359,28 @@ final class course_navigation_test extends route_testcase {
'id' => 'cm1',
],
];
yield 'With previous module outside a subsection (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 2]],
['name' => 'subsection1', 'type' => 'subsection', 'options' => ['section' => 2]],
['name' => 'cm2', 'options' => ['section' => 'subsection1']],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
],
];
yield 'With a subsection with only one module (student)' => [
'cmsdef' => [
['name' => 'subsection1', 'type' => 'subsection', 'options' => ['section' => 2]],
['name' => 'cm1', 'options' => ['section' => 'subsection1']],
],
'current' => 'cm1',
'expected' => [
'type' => 'section',
'id' => '2',
],
];
yield 'With previous module being a label and subsections (student)' => [
'cmsdef' => [
['name' => 'subsection1', 'type' => 'subsection'],
@@ -275,8 +400,8 @@ final class course_navigation_test extends route_testcase {
],
'current' => 'cm2',
'expected' => [
'type' => 'error',
'statuscode' => 404,
'type' => 'section',
'id' => '2',
],
];
yield 'With module that does not exist (student)' => [
@@ -290,6 +415,67 @@ final class course_navigation_test extends route_testcase {
'statuscode' => 404,
],
];
yield 'Sections - Simple case (teacher)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm2',
'expected' => [
'type' => 'section',
'id' => '2',
],
'role' => 'teacher',
];
yield 'Sections - Simple case (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm2',
'expected' => [
'type' => 'section',
'id' => '2',
],
];
yield 'Sections - Hidden section (student)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm2',
'expected' => [
'type' => 'section',
'id' => '2',
],
'hiddensections' => [1],
];
yield 'Sections - Hidden section (editingteacher)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm2',
'expected' => [
'type' => 'section',
'id' => '2',
],
'role' => 'editingteacher',
'hiddensections' => [1],
];
yield 'Sections - With module in a hidden section (editingteacher)' => [
'cmsdef' => [
['name' => 'cm1', 'options' => ['section' => 1]],
['name' => 'cm2', 'options' => ['section' => 2]],
],
'current' => 'cm2',
'expected' => [
'type' => 'section',
'id' => '2', // Teachers can see the hidden section.
],
'role' => 'editingteacher',
'hiddensections' => [2],
];
}
/**
@@ -300,6 +486,8 @@ final class course_navigation_test extends route_testcase {
* @param array $expected
* @param string $role
* @param string $direction
* @param int $numsections
* @param array $hiddensections
*/
protected function execute_cm_navigation_test(
array $cmsdef,
@@ -307,10 +495,16 @@ final class course_navigation_test extends route_testcase {
array $expected,
string $role = 'student',
string $direction = 'next',
int $numsections = 2,
array $hiddensections = [],
): void {
$this->resetAfterTest();
$generator = $this->getDataGenerator();
$course = $generator->create_course(['numsections' => 2]);
$course = $generator->create_course(['numsections' => $numsections]);
foreach ($hiddensections as $sectiontohide) {
$sectioninfo = get_fast_modinfo($course)->get_section_info($sectiontohide);
\core_courseformat\formatactions::section($course)->update($sectioninfo, ['visible' => false]);
}
$user = $generator->create_and_enrol($course, $role);
$cms = [];
foreach ($cmsdef as $cmdef) {
@@ -322,6 +516,7 @@ final class course_navigation_test extends route_testcase {
);
}
$cmid = $cms[$current]->cmid ?? 9999; // If we cannot find it we will test the error case of not found.
$this->setUser($user);
$response = $this->process_request(
'GET',
@@ -362,22 +557,37 @@ final class course_navigation_test extends route_testcase {
string $location
): void {
$coursemodinfo = modinfo::instance($courseid);
if ($elementtype === 'cm') {
$cms = $coursemodinfo->get_cms();
$cm = null;
foreach ($cms as $activitycm) {
if ($activitycm->get_name() == $elementid) {
$cm = $activitycm;
break;
switch ($elementtype) {
case 'cm':
$cms = $coursemodinfo->get_cms();
$cm = null;
foreach ($cms as $activitycm) {
if ($activitycm->get_name() == $elementid) {
$cm = $activitycm;
break;
}
}
}
$this->assertNotEmpty($cm, "The course module with name {$elementid} should be found.");
$this->assertEquals(
$cm->url,
new url($location)
);
} else {
$this->fail('Unknown expected element type ' . $elementtype);
$this->assertNotEmpty($cm, "The course module with name {$elementid} should be found.");
$this->assertEquals(
$cm->url,
new url($location)
);
break;
case 'section':
$sectioninfo = $coursemodinfo->get_section_info($elementid);
$this->assertEquals(
new url('/course/section.php', ['id' => $sectioninfo->id]),
new url($location)
);
break;
case 'course':
$this->assertEquals(
new url('/course/view.php', ['id' => $courseid]),
new url($location)
);
break;
default:
$this->fail('Unknown expected element type ' . $elementtype);
}
}