V2.3 coordination

[marcelfolaron]

Hi @diderich @phulstaert @broskees:

You all have 3 very large features and PRs that I am incredibly excited to merge. Let's use this thread to coordinate these efforts.
A summary of the changes I know so far:

@phulstaert worked on reworking the application start, configuration files and installation routines. This streamlines a lot of the class flow and will make things a lot easier. https://github.com/phulstaert/leantime/tree/New-bootprocess

@broskees expanded significantly on my initial plugin and event management system.
#1079

@diderich reworked our canvas modules and included a lot more canvas strategies. He also worked on the folder structure to allow developers to include customizations as well as themes and plugins.
https://github.com/diderich/leantime/tree/dts

I will add changes to the folder structure based on @diderich work.
I still have an API proof of concept open that I'd like to include in 2.3.

All of these features are somewhat connected and will need to be merged in the right order.

Here is my suggestion in terms of merge order:

@diderich probably made the most significant changes in terms of folder structure. It would be easiest if we start there.

Next we should merge @phulstaert PR to build on top of the new structure.

Lastly @broskees. This should have the least issues.

I can squeeze in my changes at the end. I created a new branch called Leantime-V2.3. Lets push our 2.3 changes into that one first while we resolve conflicts.

@gloriafolaron FYI

[diderich]

Hi all:

My latest contribution includes the following:

• New canvas:
  • SWOT Analysis
  • Environment Analysis
  • Risk Analysis
  • Empathy Map
  • Observe / Learn - Insights
  • Strategy Brief
  • Lightweight Business Model, Osterwalder's Business Model, and Detailed Business Model
  • Porter's Strategy Questions
  • Competitive Positioning Canvas
  • Strategy Messaging
  • Note: Due to copyright restrictions, the Value Proposition Canvas cannot be included:
    https://strategyzer.uservoice.com/knowledgebase/articles/506842-can-i-use-the-business-model-canvas-or-value-propo
• Revised canvas:
  • Lean Canvas (full version only)
  • Retrospective (called retroscanvas)
• Generate PDF files from any canvas
• Support for easily configurable multi-layer menu structure (support for project-specific menu structures can be enabled
  via configuration)
• Refactored generic canvas code in src/domain/canvas allowing to create a new canvas by simply extending/including
  code
• Engine to translate language string using AI/machine learning from https://www.deepl.com (registration needed, 500'000
  characters per month are free)
• Customization support for languages, templates, and class repositories
• XML import and export of canvas
• Re-organized the directory structure to support themes and plugins

See https://github.com/diderich/leantime/blob/dts/CHANGELOG-2.3.0.md for further details
See https://github.com/diderich/leantime/blob/dts/ISSUES.md for open issues

[diderich]

@phulstaert . Looked at your draft work. Very interesting. Some comments/ideas/questions from my side (with the goal for improvement, not criticism) which you may or may not agree with.

• Why not use a sectioned ini file for the configuration, rather than the archaic define() statements. In addition, I personally prefer accessing parameters through a config() class (ideally through functions rather than variables, e.g., $config->getDefaultTmezone(), rather than using global variables (best programming practice typically speaks against global constants). @marcelfolaron, what is your opinion on this?

[general]
sitename  = "Leantime"
language = "en-US"
...

[database]
host = "localhost"
user = "leantime"
...

[smtp]
...

• In your LT_setup.php you use hard-coded HTML, rather than the template engine. This will likely break the system going forward (notably due to the introduction of themes). I would recommend considering putting you setup engine in the system's default structure, ie.., by creating a setup domain, with controllers and templates. In addition this would allow offering the setup in the language of the user's browser, or a selected language (and use the existing engine, not having to re-invent the wheel). The only thing to check (and maybe adjust) would be ensuring that the database is not accessed by the leantime engine functionalities you are using during setup.

• Another idea would be to have a page, e.g, domain/setup/controllers/class.serverConfiguration.php and domain/setup/templates/serverConfiguration.php, that would show the current configuration and mark potential issues in red (for example, accessible in the Administration menu below Company Settings). Such a screen helps a lot identifying potential installation issues. Here a screenshot from another system that uses such an approach:
  
  
  or a simplified version from a photo database system I developed myself:
  

Keep up the good work.

[phulstaert]

Hi @diderich,

Marcel and I did some debugging and tracing of the Leantime code, speed and memory usage and we both where stumped by the results. Everything was in ini files and the time it took to load ini files was gigantic. Less noticeable on development machines with a NVME SSD, but on cheap shared hosting for example when using an iscsi drive to a nas with spinning disks, it was >80% of the load time of a page.
When you define those things in a php file, that file is compiled and cached for each user that requests it. And as you might know, not much can beat compiled, in-memory code. :-)
Also, when a webserver is badly configured (what happens more then less), an ini file is "downloaded", a php file is interpreted. The php won't disclose your precious variables (like passwords, keys, etc).

