salesagility / SuiteCRM-Core

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
182 stars 134 forks source link

Multiple display logics of a field not working #563

Open xkrdudrlf opened 4 weeks ago

xkrdudrlf commented 4 weeks ago

Issue

When a user configures multiple display logics for a field, currently it is not working properly.

For example, if we have set 2 display logics for a field 'status' under 'Lead' module such as follows:

$dictionary['Lead']['fields']['status']['displayLogic'] = [
    'readonly_when_converted' => [
        'key' => 'displayType',
        'modes' => ['edit'],
        'params' => [
            'fieldDependencies' => [
                'title'
            ],
            'targetDisplayType' => 'show',
            'activeOnFields' =>  [
                'title' => [ 'Converted' ]
            ]
        ]
    ],
    'readonly_when_dead' => [
        'key' => 'displayType',
        'modes' => ['edit'],
        'params' => [
            'fieldDependencies' => [
                'title'
            ],
            'targetDisplayType' => 'none',
            'activeOnFields' =>  [
                'title' => [ 'Dead' ]
            ]
        ] 
    ]
];

'status' field should be shown when 'title' field value is 'Converted' and should be hidden when 'title' field value is 'Dead'

However, the field is currently not being shown when the value is 'Converted' and it turns out that whenever any display logic condition holds, the field will always be hidden.

Possible Fix

I've found 2 problems in how we set "field.display" value in runAll() of 'FieldLogicDisplayManager' class

    runAll(field: Field, record: Record, mode: ViewMode): void {
        let toDisplay: DisplayType = 'show';

        if(!field.displayLogic) {
            return;
        }

        const validModeLogic = Object.values(field.displayLogic).filter(logic => {
            const allowedModes = logic['modes'] ?? [];
            return !!(allowedModes.length && allowedModes.includes(mode));
        });

        if (!validModeLogic || !validModeLogic.length) {
            field.display = toDisplay;
            return;
        }

        let defaultDisplay = field.defaultDisplay ?? 'show';

        let targetDisplay: DisplayType = 'none'; // <= P1. it does not reflect what's in displayLogic['params']['targetDisplayType']

        if (defaultDisplay === 'none') {
            targetDisplay = 'show';
        }

        const context = {
            record,
            field,
            module: record.module
        } as ActionContext;

        const isActive = validModeLogic.some(logic => {
            const data: FieldLogicDisplayActionData = this.buildActionData(logic, context);
            return this.actions[mode][logic.key].run(data, logic);
        });

        if (isActive) {
            defaultDisplay = targetDisplay; // <= P2. there can be multiple targetDisplay from multiple displayLogics

        }

        toDisplay = defaultDisplay as DisplayType;

        if (defaultDisplay === 'show') {
            toDisplay = 'show';
        }

        field.display = toDisplay;
    }

P1. As we can see in the above, the targetDisplay does not reflect the value we set in displayLogic['params']['targetDisplayType']

P2. Also, when there are multiple displayLogics for a field, according to the CRM official documentation, all these displayLogics will be applied as 'OR' condition. For example, when displayLogic A is 'active'(which means the display logic condition holds), the displayLogic A's 'targetDisplayType' should be applied. Also, when displayLogic B is also 'active', the displayLogic B's 'targetDisplayType' should be applied too. My current fix is currently take the last specified displayLogic's targetDisplayType since there can be a conflict for deciding the value. The current codebase does not handle the case where there can be multiple displayLogics for a field in terms of deciding targetDisplay. So, I rewrote runAll() of FieldLogicDisplayManager to tackle aforementioned 2 problems as follows:

     runAll(field: Field, record: Record, mode: ViewMode): void {
        if (!field.displayLogic) {
            return;
        }

        // 1. Set default display 
        const defaultDisplay: DisplayType = (field.defaultDisplay ?? "show") as DisplayType;

        // 2. Set target display
        let targetDisplay: DisplayType = null;

        // 2-1. Get valid mode logics - only the mode logics relevant to the current 'mode'
        const validModeLogics = Object.values(field.displayLogic).filter(logic => {
            const allowedModes = logic['modes'] ?? [];
            return !!(allowedModes.length && allowedModes.includes(mode));
        });
        if (validModeLogics.length === 0) {
            return;
        }

        // 2-2. Set target display by applying the valid mode logics
        const context = {
            record,
            field,
            module: record.module
        } as ActionContext;        
        for (const modeLogic of validModeLogics) {
            const data: FieldLogicDisplayActionData = this.buildActionData(modeLogic, context);
            const isModeLogicActive: boolean = Boolean(this.actions[mode][modeLogic.key].run(data, modeLogic));
            // Take targetDisplayType of the last mode logic by overwriting targetDisplay in case there are multiple active mode logics
            if (isModeLogicActive) {
                targetDisplay = modeLogic['params']['targetDisplayType'];
            }
        }

        // 3. Set field display
        field.display = targetDisplay === null ? defaultDisplay : targetDisplay; 
    }

Steps to Reproduce the Issue

1. Try setting multiple displayLogics for any field as follows:

$dictionary['Lead']['fields']['status']['displayLogic'] = [
    'readonly_when_converted' => [
        'key' => 'displayType',
        'modes' => ['edit'],
        'params' => [
            'fieldDependencies' => [
                'title'
            ],
            'targetDisplayType' => 'show',
            'activeOnFields' =>  [
                'title' => [ 'Converted' ]
            ]
        ]
    ],
    'readonly_when_dead' => [
        'key' => 'displayType',
        'modes' => ['edit'],
        'params' => [
            'fieldDependencies' => [
                'title'
            ],
            'targetDisplayType' => 'none',
            'activeOnFields' =>  [
                'title' => [ 'Dead' ]
            ]
        ] 
    ]
];
  1. Test whether a field is shown correctly in the page according to the display type set in displayLogics

    For example, in the above case, 'status' field should be shown when 'title' field value is 'Converted' and should be hidden when 'title' field value is 'Dead'

Context

No response

Version

8.7.0

What browser are you currently using?

Chrome

Browser Version

No response

Environment Information

PHP 8.2

Operating System and Version

Ubuntu 23.10