Reintroduce Conf/DeprecationHandler

[mdeweerd]

@eldy
What is the issue?

IMHO, a test case should demonstrate the issue so that the real cause can be determined.

From reading the code I can only see the following as a possible cause, and an overlooked deprecation mapping (I could not find any):

dolibarr/htdocs/core/class/conf.class.php

Lines 570 to 572 in 56f9ec4

	$mapping = $this->deprecatedProperties();
	if (array_key_exists($modulename, $mapping)) {
	$newmodulename = $mapping[$modulename];

Possibly $mapping should be restricted to MODULE_MAPPING and not the entire deprecation list.

[eldy]

Got several issues:

• Some properties conf->xx->dir_output were no more defined, for example conf->facture->dir_output, breaking a lot of external module code. A transition step is required before breaking zxternal module.
• directories where files were stored was also renamed, for example all pdf genrated of files uploaded of invoices were saved into a new dir, making disappear all upload files
• some properties ->enabled was empty when module is on breaking other point of modules.
• some menus also disappeared, for exemple the menu of purchase orders.
• got also trouble in storing data into llx_ecmfiles where the id of module change without updating old values, same for some link in llx_element_element, making old data no more consistent with code

It seems conf class is too critical for the moment to use doldeprecatedhandler.
Also there is some prerequisite to achieve before making refactoring on this class .
For exemple we must first replace the hardcoded conf->xxxx->dir_ouptut znd ->multidir_output with a function (we already have the function but the replacement is still in progress) so playing with conf->xxx will be easier to deal for those properties (like we started to do with isModEnabled).

[mdeweerd]

This seems strange as $conf->facture should access $conf->invoice and hence have the directory defined.

The idea of the handler is not have any prerequisite on code changes. I guess that a test case should be able to show the issue so that the real reason can be discovered - maybe something in the deprecationhander, maybe something else.

[hregis]

@mdeweerd @eldy it's the pure reality of a development from 2003 which seems incomprehensible to a pre-pubescent developer! ;-)

[hregis]

@mdeweerd warning for " ->multidir_output" !! this is for multicompany ! and multicompany it's me ! i'm nervous break down !! ;-)

[mdeweerd]

• Some properties conf->xx->dir_output were no more defined, for example conf->facture->dir_output, breaking a lot of external module code. A transition step is required before breaking zxternal module.
• directories where files were stored was also renamed, for example all pdf genrated of files uploaded of invoices were saved into a new dir, making disappear all upload files

These two items are fixed. The issue was with the initialisation of the dir_output which was using the new name.

I created a test case that currently verifies that the directory is the same for the old and the new module name.
It should be extended with the "exact" expected root directories for the modules.

The develop branch currently fails this test, with the deprecation handler enables on conf it does not!

@hregis The multicompany is not subject to deprecation and should not be handled differently.

I do not know if the next 3 mentionned items still fail with this change. A test case would be useful there too.

A mapping of "newmodulename => rootpath" would be useful both for testing and the initialisations in conf.

[mdeweerd]

@eldy I guest that the item "some menus also disappeared, for exemple the menu of purchase orders." is not an issue when I have the following in the Vendor tab:

If yes, that was also an effect of the module initialisation.

[eldy]

@eldy I guest that the item "some menus also disappeared, for exemple the menu of purchase orders." is not an issue when I have the following in the Vendor tab:

If yes, that was also an effect of the module initialisation.

I remember that i fixed this point by adding the code after the line

// Duplicate entry with the new name

into conf.class.php

[mdeweerd]

I remember that i fixed this point by adding the code after the line

// Duplicate entry with the new name

into conf.class.php

Good, then that is fixed by the change in the module initialisation as weel (in this PR).

Now I am trying to understand the following statement:

got also trouble in storing data into llx_ecmfiles where the id of module change without updating old values, same for some link in llx_element_element, making old data no more consistent with code

• id of the module = 'public $numero'?
• Building a test test case ... (how to use ecm, and make a test demonstrating that it did not work before the fix).

[mdeweerd]

I found that the ecm module is used for the document management and I was able to reproduce the issue interactively.

The reason for the issue that was still present after the change in the initialisation was that the Conf class was referencing the old module names itself thereby accessing the private properties directly without triggering the deprecationhandler.

So that is fixed by removing the private fields for the old names. I tested this locally.

So IMHO the reported issues are fixed and the method can be reinstated.

This means that to avoid side effects, private 'old' names can not be used.

[mdeweerd]

Extra note: I think it would be a good idea to define the expected subdirectory in the module and use that information in the Conf class rather than having the information in the Conf class itself.

Maybe implement a getter function that would have a default implementation in DolibarrModules that returns the name of the module. A module can then override this getter as needed to return another path.