Regarding the global variables, I agree this is a bad practice in most cases. BUT the global variables in PHP are not the same as in other programming languages. In PHP, those are variables that are accessible to each function during one request. SESSION variables are the same as global variables, but are stored between multiple requests and are often written to the disk (again, bad for latency). The POST, GET or REQUEST variables are as "bad" as the global variables, only those can be early changed.
I did not use many global variables in my script. I did use named constant variables. Are you referring to those as "global variables"?

The code might not be pretty for now, but it works, is build to be expandable, fast and uses way less memory.
Best coding practices are only good when it does not interfere with the operation of the app. I will never have a smile on my face when I follow each best coding practice, but the app is slow or memory intensive for each user on each request. We have to "choose our battles" sometimes, even it that makes programming less clean.

I looked at Wordpress and Laravel as a source of inspiration. They both use named constants and define almost everything in php files.

I don't have the time now to respond to all your questions or remarks, but I'll finish tonight (lunchtime here...)

[broskees]

@phulstaert I'm surprised that ini files are so slow! Have we done any testing with .env files? Those files need a lot less interpretation as they're just key-value pairs and don't have a demential component, so they could possibly be faster.

It would also be minimal changes to your PR. So you could do something as simple as:

<?php

/** INFORMATION **/
/*****************/

/**
 * In this file you can configure your application. 
 * For more information, check out the documentation at https://docs.leantime.io/
 * 
 * Each option is preceded by STRING, BOOLEAN or NUMBER. 
 * This indicates what type of value you have to enter. 
 * It is crucial that you do this correctly.
 * 
 * STRING: This is a piece of text and is always between single quotes. Even if the text is a number.
 * ex: define( 'TEST', 'This is a test string')
 * If you want to use a quote as part of your text, you need to escape this. 
 * This is done by using an escape character. This character is the backslash.
 * ex: define( 'TEST', 'Gloria\'s projects')
 * 
 * NUMBER: This is a number and is written without quotes and only digits. No points, comma's or spaces.
 * ex: define( 'TEST', 123456 )
 * 
 * BOOLEAN: This can be true or false. All lowercase, no quotes. So the options are
 * ex: define( 'TEST', true )
 *     define( 'TEST', false )
 */

/**
 * Load Config from .env
 */
$dotenv = \Dotenv\Dotenv\createImmutable(__DIR__);
$dotenv->load();


/** Database **/
/**************/

    //STRING | Database hostname
    define( 'LT_DB_HOST', getenv('LT_DB_HOST') ?: 'localhost' );

    //STRING | Database port
    define( 'LT_DB_PORT', getenv('LT_DB_PORT') ?: '3306' );

    //STRING | Database username
    define( 'LT_DB_USER', getenv('LT_DB_USER') ?: '' );

    //STRING | Database password
    define( 'LT_DB_PASSWORD', getenv('LT_DB_PASSWORD') ?: '' ); 

    //STRING | Database name
    define( 'LT_DB_NAME', getenv('LT_DB_NAME') ?: 'leantime' );

    //STRING | Database Charset - should always be utf8
    define( 'LT_DB_CHARSET', getenv('LT_DB_CHARSET') ?: 'utf8' );

    //STRING | Database collate type - don't change this
    define( 'LT_DB_COLLATE', getenv('LT_DB_COLLATE') ?: '' );

Or something along those lines. Personally though, I don't really mind defining my environment with constants or feel that it isn't clean, I have quite a few applications that use the same setup.

[marcelfolaron]

I agree withn @phulstaert in that constants in php are a little different but so much nicer to use. I like the addition to support .env files as this could allow us to run different environments and will also make docker set up easier.

[diderich]

Currently, when adding a new canvas/kanban, it needs to be "sort of registered" in the domain/projects/services/class.project.php, by initializing a session variable $_SESSION['currentXXXCanvas']. This means, when adding, for example, a new canvas (a proprietary one) as a custom canvas (or as a plugin), one has to modify domain/projects/services/class.project.php, which goes against the idea of customization and plugins.

Do you have any ideas or suggestions how to handle this situation (and potentially similar ones) in a clean way?
Any suggestions @marcelfolaron , @broskees?

[diderich]

Mayb/probably. Would need to think through how this could work if you only want to do some changes. For example, take the Lean Canvas and only change the layout of the fields on screen, how would that work with your logic?

With the current logic I have implemented, there is not need to register anything. But it requires a fixed directory structure.

I think at then end the question/trade-off is registry mechanism versus fixed directory structure. To find out what works best, why not go for a dual structure, the plugins under /src/plugin/ use your registry mechanism, and the domains under /src/domain work with a fixed directory structure as of today.

