moodle-tool_mergeusers icon indicating copy to clipboard operation
moodle-tool_mergeusers copied to clipboard

FR: Implement Moodle type hook to let plugins extend its configuration

Open danowar2k opened this issue 1 year ago • 2 comments

For now you need users to write config.local.php inside the plugin's directory, which isn't a very clean way of configuring it.

Nicer for plugin developers who want to provide additional configuration for better plugin compatibility would be to add a hook to config.php, e.g. here: https://github.com/jpahullo/moodle-tool_mergeusers/blob/227e3bf8dd119c64a7834ed7631c45b3c2537eb2/classes/tool_mergeusers_config.php#L70

private function __construct() {
    $config = include dirname(__DIR__) . '/config/config.php';

    $pluginswithfunction = get_plugins_with_function('tool_mergeusers_extend_config', 'lib.php');
    foreach ($pluginswithfunction as $plugins) {
        foreach ($plugins as $function) {
           try {
              $config = $function($config);
           } catch (Throwable $e) {
           debugging("Exception calling '$function'", DEBUG_DEVELOPER, $e->getTrace());
        }
    }
    $this->config = $config;
}

This way each plugin could decide what to add or even write special functions that would be called when merging.

danowar2k avatar Aug 30 '23 21:08 danowar2k

Thanks for the feedback.

It would be welcome a PR with this proposal. Actually it follows up the idea I have for this plugin: that each component of Moodle has its own control of how merging should be addressed.

And this issue is a key part on this process.

Please, keep loading config.local.php for backwards compatibility.

In fact, the loading order should be: config.php, function overriding on lib.php (this issue), and finally the config.local.php. This will keep the precedence of settings overriding.

Thanks for all,

Jordi

El El mié, 30 ago 2023 a las 23:34, Daniel Poggenpohl < @.***> escribió:

For now you need users to write config.local.php inside the plugin's directory, which isn't a very clean way of configuring it.

Nicer for plugin developers who want to provide additional configuration for better plugin compatibility would be to add a hook to config.php, e.g. here: https://github.com/jpahullo/moodle-tool_mergeusers/blob/227e3bf8dd119c64a7834ed7631c45b3c2537eb2/classes/tool_mergeusers_config.php#L70

private function __construct() { $config = include dirname(DIR) . '/config/config.php';

$pluginswithfunction = get_plugins_with_function('mergeusers_extend_config', 'lib.php');
foreach ($pluginswithfunction as $plugins) {
    foreach ($plugins as $function) {
       try {
          $config = $function($config);
       } catch (Throwable $e) {
       debugging("Exception calling '$function'", DEBUG_DEVELOPER, $e->getTrace());
    }
}
$this->config = $config;

}

This way each plugin could decide what to add or even write special functions that would be called when merging.

— Reply to this email directly, view it on GitHub https://github.com/jpahullo/moodle-tool_mergeusers/issues/254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPUCKBTVCLXCOIQP43N6RLXX6WVVANCNFSM6AAAAAA4FEB2E4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jpahullo avatar Aug 31 '23 08:08 jpahullo

Hi @danowar2k ,

Would you mind you could provide a PR for this?

I am very interested on this.

Thanks a lot for reporting,

Jordi

jpahullo avatar Dec 01 '23 09:12 jpahullo