From 9468fe44e2d9a0a75b07b5b0950c77d9e5f8e215 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Wed, 19 Mar 2025 13:05:34 -0500 Subject: [PATCH 01/15] DEF-2790: Adding support for sub plugins --- classes/plugininfo/trigger.php | 34 ++++++++++++++++++++++++++++++++ classes/workflow_manager.php | 36 ++++++++++++++++++++++++++-------- custom/.gitkeep | 0 db/subplugins.json | 5 +++++ 4 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 classes/plugininfo/trigger.php create mode 100644 custom/.gitkeep create mode 100644 db/subplugins.json diff --git a/classes/plugininfo/trigger.php b/classes/plugininfo/trigger.php new file mode 100644 index 0000000..fd771b8 --- /dev/null +++ b/classes/plugininfo/trigger.php @@ -0,0 +1,34 @@ +. + +/** + * Subplugin info class. + * + * @package tool_trigger + * @author David Castro + * @copyright 2024 Moodle US + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace tool_trigger\plugininfo; + +use core\plugininfo\base, moodle_url, part_of_admin_tree, admin_settingpage; + +/** + * Plugin info class for logging store plugins. + */ +class trigger extends base { + +} diff --git a/classes/workflow_manager.php b/classes/workflow_manager.php index 69dd2fb..f5729ca 100644 --- a/classes/workflow_manager.php +++ b/classes/workflow_manager.php @@ -226,16 +226,36 @@ public function get_step_class_names($steptype = null) { $matchedsteps = array(); $matches = array(); - foreach ($steptypes as $steptype) { - $stepdir = __DIR__ . '/steps/' . $steptype; - $handle = opendir($stepdir); - while (($file = readdir($handle)) !== false) { - preg_match('/\b(?!base)(.*step)/', $file, $matches); - foreach ($matches as $classname) { - $matchedsteps[] = '\tool_trigger\steps\\' . $steptype . '\\' . $classname; + $plugins = \core_component::get_plugin_list('trigger'); + + $dirs = [ + (object)[ + 'path' => __DIR__, + 'namespace' => 'tool_trigger', + ] + ]; + foreach ($plugins as $plugin => $dir) { + $dirs[] = (object)[ + 'path' => $dir . '/classes', + 'namespace' => "trigger_$plugin", + ]; + } + + foreach ($dirs as $dir) { + foreach ($steptypes as $steptype) { + $stepdir = $dir->path . '/steps/' . $steptype; + if (!is_dir($stepdir)) { + continue; + } + $handle = opendir($stepdir); + while (($file = readdir($handle)) !== false) { + preg_match('/\b(?!base)(.*step)/', $file, $matches); + foreach ($matches as $classname) { + $matchedsteps[] = '\\' . $dir->namespace . '\steps\\' . $steptype . '\\' . $classname; + } } + closedir($handle); } - closedir($handle); } $matchedsteps = array_unique($matchedsteps); diff --git a/custom/.gitkeep b/custom/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/db/subplugins.json b/db/subplugins.json new file mode 100644 index 0000000..f0b3302 --- /dev/null +++ b/db/subplugins.json @@ -0,0 +1,5 @@ +{ + "plugintypes": { + "trigger": "admin\/tool\/trigger\/custom" + } +} \ No newline at end of file From bca723784d7a6279a4477245f8b418cc3e7111b8 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Mon, 5 Aug 2024 13:29:39 -0500 Subject: [PATCH 02/15] DEF-2790: Adding support for subplugins admin setting page --- classes/plugininfo/trigger.php | 24 ++++++++++++++++++++++++ settings.php | 4 ++++ 2 files changed, 28 insertions(+) diff --git a/classes/plugininfo/trigger.php b/classes/plugininfo/trigger.php index fd771b8..afabd20 100644 --- a/classes/plugininfo/trigger.php +++ b/classes/plugininfo/trigger.php @@ -30,5 +30,29 @@ * Plugin info class for logging store plugins. */ class trigger extends base { +/** + * Loads plugin settings to the settings tree + * + * This function usually includes settings.php file in plugins folder. + * Alternatively it can create a link to some settings page (instance of admin_externalpage) + * + * @param \part_of_admin_tree $adminroot + * @param string $parentnodename + * @param bool $hassiteconfig whether the current user has moodle/site:config capability + */ + public function load_settings(\part_of_admin_tree $adminroot, $parentnodename, $hassiteconfig) { + global $CFG, $USER, $DB, $OUTPUT, $PAGE; // In case settings.php wants to refer to them. + $ADMIN = $adminroot; // May be used in settings.php. + $plugininfo = $this; // Also can be used inside settings.php. + + if (!$this->is_installed_and_upgraded()) { + return; + } + + if (!$hassiteconfig or !file_exists($this->full_path('settings.php'))) { + return; + } + include($this->full_path('settings.php')); + } } diff --git a/settings.php b/settings.php index 4aaf60d..c11264e 100644 --- a/settings.php +++ b/settings.php @@ -91,5 +91,9 @@ $ADMIN->add('tool_trigger', $settings); $ADMIN->add('tool_trigger', $workflowsettings); + foreach (core_plugin_manager::instance()->get_plugins_of_type('trigger') as $plugin) { + $plugin->load_settings($ADMIN, 'trigger', $hassiteconfig); + } + $settings = null; } From b78dc232b01c9b230251dd62e46326ddaa67c697 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Thu, 24 Oct 2024 10:55:06 -0500 Subject: [PATCH 03/15] DEF-3316: Adding form setting to fix validation on ws params --- classes/plugininfo/trigger.php | 5 ++-- .../steps/actions/webservice_action_step.php | 27 ++++++++++++++++--- lang/en/tool_trigger.php | 2 ++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/classes/plugininfo/trigger.php b/classes/plugininfo/trigger.php index afabd20..9f30fd3 100644 --- a/classes/plugininfo/trigger.php +++ b/classes/plugininfo/trigger.php @@ -30,8 +30,9 @@ * Plugin info class for logging store plugins. */ class trigger extends base { -/** - * Loads plugin settings to the settings tree + + /** + * Loads plugin settings to the settings tree. * * This function usually includes settings.php file in plugins folder. * Alternatively it can create a link to some settings page (instance of admin_externalpage) diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index f937316..c637822 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -50,6 +50,15 @@ class webservice_action_step extends base_action_step { 'exception', ]; + /** + * Special parameter that are only alpha(letters). + * + * @var array + */ + private static $specialparams = [ + 'country' + ]; + protected function init() { $this->functionname = $this->data['functionname']; $this->username = $this->data['username']; @@ -201,6 +210,12 @@ public function form_definition_extra($form, $mform, $customdata) { $mform->addElement('textarea', 'params', get_string('webserviceactionparams', 'tool_trigger'), $attributes); $mform->setType('params', PARAM_RAW_TRIMMED); $mform->addHelpButton('params', 'webserviceactionparams', 'tool_trigger'); + + // Params. + $attributes = ['cols' => '50', 'rows' => '5']; + $mform->addElement('textarea', 'alphaparams', get_string('webserviceactionalphaparmas', 'tool_trigger'), $attributes); + $mform->setType('alphaparams', PARAM_RAW_TRIMMED); + $mform->addHelpButton('alphaparams', 'webserviceactionalphaparmas', 'tool_trigger'); } /** @@ -249,10 +264,14 @@ public function form_validation($data, $files) { $function = external_api::external_function_info($data['functionname']); $errorfield = 'params'; - - // Fill template fields with a number. - $transformcallback = function() { - return 0; + $alphaparams = explode(',', $data['alphaparams']); + // Fill template fields with a number. Some params are special and only allow letters. + $transformcallback = function($matches) use($alphaparams) { + if (in_array($matches[1], $alphaparams)) { + return ''; + } else { + return 0; + } }; // Cannot use redner_datafields since we need to know of the diff --git a/lang/en/tool_trigger.php b/lang/en/tool_trigger.php index e689ae9..ade01fe 100644 --- a/lang/en/tool_trigger.php +++ b/lang/en/tool_trigger.php @@ -292,6 +292,8 @@ $string['webserviceactionfunctionname'] = 'Function'; $string['webserviceactionfunctionname_help'] = 'The webservice function to be called. See the API Documentation'; +$string['webserviceactionalphaparmas'] = 'WS alpha parameters'; +$string['webserviceactionalphaparmas_help'] = 'Almost all webservices parameters are ALPHANUMERIC(can contain number and letters), when validating the form a mock array is created and the values are set to 0. That works for most of the cases, but if the parameter is just an ALPHA(just letters) this will not work. This field is to set the ALPHA parameters mock as a string when validating. Add here the parameters separated by comma if you are getting an "alpha" type error when saving the form.'; $string['webserviceactionusername'] = 'Who'; $string['webserviceactionusername_help'] = 'The user (username) who this step will be performed in the context of. This defaults to the main admin user if not explicitly set'; $string['webserviceactionparams'] = 'Parameters'; From 7db832b818f288cab423f90b57aec3ac243bc6ad Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Fri, 25 Oct 2024 08:33:58 -0500 Subject: [PATCH 04/15] DEF-3316: Changing alpha parameter help text --- classes/steps/actions/webservice_action_step.php | 9 --------- lang/en/tool_trigger.php | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index c637822..3b81826 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -50,15 +50,6 @@ class webservice_action_step extends base_action_step { 'exception', ]; - /** - * Special parameter that are only alpha(letters). - * - * @var array - */ - private static $specialparams = [ - 'country' - ]; - protected function init() { $this->functionname = $this->data['functionname']; $this->username = $this->data['username']; diff --git a/lang/en/tool_trigger.php b/lang/en/tool_trigger.php index ade01fe..b12951e 100644 --- a/lang/en/tool_trigger.php +++ b/lang/en/tool_trigger.php @@ -293,7 +293,7 @@ $string['webserviceactionfunctionname'] = 'Function'; $string['webserviceactionfunctionname_help'] = 'The webservice function to be called. See the API Documentation'; $string['webserviceactionalphaparmas'] = 'WS alpha parameters'; -$string['webserviceactionalphaparmas_help'] = 'Almost all webservices parameters are ALPHANUMERIC(can contain number and letters), when validating the form a mock array is created and the values are set to 0. That works for most of the cases, but if the parameter is just an ALPHA(just letters) this will not work. This field is to set the ALPHA parameters mock as a string when validating. Add here the parameters separated by comma if you are getting an "alpha" type error when saving the form.'; +$string['webserviceactionalphaparmas_help'] = 'The form validation may fail due to some ws parameters type, if you are getting an "ALPHA" type error, add the parameter here.'; $string['webserviceactionusername'] = 'Who'; $string['webserviceactionusername_help'] = 'The user (username) who this step will be performed in the context of. This defaults to the main admin user if not explicitly set'; $string['webserviceactionparams'] = 'Parameters'; From 0ff8f027e4bd9e127540c59559c6813b2ec0c2ad Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Wed, 19 Mar 2025 13:07:03 -0500 Subject: [PATCH 05/15] DEF-3316: Ws issue fix --- .../steps/actions/webservice_action_step.php | 100 +++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index 3b81826..b551baf 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -111,7 +111,7 @@ private function run_function() { $params = $this->render_datafields($this->params); // Execute the provided function name passing with the given parameters. - $response = external_api::call_external_function($functionname, json_decode($params, true)); + $response = self::call_external_function($functionname, json_decode($params, true)); return $response; } @@ -334,4 +334,102 @@ public function transform_form_data($data) { } return $data; } + + /** + * Call an external function validating all params/returns correctly. + * + * Note that an external function may modify the state of the current page, so this wrapper + * saves and restores tha PAGE and COURSE global variables before/after calling the external function. + * This is a fork of the external_api::call_external_function method. + * Due the nature of the WS API, without a real user session the function may not work as expected. + * + * @param string $function A webservice function name. + * @param array $args Params array (named params) + * @param boolean $ajaxonly If true, an extra check will be peformed to see if ajax is required. + * @return array containing keys for error (bool), exception and data. + */ + public static function call_external_function($function, $args, $ajaxonly=false) { + global $PAGE, $COURSE, $CFG, $SITE; + + require_once($CFG->libdir . "/pagelib.php"); + + $externalfunctioninfo = \external_api::external_function_info($function); + + // Eventually this should shift into the various handlers and not be handled via config. + $readonlysession = $externalfunctioninfo->readonlysession ?? false; + if (!$readonlysession || empty($CFG->enable_read_only_sessions)) { + \core\session\manager::restart_with_write_lock($readonlysession); + } + + $currentpage = $PAGE; + $currentcourse = $COURSE; + $response = array(); + + try { + // Taken straight from from setup.php. + if (!empty($CFG->moodlepageclass)) { + if (!empty($CFG->moodlepageclassfile)) { + require_once($CFG->moodlepageclassfile); + } + $classname = $CFG->moodlepageclass; + } else { + $classname = 'moodle_page'; + } + $PAGE = new $classname(); + $COURSE = clone($SITE); + + // Validate params, this also sorts the params properly, we need the correct order in the next part. + $callable = array($externalfunctioninfo->classname, 'validate_parameters'); + $params = call_user_func($callable, + $externalfunctioninfo->parameters_desc, + $args); + $params = array_values($params); + + // Allow any Moodle plugin a chance to override this call. This is a convenient spot to + // make arbitrary behaviour customisations. The overriding plugin could call the 'real' + // function first and then modify the results, or it could do a completely separate + // thing. + $callbacks = get_plugins_with_function('override_webservice_execution'); + $result = false; + foreach ($callbacks as $plugintype => $plugins) { + foreach ($plugins as $plugin => $callback) { + $result = $callback($externalfunctioninfo, $params); + if ($result !== false) { + break 2; + } + } + } + + // If the function was not overridden, call the real one. + if ($result === false) { + $callable = array($externalfunctioninfo->classname, $externalfunctioninfo->methodname); + $result = call_user_func_array($callable, $params); + } + + // Validate the return parameters. + if ($externalfunctioninfo->returns_desc !== null) { + $callable = array($externalfunctioninfo->classname, 'clean_returnvalue'); + $result = call_user_func($callable, $externalfunctioninfo->returns_desc, $result); + } + + $response['error'] = false; + $response['data'] = $result; + } catch (\Throwable $e) { + $exception = get_exception_info($e); + unset($exception->a); + $exception->backtrace = format_backtrace($exception->backtrace, true); + if (!debugging('', DEBUG_DEVELOPER)) { + unset($exception->debuginfo); + unset($exception->backtrace); + } + $response['error'] = true; + $response['exception'] = $exception; + // Do not process the remaining requests. + } + + $PAGE = $currentpage; + $COURSE = $currentcourse; + + return $response; + } } From 716fce95a48cb215a8b456bbf486abb03a10070b Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Mon, 23 Dec 2024 16:17:38 +0000 Subject: [PATCH 06/15] DEF-3459: Fixed return empty value. --- classes/helper/datafield_manager.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/classes/helper/datafield_manager.php b/classes/helper/datafield_manager.php index 71fa316..32e9de0 100644 --- a/classes/helper/datafield_manager.php +++ b/classes/helper/datafield_manager.php @@ -162,8 +162,13 @@ public function render_datafields($templatestr, $event = null, $stepresults = nu return $value; } else { - // No match! Leave the template string in place. - return $matches[0]; + // If there is no value, return the empty value. + if (preg_match('/\{.*?\}/', $matches[0])) { + return ''; + } else { + // No match! Leave the template string in place. + return $matches[0]; + } } }; From a01afa1f2f297e063d5ebdcb0bf1e1a6f5f59c47 Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Fri, 7 Feb 2025 15:33:41 +0000 Subject: [PATCH 07/15] DEF-3564: Fix Webservice function so it returns a previous step result. --- .../steps/actions/webservice_action_step.php | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index b551baf..b646ada 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -150,11 +150,12 @@ public function execute($step, $trigger, $event, $stepresults) { try { $response = $this->run_function(); if ($response['error']) { - // Throw an exception to be propagated for proper error capture. - throw new \coding_exception(json_encode($response['exception'])); - } - $status = [true, $response]; + // Throw an error if step results are not being returned. + $stepresults['error'] = json_encode($response['exception']); + + return [false, $stepresults]; + } } catch (\Throwable $e) { // Restore the previous user to avoid any side-effects occuring in later steps / code. \core\session\manager::set_user($previoususer); @@ -174,8 +175,8 @@ public function execute($step, $trigger, $event, $stepresults) { \core\session\manager::set_user($previoususer); $SESSION = $session; - // Return the function call response as is. The shape is already normalised. - return $status; + // Return stepresults. + return [true, $stepresults]; } /** @@ -255,6 +256,7 @@ public function form_validation($data, $files) { $function = external_api::external_function_info($data['functionname']); $errorfield = 'params'; + $alphaparams = explode(',', $data['alphaparams']); // Fill template fields with a number. Some params are special and only allow letters. $transformcallback = function($matches) use($alphaparams) { @@ -357,6 +359,7 @@ public static function call_external_function($function, $args, $ajaxonly=false) // Eventually this should shift into the various handlers and not be handled via config. $readonlysession = $externalfunctioninfo->readonlysession ?? false; + if (!$readonlysession || empty($CFG->enable_read_only_sessions)) { \core\session\manager::restart_with_write_lock($readonlysession); } @@ -375,22 +378,25 @@ public static function call_external_function($function, $args, $ajaxonly=false) } else { $classname = 'moodle_page'; } + $PAGE = new $classname(); $COURSE = clone($SITE); // Validate params, this also sorts the params properly, we need the correct order in the next part. $callable = array($externalfunctioninfo->classname, 'validate_parameters'); + $params = call_user_func($callable, $externalfunctioninfo->parameters_desc, $args); - $params = array_values($params); + $params = array_values($params); // Allow any Moodle plugin a chance to override this call. This is a convenient spot to // make arbitrary behaviour customisations. The overriding plugin could call the 'real' // function first and then modify the results, or it could do a completely separate // thing. $callbacks = get_plugins_with_function('override_webservice_execution'); $result = false; + foreach ($callbacks as $plugintype => $plugins) { foreach ($plugins as $plugin => $callback) { $result = $callback($externalfunctioninfo, $params); From 20e5468d3947f5024a3b4746cc7cdf641c38e717 Mon Sep 17 00:00:00 2001 From: David Castro Date: Thu, 23 May 2024 12:39:05 -0500 Subject: [PATCH 08/15] Adding course custom fields. --- classes/steps/lookups/course_lookup_step.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/classes/steps/lookups/course_lookup_step.php b/classes/steps/lookups/course_lookup_step.php index 1b3f324..5dfb3e5 100644 --- a/classes/steps/lookups/course_lookup_step.php +++ b/classes/steps/lookups/course_lookup_step.php @@ -125,6 +125,17 @@ public function execute($step, $trigger, $event, $stepresults) { foreach ($coursedata as $key => $value) { $stepresults[$this->outputprefix . $key] = $value; } + + $handler = \core_customfield\handler::get_handler('core_course', 'course'); + $datas = $handler->get_instance_data($courseid); + foreach ($datas as $data) { + if (empty($data->get_value())) { + continue; + } + $key = $data->get_field()->get('shortname'); + $stepresults[$this->outputprefix . $key] = $data->get_value(); + } + return [true, $stepresults]; } @@ -175,7 +186,11 @@ public static function get_privacyfields() { * @return array $stepfields The fields this step provides. */ public static function get_fields() { - return self::$stepfields; + $handler = \core_customfield\handler::get_handler('core_course', 'course'); + $customfields = array_walk($handler->get_fields(), function(&$field) { + $field = $field->get('shortname'); + }); + return self::$stepfields + $customfields; } } From ddad73db9d0c2c3550a74cbf0a10232cdaa6ef4e Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 18 Mar 2025 14:20:29 -0500 Subject: [PATCH 09/15] Fixing course custom fields --- classes/steps/lookups/course_lookup_step.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/classes/steps/lookups/course_lookup_step.php b/classes/steps/lookups/course_lookup_step.php index 5dfb3e5..ec6cfb0 100644 --- a/classes/steps/lookups/course_lookup_step.php +++ b/classes/steps/lookups/course_lookup_step.php @@ -187,10 +187,11 @@ public static function get_privacyfields() { */ public static function get_fields() { $handler = \core_customfield\handler::get_handler('core_course', 'course'); - $customfields = array_walk($handler->get_fields(), function(&$field) { - $field = $field->get('shortname'); - }); + $customfields = []; + foreach ($handler->get_fields() as $field) { + $customfields[] = $field->get('shortname'); + } - return self::$stepfields + $customfields; + return array_merge(self::$stepfields, $customfields); } } From 421a68bc8a6161e70462e31e564def9da142ac5d Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 10:04:23 -0500 Subject: [PATCH 10/15] DEF-3115: Adding custom course fields test --- tests/course_lookup_step_test.php | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/course_lookup_step_test.php b/tests/course_lookup_step_test.php index ac21c1c..2bc5a74 100644 --- a/tests/course_lookup_step_test.php +++ b/tests/course_lookup_step_test.php @@ -90,6 +90,33 @@ public function test_execute_basic() { $this->assertEquals($context->id, $stepresults['course_contextid']); } + /** + * Basic test, but this time with additional custom profile field. + */ + public function test_execute_basic_with_custom_profile_fields() { + // Create user profile fields. + $customfieldg = $this->getDataGenerator()->get_plugin_generator('core_customfield'); + $category = $customfieldg->create_category(); + + $customfield = $customfieldg->create_field([ + 'categoryid' => $category->get('id'), + 'type' => 'text', + 'shortname' => 'testfield1', + 'configdata' => [], + ]); + $this->add_course_custom_profile_field_data($customfield->get('id'), $this->course->id, 'CourseFieldValue'); + $step = new \tool_trigger\steps\lookups\course_lookup_step( + json_encode([ + 'courseidfield' => 'objectid', + 'outputprefix' => 'course_' + ]) + ); + + list($status, $stepresults) = $step->execute(null, null, $this->event, []); + $this->assertTrue($status); + $this->assertEquals('CourseFieldValue', $stepresults['course_testfield1']); + } + /** * Test for exception if an invalid field name is entered. */ @@ -222,4 +249,22 @@ public function test_execute_course_id_string() { $this->assertEquals($this->course->fullname, $stepresults['course_fullname']); $this->assertEquals($context->id, $stepresults['course_contextid']); } + + public function add_course_custom_profile_field_data($fieldid, $courseid, $customfielddata) { + global $DB; + + // Add data to the custom profile fields. + $data = new \stdClass(); + $data->fieldid = $fieldid; + $data->instanceid = $courseid; + $data->value = $customfielddata; + $data->charvalue = $customfielddata; + $data->timecreated = time(); + $data->timemodified = time(); + $data->valueformat = 0; + $data->valuetrust = 0; + $data->contextid = \context_course::instance($courseid)->id; + + return $DB->insert_record('customfield_data', $data); + } } From 03299e4e4d6c77f6855335d2a333cd5067861d86 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 10:20:40 -0500 Subject: [PATCH 11/15] DEF-3115: Adding require for external_api class --- classes/steps/actions/webservice_action_step.php | 1 + 1 file changed, 1 insertion(+) diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index b646ada..d927e59 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -16,6 +16,7 @@ namespace tool_trigger\steps\actions; +require_once($CFG->libdir . '/externallib.php'); use core_external\external_api; /** From 8c656e68f4d87bd8d390f77a2406566f07f0e022 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 10:34:17 -0500 Subject: [PATCH 12/15] DEF-3115: Reverting unnecesary changes --- classes/helper/datafield_manager.php | 9 +- .../steps/actions/webservice_action_step.php | 135 ++---------------- lang/en/tool_trigger.php | 2 - tests/course_lookup_step_test.php | 14 +- 4 files changed, 20 insertions(+), 140 deletions(-) diff --git a/classes/helper/datafield_manager.php b/classes/helper/datafield_manager.php index 32e9de0..71fa316 100644 --- a/classes/helper/datafield_manager.php +++ b/classes/helper/datafield_manager.php @@ -162,13 +162,8 @@ public function render_datafields($templatestr, $event = null, $stepresults = nu return $value; } else { - // If there is no value, return the empty value. - if (preg_match('/\{.*?\}/', $matches[0])) { - return ''; - } else { - // No match! Leave the template string in place. - return $matches[0]; - } + // No match! Leave the template string in place. + return $matches[0]; } }; diff --git a/classes/steps/actions/webservice_action_step.php b/classes/steps/actions/webservice_action_step.php index d927e59..f937316 100644 --- a/classes/steps/actions/webservice_action_step.php +++ b/classes/steps/actions/webservice_action_step.php @@ -16,7 +16,6 @@ namespace tool_trigger\steps\actions; -require_once($CFG->libdir . '/externallib.php'); use core_external\external_api; /** @@ -112,7 +111,7 @@ private function run_function() { $params = $this->render_datafields($this->params); // Execute the provided function name passing with the given parameters. - $response = self::call_external_function($functionname, json_decode($params, true)); + $response = external_api::call_external_function($functionname, json_decode($params, true)); return $response; } @@ -151,12 +150,11 @@ public function execute($step, $trigger, $event, $stepresults) { try { $response = $this->run_function(); if ($response['error']) { - - // Throw an error if step results are not being returned. - $stepresults['error'] = json_encode($response['exception']); - - return [false, $stepresults]; + // Throw an exception to be propagated for proper error capture. + throw new \coding_exception(json_encode($response['exception'])); } + + $status = [true, $response]; } catch (\Throwable $e) { // Restore the previous user to avoid any side-effects occuring in later steps / code. \core\session\manager::set_user($previoususer); @@ -176,8 +174,8 @@ public function execute($step, $trigger, $event, $stepresults) { \core\session\manager::set_user($previoususer); $SESSION = $session; - // Return stepresults. - return [true, $stepresults]; + // Return the function call response as is. The shape is already normalised. + return $status; } /** @@ -203,12 +201,6 @@ public function form_definition_extra($form, $mform, $customdata) { $mform->addElement('textarea', 'params', get_string('webserviceactionparams', 'tool_trigger'), $attributes); $mform->setType('params', PARAM_RAW_TRIMMED); $mform->addHelpButton('params', 'webserviceactionparams', 'tool_trigger'); - - // Params. - $attributes = ['cols' => '50', 'rows' => '5']; - $mform->addElement('textarea', 'alphaparams', get_string('webserviceactionalphaparmas', 'tool_trigger'), $attributes); - $mform->setType('alphaparams', PARAM_RAW_TRIMMED); - $mform->addHelpButton('alphaparams', 'webserviceactionalphaparmas', 'tool_trigger'); } /** @@ -258,14 +250,9 @@ public function form_validation($data, $files) { $errorfield = 'params'; - $alphaparams = explode(',', $data['alphaparams']); - // Fill template fields with a number. Some params are special and only allow letters. - $transformcallback = function($matches) use($alphaparams) { - if (in_array($matches[1], $alphaparams)) { - return ''; - } else { - return 0; - } + // Fill template fields with a number. + $transformcallback = function() { + return 0; }; // Cannot use redner_datafields since we need to know of the @@ -337,106 +324,4 @@ public function transform_form_data($data) { } return $data; } - - /** - * Call an external function validating all params/returns correctly. - * - * Note that an external function may modify the state of the current page, so this wrapper - * saves and restores tha PAGE and COURSE global variables before/after calling the external function. - * This is a fork of the external_api::call_external_function method. - * Due the nature of the WS API, without a real user session the function may not work as expected. - * - * @param string $function A webservice function name. - * @param array $args Params array (named params) - * @param boolean $ajaxonly If true, an extra check will be peformed to see if ajax is required. - * @return array containing keys for error (bool), exception and data. - */ - public static function call_external_function($function, $args, $ajaxonly=false) { - global $PAGE, $COURSE, $CFG, $SITE; - - require_once($CFG->libdir . "/pagelib.php"); - - $externalfunctioninfo = \external_api::external_function_info($function); - - // Eventually this should shift into the various handlers and not be handled via config. - $readonlysession = $externalfunctioninfo->readonlysession ?? false; - - if (!$readonlysession || empty($CFG->enable_read_only_sessions)) { - \core\session\manager::restart_with_write_lock($readonlysession); - } - - $currentpage = $PAGE; - $currentcourse = $COURSE; - $response = array(); - - try { - // Taken straight from from setup.php. - if (!empty($CFG->moodlepageclass)) { - if (!empty($CFG->moodlepageclassfile)) { - require_once($CFG->moodlepageclassfile); - } - $classname = $CFG->moodlepageclass; - } else { - $classname = 'moodle_page'; - } - - $PAGE = new $classname(); - $COURSE = clone($SITE); - - // Validate params, this also sorts the params properly, we need the correct order in the next part. - $callable = array($externalfunctioninfo->classname, 'validate_parameters'); - - $params = call_user_func($callable, - $externalfunctioninfo->parameters_desc, - $args); - - $params = array_values($params); - // Allow any Moodle plugin a chance to override this call. This is a convenient spot to - // make arbitrary behaviour customisations. The overriding plugin could call the 'real' - // function first and then modify the results, or it could do a completely separate - // thing. - $callbacks = get_plugins_with_function('override_webservice_execution'); - $result = false; - - foreach ($callbacks as $plugintype => $plugins) { - foreach ($plugins as $plugin => $callback) { - $result = $callback($externalfunctioninfo, $params); - if ($result !== false) { - break 2; - } - } - } - - // If the function was not overridden, call the real one. - if ($result === false) { - $callable = array($externalfunctioninfo->classname, $externalfunctioninfo->methodname); - $result = call_user_func_array($callable, $params); - } - - // Validate the return parameters. - if ($externalfunctioninfo->returns_desc !== null) { - $callable = array($externalfunctioninfo->classname, 'clean_returnvalue'); - $result = call_user_func($callable, $externalfunctioninfo->returns_desc, $result); - } - - $response['error'] = false; - $response['data'] = $result; - } catch (\Throwable $e) { - $exception = get_exception_info($e); - unset($exception->a); - $exception->backtrace = format_backtrace($exception->backtrace, true); - if (!debugging('', DEBUG_DEVELOPER)) { - unset($exception->debuginfo); - unset($exception->backtrace); - } - $response['error'] = true; - $response['exception'] = $exception; - // Do not process the remaining requests. - } - - $PAGE = $currentpage; - $COURSE = $currentcourse; - - return $response; - } } diff --git a/lang/en/tool_trigger.php b/lang/en/tool_trigger.php index b12951e..e689ae9 100644 --- a/lang/en/tool_trigger.php +++ b/lang/en/tool_trigger.php @@ -292,8 +292,6 @@ $string['webserviceactionfunctionname'] = 'Function'; $string['webserviceactionfunctionname_help'] = 'The webservice function to be called. See the API Documentation'; -$string['webserviceactionalphaparmas'] = 'WS alpha parameters'; -$string['webserviceactionalphaparmas_help'] = 'The form validation may fail due to some ws parameters type, if you are getting an "ALPHA" type error, add the parameter here.'; $string['webserviceactionusername'] = 'Who'; $string['webserviceactionusername_help'] = 'The user (username) who this step will be performed in the context of. This defaults to the main admin user if not explicitly set'; $string['webserviceactionparams'] = 'Parameters'; diff --git a/tests/course_lookup_step_test.php b/tests/course_lookup_step_test.php index 2bc5a74..b8ee165 100644 --- a/tests/course_lookup_step_test.php +++ b/tests/course_lookup_step_test.php @@ -91,20 +91,21 @@ public function test_execute_basic() { } /** - * Basic test, but this time with additional custom profile field. + * Basic test, but this time with additional custom course field. */ public function test_execute_basic_with_custom_profile_fields() { - // Create user profile fields. + // Create custom course field. $customfieldg = $this->getDataGenerator()->get_plugin_generator('core_customfield'); $category = $customfieldg->create_category(); - $customfield = $customfieldg->create_field([ 'categoryid' => $category->get('id'), 'type' => 'text', 'shortname' => 'testfield1', 'configdata' => [], ]); - $this->add_course_custom_profile_field_data($customfield->get('id'), $this->course->id, 'CourseFieldValue'); + + // Add data to the customfield_data table. + $this->add_course_custom_course_field_data($customfield->get('id'), $this->course->id, 'CourseFieldValue'); $step = new \tool_trigger\steps\lookups\course_lookup_step( json_encode([ 'courseidfield' => 'objectid', @@ -114,6 +115,7 @@ public function test_execute_basic_with_custom_profile_fields() { list($status, $stepresults) = $step->execute(null, null, $this->event, []); $this->assertTrue($status); + // Check that the custom field data is returned as a step result. $this->assertEquals('CourseFieldValue', $stepresults['course_testfield1']); } @@ -250,10 +252,10 @@ public function test_execute_course_id_string() { $this->assertEquals($context->id, $stepresults['course_contextid']); } - public function add_course_custom_profile_field_data($fieldid, $courseid, $customfielddata) { + public function add_course_custom_course_field_data($fieldid, $courseid, $customfielddata) { global $DB; - // Add data to the custom profile fields. + // Add data to the customfield_data table. $data = new \stdClass(); $data->fieldid = $fieldid; $data->instanceid = $courseid; From fed76388fed6561f3a7c21cb5463cafd6e794725 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 12:02:28 -0500 Subject: [PATCH 13/15] DEF-3115: Adding testing for subplugins --- classes/workflow_manager.php | 3 + .../lookups/subplugin_lookup_test_step.php | 91 +++++++++++++++++++ .../testplugin/lang/en/trigger_testplugin.php | 31 +++++++ custom/testplugin/version.php | 33 +++++++ tests/subplugin_step_test.php | 78 ++++++++++++++++ tests/workflow_manager_test.php | 14 +++ 6 files changed, 250 insertions(+) create mode 100644 custom/testplugin/classes/steps/lookups/subplugin_lookup_test_step.php create mode 100755 custom/testplugin/lang/en/trigger_testplugin.php create mode 100755 custom/testplugin/version.php create mode 100644 tests/subplugin_step_test.php diff --git a/classes/workflow_manager.php b/classes/workflow_manager.php index f5729ca..8f7fff7 100644 --- a/classes/workflow_manager.php +++ b/classes/workflow_manager.php @@ -235,6 +235,9 @@ public function get_step_class_names($steptype = null) { ] ]; foreach ($plugins as $plugin => $dir) { + if (!PHPUNIT_TEST && $plugin == 'testplugin') { + continue; + } $dirs[] = (object)[ 'path' => $dir . '/classes', 'namespace' => "trigger_$plugin", diff --git a/custom/testplugin/classes/steps/lookups/subplugin_lookup_test_step.php b/custom/testplugin/classes/steps/lookups/subplugin_lookup_test_step.php new file mode 100644 index 0000000..39b5784 --- /dev/null +++ b/custom/testplugin/classes/steps/lookups/subplugin_lookup_test_step.php @@ -0,0 +1,91 @@ +. + +namespace trigger_testplugin\steps\lookups; +use tool_trigger\steps\lookups\base_lookup_step; + +/** + * A lookup step for testing purposes. + * + * @package trigger_testplugin + * @copyright 2025 Moodle US + * @author Oscar Nadjar + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class subplugin_lookup_test_step extends base_lookup_step { + + /** + * The fields supplied by this step. + * A string containing all the cohorts a user is assigned to. + * + * @var array + */ + private static $stepfields = array( + 'testlookupresult' + ); + + protected function init() { + } + + /** + * {@inheritDoc} + * @see \tool_trigger\steps\base\base_step::execute() + */ + public function execute($step, $trigger, $event, $stepresults) { + global $DB; + $stepresults = [ 'testlookupresult' => 'test' ]; + return [true, $stepresults]; + } + + /** + * {@inheritDoc} + * @see \tool_trigger\steps\base\base_step::form_definition_extra() + */ + public function form_definition_extra($form, $mform, $customdata) { + } + + /** + * {@inheritDoc} + * @see \tool_trigger\steps\base\base_step::get_step_desc() + */ + public static function get_step_desc() { + return get_string('subplugin_lookup_test_step_desc', 'trigger_testplugin'); + } + + /** + * {@inheritDoc} + * @see \tool_trigger\steps\base\base_step::get_step_name() + */ + public static function get_step_name() { + return get_string('subplugin_lookup_test_step_name', 'trigger_testplugin'); + } + + /** + * {@inheritDoc} + * @see \tool_trigger\steps\base\base_step::get_privacyfields() + */ + public static function get_privacyfields() { + } + + /** + * Get a list of fields this step provides. + * + * @return array $stepfields The fields this step provides. + */ + public static function get_fields() { + return self::$stepfields; + } +} diff --git a/custom/testplugin/lang/en/trigger_testplugin.php b/custom/testplugin/lang/en/trigger_testplugin.php new file mode 100755 index 0000000..a3cf314 --- /dev/null +++ b/custom/testplugin/lang/en/trigger_testplugin.php @@ -0,0 +1,31 @@ +. + +/** + * Language file for trigger_testplugin. + * + * @package trigger_testplugin + * @copyright 2025 Moodle US + * @author Oscar Nadjar + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$string['pluginname'] = 'Test Trigger Subplugin'; +$string['pluginname_help'] = 'This plugin is used for testing the subplugin functionality of the Tool Trigger plugin.'; +$string['subplugin_lookup_test_step_desc'] = 'This is a test subplugin step for the Tool Trigger plugin. It is used to test the functionality of the subplugin system.'; +$string['subplugin_lookup_test_step_name'] = 'Test Subplugin Step'; diff --git a/custom/testplugin/version.php b/custom/testplugin/version.php new file mode 100755 index 0000000..84c2021 --- /dev/null +++ b/custom/testplugin/version.php @@ -0,0 +1,33 @@ +. + +/** + * Plugin version and other meta-data are defined here. + * + * @package trigger_testplugin + * @copyright 2025 Moodle US + * @author Oscar Nadjar + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$plugin->component = 'trigger_testplugin'; +$plugin->version = 2025042200; +$plugin->requires = 2021051701; +$plugin->supported = [404, 405]; +$plugin->maturity = MATURITY_STABLE; +$plugin->release = '1.0.0'; diff --git a/tests/subplugin_step_test.php b/tests/subplugin_step_test.php new file mode 100644 index 0000000..14c68d1 --- /dev/null +++ b/tests/subplugin_step_test.php @@ -0,0 +1,78 @@ +. + +/** + * "Fail" filter step's unit tests. + * + * @package tool_trigger + * @author Aaron Wells + * @copyright Catalyst IT 2018 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace tool_trigger; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +class subplugin_step_test extends \advanced_testcase { + + /** + * Test user. + * @var + */ + protected $user; + + /** + * Test event. + * @var + */ + protected $event; + + /** + * Create an event to use for testing. + */ + public function setup():void { + // Create a user event. + $this->requestssent = []; + $this->user = \core_user::get_user_by_username('admin'); + $this->event = \core\event\user_profile_viewed::create([ + 'objectid' => $this->user->id, + 'relateduserid' => $this->user->id, + 'context' => \context_user::instance($this->user->id), + 'other' => [ + 'courseid' => 1, + 'courseshortname' => 'short name', + 'coursefullname' => 'full name' + ] + ]); + } + + /** + * Basic use-case, with default values for settings. Find the + * user identified at "userid", and add their data with the + * prefix "user_". + */ + public function test_execute_basic() { + $step = new \trigger_testplugin\steps\lookups\subplugin_lookup_test_step(json_encode([])); + + list($status, $stepresults) = $step->execute(null, null, $this->event, []); + + $this->assertTrue($status); + $this->assertEquals('test', $stepresults['testlookupresult']); + } +} diff --git a/tests/workflow_manager_test.php b/tests/workflow_manager_test.php index 491652a..884b9fa 100644 --- a/tests/workflow_manager_test.php +++ b/tests/workflow_manager_test.php @@ -79,6 +79,20 @@ public function test_get_steps_by_type() { ); } + /** + * Test getting step from custom plugin. + */ + public function test_custom_step_names() { + + $stepclasses = ['\trigger_testplugin\steps\lookups\subplugin_lookup_test_step']; + $stepobj = new \tool_trigger\workflow_manager(); + $steps = $stepobj->lookup_step_names($stepclasses); + $this->assertEquals( + get_string('subplugin_lookup_test_step_name', 'trigger_testplugin'), + $steps['\trigger_testplugin\steps\lookups\subplugin_lookup_test_step'] + ); + } + /** * Test the code for validating the name of a step class and instantiating it. * From 3f7fe55e36b122e7fed3113c4e37be0bdb17c8e1 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 12:14:14 -0500 Subject: [PATCH 14/15] DEF-3115: Fixing tests --- tests/subplugin_step_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/subplugin_step_test.php b/tests/subplugin_step_test.php index 14c68d1..9e9d9fd 100644 --- a/tests/subplugin_step_test.php +++ b/tests/subplugin_step_test.php @@ -47,8 +47,8 @@ class subplugin_step_test extends \advanced_testcase { * Create an event to use for testing. */ public function setup():void { + // Create a user event. - $this->requestssent = []; $this->user = \core_user::get_user_by_username('admin'); $this->event = \core\event\user_profile_viewed::create([ 'objectid' => $this->user->id, From b9d0032ce5ac10c0ed2e1b9d636518c5270c71f5 Mon Sep 17 00:00:00 2001 From: Oscar Nadjar Date: Tue, 22 Apr 2025 12:21:26 -0500 Subject: [PATCH 15/15] DEF-3115: Fixing pipeline --- tests/subplugin_step_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/subplugin_step_test.php b/tests/subplugin_step_test.php index 9e9d9fd..caf308a 100644 --- a/tests/subplugin_step_test.php +++ b/tests/subplugin_step_test.php @@ -30,7 +30,7 @@ global $CFG; class subplugin_step_test extends \advanced_testcase { - + /** * Test user. * @var