I am really open to your idea, but still cannot grasp the details of what it means in practice. I would really like to see how implementing a canvas, as an example would work in your structure (with all the details solved) and compare it to the old-fashioned way. I could imagine that your solution may be cleaner (which I typically like) but I have a hard time seeing the practical implications (and there are numerous to handle) at the moment.

[marcelfolaron]

@broskees solution should work well and if a user wants to make layout changes they should just go the plugin route. There is a cost to making development work that much easier and I have a feeling that only a fraction of people will actually create new layouts.

[broskees]

@diderich if you know anything about the observer pattern, it will make clearer what I'm doing here. But basically filters modify variables, and events execute actions.

You can register filters/events in your plugin, which are loaded during the application's initialization, and stored. Later, they are executed wherever and on whatever you decide to dispatch that event/filter.

The either/or solution can work. I did something similar in the events class when it came to how event/filter listeners are executed.

I think I would like to figure out a way where they can live in the same directory though.

My first thought says overwrites should live in plugins/plugin-name/modules/module-name but I think I can come up with something better. I'll take a deeper dive later.

[broskees]

@diderich to answer your question about how we would want to make layout changes of fields on screen; I'm not sure which fields of which template you're specifically referring to, if you are looking for a specific example, but here's a general example:

Module Template File:

$layoutFields = $this->get("layoutFields");

$this->dispatchTplEvent('beforeLayoutFields', $layoutFields);
foreach ($layoutFields as $field) {
    ?>
        <label for="#<?= $field['id'] ?>"><?= $field['label'] ?></label>
        <input id="<?= $field['id'] ?>" name="<?= $field['name'] ?> type="<?= $field['type'] ?>" value="<?= $field['value'] ?>" />
    <?php
}
$this->dispatchTplEvent('afterLayoutFields', $layoutFields);

So, since i've modified the leantime\\core\\template\\assign function to dispatch a filter automatically, you can filter the layoutFields variable in the example above like so:

Plugin file:

// remove the last field from display
events::add_filter_listener("core.template.assign.var.layoutFields", function ($fields) {
    array_pop($fields);
    return $fields;
});

Or I could add my own field like so:

Plugin file:

// add a notes field
events::add_event_listener("tpl.whatevermodule.whateveraction.afterLayoutFields", function ($fields) {
    ob_start();
    ?>
        <label for="notes">Write any additional notes</label>
        <textarea id="notes" name="notes" placeholder="What's the note?">
    <?php
    echo ob_get_clean();
});

What's great about this approach at the end of the day is as an end user, an implementation of changing a feature in leantime could literally only take the code I wrote in this comment, and doesn't require the user to know concepts of OOP, the nature of leantime's structure, etc. They can just hook into what they want to change and go.

This approach coupled with great documentation on the available hooks that plugin developers can hook into, will create a lot of developer community very quickly I suspect.

[marcelfolaron]

@broskees This would be a great addition.

[diderich]

With the new themes concept, it should be possible to define the logo to use and the look and fee at the theme level, rather than, or in addition to the company level. This should be moved to new features once the release 2.3 has been active and the feature has not yet been implemented. A skeleton for the logo is available as core/theme::getLogoUrl().

[broskees]

@diderich @phulstaert @marcelfolaron

I have to look a lot more into each of your PRs, but I'll open something for discussion that I'm running into on my end. in core/class.frontcontroller.php and in core/class.template.php a controller / action / template override system that I'm not particularly fond of. Currently it's as so:
frontcontroller:

            //actionname.filename
            //actionName is foldername
            $actionName = self::getActionName($completeName);

            //moduleName is filename
            $moduleName = self::getModuleName($completeName);

            //Folder doesn't exist.
            if(is_dir('../src/domain/' . $moduleName) === false || is_file('../src/domain/' . $moduleName . '/controllers/class.' . $actionName . '.php') === false) {

                self::dispatch("general.error404");
                return;

            }

            $pluginPath = ROOT.'/../src/plugins/' . $moduleName . '/controllers/class.' . $actionName.'.php';
            $domainPath = ROOT.'/../src/domain/' . $moduleName . '/controllers/class.' . $actionName.'.php';

            $controllerNs = "domain";

            //Try plugin folder first for overrides
            if(file_exists($pluginPath)) {
                $controllerNs = "plugins";
                require_once $pluginPath;

            }else if(file_exists($domainPath)) {

                require_once $domainPath;

            }else{
                self::dispatch("general.error404", 404);
                return;
            }

template:

            //frontcontroller splits the name (actionname.modulename)
            $action = $this->frontcontroller::getActionName($template);

            $module = $this->frontcontroller::getModuleName($template);

            $pluginPath = ROOT.'/../src/plugins/' . $module . '/templates/' . $action.'.tpl.php';
            $domainPath = ROOT.'/../src/domain/' . $module . '/templates/' . $action.'.tpl.php';

            //Try plugin folder first for overrides
            if(file_exists($pluginPath)) {
                require_once $pluginPath;
            }else if(file_exists($domainPath)) {
                require_once $domainPath;
            }else{
                throw new \Exception($this->__("notifications.no_template"));
            }

