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

This commit is contained in:
cescobedo
2026-03-06 11:40:15 +01:00
10 changed files with 481 additions and 28 deletions
@@ -0,0 +1,14 @@
issueNumber: MDL-87984
notes:
core_course:
- message: >-
The `cm_info` class now includes `get_navigation_url()`,
`set_navigation_url(?url $url)`, and `reset_navigation_url()` methods,
allowing activities to explicitly define, override, or suppress their
navigation URL. This customisation can be managed within the
`cm_info_dynamic callback`. By setting the navigation URL to null, a
module can be effectively excluded from the linear navigation flow,
such as the automatic "Previous" and "Next" routing URLs. In cases where
no override is specified, `get_navigation_url()` will return the default
`$cm->url` by fallback.
type: improved
+53
View File
@@ -168,6 +168,10 @@ use core\output\html_writer;
* Calculated on request
* @property-read string $content Content to display on main (view) page - calculated on request
* @property-read url|null $url URL to link to for this module, or null if it doesn't have a view page - calculated on request
* @property-read url|null $navigationurl URL to access from course navigation, or null if it doesn't have it.
* By default, this is the same as $url but it can be set separately by module if needed. For instance, when the module
* opens in a new window, $navigationurl will force the module view page, instead of the URL that opens in a new window.
* Calculated on request.
* @property-read string $extraclasses Extra CSS classes to add to html output for this activity on main page
* C'alculated on request
* @property-read string $onclick Content of HTML on-click attribute already escaped - calculated on request
@@ -456,6 +460,16 @@ class cm_info implements IteratorAggregate {
*/
private $url;
/**
* @var url|null The navigation URL for this course module, if any.
*/
private $navigationurl;
/**
* @var bool True if the navigation URL was modified, false otherwise.
*/
private $navigationurlmodified;
/**
* @var string
*/
@@ -534,6 +548,7 @@ class cm_info implements IteratorAggregate {
*/
private static $standardproperties = [
'url' => 'get_url',
'navigationurl' => 'get_navigation_url',
'content' => 'get_content',
'extraclasses' => 'get_extra_classes',
'onclick' => 'get_on_click',
@@ -727,6 +742,44 @@ class cm_info implements IteratorAggregate {
return $this->url;
}
/**
* Get the navigation URL for this course module.
* This method retrieves the navigation URL for the course module
*
* @return url|null The navigation URL for this course module, or null if navigationurl is not available.
*/
public function get_navigation_url(): ?url {
$this->obtain_dynamic_data();
// If the navigation URL was not modified, return the URL.
if (!$this->navigationurlmodified) {
return $this->url;
}
return $this->navigationurl;
}
/**
* Sets the navigation URL for this course module.
*
* @param url|null $navigationurl The navigation URL to set, or null to unset it.
*/
public function set_navigation_url(?url $navigationurl): void {
$this->check_not_view_only();
$this->navigationurl = $navigationurl;
$this->navigationurlmodified = true;
}
/**
* Resets the navigation URL for this course module.
* After calling this method, the navigation URL will be the same as the URL returned by get_url().
*/
public function reset_navigation_url(): void {
$this->check_not_view_only();
$this->navigationurl = null;
$this->navigationurlmodified = false;
}
/**
* Obtains content to display on main (view) page.
* Note: Will collect view data, if not already obtained.
@@ -77,7 +77,7 @@ class course_navigation {
for ($cmindex++; $cmindex < $cmcount; $cmindex++) {
$nextcm = $allsectioncms[$cmindex];
if ($this->is_valid_cm($nextcm)) {
return $this->redirect($response, $nextcm->get_url());
return $this->redirect($response, $nextcm->get_navigation_url());
}
}
return $this->redirect_to_course($response, $cm->get_course()->id);
@@ -126,7 +126,7 @@ class course_navigation {
for ($cmindex--; $cmindex >= 0; $cmindex--) {
$prevcm = $allsectioncms[$cmindex];
if ($this->is_valid_cm($prevcm)) {
return $this->redirect($response, $prevcm->get_url());
return $this->redirect($response, $prevcm->get_navigation_url());
}
}
return $this->redirect_to_course($response, $cm->get_course()->id);
@@ -141,7 +141,7 @@ class course_navigation {
private function is_valid_cm(cm_info $cm): bool {
return
// Skip modules that don't have a URL (like labels).
!empty($cm->get_url())
!empty($cm->get_navigation_url())
// Skip modules that are not visible to the user.
&& $cm->is_visible_on_course_page()
// Skip modules that are not displayable.
@@ -800,7 +800,7 @@ final class cmactions_test extends \advanced_testcase {
// We ignore obvious differences and also sections information as it is already tested above (and
// can differ due to section movements).
$ignoredproperties = ['id', 'url', 'instance', 'added', 'context', 'section', 'sectionid', 'sectionnum'];
$ignoredproperties = ['id', 'url', 'navigationurl', 'instance', 'added', 'context', 'section', 'sectionid', 'sectionnum'];
// Make sure they are the same, except obvious id changes.
foreach ($modinfo->get_cm($cmid) as $prop => $value) {
if (in_array($prop, $ignoredproperties, true)) {
@@ -814,7 +814,7 @@ final class cmactions_test extends \advanced_testcase {
$value = $newname;
}
}
$this->assertEquals($value, $newcm->$prop);
$this->assertEquals($value, $newcm->$prop, "Property '$prop' does not match between original and duplicated cm");
}
}
@@ -66,6 +66,9 @@ final class course_navigation_test extends route_testcase {
* @return \Generator
*/
public static function cm_next_provider(): \Generator {
global $CFG;
require_once("$CFG->libdir/resourcelib.php");
$emailavailability = '{"op":"&","c":[{"type":"profile","sf":"email","op":"isequalto","v":"';
yield 'Simple case (teacher)' => [
'cmsdef' => [
@@ -525,6 +528,157 @@ final class course_navigation_test extends route_testcase {
['section' => 2, 'available' => $emailavailability . '[email protected]"}],"showc":[false]}'],
],
];
yield 'Resource: Display auto (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_AUTO]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display embed (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_EMBED]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display frame (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_FRAME]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display new (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_NEW]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display download (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_DOWNLOAD]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display open (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_OPEN]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display popup (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'resource', 'options' => [
'display' => RESOURCELIB_DISPLAY_POPUP,
'popupwidth' => 800,
'popupheight' => 600,
]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display auto (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_AUTO]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display embed (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_EMBED]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display frame (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_FRAME]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display new (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_NEW]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display open (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_OPEN]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display popup (student)' => [
'cmsdef' => [
['name' => 'cm1'],
['name' => 'cm2', 'type' => 'url', 'options' => [
'display' => RESOURCELIB_DISPLAY_POPUP,
'popupwidth' => 800,
'popupheight' => 600,
]],
],
'current' => 'cm1',
'expected' => [
'id' => 'cm2',
'params' => ['id', 'forceview'],
],
];
yield 'With module not supporting FEATURE_CAN_DISPLAY (student)' => [
'cmsdef' => [
['name' => 'cm1'],
@@ -636,6 +790,9 @@ final class course_navigation_test extends route_testcase {
* @return \Generator
*/
public static function cm_previous_provider(): \Generator {
global $CFG;
require_once("$CFG->libdir/resourcelib.php");
$emailavailability = '{"op":"&","c":[{"type":"profile","sf":"email","op":"isequalto","v":"';
yield 'Simple case (teacher)' => [
'cmsdef' => [
@@ -1086,6 +1243,157 @@ final class course_navigation_test extends route_testcase {
['section' => 2, 'available' => $emailavailability . '[email protected]"}],"showc":[false]}'],
],
];
yield 'Resource: Display auto (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_AUTO]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display embed (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_EMBED]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display frame (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_FRAME]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display new (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_NEW]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display download (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_DOWNLOAD]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display open (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => ['display' => RESOURCELIB_DISPLAY_OPEN]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'Resource: Display popup (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'resource', 'options' => [
'display' => RESOURCELIB_DISPLAY_POPUP,
'popupwidth' => 800,
'popupheight' => 600,
]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display auto (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_AUTO]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display embed (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_EMBED]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display frame (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_FRAME]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display new (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_NEW]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display open (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => ['display' => RESOURCELIB_DISPLAY_OPEN]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'URL: Display popup (student)' => [
'cmsdef' => [
['name' => 'cm1', 'type' => 'url', 'options' => [
'display' => RESOURCELIB_DISPLAY_POPUP,
'popupwidth' => 800,
'popupheight' => 600,
]],
['name' => 'cm2'],
],
'current' => 'cm2',
'expected' => [
'id' => 'cm1',
'params' => ['id', 'forceview'],
],
];
yield 'With module not supporting FEATURE_CAN_DISPLAY (student)' => [
'cmsdef' => [
['name' => 'cm1'],
@@ -1187,6 +1495,7 @@ final class course_navigation_test extends route_testcase {
): void {
$this->resetAfterTest();
set_config('allowstealth', 1);
$this->setAdminUser();
$generator = $this->getDataGenerator();
$course = $generator->create_course(['numsections' => $numsections]);
@@ -1248,7 +1557,8 @@ final class course_navigation_test extends route_testcase {
$expected['type'] ?? 'cm',
$expected['id'] ?? '',
$course->id,
$location[0]
$location[0],
$expected['params'] ?? [],
);
}
@@ -1259,14 +1569,17 @@ final class course_navigation_test extends route_testcase {
* @param string $elementid
* @param int $courseid
* @param string $location
* @param array $expectedparams
*/
protected function assert_redirected_url(
string $elementtype,
string $elementid,
int $courseid,
string $location
string $location,
array $expectedparams = [],
): void {
$coursemodinfo = modinfo::instance($courseid);
$navigationurl = null;
switch ($elementtype) {
case 'cm':
$cms = $coursemodinfo->get_cms();
@@ -1278,27 +1591,35 @@ final class course_navigation_test extends route_testcase {
}
}
$this->assertNotEmpty($cm, "The course module with name {$elementid} should be found.");
$this->assertEquals(
$cm->url,
new url($location)
);
$navigationurl = $cm->navigationurl;
break;
case 'section':
$sectioninfo = $coursemodinfo->get_section_info($elementid);
$this->assertEquals(
course_get_url($courseid, $sectioninfo, ['navigation' => true]),
new url($location)
);
$navigationurl = course_get_url($courseid, $sectioninfo, ['navigation' => true]);
break;
case 'course':
$this->assertEquals(
course_get_url($courseid),
new url($location)
);
$navigationurl = course_get_url($courseid);
break;
default:
$this->fail('Unknown expected element type ' . $elementtype);
}
$this->assertEquals(
$navigationurl,
new url($location),
);
// Check for expected parameters in the redirection URL (only when specified).
if (!empty($expectedparams)) {
$actualparams = array_keys((new url($location))->params());
sort($actualparams);
sort($expectedparams);
$this->assertEquals(
$expectedparams,
$actualparams,
"The URL parameter names do not match.\n" .
"Expected: " . implode(', ', $expectedparams) . "\n" .
"Actual: " . implode(', ', $actualparams),
);
}
}
/**
+2 -1
View File
@@ -160,9 +160,10 @@ class util {
ResponseInterface $response,
string|url $url,
): ResponseInterface {
$location = ($url instanceof url) ? $url->out(false) : $url;
return $response
->withStatus(302)
->withHeader('Location', (string) $url);
->withHeader('Location', $location);
}
/**
+13
View File
@@ -624,3 +624,16 @@ function mod_resource_get_path_from_pluginfile(string $filearea, array $args): a
'filepath' => $filepath,
];
}
/**
* Sets dynamic information about a course module.
*
* @param cm_info $cm
*/
function mod_resource_cm_info_dynamic(cm_info $cm) {
// Update the navigation URL to guarantee the user will see the content even if the module
// is set to open in a new window or popup.
$cm->set_navigation_url(
new url($cm->get_navigation_url(), ['forceview' => 1])
);
}
+23 -4
View File
@@ -25,9 +25,6 @@
*/
namespace mod_resource;
defined('MOODLE_INTERNAL') || die();
/**
* Unit tests for mod_resource lib
*
@@ -38,7 +35,6 @@ defined('MOODLE_INTERNAL') || die();
* @since Moodle 3.0
*/
final class lib_test extends \advanced_testcase {
/**
* Prepares things before this test case is initialised
* @return void
@@ -263,6 +259,29 @@ final class lib_test extends \advanced_testcase {
$this->assertTrue($actionevent2->is_actionable());
}
/**
* Test that mod_resource_cm_info_dynamic overrides the navigation URL.
*
* @covers ::mod_resource_cm_info_dynamic
*/
public function test_cm_info_dynamic(): void {
$this->resetAfterTest();
$this->setAdminUser();
// Create the activity.
$course = $this->getDataGenerator()->create_course();
$module = $this->getDataGenerator()->create_module('resource', ['course' => $course->id]);
$cminfo = \cm_info::create(get_coursemodule_from_instance('resource', $module->id));
$navigationurl = $cminfo->get_navigation_url();
$this->assertInstanceOf(\core\url::class, $navigationurl);
$this->assertNotEquals(
$cminfo->get_url(),
$navigationurl,
);
}
/**
* Creates an action event.
*
+13
View File
@@ -412,3 +412,16 @@ function mod_url_core_calendar_provide_event_action(calendar_event $event,
true
);
}
/**
* Sets dynamic information about a course module.
*
* @param cm_info $cm
*/
function mod_url_cm_info_dynamic(cm_info $cm) {
// Update the navigation URL to guarantee the user will see the content even if the module
// is set to open in a new window or popup.
$cm->set_navigation_url(
new \core\url($cm->get_navigation_url(), ['forceview' => 1])
);
}
+23 -4
View File
@@ -24,9 +24,6 @@
*/
namespace mod_url;
defined('MOODLE_INTERNAL') || die();
/**
* mod_url tests
*
@@ -36,7 +33,6 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
final class lib_test extends \advanced_testcase {
/**
* Prepares things before this test case is initialised
* @return void
@@ -246,6 +242,29 @@ final class lib_test extends \advanced_testcase {
$this->assertNull($actionevent);
}
/**
* Test that mod_url_cm_info_dynamic overrides the navigation URL.
*
* @covers ::mod_url_cm_info_dynamic
*/
public function test_cm_info_dynamic(): void {
$this->resetAfterTest();
$this->setAdminUser();
// Create the activity.
$course = $this->getDataGenerator()->create_course();
$module = $this->getDataGenerator()->create_module('url', ['course' => $course->id]);
$cminfo = \cm_info::create(get_coursemodule_from_instance('url', $module->id));
$navigationurl = $cminfo->get_navigation_url();
$this->assertInstanceOf(\core\url::class, $navigationurl);
$this->assertNotEquals(
$cminfo->get_url(),
$navigationurl,
);
}
/**
* Creates an action event.
*