7316 fix(ModuleInstall) - delete Vardefs files after deleting custom field

[re8260]

Description

Fix ISSUE 7316 Delete field with class ModuleInstaller and uninstall_custom_fields() doesn't delete Vardefs files

Motivation and Context

DynamicField class require module name in constructor to initialize class variable base_path. Its works correct when field deleted from studio because in /modules/ModuleBuilder/controller.php function action_DeleteField Module Name passed to DynamicField constructor correct $df = new DynamicField($moduleName) ; but in /ModuleInstall/ModuleInstaller.php Module Name function uninstall_custom_fields was missing: $dyField = new DynamicField();

How To Test This

Steps to Reproduce 7316

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

• [x] My code follows the code style of this project found here.
• [ ] My change requires a change to the documentation.
• [x] I have read the How to Contribute guidelines.

[Mac-Rae]

Hi @re8260

This appears to have failed on 2 of our testers, could you provide more detailed steps on how to replicate this issue?

Thanks

[re8260]

Hello @Mac-Rae.

As you can see from the constructor of DynamicField if $module is empty and $_REQUEST['module'] is empty because I'm calling it from PHP code with multiple fields of different modules, constructor will assign wrong value to $this->base_path : custom/Extension/modules//Ext/Vardefs. as a result the file will stay in file system.

/**
 * DynamicField constructor.
 * @param string $module
 */
public function __construct($module = '')
{
    global $sugar_config;
    $this->module = (!empty($module)) ? $module 
        : ((isset($_REQUEST['module']) && !empty($_REQUEST['module'])) ? $_REQUEST ['module'] 
        : '');
    $this->base_path = "custom/Extension/modules/{$this->module}/Ext/Vardefs";
    if (isset($sugar_config['dbconfig'])) {
        $this->db = DBManagerFactory::getInstance();
    }
}

and now in function that should remove all files it fail to delete because the path is incorrect, path doesn't include module name:

    /**
     * @param $field
     */
    protected function removeVardefExtension($field)
    {
        $file_loc = "$this->base_path/sugarfield_{$field->name}.php";

        if (is_file($file_loc)) {
            unlink($file_loc);
        }
    }

Simple Code to reproducer the issue that create custom fields and then try to delete it:

<?php
define('sugarEntry', true);

chdir(dirname(__FILE__));

require_once('include/entryPoint.php');
require_once('ModuleInstall/ModuleInstaller.php');
require_once('modules/ModuleBuilder/Module/StudioModuleFactory.php');
/**
 * Allow to run only from CLI
 */
$sapi_type = php_sapi_name();
if (substr($sapi_type, 0, 3) != 'cli') {
    die("migrate.php is CLI only.");
}

$fieldsToCreate = [];
$fieldsToDelete = [];

$fieldsToCreate[] = [
    'name' => 'test_field_a_c',
    'label' => 'LBL_TEST_FIELD_A',
    'type' => 'varchar',
    'module' => 'Cases',
    'help' => '',
    'comment' => '',
    'default_value' => '',
    'max_size' => 255,
    'required' => false,
    'reportable' => true,
    'audited' => true,
    'importable' => 'true',
    'duplicate_merge' => false,
];
$fieldsToCreate[] = [
    'name' => 'test_field_b_c',
    'label' => 'LBL_TEST_FIELD_B',
    'type' => 'varchar',
    'module' => 'Accounts',
    'help' => '',
    'comment' => '',
    'default_value' => '',
    'max_size' => 255,
    'required' => false,
    'reportable' => true,
    'audited' => true,
    'importable' => 'true',
    'duplicate_merge' => false,
];

$fieldsToDelete[] = array(
    'module' => 'Cases',
    'name' => 'test_field_a_c'
);
$fieldsToDelete[] = array(
    'module' => 'Accounts',
    'name' => 'test_field_b_c'
);

$moduleInstaller = new ModuleInstaller();
/**
 * Create custom fields
 */
@$moduleInstaller->install_custom_fields($fieldsToCreate);

/**
 * Delete custom fields
 */
@$moduleInstaller->uninstall_custom_fields($fieldsToDelete);

After running this code, no new files should stay in the project because this code create and the delete new fields, but as you can see before my fix it leave them undeleted:

[SuiteBot]

All committers have signed the CLA.

[serhiisamko091184]

Hello @re8260

The PR has been marked as stale because there has been no recent activity. It will be closed if no further activity occurs. Thanks for your contributions.

Regards, Serhii

[serhiisamko091184]

Hello @re8260,

I've just removed the label stale as I put it by accident,

Thanks for your contribution, the fix is still relevant, would you be so kind to update your branch to make it up to date with the current hotfix?

Many thanks in advance!

Regards, Serhii

[serhiisamko091184]

@re8260, one more thing - could you please update the commit message to the following format: Fix Issue number - Commit message? For example:

For more information, visit our documentation: https://docs.suitecrm.com/community/contributing-code/bugs/

Thank you!

Regards, Serhii