Personally, I don't like how this forces plugin developers to adhere to specific folder/file structures. Personally, I prefer WordPress's methodology with plugins, which is basically "modify your instance however you like". It gives freedom to developers to make more amazing things, which I think is the main reason for its success in building the community that surrounds it.

It also doesn't allow plugin authors to create their own modules, but only overwrite existing ones.

But before I rework this to allow plugin developers to "register" their own modules and template paths, I'd like to discuss this concept with you all and see your thoughts.

How I would refactor frontcontroller for example:

            //actionname.filename
            //actionName is foldername
            $actionName = self::getActionName($completeName);

            //moduleName is filename
            $moduleName = self::getModuleName($completeName);

            $actionPath = events::dispatch_filter(
                "requests.$actionName.$moduleName",
                ROOT . "/../src/domain/$moduleName/controllers/class.$actionName.php",
                [
                    'action' => $actionName,
                    'module' => $moduleName
                ]
            );

            //Folder doesn't exist.
            if(!file_exists($actionPath)) {

                self::dispatch("general.error404");
                return;

            }

            require_once $actionPath;

            //Initialize Action
            try {

                $classname = events::dispatch_filter(
                    "action_classname",
                    "leantime\\domain\\controllers\\".$actionName,
                    [
                        'module' => $moduleName,
                        'action' => $actionName
                    ]
                );

                $action = new $classname();
                
                // ....

That way, if a plugin developer wanted a url that looked like "/newmodule/show" then they could simply register:

events::add_filter_listener('core.frontcontroller.executeAction.requests.newmodule.show', function ($filepath, $params) {
   if (empty($params['module'])
        || empty($params['action'])
   ) {
       return $filepath;
   }

   $newfilepath = ROOT . "/../src/plugins/my/own/${params['module']}/and/${params['action']}.php";

    if (!file_exists($newfilepath)) {
        return $filepath;
    }

    return $newfilepath;
});

events::add_filter_listener('core.frontcontroller.executeAction.action_classname', function ($classname, $params) {
    return "myPluginNs\\{$params['module']}\\whatever\\{$params['action']}";
});

[diderich]

I just had a second look at your POC. The more I look at it, the more I do like it. There is (at least) one think that I would suggest to change. The system should deal with the root part of the filenames, that is the part ROOT.'/../src/plugin'. This is relevant from a security perspective to avoid that users/developers register files that are outside of the leantime installation.

For the menu registry, could you imagine a structure where an array with the intended menu structure, as in the example below is registered as a new menu structure?

        private array $menuStructures = [ 
            'dts' => [
                11 => [ 'type' => 'item', 'module' => 'dashboard',  'title' => 'menu.dashboard',  'href' => '/dashboard/show',     'active' => [ 'show' ] ],
                21 => [ 'type' => 'item', 'module' => 'tickets',    'title' => 'menu.todos',                                       'active' => [ 'showKanban', 'showAll', 'showTicket' ] ],
                31 => [ 'type' => 'item', 'module' => 'tickets',    'title' => 'menu.milestones', 'href' => '/tickets/roadmap',    'active' => [ 'roadmap' ] ],
                50 => [ 'type' => 'submenu', 'id' => 'dts-process', 'title' => 'menu.dts.process', 'visual' => 'open',
                        'submenu' => [
                            51 => [ 'type' => 'item',    'module' => 'insightscanvas', 'title' => 'menu.insightscanvas', 'href' => '/insightscanvas/showCanvas' ],
                            52 => [ 'type' => 'item',    'module' => 'ideas',          'title' => 'menu.ideation',       'href' => '/ideas/showBoards' ] ] ],
                60  => [ 'type' => 'submenu', 'id' => 'dts-frameworks', 'title' => 'menu.dts.frameworks', 'visual' => 'open',
                         'submenu' => [
                             61 => [ 'type' => 'header', 'title' => 'menu.dts.observe' ],
                             62 => [ 'type' => 'item',   'module' => 'sbcanvas',    'title' => 'menu.sbcanvas',    'href' => '/sbcanvas/showCanvas' ],
                             63 => [ 'type' => 'item',   'module' => 'riskscanvas', 'title' => 'menu.riskscanvas', 'href' => '/riskscanvas/showCanvas' ],
                             64 => [ 'type' => 'item',   'module' => 'eacanvas',    'title' => 'menu.eacanvas',    'href' => '/eacanvas/showCanvas' ],
                             65 => [ 'type' => 'header', 'title' => 'menu.dts.design' ],
                             66 => [ 'type' => 'item',   'module' => 'lbmcanvas',   'title' => 'menu.lbmcanvas',   'href' => '/lbmcanvas/showCanvas' ],
                             67 => [ 'type' => 'item',   'module' => 'dbmcanvas',   'title' => 'menu.dbmcanvas',   'href' => '/dbmcanvas/showCanvas' ],
                             68 => [ 'type' => 'item',   'module' => 'cpcanvas',    'title' => 'menu.cpcanvas',    'href' => '/cpcanvas/showCanvas' ],
                             69 => [ 'type' => 'header', 'title' => 'menu.dts.validate' ],
                             70 => [ 'type' => 'item',   'module' => 'sqcanvas',    'title' => 'menu.sqcanvas',    'href' => '/sqcanvas/showCanvas' ],
                             71 => [ 'type' => 'item',   'module' => 'smcanvas',    'title' => 'menu.smcanvas',    'href' => '/smcanvas/showCanvas' ] ] ],
                80 => [ 'type' => 'submenu', 'id' => 'dts-admin', 'title' => 'menu.dts.admin', 'visual' => 'open', 
                        'submenu' => [
                            81 => [ 'type' => 'item', 'module' => 'wiki','title' => 'menu.documents', 'href' => '/wiki/show' ],
                            82 => [ 'type' => 'item', 'module' => 'retroscanvas','title' => 'menu.retroscanvas', 'href' => '/retroscanvas/showCanvas' ],
                            83 => [ 'type' => 'item', 'module' => 'reports','title' => 'menu.reports', 'href' => '/reports/show', 'role' => 'editor' ] ] ] ]
    ];

If I understand you, that could be something like

events::register_new_menu('core.menu.dts', function () {
    return $menuStructure;
}

Another question to think about is how one would change and register a showCanvas.tpl.php that currently looks as follows (especially the $this->getTemplatePath() which allows included standard components from the system? Maybe by something like this $this->getTemplateInclude('core.template.display.tpl.canvas.block.XXX') where XXX would be something like showCanvasTop, element, and showCanvasBottom, components pre-registered by the system? Or even allowing to support optional parameters when registering a template, i.e., you could include a template part through $this->getTemplateInclude('core.template.display.tpl.canvas.block.element', [' elementName' => 'swot_strength'] );

<?php
/**
 * Template
 */
defined('RESTRICTED') or die('Restricted access');

$canvasName = 'swot';
?>

<?php require($this->getTemplatePath('canvas' ,'showCanvasTop.inc.php')); ?>

    <?php if(count($this->get('allCanvas')) > 0) { ?>

        <div id="sortableCanvasKanban" class="sortableTicketList disabled">
          <div class="row-fluid"><div class="column" style="width: 100%; min-width: calc(2 * 250px);">

            <div class="row canvas-row">
                <div class="column" style="width: 50%">
			      <h4 class="widgettitle title-primary center canvas-title-only">
			        <large><i class="far fa-thumbs-up"></i> <?=$this->__('box.header.swot.helpful') ?></large>
                  </h4>
                </div>
                <div class="column" style="width: 50%">
			      <h4 class="widgettitle title-primary center" style="border-radius: var(--box-radius-small);">
			        <large><i class="far fa-thumbs-down"></i> <?=$this->__('box.header.swot.harmful') ?></large>
                  </h4>
                </div>
            </div>
			  
            <div class="row canvas-row" id="firstRow">
                <div class="column" style="width: 50%">
                    <?php $elementName = 'swot_strengths'; require($this->getTemplatePath('canvas' ,'element.inc.php')); ?>
                </div>
                <div class="column"  style="width: 50%">
                    <?php $elementName = 'swot_weaknesses'; require($this->getTemplatePath('canvas' ,'element.inc.php')); ?>
                </div>
            </div>

            <div class="row canvas-row" id="secondRow">
                <div class="column" style="width: 50%">
                    <?php $elementName = 'swot_opportunities'; require($this->getTemplatePath('canvas' ,'element.inc.php')); ?>
                </div>
                <div class="column" style="width: 50%">
                    <?php $elementName = 'swot_threats'; require($this->getTemplatePath('canvas' ,'element.inc.php')); ?>
                </div>
            </div>

          </div></div>
        </div>
        <div class="clearfix"></div>
    <?php } ?>

<?php require($this->getTemplatePath('canvas' ,'showCanvasBottom.inc.php')); ?>

Regarding the localization, maybe have something like events::localize('core.language.svccancvs', function () { return '/svccanvas/whereIHaveMyLanguageIniFiles'; } where the returned directory would contain 'en-US.ini', ... files defining/localizing the language keys specific to the plugin?

Does that correspond to your way of thinking or do I miss something fundamentally?

[broskees]

Whew, a lot to hit on here. Maybe we should get a slack going to make things easier.

---

I believe the comparison with WordPress is misleading. I believe that there is much more a need to extend existing functionalities and integrate with the project management features than is the case in WordPress where themes and plugins are (completely) independent of each other. This claim is based on the list of feature requests I am reading in the issues threat.

Yeah I agree there is a ways to go before we get there. However, we might be getting more help from the community on these features with a well defined plugin development system!

---

I full agree with this, but there is a long way to there! What I recommend/suggest is building on the existing system in a way that does not break it and move functionalities over time out of the domain into the plugin framework, rather than trying a big-bang approach. Ie., run the current and register.php approach in parallel for some time, no longer develop functionalities using the current approach going forward, and gradually move existing functionality over to the register approach (while addressing the challenges identified on the road).

I'm fine with this. The only thing I'm a tad worried about with this is people utilizing the /custom system and then later having to convert all their code to the new system. So as long as we're clear that this will go away at some point though, that would be fine.

---

There is (at least) one think that I would suggest to change. The system should deal with the root part of the filenames, that is the part ROOT.'/../src/plugin'. This is relevant from a security perspective to avoid that users/developers register files that are outside of the leantime installation.

I agree, I think adding a static "helpers" class/file that can be used with plugins or in core with functions like getPluginDirectory() or getDomainDirectory() would be a good solution to this in the future. However, if the ROOT constant doesn't reference the users leantime directory on their system, then more than likely leantime won't work at all, with or without plugins. Perhaps that constant should be renamed LT_ROOT.

---

If I understand you, that could be something like

events::register_new_menu('core.menu.dts', function () {
    return $menuStructure;
}

Close, but not quite. Probably would be more like:
events::add_filter_listener('core.menu.menu_registry', function ($menus) { $menus['dts'][...] }); (again I don't have a menu module in my install locally).
The only two functions you should need to register any functionality you add would be events::add_filter_listener and events::add_event_listener. I also realize now that I used events::dispatch_filter in my plugin example, but that's wrong as I kinda rushed it. Those should have been add_filter_listener. We could however, include a helper like in the example I mentioned above that wrapped the filter_listener called register_new_menu.

---

Another question to think about is how one would change and register a showCanvas.tpl.php that currently looks as follows (especially the $this->getTemplatePath() which allows included standard components from the system? Maybe by something like this $this->getTemplateInclude('core.template.display.tpl.canvas.block.XXX') where XXX would be something like showCanvasTop, element, and showCanvasBottom, components pre-registered by the system? Or even allowing to support optional parameters when registering a template, i.e., you could include a template part through $this->getTemplateInclude('core.template.display.tpl.canvas.block.element', [' elementName' => 'swot_strength'] );

Is this a function specific to your PR? I don't have $this->getTemplatePath() function yet. If its completely new functionality, I would have to dispatch events and filters within it so that it can be hooked into. However, if its a wrapper function for the current displayPartial function in core/class.template.php, all you would need to do is register:

events::add_filter_listener('core.template.display.template_path.canvas.showCanvas', function ($oldPath) {
    $newpath = '.......';
    return $newpath;
});

---

Regarding the localization, maybe have something like events::localize('core.language.svccancvs', function () { return '/svccanvas/whereIHaveMyLanguageIniFiles'; } where the returned directory would contain 'en-US.ini', ... files defining/localizing the language keys specific to the plugin?

This is a good opportunity to explain how this all works in a more concise example. Currently, languages files are loaded like so in core/class.language.php:

                if (file_exists('' . $this->iniFolder . 'languagelist.ini') === true) {
                    $this->langlist = parse_ini_file('' . $this->iniFolder . 'languagelist.ini');
                    $_SESSION['cache.langlist'] =  $this->langlist;
                } else {
                    throw new Exception("Language list missing");
                }

If I want this code to be extendable with a plugin, I'll refactor to add some functionality a bit:

                $parsedLangList = [];
                $language_lists = events::dispatch_filter(
                    'language_lists',
                    [
                        '' . $this->iniFolder . 'languagelist.ini'
                    ]
                );

                if (is_array($language_lists) && !empty($language_lists)) {
                    foreach ($language_lists as $list) {
                        if (file_exists($list)) {
                            $parsedLangList = array_merge($parsedLangList, parse_ini_file($list));
                        }
                    }
                }

                if (empty($parsedLangList)) {
                    throw new Exception("Language list missing");
                }

                $this->langlist = $_SESSION['cache.langlist'] = $parsedLangList;

Great, we've added a filter to add more language lists now. These types of changes basically encompass what's contained in my entire PR. Now when a developer wants to add a lang list, in their plugin register.php file they'll add:

events::add_filter_listener('core.language.__construct.language_lists', function($langlistArr) {
    $langlistArr[] = ROOT. '/../src/plugins/svccanvas/whereIHaveMyLanguageIniFiles/lang.ini';
    return $langlistArr;
});

Then, as we make things even more dev friendly, we could add a helper like so:

function add_language_file($language_location) {
    events::add_filter_listener(
        'core.language.__construct.language_list_locations', 
        function($langlistArr) use ($language_location) {
            $langlistArr[] = $language_location;
            return $langlistArr;
        }
    );
}

Then things could be as easy as:

add_language_file(
    get_plugin_directory() . '/svccanvas/whereIHaveMyLanguageIniFiles/lang.ini'
);

I hope this adds some clarity.

[marcelfolaron]

Wow guys there is a lot to digest here but overall I think your thoughts are on point. I do agree with @diderich that a step by step approach is better at this stage of Leantime. Large (potentially breaking) changes are currently difficult to manage even just from a support perspective.

FYI: We have a discord server and I could create a new channel for 2.3 https://discord.gg/4zMzJtAq9z

[marcelfolaron]

Alright I had a chance to go through this some more. Here my thoughts:

Menu Registry:
Love this and we should do that. I think it should be part of the even system but called as a sepearate event dispatcher: events::add_menu($menuPosition, $sortOrder, function $menuItem(){ return "" })

On the frontcontroller filestructure:
I know this seems very static but I believe the ease of use within Leantime is based on a "common" file structure that forces developers to adhere to certain coding practices. If we break with this we might run into the same issue that wordpress runs into with regards to low quality plugins.
I am also afraid that this might cause the load time to shoot up quite significantly. @phulstaert might have some thoughts on this.
Suggestion:
I think Leantime currently suffers from one significant standard deviation: PSR-4.
I would suggest to keep the basic folder structure and loading mechanism intact but adjust it to follow psr-4 standards.
This would mean we update our namespaces and revers the order on some:

leantime\domain\module\controller
leantime\domain\module\service
leantime\domain\module\...

leantime\plugins\pluginName\controller

basically everything after leantime follows the structure within the src/ folder path. The nice thing about this is that we/developers can add additional folders/classes without having to worry about loading them. I have been struggling in the past to have multiple services within the same module because of the static structure, so this would resolve that.

I don't think the template manager needs to change signficicantly with the exception of addint an optional field for "plugin/domain"

Additionally we can add the PSR-4 example class https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-4-autoloader-examples.md for additional path definitions.

Language
Yes, please!

[diderich]

Fully agree. And it will make correctly finding/loading classes easier. No longer need to twist your brain 😉, because domain/service/peter/pan means domain/peter/service/pan.

[diderich]

Here is an example of how a custom canvas could be added to the system by deploying it in the ' /custom/` directory.
custom.tar.gz

[broskees]

interesting, so does the /custom/ directory live alongside the /plugins/ directory in my PR?

[diderich]

No, /custom lives at the root level, ie., same level as /src.

You could even imagine a customization of a plugin in /custom/plugin. This could be useful if developer A offers a plugin X (under /src/plugin/X) and a company wants/needs a slight modification of the plugin X, it could copy the files from /src/plugin/X into /custom/plugin/X. They would automatically get precedence when loading (which could be handled in the events listener set-up you are proposing).

What I have come across is often that people (typically managers) say, yes I like you solution, BUT could you change the order of the fields (as an example). This could, under this logic by easily implemented by making a custom copy of the related file (any only those) and making the change there (for a typical example, look at class.pdf.php in my example above of how such a customization can easily be done).

The solution nevertheless comes with a disclaimer: If the developer of the plugin (or in my case, files under src) changes their code, the customization may no longer work and would need back-porting to the updated files.

The other advantage with custom is that you could do those proprietary stuff that some companies want in their project management system. For example, adding a firm-specific canvas could be done that way without "disturbing/touching" the rest of the system. That is how I did it in my example above to add a custom canvas "Service Value Chain" without having to touch any system file (except for the open issue of the $_SESSION['currentCanvas'] variable).

[broskees]

Okay, interesting. Let me put some thought into this.

[diderich]

The advantage of having /custom basically mirroring /src (except for /src/core) is that you get a clean separation what is supported system, and what is* unsupported customization*. I have used this approach over many years customizing opensource CRM systems for clients. No need to touch the supported system, but having the option to make any adjustments you (typically the paying client, in my case - and they can come-up with very unique requirements - I often won business because competitors said that change will break the system and I could do it in a simple non-breakable way) wants.

Note that the /custom approach is not incompatible with your plugin/registering approach, as long as it would support the mirror concept in a transparent way, i.e., if a repository X is registered and located in /src/plugin/PeterPan/myRegistry/class.james.php, the system would honor /custom/plugin/PeterPan/myRegistry/class.james.php instead of /src/plugin/PeterPan/myRegistry/class.james.php if it exists without having to touch the registration.

[diderich]

HI all. I would like to summarize (my understanding of) the discussion I had beforehand with @marcelfolaron as well as in this thread regarding the directory structure to ensure we have a common understanding.

Basic Principles

• The high level directory structure is independent of the engine used to access it, whether through a "hard coded" directory structure, as is the case today, or an event driven engine, as @broskees is currently developing, or a combination of both (which for practical purposes is probably the way to go in the short term)
• We distinguish between functionality and styling (where styling includes the high level layout of a page, but not the details)
• We want to keep as much of the functionality open source (GPL), but allow for developers to sell specific functionalities that go beyond the core functionality of a project management system
• We want to allow customer/integrators to customize the functionality of the system so that it meets specific needs. This will support a broad user base

Resulting directory structure

• /src - Contains the functionality, including language translations, of the system
• /public/theme - Contains the styling part of the system that warrants changing. Themes do not have any php code associated with them
• /custom - Contains the customization

Incorporating licensing

• /src/domain, /src/language, /src/core - Contains functionality that is made available under the GPL. We should encourage everyone to submit their functional extensions for consideration here (if they are rejected, they can either be implemented as a plugin (preferred version) or implemented as custom functions. Community support is provided for functionality.
• /src/plugin - Contains functionality that is not bound to the GPL license. Functionality here can be sold. Owner of code here is responsible to adapt their code to changes made in /src/core, /src/domain, when and if needed. Community support may or may not be provided
• /custom/domain, /custom/language (but not /custom/core) - Contains custom changes to existing functionality from /src/domain and /src/language (for example a company-specific dialect). It is not covered by the GPL and is typically installation specific. It is not supported by the community.
• /custom/plugin - Contains custom changes to existing functionality from /src/plugin. It is not covered by the GPL and is typically installation specific. It is not supported by the community.
• /public/themes - It is my understanding that the themes fall under the GPL, but I think this is still an unaddressed point.

Incorporating customization

• My experience with integrating systems is that often the selection of a given system depends on the possibility to customize certain aspects of it.
  • Example: Consider X selling a plug-in that produces invoices for the hours logged in the system and exports them. It is installed under /src/plugin/invoicing. Now customer/integrator Y who has bought the plug-in (because it is a useful feature to him) wants to transmit the invoices via API into his accounting system, rather than exporting them. This would be typically a customization that is not of value to other users or the developer of the plug-in, but may be key for Y to adopt the system (and should not fall under the GPL).
• Customization must be the sole responsibility of the customer running the system (or their integrator)
• Typically there are multiple approaches to customization
  • The files/functions to be customized are changed in the core system, ie. files under /src/: This is a bad idea, because each time the system is updated the customization is lost
  • The files/functions to be customized are copied/duplicated, e.g., to customize src/domain/leancanvas, one would make a copy to /src/domain/customleancanvas and make the changes there: This is also sub-optimal as changes to src/domain/leancanvas unrelated to the customization are not taken into account
  • The system provides hooks for customization. Although this sounds great, it only works if those hooks foresee where customization will be made (and the problem is that Murphy's law typically applies here)
  • The customization is done by cloning/mirroring those files that need to change from /src/... into /custom/... and making the changes there. The system will (as is implemented in my PR) first look for classes and templates under /custom/..., before looking under /src/...: This solution is the lesser evil of both other alternatives and has the huge advantage that it ensures ownership (/src is owned by the system, /custom is owned by the implementor/user of the system)

@marcelfolaron , is that a correct representation of the discussion we had? If not, please ammend :-)

[marcelfolaron]

Yes, correct!
Themes: I would like developers to allow selling themes under commercial licenses. As such themes would also be exempt from GPL.
In general we need to add individual license files for the subfolders within the exempt folder.

Eg themes/default <-- GPL License file
themes/dark <-- GPL License file

[marcelfolaron]

Now looking at release timelines.
@broskees Do you know when you might be able to finalize your PR?

@phulstaert Were you able to adjust the folder structure as discussed?

[broskees]

@marcelfolaron pretty close to done. Trying to settle all the merge conflicts with @diderich's PR right now.

If you have time for a chat I'd like to discuss some of the stuff I'm working on and how to have it settle in nicely with the vision you have for this.

[broskees]

@marcelfolaron update: Majority of the code is done. Just testing remains.

I have a plan for a more extensive filtering/event system specific to templates that can be implemented as we include twig/blade.

[marcelfolaron]

That's exciting!

…

On Wed, Nov 9, 2022 at 11:40 PM broskees ***@***.***> wrote: @marcelfolaron <https://github.com/marcelfolaron> update: Majority of the code is done. Just testing remains. I have a plan for a more extensive filtering/event system specific to templates that can be implemented as we include twig/blade. — Reply to this email directly, view it on GitHub <#1095 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALG4EFQQZVWFIZYJ2KJ62IDWHR4ENANCNFSM6AAAAAARSL7VU4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[broskees]

@marcelfolaron Did you see this ask?

If you have time for a chat I'd like to discuss some of the stuff I'm working on and how to have it settle in nicely with the vision you have for this.