From eab478b94d05e761a64d7786d6e1ea91f13e329d Mon Sep 17 00:00:00 2001 From: bernhard Date: Fri, 2 Jun 2023 18:01:51 +0200 Subject: [PATCH 1/4] Issue #2327: get started on the method JSONReply Use it for the ContactsWD dynamic field. Use the method Attachment() internally. --- Kernel/Modules/AgentContactWDSearch.pm | 57 ++++++++------------------ Kernel/Output/HTML/Layout.pm | 35 ++++++++++++++++ 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/Kernel/Modules/AgentContactWDSearch.pm b/Kernel/Modules/AgentContactWDSearch.pm index 66a7cdf61c..2304a5250b 100644 --- a/Kernel/Modules/AgentContactWDSearch.pm +++ b/Kernel/Modules/AgentContactWDSearch.pm @@ -36,9 +36,10 @@ sub new { sub Run { my ( $Self, %Param ) = @_; - my $JSON = ''; + my $LayoutObject = $Kernel::OM->Get('Kernel::Output::HTML::Layout'); - # search customers + # search contacts + my @Results; if ( !$Self->{Subaction} ) { # get needed params @@ -46,8 +47,6 @@ sub Run { my $Search = $ParamObject->GetParam( Param => 'Term' ) || ''; my $MaxResults = int( $ParamObject->GetParam( Param => 'MaxResults' ) || 20 ); my $FieldName = $ParamObject->GetParam( Param => 'Field' ); - - my $LayoutObject = $Kernel::OM->Get('Kernel::Output::HTML::Layout'); if ( !$FieldName || @@ -57,13 +56,11 @@ sub Run { ) ) { - return $Self->_ReturnJSON( - Content => $LayoutObject->JSONEncode( - Data => { - Success => 0, - Messsage => 'Need Field!', - } - ), + return $LayoutObject->JSONReply( + Data => { + Success => 0, + Messsage => 'Need Field!', + } ); } else { @@ -80,13 +77,11 @@ sub Run { || $DynamicField->{ValidID} ne 1 ) { - return $Self->_ReturnJSON( - Content => $LayoutObject->JSONEncode( - Data => { - Success => 0, - Messsage => 'Error reading dynamic field!', - } - ), + return $LayoutObject->JSONReply( + Data => { + Success => 0, + Messsage => 'Error reading dynamic field!', + } ); } @@ -103,7 +98,6 @@ sub Run { $Search =~ s{ Z \z }{}xms; # get possible contacts - my @Data; my $MaxResultCount = $MaxResults; my $PossibleContacts = $DynamicField->{Config}->{ContactsWithData}; my $SearchableFields = $DynamicField->{Config}->{SearchableFieldsComputed}; @@ -121,37 +115,18 @@ sub Run { next CONTACT if !$ContactData{ValidID}; next CONTACT if $ContactData{ValidID} ne 1; - push @Data, { + push @Results, { Key => $Contact, Value => $ContactData{Name}, }; $MaxResultCount--; last CONTACT if $MaxResultCount <= 0; } - - # build JSON output - $JSON = $LayoutObject->JSONEncode( - Data => \@Data, - ); } - return $Self->_ReturnJSON( - Content => $JSON, + return $LayoutObject->JSONReply( + Data => \@Results, ); } -sub _ReturnJSON { - my ( $Self, %Param ) = @_; - - # send JSON response - my $LayoutObject = $Kernel::OM->Get('Kernel::Output::HTML::Layout'); - return $LayoutObject->Attachment( - ContentType => 'application/json', - Content => $Param{Content} || '', - Type => 'inline', - NoCache => 1, - ); - -} - 1; diff --git a/Kernel/Output/HTML/Layout.pm b/Kernel/Output/HTML/Layout.pm index c69f811cf8..6c6b25f29a 100644 --- a/Kernel/Output/HTML/Layout.pm +++ b/Kernel/Output/HTML/Layout.pm @@ -2692,6 +2692,7 @@ sub Attachment { Priority => 'error', Message => "Got no $_!", ); + $Self->FatalError(); } } @@ -2773,6 +2774,40 @@ sub Attachment { return $Param{Content}; } +=head2 JSONReply() + +Give back a data structure as a JSON response. +A bit like the method C. +As a side effect headers of the HTTP response are set in the object C. + +=cut + +sub JSONReply { + my ( $Self, %Param ) = @_; + + # check needed params + for my $Needed (qw(Data)) { + if ( !defined $Param{$Needed} ) { + $Kernel::OM->Get('Kernel::System::Log')->Log( + Priority => 'error', + Message => "Got no $Needed!", + ); + + $Self->FatalError(); + } + } + my $Content = $Self->JSONEncode( + Data => \$Param{Data}, + ); + + return $Self->Attachment( + ContentType => 'application/json', + Content => $Content || '', + Type => 'inline', + NoCache => 1, + ); +} + =head2 PageNavBar() generates a page navigation bar From feb1e81c03345dbcd29ca900175771ea15db78fd Mon Sep 17 00:00:00 2001 From: bernhard Date: Fri, 2 Jun 2023 18:03:30 +0200 Subject: [PATCH 2/4] Issue #2327: try to enhance POD of JSONEncode() --- Kernel/Output/HTML/Layout.pm | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Kernel/Output/HTML/Layout.pm b/Kernel/Output/HTML/Layout.pm index 6c6b25f29a..6a0dabb875 100644 --- a/Kernel/Output/HTML/Layout.pm +++ b/Kernel/Output/HTML/Layout.pm @@ -521,20 +521,36 @@ sub Block { =head2 JSONEncode() -Encode perl data structure to JSON string +Serialize a Perl data structure as JSON. + my %Hash = ( + Key1 => 'Something', + Key2 => [ "Foo", "Bar" ], + ); my $JSON = $LayoutObject->JSONEncode( - Data => $Data, - NoQuotes => 0|1, # optional: no double quotes at the start and the end of JSON string + Data => \%Hash, + NoQuotes => 0, # optional: 0|1 no double quotes at the start and the end of JSON string, default is 0 ); +Returns: + + $JSON = <<'END_JSON'; + { + "Key1" : "Something", + "Key2" : [ + "Foo", + "Bar" + ], + } + END_JSON + =cut sub JSONEncode { my ( $Self, %Param ) = @_; # check for needed data - return if !defined $Param{Data}; + return unless defined $Param{Data}; # get JSON encoded data my $JSON = $Kernel::OM->Get('Kernel::System::JSON')->Encode( @@ -2667,7 +2683,7 @@ returns browser output to display/download a attachment. # scripts, flash etc. ); -Or for AJAX html snippets: +Or for AJAX HTML snippets: $HTML = $LayoutObject->Attachment( Type => 'inline', # optional, default: attachment, possible: inline|attachment From 60743f180d8b81ba8b1e636348c0c239e8e7364a Mon Sep 17 00:00:00 2001 From: bernhard Date: Sat, 3 Jun 2023 12:06:41 +0200 Subject: [PATCH 3/4] Issue #2327: Add empty line before FatalError() because FatalError() changes the control flow. --- Kernel/Output/HTML/Layout.pm | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/Kernel/Output/HTML/Layout.pm b/Kernel/Output/HTML/Layout.pm index 6a0dabb875..cc651eed8a 100644 --- a/Kernel/Output/HTML/Layout.pm +++ b/Kernel/Output/HTML/Layout.pm @@ -474,7 +474,8 @@ sub SetEnv { Priority => 'error', Message => "Need $_!" ); - $Self->FatalError(); + + $Self->FatalError; } } $Self->{EnvNewRef}->{ $Param{Key} } = $Param{Value}; @@ -2472,14 +2473,16 @@ sub BuildSelection { Priority => 'error', Message => 'Need Depend Param Ajax option!', ); - $Self->FatalError(); + + $Self->FatalError; } if ( !$Param{Ajax}->{Update} ) { $Kernel::OM->Get('Kernel::System::Log')->Log( Priority => 'error', Message => 'Need Update Param Ajax option()!', ); - $Self->FatalError(); + + $Self->FatalError; } my $Selector = $Param{ID} || $Param{Name}; $Param{OnChange} = "Core.AJAX.FormUpdate(\$('#" @@ -2531,7 +2534,8 @@ sub BuildSelection { Priority => 'error', Message => 'Each Filter must provide Name and Values!', ); - $Self->FatalError(); + + $Self->FatalError; } $Index++; } @@ -2619,7 +2623,8 @@ sub Permission { Priority => 'error', Message => "Got no $Needed!", ); - $Self->FatalError(); + + $Self->FatalError; } } @@ -2709,7 +2714,7 @@ sub Attachment { Message => "Got no $_!", ); - $Self->FatalError(); + $Self->FatalError; } } @@ -2809,7 +2814,7 @@ sub JSONReply { Message => "Got no $Needed!", ); - $Self->FatalError(); + $Self->FatalError; } } my $Content = $Self->JSONEncode( @@ -4498,7 +4503,7 @@ sub CustomerFatalError { my ( $Self, %Param ) = @_; # Prevent endless recursion in case of problems with Template engine. - return if ( $Self->{InFatalError}++ ); + return if $Self->{InFatalError}++; if ( $Param{Message} ) { $Kernel::OM->Get('Kernel::System::Log')->Log( @@ -4715,7 +4720,7 @@ sub CustomerNavigationBar { # load module if ( !$MainObject->Require( $Jobs{$Job}->{Module} ) ) { - $Self->FatalError(); + $Self->FatalError; } my $Object = $Jobs{$Job}->{Module}->new( %{$Self}, @@ -6290,7 +6295,8 @@ sub SetRichTextParameters { Priority => 'error', Message => "Need HashRef in Param Data! Got: '" . ref( $Param{Data} ) . "'!", ); - $Self->FatalError(); + + $Self->FatalError; } # get needed objects @@ -6428,7 +6434,8 @@ sub CustomerSetRichTextParameters { Priority => 'error', Message => "Need HashRef in Param Data! Got: '" . ref( $Param{Data} ) . "'!", ); - $Self->FatalError(); + + $Self->FatalError; } # get needed objects From fe577b16a846d9fc944130219ddb16bfda069cda Mon Sep 17 00:00:00 2001 From: bernhard Date: Sat, 3 Jun 2023 12:27:09 +0200 Subject: [PATCH 4/4] Issue #2327: tidying Add some comments. Write the check for Subactions as a guard clause. Limit scope of some variables. Try to make the control flow more obvious. --- Kernel/Modules/AgentContactWDSearch.pm | 116 +++++++++++++------------ 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/Kernel/Modules/AgentContactWDSearch.pm b/Kernel/Modules/AgentContactWDSearch.pm index 2304a5250b..02ab96e12d 100644 --- a/Kernel/Modules/AgentContactWDSearch.pm +++ b/Kernel/Modules/AgentContactWDSearch.pm @@ -27,10 +27,7 @@ sub new { my ( $Type, %Param ) = @_; # allocate new hash for object - my $Self = {%Param}; - bless( $Self, $Type ); - - return $Self; + return bless {%Param}, $Type; } sub Run { @@ -38,52 +35,55 @@ sub Run { my $LayoutObject = $Kernel::OM->Get('Kernel::Output::HTML::Layout'); - # search contacts - my @Results; - if ( !$Self->{Subaction} ) { - - # get needed params - my $ParamObject = $Kernel::OM->Get('Kernel::System::Web::Request'); - my $Search = $ParamObject->GetParam( Param => 'Term' ) || ''; - my $MaxResults = int( $ParamObject->GetParam( Param => 'MaxResults' ) || 20 ); - my $FieldName = $ParamObject->GetParam( Param => 'Field' ); - if ( - !$FieldName - || - ( - $FieldName !~ m{ \A Autocomplete_DynamicField_ (.*) \z }xms - && $FieldName !~ m{ \A Search_DynamicField_ (.*) \z }xms - ) - ) - { - return $LayoutObject->JSONReply( - Data => { - Success => 0, - Messsage => 'Need Field!', - } - ); - } - else { - $FieldName = $1; # effectively remove prefix Autocomplete_DynamicField_ or Search_DynamicField_ - } + # Return empty list when Subaction was passed as subactions are not supported here. + return $LayoutObject->JSONReply( Data => [] ) if $Self->{Subaction}; + + # Get FieldName from $ParamObject, bail out when not found + my $ParamObject = $Kernel::OM->Get('Kernel::System::Web::Request'); + my $FieldName = $ParamObject->GetParam( Param => 'Field' ); + if ( + !$FieldName + || + ( + $FieldName !~ m{ \A Autocomplete_DynamicField_ (.*) \z }xms + && $FieldName !~ m{ \A Search_DynamicField_ (.*) \z }xms + ) + ) + { + return $LayoutObject->JSONReply( + Data => { + Success => 0, + Messsage => 'Need Field!', + } + ); + } + else { + $FieldName = $1; # effectively remove prefix Autocomplete_DynamicField_ or Search_DynamicField_ + } - # get dynamic field - my $DynamicField = $Kernel::OM->Get('Kernel::System::DynamicField')->DynamicFieldGet( - Name => $FieldName, + # Get config for the dynamic field and check the sanity + my $DynamicField = $Kernel::OM->Get('Kernel::System::DynamicField')->DynamicFieldGet( + Name => $FieldName, + ); + if ( + !IsHashRefWithData($DynamicField) + || $DynamicField->{FieldType} ne 'ContactWD' + || $DynamicField->{ValidID} ne 1 + ) + { + return $LayoutObject->JSONReply( + Data => { + Success => 0, + Messsage => 'Error reading dynamic field!', + } ); - if ( - !IsHashRefWithData($DynamicField) - || $DynamicField->{FieldType} ne 'ContactWD' - || $DynamicField->{ValidID} ne 1 - ) - { - return $LayoutObject->JSONReply( - Data => { - Success => 0, - Messsage => 'Error reading dynamic field!', - } - ); - } + } + + # search contacts, rthe results will be returned as JSON + my @Results; + { + my $Search = $ParamObject->GetParam( Param => 'Term' ) || ''; + my $RemainingResultCount = int( $ParamObject->GetParam( Param => 'MaxResults' ) || 20 ); # make search safe and use '*' as wildcard $Search =~ s{ \A \s+ }{}xms; @@ -98,29 +98,37 @@ sub Run { $Search =~ s{ Z \z }{}xms; # get possible contacts - my $MaxResultCount = $MaxResults; my $PossibleContacts = $DynamicField->{Config}->{ContactsWithData}; my $SearchableFields = $DynamicField->{Config}->{SearchableFieldsComputed}; CONTACT: for my $Contact ( sort { lc($a) cmp lc($b) } keys %{$PossibleContacts} ) { + + # check whether the contact matches the search term my %ContactData = %{ $PossibleContacts->{$Contact} }; my $SearchMatch; FIELD: for my $Field ( @{$SearchableFields} ) { next FIELD if $ContactData{$Field} !~ m{ $Search }xmsi; + $SearchMatch = 1; + last FIELD; } - next CONTACT if !$SearchMatch; - next CONTACT if !$ContactData{ValidID}; - next CONTACT if $ContactData{ValidID} ne 1; + next CONTACT unless $SearchMatch; + + # only valid contacts are returned + next CONTACT unless $ContactData{ValidID}; + next CONTACT unless $ContactData{ValidID} eq 1; + push @Results, { Key => $Contact, Value => $ContactData{Name}, }; - $MaxResultCount--; - last CONTACT if $MaxResultCount <= 0; + $RemainingResultCount--; + + # only a limited number of contacts are returned + last CONTACT if $RemainingResultCount <= 0; } }