View Issue Details

IDProjectCategoryView StatusLast Update
15886Development Pluginspublic2020-02-26 18:05
Reporterdschirge Assigned To 
PrioritynoneSeverityfeature 
Status newResolutionopen 
Product Version3.x 
Summary15886: Plugin development
Description

I already wrote several plugins for LS and noticed some problems.

  • It isn't possible to use namespaces or at least I wasn't able to. As soon as I created a namespace LS wasn't able to load it anymore. This way it isn't safe to use multiple classes because PHP isn't able to distinct the correct classe if multiple plugins are using different classes with the same name (for example PluginHelper). If 2 or more plugins are having their own PluginHelper class no one can tells which class is used. So it is possible to create naming conflicts.

  • the same counts for JavaScript. There are no "namespaces" created in the LS plugin system. If there are two plugins with the same function called for example "downloadFile" the second plugin will overwrite the first plugin's "downloadFile" function. Of course there are ways to avoid it but it would be good if LS would provide a function like the ExtJs function Ext.ns();

  • there is no default way (or at least I don't know it) for loading javascript & css ressources and execute the JavaScript. Every developer tries to get it working in it's own way so there is chaos in the DOM if you are using multiple plugins. In my view you shoul dcreate an Interface called PluginInterface which provides the following functions:

    function loadJavaScriptFiles():array;

    function loadCssFiles():array;

    function executeJavaScript():array

You could do in your implementation the following:

class ExamplePlugin extens PluginInterface

{

public function loadJavaScriptFiles(): array

{

   return [

                   'assets/js/InternalClass'

               ]

}

public function executeJavaScript():array

{

return [

      'myCompany.Example.MyClass.MyClassInstance = new myCompany.Example.MyClass.MyClass();'

];

}

public function loadCssFiles():array
{
return [];
}

}

Of course there is more you could do this way, it's just a first example.

In the background you could change your method for executing the plugins to the following:

if ((!isset($this->plugins[$id]) || get_class($this->plugins[$id]) !== $pluginName)) {

            if ($this->getPluginInfo($pluginName) !== false) {

                if (class_exists($pluginName)) {
                try {
                    $this->plugins[$id] = new $pluginName($this, $id);

                     if (method_exists($this->plugins[$id], 'init')) {

                        $this->plugins[$id]->init();

                    }
                    if($this->context !== LimeSurveyContext::WEB_APP)
                    {
                            return;
                     }
              $scripts =    $this->plugins[$id]->loadJavaScriptFiles();
          $commands = $this->plugin[$id]->executeJavaScript();
      $this->loadPluginJsScripts($scripts);               
      $this->executePluginScripts($commands);

            }
            catch(Exception $ex)
            {
                //Insert some logging or what ever is neccessary
            }
                } else {

                    $this->plugins[$id] = null;

                }

            } else {

                $this->plugins[$id] = null;

            }

        }
  • there is no default way for a plugin to extend the LS masks. It should be solved via Interface as well. For example have a function "function GetMaskExtensions():array". The array should consist of objects which describes what you want to extend.
    For example could a object look this way:

class MyPluginToolMenuItem implements MaskExtensionInterface
{
//The tag sets the position of the menu extension
public $tag = "ToolMenu";
//The html is of course the html which will be applied to the menu.
public $html;
}

This is of course a very simple example and maybe not enough for a "mask extension". But if you would develop a system I suggested you would take back the control from the plugin developer to LS. Please let me know your thoughts.
By the way I added a try catch to the plugin execution. At the moment an init function of a plugin would in case on an error cancel the execution of all the following plugins.

TagsNo tags attached.

Activities

Mazi

Mazi

2020-02-18 11:52

partner   ~56066

@DenisChenu: What do you think about the suggested approach? Wouldn't a proper implementation like this make the plugin sytem more stable and future safe?

@ollehar: Who at your development team is responsible for deadling with plugin related details?

DenisChenu

DenisChenu

2020-02-18 12:01

developer   ~56068

PHP NameSpace and JS NameSpace ?
Sample : https://github.com/LimeSurvey/LimeSurvey/tree/master/plugins/Demo/demoAddEmFunction

Then:

It isn't possible to use namespaces or at least I wasn't able to.

It's possible : usage of Yii::setPathOfAlias is the trick

there is no default way (or at least I don't know it) for loading javascript & css ressources and execute the JavaScript.

Twig have function, expression manager have own function , and Yii have regsiterScript and registerPackage (personnaly : prefer registerPackage for plugin ..)

Else : current LimeSurveyAPI exist but is unusuable. If you want to improve and add more function : please do ...
But in my opinion : LimeSurvey is constructed on Yii : the n use Yii are the best solution ...

About extend HTML (admin and public) : there are no way. Can be great to have 2 system to add element on any HTML part but please .... no 4 different ways again (like menu ... for example).

dschirge

dschirge

2020-02-18 12:29

reporter   ~56070

Hi DenisChenu,

I am aware for example of Yii's registerScript.
But I still think it would be better to define an interface where the plugin provides the files etc. you need to work with and let LS decide the proper way of including them. In my opinion it isn't the job of a developer to decide in an existing system how to include ressources but the system.
As far as I know there are plans to migrate to a future Yii version. All plugins which are using registerScript, registerPackage and registerCss won't work anymore because there are other functions.
But nevertheless - if you want to have a reliable plugin system you need to do some changes - at least in my opinion.
Alias: Maybe I don't know the Yii 1 system well enough but in Yii2 you could register the plugin directory that way that all namespaces within this directory could work without any further aliases.

Extending HTML: I don't know of 4 different ways. It like on the points above: LS should decide how to append new HTML.

Sorry, but I get the feeling you don't like to get suggestions how to improve LS. I don't criticize anybody here or in HH. No software is perfect, no developer is perfect and a look after some weeks on my own code mostly displays my own imperfection.
And maybe my suggestions aren't perfect but I try to show a way how to solve problems I detected.
So why this angry tone?

DenisChenu

DenisChenu

2020-02-18 12:44

developer   ~56071

Last edited: 2020-02-18 12:45

View 2 revisions

Else about API stabilization (make the plugin sytem more stable and future safe) : best solution create test : https://github.com/LimeSurvey/LimeSurvey/tree/master/tests/functional/acceptance/15246-fixed-em-function

Sorry, but I get the feeling you don't like to get suggestions how to improve LS

It's not about your suggestion . In fact : i don't care, because when i create a plugin: a lot of people ask for previous version compatibility ... . My opinion : do it, add a test, add a plugin using it , make the pull request and let's see.

About javascript if we talk for javascript : there are official way for theme (admin or public). (config.xml : https://github.com/LimeSurvey/LimeSurvey/blob/ae4638e2b61ea2d0a895459965793048828d8b1a/themes/admin/Sunset_Orange/config.xml#L32 or https://github.com/LimeSurvey/LimeSurvey/blob/ae4638e2b61ea2d0a895459965793048828d8b1a/themes/survey/vanilla/config.xml#L48 etc ...)

I don't understand why LimeSurveyAPI need new functions .... because : WHO manage LimeSurveyAPI file ?

See 14477: LimeSurvey api createUrl didn't use publicurl setting issue for example.

My idea: more function in API => More unresolved bug ....

ollehar

ollehar

2020-02-26 16:25

administrator   ~56205

So why this angry tone?

This is French friendly tone. ;D

Yeah, plugin dev is a bit hackish. Usually you can do anything you want, though. Cleaning up the plugin system is not a priority, since there so much to clean in the general code base already.

ollehar

ollehar

2020-02-26 16:27

administrator   ~56206

Also note that no new features will be added into LS3, only (more serious) bug fixes.

dschirge

dschirge

2020-02-26 17:58

reporter   ~56207

Hi Ollehar,
I assume it's the same for LS4 but I can't choose both versions. ;-)
And that's true. If you look at the source code there are dozens of tasks to do.

DenisChenu

DenisChenu

2020-02-26 18:02

developer   ~56208

If you look at the source code there are dozens of tasks to do.

More and more than dozen ....

dschirge

dschirge

2020-02-26 18:05

reporter   ~56209

Yes. In German you express it with dozens. You of course could say tons of tasks but it doesn't change the fact that it's urgent to look at some coding principles - and I maybe would throw in "design patterns" like MVC.

Issue History

Date Modified Username Field Change
2020-02-18 10:11 dschirge New Issue
2020-02-18 11:52 Mazi Note Added: 56066
2020-02-18 12:01 DenisChenu Note Added: 56068
2020-02-18 12:29 dschirge Note Added: 56070
2020-02-18 12:44 DenisChenu Note Added: 56071
2020-02-18 12:45 DenisChenu Note Edited: 56071 View Revisions
2020-02-26 16:25 ollehar Note Added: 56205
2020-02-26 16:27 ollehar Note Added: 56206
2020-02-26 17:58 dschirge Note Added: 56207
2020-02-26 18:02 DenisChenu Note Added: 56208
2020-02-26 18:05 dschirge Note Added: 56209