From 30cdddb017894966c02fa4672bcb630390cee323 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 21 Sep 2016 12:59:32 +0100 Subject: [PATCH] MDL-55129 mod_assign: Allow plugins to handle configs for external MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now in the get_assignments WS all the settings for assignment plugins (submission and feedback) are returned, this was bad because: - It was returning settings for plugins that may not be enabled - It was returning settings that would be a security risk (a third party plugin including a password or credentials for connecting a remote site) - Some values was returned "raw" without further processing, for example, maxsubmissionsizebytes returned 0 when using the plugin default value instead a valid size - Assignment plugins implemented an API for communication with the external API, see for example: get_external_parameters, it didn’t make sense had an API for the received parameters and not the returned data. --- mod/assign/assignmentplugin.php | 11 ++++++ mod/assign/externallib.php | 36 +++++++++++-------- mod/assign/feedback/comments/locallib.php | 9 +++++ mod/assign/feedback/editpdf/locallib.php | 10 ++++++ mod/assign/feedback/file/locallib.php | 9 +++++ mod/assign/feedback/offline/locallib.php | 9 +++++ mod/assign/submission/comments/locallib.php | 10 ++++++ mod/assign/submission/file/locallib.php | 19 ++++++++++ mod/assign/submission/onlinetext/locallib.php | 9 +++++ mod/assign/tests/externallib_test.php | 1 + mod/assign/upgrade.txt | 2 ++ 11 files changed, 110 insertions(+), 15 deletions(-) diff --git a/mod/assign/assignmentplugin.php b/mod/assign/assignmentplugin.php index ca62eb93b10..8861aece6aa 100644 --- a/mod/assign/assignmentplugin.php +++ b/mod/assign/assignmentplugin.php @@ -677,4 +677,15 @@ abstract class assign_plugin { public function is_configurable() { return true; } + + /** + * Return the plugin configs for external functions, + * in some cases the configs will need formatting or be returned only if the current user has some capabilities enabled. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return array(); + } } diff --git a/mod/assign/externallib.php b/mod/assign/externallib.php index ce4451e3b6d..f87d91d6b0a 100644 --- a/mod/assign/externallib.php +++ b/mod/assign/externallib.php @@ -401,19 +401,27 @@ class mod_assign_external extends external_api { ); continue; } - $configrecords = $DB->get_recordset('assign_plugin_config', array('assignment' => $module->assignmentid)); + + $assign = new assign($context, null, null); + + // Get configurations for only enabled plugins. + $plugins = $assign->get_submission_plugins(); + $plugins = array_merge($plugins, $assign->get_feedback_plugins()); + $configarray = array(); - foreach ($configrecords as $configrecord) { - $configarray[] = array( - 'id' => $configrecord->id, - 'assignment' => $configrecord->assignment, - 'plugin' => $configrecord->plugin, - 'subtype' => $configrecord->subtype, - 'name' => $configrecord->name, - 'value' => $configrecord->value - ); + foreach ($plugins as $plugin) { + if ($plugin->is_enabled() && $plugin->is_visible()) { + $configrecords = $plugin->get_config_for_external(); + foreach ($configrecords as $name => $value) { + $configarray[] = array( + 'plugin' => $plugin->get_type(), + 'subtype' => $plugin->get_subtype(), + 'name' => $name, + 'value' => $value + ); + } + } } - $configrecords->close(); $assignment = array( 'id' => $module->assignmentid, @@ -446,8 +454,6 @@ class mod_assign_external extends external_api { ); // Return or not intro and file attachments depending on the plugin settings. - $assign = new assign($context, null, null); - if ($assign->show_intro()) { list($assignment['intro'], $assignment['introformat']) = external_format_text($module->intro, @@ -540,8 +546,8 @@ class mod_assign_external extends external_api { private static function get_assignments_config_structure() { return new external_single_structure( array( - 'id' => new external_value(PARAM_INT, 'assign_plugin_config id'), - 'assignment' => new external_value(PARAM_INT, 'assignment id'), + 'id' => new external_value(PARAM_INT, 'assign_plugin_config id', VALUE_OPTIONAL), + 'assignment' => new external_value(PARAM_INT, 'assignment id', VALUE_OPTIONAL), 'plugin' => new external_value(PARAM_TEXT, 'plugin'), 'subtype' => new external_value(PARAM_TEXT, 'subtype'), 'name' => new external_value(PARAM_TEXT, 'name'), diff --git a/mod/assign/feedback/comments/locallib.php b/mod/assign/feedback/comments/locallib.php index ab5a7c09aeb..db2deae6a9c 100644 --- a/mod/assign/feedback/comments/locallib.php +++ b/mod/assign/feedback/comments/locallib.php @@ -517,4 +517,13 @@ class assign_feedback_comments extends assign_feedback_plugin { return array('assignfeedbackcomments_editor' => $editorstructure); } + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/feedback/editpdf/locallib.php b/mod/assign/feedback/editpdf/locallib.php index a9a74e5e027..6f38686c8bf 100644 --- a/mod/assign/feedback/editpdf/locallib.php +++ b/mod/assign/feedback/editpdf/locallib.php @@ -375,4 +375,14 @@ class assign_feedback_editpdf extends assign_feedback_plugin { public function supports_review_panel() { return true; } + + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/feedback/file/locallib.php b/mod/assign/feedback/file/locallib.php index 41e0c6c058d..d5c3973346d 100644 --- a/mod/assign/feedback/file/locallib.php +++ b/mod/assign/feedback/file/locallib.php @@ -688,4 +688,13 @@ class assign_feedback_file extends assign_feedback_plugin { ); } + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/feedback/offline/locallib.php b/mod/assign/feedback/offline/locallib.php index f41848b5b19..f293ce1bd09 100644 --- a/mod/assign/feedback/offline/locallib.php +++ b/mod/assign/feedback/offline/locallib.php @@ -406,4 +406,13 @@ class assign_feedback_offline extends assign_feedback_plugin { return false; } + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/submission/comments/locallib.php b/mod/assign/submission/comments/locallib.php index 787f3f5f666..16ac94d7c81 100644 --- a/mod/assign/submission/comments/locallib.php +++ b/mod/assign/submission/comments/locallib.php @@ -188,4 +188,14 @@ class assign_submission_comments extends assign_submission_plugin { public function is_configurable() { return false; } + + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/submission/file/locallib.php b/mod/assign/submission/file/locallib.php index c55f48a0fb3..f0943bca99b 100644 --- a/mod/assign/submission/file/locallib.php +++ b/mod/assign/submission/file/locallib.php @@ -547,4 +547,23 @@ class assign_submission_file extends assign_submission_plugin { ) ); } + + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + global $CFG; + + $configs = $this->get_config(); + + // Get a size in bytes. + if ($configs->maxsubmissionsizebytes == 0) { + $configs->maxsubmissionsizebytes = get_max_upload_file_size($CFG->maxbytes, $this->assignment->get_course()->maxbytes, + get_config('assignsubmission_file', 'maxbytes')); + } + return (array) $configs; + } } diff --git a/mod/assign/submission/onlinetext/locallib.php b/mod/assign/submission/onlinetext/locallib.php index aa852ee56bb..8cd05284fd2 100644 --- a/mod/assign/submission/onlinetext/locallib.php +++ b/mod/assign/submission/onlinetext/locallib.php @@ -665,6 +665,15 @@ class assign_submission_onlinetext extends assign_submission_plugin { } } + /** + * Return the plugin configs for external functions. + * + * @return array the list of settings + * @since Moodle 3.2 + */ + public function get_config_for_external() { + return (array) $this->get_config(); + } } diff --git a/mod/assign/tests/externallib_test.php b/mod/assign/tests/externallib_test.php index fb87d8da48b..d4c17e040fb 100644 --- a/mod/assign/tests/externallib_test.php +++ b/mod/assign/tests/externallib_test.php @@ -229,6 +229,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->assertEquals($course1->id, $assignment['course']); $this->assertEquals('lightwork assignment', $assignment['name']); $this->assertContains('the assignment intro text here', $assignment['intro']); + $this->assertNotEmpty($assignment['configs']); // Check the url of the file attatched. $this->assertRegExp('@"' . $CFG->wwwroot . '/webservice/pluginfile.php/\d+/mod_assign/intro/intro\.txt"@', $assignment['intro']); $this->assertEquals(1, $assignment['markingworkflow']); diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index 23d65745b87..2b60118c6d5 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -13,6 +13,8 @@ This files describes API changes in the assign code. Please, note that previously the filename was part of the filepath field, now they are separated. * Submission and feedback plugins can now specify file areas related to their configuration data, which will then be included in backup and restore; see assign_plugin::get_config_file_areas(). +* Submission and feedback plugins must now return the specific list of configs available for external functions, + this can be done implementing the new assign plugin method get_config_for_external() === 3.1 === * The feedback plugins now need to implement the is_feedback_modified() method. The default is to return true