Skip to content

Commit

Permalink
MDL-62599 mod_lti: Revert service configuration parameters
Browse files Browse the repository at this point in the history 
Code to add configuration settings for LTI services reverted to current release code which injects them directly into the edit form.  This has the risk of name clashes and changes being made to elements not created by the service.
  • Loading branch information
@spvickers @snake
spvickers authored and snake committed May 6, 2019
1 parent d930ef0 commit a464331
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 141 deletions.
17 changes: 2 additions & 15 deletions mod/lti/classes/local/ltiservice/service_base.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,9 @@ public function get_permitted_scopes() {
* Returns the configuration options for this service.
*
* @param \MoodleQuickForm $mform Moodle quickform object definition
* @deprecated since Moodle 3.7 MDL-62599 - please do not use this function any more.
* @see service_base::get_configuration_elements()
*/
public function get_configuration_options(&$mform) {
debugging('get_configuration_options() has been deprecated, ' .
'please use service_base::get_configuration_elements() instead.', DEBUG_DEVELOPER);

}

/**
Expand All @@ -235,17 +232,7 @@ public function get_configuration_options(&$mform) {
* @return array Names list of the parameters that the service will be saving in the configuration
*/
public function get_configuration_parameter_names() {
debugging('get_configuration_options() has been deprecated, ' .
'please use service_base::get_configuration_elements() instead.', DEBUG_DEVELOPER);
return array();
}

/**
* Create form element for gradebook sync add/edit page.
*
* @return array of \HTML_QuickForm_element Form elements
*/
public function get_configuration_elements() {
debugging('get_configuration_parameter_names() has been deprecated.', DEBUG_DEVELOPER);
return array();
}

Expand Down
10 changes: 1 addition & 9 deletions mod/lti/db/upgrade.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,6 @@ function xmldb_lti_upgrade($oldversion) {
}

if ($oldversion < 2019031302) {
$DB->set_field('lti_types_config', 'name', 'ltiservice_memberships', array('name' => 'memberships'));
$DB->set_field('lti_types_config', 'name', 'ltiservice_gradebookservices', array('name' => 'gradesynchronization'));

// Lti savepoint reached.
upgrade_mod_savepoint(true, 2019031302, 'lti');
}

if ($oldversion < 2019031303) {
// Define field typeid to be added to lti_tool_settings.
$table = new xmldb_table('lti_tool_settings');
$field = new xmldb_field('typeid', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'toolproxyid');
Expand All @@ -231,7 +223,7 @@ function xmldb_lti_upgrade($oldversion) {
$dbman->add_key($table, $key);

// Lti savepoint reached.
upgrade_mod_savepoint(true, 2019031303, 'lti');
upgrade_mod_savepoint(true, 2019031302, 'lti');
}

return true;
Expand Down
41 changes: 2 additions & 39 deletions mod/lti/edit_form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ public function definition() {
$mform->addHelpButton('lti_secureicon', 'secure_icon_url', 'lti');

if (!$istool) {
$this->get_lti_services($mform);
// Display the lti advantage services.
$this->get_lti_advantage_services($mform);
}

if (!$istool) {
Expand Down Expand Up @@ -328,12 +329,8 @@ public function get_data() {
* Generates the lti advantage extra configuration adding it to the mform
*
* @param MoodleQuickForm $mform
* @deprecated since Moodle 3.7 MDL-62599 - please do not use this function any more.
* @see mod_lti_edit_types_form::get_lti_services()
*/
public function get_lti_advantage_services(&$mform) {
debugging('get_lti_advantage_services() has been deprecated, ' .
'please use mod_lti_edit_types_form::get_lti_services() instead.', DEBUG_DEVELOPER);
// For each service add the label and get the array of configuration.
$services = lti_get_services();
$mform->addElement('header', 'services', get_string('services', 'lti'));
Expand All @@ -342,38 +339,4 @@ public function get_lti_advantage_services(&$mform) {
$service->get_configuration_options($mform);
}
}

/**
* Generates the lti services extra configuration adding it to the mform
*
* @param MoodleQuickForm $mform
*/
public function get_lti_services(&$mform) {
$services = lti_get_services();
$mform->addElement('header', 'services', get_string('services', 'lti'));
foreach ($services as $service) {
$elements = $service->get_configuration_elements();
if (!empty($elements)) {
foreach ($elements as $name => $element) {
$id = $service->get_component_id();
if (!empty($name)) {
$id = "{$id}_{$name}";
}
$element->setName($id);
$mform->addelement($element);
$type = PARAM_TEXT;
if ($element instanceof \MoodleQuickForm_select) {
if (is_int($element->_options[0]['attr']['value'])) {
$type = PARAM_INT;
}
}
$mform->setType($id, $type);
$mform->setDefault($id, 0);
if (empty($name)) {
$mform->addHelpButton($id, $id, $id);
}
}
}
}
}
}
6 changes: 2 additions & 4 deletions mod/lti/locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2522,10 +2522,8 @@ function lti_get_type_type_config($id) {
}

// Get the parameters from the LTI services.
$services = lti_get_services();
foreach ($services as $service) {
$name = $service->get_component_id();
if (isset($config[$name])) {
foreach ($config as $name => $value) {
if (strpos($name, 'ltiservice_') === 0) {
$type->{$name} = $config[$name];
}
}
Expand Down
30 changes: 0 additions & 30 deletions mod/lti/service/basicoutcomes/classes/local/service/basicoutcomes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,34 +81,4 @@ public function get_permitted_scopes() {

}

/**
* Return an array of key/values to add to the launch parameters.
*
* @param string $messagetype 'basic-lti-launch-request' or 'ContentItemSelectionRequest'.
* @param string $courseid The course id.
* @param string $user The user id.
* @param string $typeid The tool lti type id.
* @param string $modlti The id of the lti activity.
*
* The type is passed to check the configuration
* and not return parameters for services not used.
*
* @return array of key/value pairs to add as launch parameters.
*/
public function get_launch_parameters($messagetype, $courseid, $user, $typeid, $modlti = null) {
global $COURSE;

$launchparameters = array();
$tool = lti_get_type_type_config($typeid);
if (isset($tool->{$this->get_component_id()})) {
if ($tool->{$this->get_component_id()} == parent::SERVICE_ENABLED) {
if ($messagetype === 'basic-lti-launch-request') {
$launchparameters['lis_outcome_service_url'] = '$BasicOutcome.url';
$launchparameters['lis_result_sourcedid'] = '$BasicOutcome.sourcedId';
}
}
}
return $launchparameters;
}

}
36 changes: 8 additions & 28 deletions mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,41 +116,21 @@ public function get_permitted_scopes() {
* Adds form elements for gradebook sync add/edit page.
*
* @param \MoodleQuickForm $mform Moodle quickform object definition
* @deprecated since Moodle 3.7 MDL-62599 - please do not use this function any more.
* @see gradebookservices::get_configuration_elements()
*/
public function get_configuration_options(&$mform) {
debugging('get_configuration_options() has been deprecated, ' .
'please use service_base::get_configuration_elements() instead.', DEBUG_DEVELOPER);

$selectelementname = 'ltiservice_gradesynchronization';
$identifier = 'grade_synchronization';
$options = [
$this->get_string('nevergs'),
$this->get_string('partialgs'),
$this->get_string('alwaysgs')
];
$mform->addElement('select', $selectelementname, $this->get_string($identifier), $options);
$mform->setType($selectelementname, 'int');
$mform->setDefault($selectelementname, 0);
$mform->addHelpButton($selectelementname, $identifier, self::SERVICE_NAME);
}

/**
* Create form element for gradebook sync add/edit page.
*
* @return array of \HTML_QuickForm_element Form elements
*/
public function get_configuration_elements() {
$elements = array();
$options = [
get_string('nevergs', $this->get_component_id()),
get_string('partialgs', $this->get_component_id()),
get_string('alwaysgs', $this->get_component_id())
];
$elements[''] = new \MoodleQuickForm_select($this->get_component_id(), get_string($this->get_component_id(),
$this->get_component_id()), $options);

return $elements;
$mform->addElement('select', $selectelementname, get_string($identifier, $this->get_component_id()), $options);
$mform->setType($selectelementname, 'int');
$mform->setDefault($selectelementname, 0);
$mform->addHelpButton($selectelementname, $identifier, $this->get_component_id());
}

/**
Expand All @@ -173,9 +153,9 @@ public function get_launch_parameters($messagetype, $courseid, $user, $typeid, $
$this->set_type(lti_get_type($typeid));
$this->set_typeconfig(lti_get_type_config($typeid));
// Only inject parameters if the service is enabled for this tool.
if (isset($this->get_typeconfig()[$this->get_component_id()])) {
if ($this->get_typeconfig()[$this->get_component_id()] == self::GRADEBOOKSERVICES_READ ||
$this->get_typeconfig()[$this->get_component_id()] == self::GRADEBOOKSERVICES_FULL) {
if (isset($this->get_typeconfig()['ltiservice_gradesynchronization'])) {
if ($this->get_typeconfig()['ltiservice_gradesynchronization'] == self::GRADEBOOKSERVICES_READ ||
$this->get_typeconfig()['ltiservice_gradesynchronization'] == self::GRADEBOOKSERVICES_FULL) {
// Check for used in context is only needed because there is no explicit site tool - course relation.
if ($this->is_allowed_in_context($typeid, $courseid)) {
if (is_null($modlti)) {
Expand Down
4 changes: 2 additions & 2 deletions mod/lti/service/gradebookservices/lang/en/ltiservice_gradebookservices.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
*/

$string['alwaysgs'] = 'Use this service for grade sync and column management ';
$string['ltiservice_gradebookservices'] = 'IMS LTI Assignment and Grade Services';
$string['ltiservice_gradebookservices_help'] = 'Whether to use the IMS LTI Assignment and Grade Services to synchronise grades instead of the Basic Outcomes service.
$string['grade_synchronization'] = 'IMS LTI Assignment and Grade Services';
$string['grade_synchronization_help'] = 'Whether to use the IMS LTI Assignment and Grade Services to synchronise grades instead of the Basic Outcomes service.
* **Do not use this service** - Basic Outcomes features and configuration will be used
* **Use this service for grade sync only** - The service will populate the grades in an already existing gradebook column, but it will not be able to create new columns
Expand Down
15 changes: 8 additions & 7 deletions mod/lti/service/memberships/classes/local/service/memberships.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,20 +493,21 @@ private static function is_allowed_field_set($toolconfig, $instanceconfig, $fiel
}

/**
* Create form element for membership add/edit page.
* Adds form elements for membership add/edit page.
*
* @return array of \HTML_QuickForm_element Form elements
* @param \MoodleQuickForm $mform
*/
public function get_configuration_elements() {
$elements = array();
public function get_configuration_options(&$mform) {
$elementname = $this->get_component_id();
$options = [
get_string('notallow', $this->get_component_id()),
get_string('allow', $this->get_component_id())
];
$elements[''] = new \MoodleQuickForm_select($this->get_component_id(), get_string($this->get_component_id(),
$this->get_component_id()), $options);

return $elements;
$mform->addElement('select', $elementname, get_string($elementname, $this->get_component_id()), $options);
$mform->setType($elementname, 'int');
$mform->setDefault($elementname, 0);
$mform->addHelpButton($elementname, $elementname, $this->get_component_id());
}

/**
Expand Down
15 changes: 8 additions & 7 deletions mod/lti/service/toolsettings/classes/local/service/toolsettings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,21 @@ public static function settings_to_json($settings, $simpleformat, $type, $resour
}

/**
* Create form element for membership add/edit page.
* Adds form elements for membership add/edit page.
*
* @return array of \HTML_QuickForm_element Form elements
* @param \MoodleQuickForm $mform
*/
public function get_configuration_elements() {
$elements = array();
public function get_configuration_options(&$mform) {
$elementname = $this->get_component_id();
$options = [
get_string('notallow', $this->get_component_id()),
get_string('allow', $this->get_component_id())
];
$elements[''] = new \MoodleQuickForm_select($this->get_component_id(), get_string($this->get_component_id(),
$this->get_component_id()), $options);

return $elements;
$mform->addElement('select', $elementname, get_string($elementname, $this->get_component_id()), $options);
$mform->setType($elementname, 'int');
$mform->setDefault($elementname, 0);
$mform->addHelpButton($elementname, $elementname, $this->get_component_id());
}

/**
Expand Down

0 comments on commit a464331

Please sign in to comment.