FreePBX / framework

This module provides a facility to install bug fixes to the framework code that is not otherwise housed in a module
http://www.freepbx.org
GNU General Public License v3.0
63 stars 59 forks source link

[bug]: breaking section in module.xml #155

Open vsc55 opened 2 weeks ago

vsc55 commented 2 weeks ago

FreePBX Version

FreePBX 17

Issue Description

I am adding a breaking section to the modele.xml to be able to add a conflict condition with another module but the following error is generated. This would be an example of the modification:

<module>
    <rawname>module</rawname>
    .....
    <breaking>
        <module>
            <rawname>cdr</rawname>
                <type>conflict</type>
        </module>
    </breaking>
    ...
</module>

I tested it by enabling and disabling the module to which I added the previous code in the module.xml file, so the exception occurs in the 'enable' section. However, the same code is in install/uninstall/etc., so I assume it will give the same error.

I tested this on FreePBX 17, but I understand that the same happens in version 16.

Operating Environment

Whoops\Exception\ErrorException thrown with message "Undefined array key "OSS PBX End Point Manager""

Stacktrace:

2 Whoops\Exception\ErrorException in /usr/src/freepbx/framework/amp_conf/htdocs/admin/page.modules.php:645

1 Whoops\Run:handleError in /usr/src/freepbx/framework/amp_conf/htdocs/admin/page.modules.php:645

0 include in /usr/src/freepbx/framework/amp_conf/htdocs/admin/config.php:481

Relevant log output

I have made some bugs for $conflicterrors and $module.
These are the data.

$conflicterrors = $FreePBX->Modules->checkConflicts($modules[$module]);

$module => endpointman
$conflicterrors =>

 Array
(
    [breaking] => 1
    [issues] => Array
        (
            [cdr] => Array
                (
                    [0] => The currently installed module CDR Reports [cdr] conflicts with OSS PBX End Point Manager [endpointman]. Having both installed may cause issues with the functionality of your system.
                )

        )

    [replacements] => Array
        (
        )

)

If more modules are added in the brocket section, $conflicterrors is null.
vsc55 commented 2 weeks ago

By the way, there is no information regarding this option in the wiki documentation. https://sangomakb.atlassian.net/wiki/spaces/FP/pages/10289830/module.xml

Adding something like this would be nice.

<module>
    <rawname>reset</rawname>
    <name>Reset Module</name>
    <breaking>
        <module>
            <rawname>target_module_1</rawname>
            <type>conflict</type>
            <version>1.0.0</version>
            <errormessage>The module RESET conflicts with TARGET_MODULE_1. Having both installed may cause issues with the functionality of your system.</errormessage>
        </module>
        <module>
            <rawname>target_module_2</rawname>
            <type>conflict</type>
            <version>2.0.0</version>
            <errormessage>The module RESET conflicts with TARGET_MODULE_2. Having both installed may cause issues with the functionality of your system.</errormessage>
        </module>
        <module>
            <rawname>target_module_3</rawname>
            <type>conflict</type>
            <!-- No version or errormessage provided -->
        </module>
    </breaking>
</module>
kguptasangoma commented 1 week ago

We have callback logic to check the conflict module during the install time so why you like to add into the module.xml ?

Ref - https://github.com/FreePBX/ucp/blob/release/16.0/install.inc.php#L8

vsc55 commented 1 week ago

Hi @kguptasangoma, Yes, I already saw that logic, but it seemed cleaner to me to have it in the module.xml just like the depends and requirements. I also saw that the breaking option already existed, so I decided to create a PR to make this option operational.