diff --git a/enrol/lti/classes/local/ltiadvantage/entity/ags_info.php b/enrol/lti/classes/local/ltiadvantage/entity/ags_info.php index bd900ad4499..3b9ce728122 100644 --- a/enrol/lti/classes/local/ltiadvantage/entity/ags_info.php +++ b/enrol/lti/classes/local/ltiadvantage/entity/ags_info.php @@ -38,7 +38,7 @@ class ags_info { /** @var string Scope for posting scores.*/ private const SCOPES_SCORES_POST = 'https://purl.imsglobal.org/spec/lti-ags/scope/score'; - /** @var \moodle_url The service URL used to get/put lineitems*/ + /** @var \moodle_url|null The service URL used to get/put lineitems, if supported*/ private $lineitemsurl; /** @var \moodle_url|null The lineitemurl, which is only present when a single lineitem is supported.*/ @@ -56,11 +56,17 @@ class ags_info { /** * The ags_info constructor. * - * @param \moodle_url $lineitemsurl The service URL used to get/put lineitems. + * @param \moodle_url|null $lineitemsurl The service URL used to get/put lineitems, if supported. * @param \moodle_url|null $lineitemurl The lineitemurl, which is only present when a single lineitem is supported. * @param array $scopes The array of supported scopes for this service instance. */ - private function __construct(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl, array $scopes) { + private function __construct(?\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl, array $scopes) { + + // Platforms may support just lineitemurl, just lineitemsurl or both. At least one of the two is required. + if (is_null($lineitemsurl) && is_null($lineitemurl)) { + throw new \coding_exception("Missing lineitem or lineitems URL"); + } + $this->lineitemsurl = $lineitemsurl; $this->lineitemurl = $lineitemurl; $this->validate_scopes($scopes); @@ -69,12 +75,12 @@ class ags_info { /** * Factory method to create a new ags_info instance. * - * @param \moodle_url $lineitemsurl The service URL used to get/put lineitems. + * @param \moodle_url|null $lineitemsurl The service URL used to get/put lineitems, if supported. * @param \moodle_url|null $lineitemurl The lineitemurl, which is only present when a single lineitem is supported. * @param array $scopes The array of supported scopes for this service instance. * @return ags_info the object instance. */ - public static function create(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl = null, + public static function create(?\moodle_url $lineitemsurl = null, ?\moodle_url $lineitemurl = null, array $scopes = []): ags_info { return new self($lineitemsurl, $lineitemurl, $scopes); } @@ -110,11 +116,11 @@ class ags_info { } /** - * Get the url for querying line items. + * Get the url for querying line items, if supported. * * @return \moodle_url the url. */ - public function get_lineitemsurl(): \moodle_url { + public function get_lineitemsurl(): ?\moodle_url { return $this->lineitemsurl; } diff --git a/enrol/lti/classes/local/ltiadvantage/entity/resource_link.php b/enrol/lti/classes/local/ltiadvantage/entity/resource_link.php index d6e0a81e9fe..9acd18e4265 100644 --- a/enrol/lti/classes/local/ltiadvantage/entity/resource_link.php +++ b/enrol/lti/classes/local/ltiadvantage/entity/resource_link.php @@ -163,11 +163,11 @@ class resource_link { /** * Add grade service information to this resource_link instance. * - * @param \moodle_url $lineitemsurl the service URL for get/put of line items. + * @param \moodle_url|null $lineitemsurl the service URL for get/put of line items, if supported. * @param \moodle_url|null $lineitemurl the service URL if only a single line item is present in the platform. * @param string[] $scopes the string array of grade service scopes which may be used by the service. */ - public function add_grade_service(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl = null, array $scopes = []) { + public function add_grade_service(?\moodle_url $lineitemsurl = null, ?\moodle_url $lineitemurl = null, array $scopes = []) { $this->gradeservice = ags_info::create($lineitemsurl, $lineitemurl, $scopes); } diff --git a/enrol/lti/classes/local/ltiadvantage/repository/resource_link_repository.php b/enrol/lti/classes/local/ltiadvantage/repository/resource_link_repository.php index 4972b7a96ef..975924b92b8 100644 --- a/enrol/lti/classes/local/ltiadvantage/repository/resource_link_repository.php +++ b/enrol/lti/classes/local/ltiadvantage/repository/resource_link_repository.php @@ -50,7 +50,7 @@ class resource_link_repository { $record->id ); - if ($record->lineitemsservice) { + if ($record->lineitemsservice || $record->lineitemservice) { $scopes = []; if ($record->lineitemscope) { $lineitemscopes = json_decode($record->lineitemscope); @@ -65,7 +65,7 @@ class resource_link_repository { $scopes[] = $record->scorescope; } $resourcelink->add_grade_service( - new \moodle_url($record->lineitemsservice), + $record->lineitemsservice ? new \moodle_url($record->lineitemsservice) : null, $record->lineitemservice ? new \moodle_url($record->lineitemservice) : null, $scopes ); @@ -112,7 +112,7 @@ class resource_link_repository { 'ltideploymentid' => $resourcelink->get_deploymentid(), 'resourceid' => $resourcelink->get_resourceid(), 'lticontextid' => $resourcelink->get_contextid(), - 'lineitemsservice' => $gradeservice ? $gradeservice->get_lineitemsurl()->out(false) : null, + 'lineitemsservice' => null, 'lineitemservice' => null, 'lineitemscope' => null, 'resultscope' => $gradeservice ? $gradeservice->get_resultscope() : null, @@ -121,7 +121,10 @@ class resource_link_repository { 'nrpsserviceversions' => $nrpservice ? json_encode($nrpservice->get_service_versions()) : null ]; - if ($gradeservice && ($lineitemurl = $gradeservice->get_lineitemurl())) { + if ($gradeservice && ($lineitemsurl = $gradeservice->get_lineitemsurl())) { + $record['lineitemsservice'] = $lineitemsurl->out(false); + } + if ($gradeservice && ($lineitemurl = $gradeservice->get_lineitemurl())) { $record['lineitemservice'] = $lineitemurl->out(false); } if ($gradeservice && ($lineitemscopes = $gradeservice->get_lineitemscope())) { diff --git a/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php b/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php index 4a7de30821f..ec6ddf0d1ac 100644 --- a/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php +++ b/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php @@ -163,12 +163,12 @@ class tool_launch_service { $context ? $context->get_id() : null ); } - // AGS. If the lineitemsurl is missing, it means the tool has no access to the endpoint. + // Add the AGS configuration for the resource link. // See: http://www.imsglobal.org/spec/lti-ags/v2p0#assignment-and-grade-service-claim. - if ($launchdata->ags && $launchdata->ags['lineitems']) { + if ($launchdata->ags && (!empty($launchdata->ags['lineitems']) || !empty($launchdata->ags['lineitem']))) { $resourcelink->add_grade_service( - new \moodle_url($launchdata->ags['lineitems']), - isset($launchdata->ags['lineitem']) ? new \moodle_url($launchdata->ags['lineitem']) : null, + !empty($launchdata->ags['lineitems']) ? new \moodle_url($launchdata->ags['lineitems']) : null, + !empty($launchdata->ags['lineitem']) ? new \moodle_url($launchdata->ags['lineitem']) : null, $launchdata->ags['scope'] ); } diff --git a/enrol/lti/tests/local/ltiadvantage/entity/ags_info_test.php b/enrol/lti/tests/local/ltiadvantage/entity/ags_info_test.php index fd1827b567c..87d0bba422c 100644 --- a/enrol/lti/tests/local/ltiadvantage/entity/ags_info_test.php +++ b/enrol/lti/tests/local/ltiadvantage/entity/ags_info_test.php @@ -286,6 +286,35 @@ class ags_info_test extends \advanced_testcase { 'exceptionmessage' => "Scope must be a string value" ] ], + 'Claim contains a single lineitem URL only with valid scopes' => [ + 'args' => [ + 'lineitemsurl' => null, + 'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), + 'scopes' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/score' + ] + ], + 'expectations' => [ + 'valid' => true, + 'lineitemscope' => null, + 'scorescope' => 'https://purl.imsglobal.org/spec/lti-ags/scope/score', + 'resultscope' => null + ] + ], + 'Claim contains no lineitems URL or lineitem URL' => [ + 'args' => [ + 'lineitemsurl' => null, + 'lineitemurl' => null, + 'scopes' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/score' + ] + ], + 'expectations' => [ + 'valid' => false, + 'exception' => \coding_exception::class, + 'exceptionmessage' => "Missing lineitem or lineitems URL" + ] + ], ]; } } diff --git a/enrol/lti/tests/local/ltiadvantage/entity/resource_link_test.php b/enrol/lti/tests/local/ltiadvantage/entity/resource_link_test.php index 887720d5698..33963c63345 100644 --- a/enrol/lti/tests/local/ltiadvantage/entity/resource_link_test.php +++ b/enrol/lti/tests/local/ltiadvantage/entity/resource_link_test.php @@ -115,23 +115,96 @@ class resource_link_test extends \advanced_testcase { /** * Test confirming that a grade service instance can be added to the object instance. * + * @param array $args the array of method arguments + * @param array $expected the array of expectations + * @dataProvider add_grade_service_provider * @covers ::add_grade_service */ - public function test_add_grade_service() { + public function test_add_grade_service(array $args, array $expected) { $reslink = resource_link::create('res-link-id-123', 24, 44); $this->assertNull($reslink->get_grade_service()); - $reslink->add_grade_service( - new \moodle_url('https://platform.example.org/10/lineitems'), - new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), - ['https://purl.imsglobal.org/spec/lti-ags/scope/lineitem'] - ); + + if (!$expected['valid']) { + $this->expectException($expected['exception']); + } + $reslink->add_grade_service(...array_values($args)); $gradeservice = $reslink->get_grade_service(); $this->assertInstanceOf(ags_info::class, $gradeservice); - $this->assertEquals(new \moodle_url('https://platform.example.org/10/lineitems'), - $gradeservice->get_lineitemsurl()); - $this->assertEquals(new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), - $gradeservice->get_lineitemurl()); - $this->assertEquals(['https://purl.imsglobal.org/spec/lti-ags/scope/lineitem'], $gradeservice->get_scopes()); + $this->assertEquals($args['lineitemsurl'], $gradeservice->get_lineitemsurl()); + $this->assertEquals($args['lineitemurl'], $gradeservice->get_lineitemurl()); + $this->assertEquals($args['scope'], $gradeservice->get_scopes()); + } + + /** + * Data provider for testing the add_grade_service method. + * + * @return array the array of test case data. + */ + public function add_grade_service_provider(): array { + return [ + 'Valid, both URLs, some scopes' => [ + 'args' => [ + 'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'), + 'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), + 'scope' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem', + 'https://purl.imsglobal.org/spec/lti-ags/scope/score' + ] + ], + 'expected' => [ + 'valid' => true, + ] + ], + 'Valid, only coupled line item URL, some scopes' => [ + 'args' => [ + 'lineitemsurl' => null, + 'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), + 'scope' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/score' + ] + ], + 'expected' => [ + 'valid' => true, + ] + ], + 'Valid, only decoupled line items URL, some scopes' => [ + 'args' => [ + 'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'), + 'lineitemurl' => null, + 'scope' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem', + 'https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly', + 'https://purl.imsglobal.org/spec/lti-ags/scope/score', + ] + ], + 'expected' => [ + 'valid' => true, + ] + ], + 'Valid, URLs without any scopes' => [ + 'args' => [ + 'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'), + 'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'), + 'scope' => [] + ], + 'expected' => [ + 'valid' => true, + ] + ], + 'Invalid, missing both URLs' => [ + 'args' => [ + 'lineitemsurl' => null, + 'lineitemurl' => null, + 'scope' => [ + 'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem' + ] + ], + 'expected' => [ + 'valid' => false, + 'exception' => \coding_exception::class + ] + ], + ]; } /**