DynamicField Reference Agent: Empty value doesn't work

StefanAbel-OTOBO commented 3 months ago

When you create a Dynamic Field of type (Reference) Agent the Empty value option doesn't work at all, which causes the alphabetically first agent to be selected automatically.

I reproduced ths on ov270

This is my DF configuration:

This is the result (it looks the same e.g. in a Process Activity Dialog or in AgentTicketPhone):

stefanhaerter commented 1 month ago

I managed to track the root bug down. There are a number of things which come into play to create the environment, but in the end it boils down to:

Double quote precedes minus precedes alphanumeric character

in regard to perl sort. Please feel free to ask me for further details, but I didn't want to clutter this comment.

I see two possible ways to solve the root cause:

Insert sorting function ensuring the precedence of the exact combination "" => '-' within selection data
Insert something like the following to ensure that the empty value is marked as selected if no other information is given:

sub BuildSelection {
    [...]
    if ( !defined $Param{SelectedID} && !defined $Param{SelectedValue} ) {
        $Param{SelectedID} = '';
    }
    [...]
}

stefanhaerter commented 1 month ago

For the interested reader, the place where the mentioned sorting takes place is here:

https://github.com/RotherOSS/otobo/blob/9670cb1d6814a847a241626a5428cf75291adc08/Kernel/Output/HTML/Layout.pm#L5920-L5926

svenoe commented 1 month ago

Is reproducible with a standard dropdown dynamic field including "-" as value. (Probably just " suffices.) See probably: https://github.com/RotherOSS/otobo/blob/fecfc99615e486d34aa887cd180c7b4317a2e175/var/httpd/htdocs/js/Core.UI.InputFields.js#L568 which, if no option is selected returns the first option, and line 594 where an empty key is skipped. Problem arises when stuff (") is sorted in front of the empty value (-). (See also backend sorting above.)

MichaelThumes commented 1 month ago

steps to reproduce:

create DF dropdown with 2-3 keys+values like a,b,c
for one of the fields set value to '-' or '"-"' or '"' (without surrounding single quotes)
set 'Add empty value' for the dropdown DF to True
add DF to say AgentTicketPhone
browse to AgentTicketPhone

Expected Behaviour:

DF dropdown has empty value pre-selected

Observed Behaviour:

DF dropdown has one of the weird values from above pre-selected , eg '"-"'

root cause

Combination of the backend sorting and the way the frontend handles selection (see above comments)

possible approaches

change backend sorting
change frontend

MichaelThumes commented 1 month ago

option a) patch the backend Kernel/Output/HTML/Layout.pm

diff --git a/Kernel/Output/HTML/Layout.pm b/Kernel/Output/HTML/Layout.pm
index 86b163855..ae3a6ba35 100644
--- a/Kernel/Output/HTML/Layout.pm
+++ b/Kernel/Output/HTML/Layout.pm
@@ -5919,6 +5919,9 @@ sub _BuildSelectionDataRefCreate {
         }
         else {
             @SortKeys = sort {
+                # assure the empty option, if any, is always sorted first                
+                if($a eq '' ) { return -1; }
+                if($b eq '' ) { return  1; }
                 lc( $DataLocal->{$a} // '' )
                     cmp lc( $DataLocal->{$b} // '' )
             } ( keys %{$DataLocal} );

That seems to work but adds 2 condition checks in the inner sort loop., albeit cheap ones.

Alternative is less condition checks but more typing:

diff --git a/Kernel/Output/HTML/Layout.pm b/Kernel/Output/HTML/Layout.pm
index 86b163855..fa7d013cd 100644
--- a/Kernel/Output/HTML/Layout.pm
+++ b/Kernel/Output/HTML/Layout.pm
@@ -5918,10 +5918,15 @@ sub _BuildSelectionDataRefCreate {
             # already done before the translation
         }
         else {
+            my $EmptyValue = delete $DataLocal->{''};
             @SortKeys = sort {
                 lc( $DataLocal->{$a} // '' )
                     cmp lc( $DataLocal->{$b} // '' )
             } ( keys %{$DataLocal} );
+            if( defined $EmptyValue) {
+                $DataLocal->{''} = $EmptyValue;
+                unshift @SortKeys, '';
+            }
             $OptionRef->{Sort} = 'AlphanumericValue';
         }

option b) - hack the frontend Core.UI.InputFields.js

diff --git a/var/httpd/htdocs/js/Core.UI.InputFields.js b/var/httpd/htdocs/js/Core.UI.InputFields.js
index 431745e85..7273b591e 100644
--- a/var/httpd/htdocs/js/Core.UI.InputFields.js
+++ b/var/httpd/htdocs/js/Core.UI.InputFields.js
@@ -565,7 +565,7 @@ Core.UI.InputFields = (function (TargetNS) {
                 Selection = $.unique($SelectObj.val());
                 SelectionLength = Selection.length;
             } else {
-                Selection = [ $SelectObj.val() ];
+                Selection = [ PossibleNone ? '||-' : $SelectObj.val() ];
                 SelectionLength = 1;
             }

RotherOSS / otobo