From 00c76a5915f7c86c0a7fb71ca134fd970ced62f1 Mon Sep 17 00:00:00 2001 From: Yohann Dufour Date: Fri, 8 Aug 2014 15:11:18 +0200 Subject: [PATCH] [SIGNED-OFF] Bug 12627: SQLHelper replacement - C4::Suggestions With this patch, the subroutines NewSuggestion and ModSuggestion use DBIx::Class instead of C4::SQLHelper. Moreover, the tests and the .pl have been adapted. Test plan: 1) Apply the patch. 2) Execute the unit tests by launching : prove t/db_dependent/Suggestions.t 3) The result has to be a success without error or warning : t/db_dependent/Suggestions.t .. ok All tests successful. Files=1, Tests=91, 2 wallclock secs ( 0.05 usr 0.01 sys + 1.65 cusr 0.09 csys = 1.80 CPU) Result: PASS 4) Log in the intranet, create a suggestion and verify the created suggestion. 5) Edit a suggestion from the intranet and verify the suggestion is correctly modified. 6) Log in the OPAC and verify you can add a suggestion. Signed-off-by: Bernardo Gonzalez Kriegel Test pass, suggestion created on staff and opac, suggestion edited without problems, no koha-qa errors. --- C4/Suggestions.pm | 25 ++++++++++++++++++---- opac/opac-suggestions.pl | 1 + suggestion/suggestion.pl | 48 ++++++++++++++++++++++++------------------ t/db_dependent/Suggestions.t | 2 +- 4 files changed, 50 insertions(+), 26 deletions(-) diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index b543293..ddfa1b6 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -26,7 +26,6 @@ use CGI; use C4::Context; use C4::Output; use C4::Dates qw(format_date format_date_in_iso); -use C4::SQLHelper qw(:all); use C4::Debug; use C4::Letters; use List::MoreUtils qw(any); @@ -426,8 +425,14 @@ Insert a new suggestion on database with value given on input arg. sub NewSuggestion { my ($suggestion) = @_; + + my $new_suggestion = { %$suggestion }; $suggestion->{STATUS} = "ASKED" unless $suggestion->{STATUS}; - return InsertInTable( "suggestions", $suggestion ); + $new_suggestion->{status} = $suggestion->{STATUS}; + delete $new_suggestion->{STATUS}; + + my $rs = Koha::Database->new->schema->resultset('Suggestion'); + return $rs->create($new_suggestion)->id; } =head2 ModSuggestion @@ -445,9 +450,21 @@ Note that there is no function to modify a suggestion. sub ModSuggestion { my ($suggestion) = @_; - my $status_update_table = UpdateInTable( "suggestions", $suggestion ); + return unless( $suggestion and defined($suggestion->{suggestionid}) ); + + my $mod_suggestion = { %$suggestion }; + my $status = $suggestion->{STATUS}; + delete $mod_suggestion->{STATUS}; + $mod_suggestion->{status} = $status; + + my $rs = Koha::Database->new->schema->resultset('Suggestion')->find($suggestion->{suggestionid}); + my $status_update_table = 1; + eval { + $rs->update($mod_suggestion); + }; + $status_update_table = 0 if( $@ ); - if ( $suggestion->{STATUS} ) { + if ( $status ) { # fetch the entire updated suggestion so that we can populate the letter my $full_suggestion = GetSuggestion( $suggestion->{suggestionid} ); diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl index 2801233..9c74308 100755 --- a/opac/opac-suggestions.pl +++ b/opac/opac-suggestions.pl @@ -33,6 +33,7 @@ my $input = new CGI; my $allsuggestions = $input->param('showall'); my $op = $input->param('op'); my $suggestion = $input->Vars; +delete $suggestion->{negcap}; my $negcaptcha = $input->param('negcap'); # If a spambot accidentally populates the 'negcap' field in the sugesstions form, then silently skip and return. diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index 046e347..72f40d3 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -95,6 +95,12 @@ my $tabcode = $input->param('tabcode'); # filter informations which are not suggestion related. my $suggestion_ref = $input->Vars; +# get only the columns of Suggestion +my $schema = Koha::Database->new()->schema; +my $columns = ' '.join(' ', $schema->source('Suggestion')->columns).' '; +my $suggestion_only = { map { $columns =~ / $_ / ? ($_ => $suggestion_ref->{$_}) : () } keys($suggestion_ref) }; +$suggestion_only->{STATUS} = $suggestion_ref->{STATUS}; + delete $$suggestion_ref{$_} foreach qw( suggestedbyme op displayby tabcode edit_field ); foreach (keys %$suggestion_ref){ delete $$suggestion_ref{$_} if (!$$suggestion_ref{$_} && ($op eq 'else' || $op eq 'change')); @@ -115,26 +121,26 @@ $template->param('borrowernumber' => $borrowernumber); ## Operations ## if ( $op =~ /save/i ) { - if ( $$suggestion_ref{"STATUS"} ) { - if ( my $tmpstatus = lc( $$suggestion_ref{"STATUS"} ) =~ /ACCEPTED|REJECTED/i ) { - $$suggestion_ref{ lc( $$suggestion_ref{"STATUS"}) . "date" } = C4::Dates->today; - $$suggestion_ref{ lc( $$suggestion_ref{"STATUS"}) . "by" } = C4::Context->userenv->{number}; + if ( $suggestion_only->{"STATUS"} ) { + if ( my $tmpstatus = lc( $suggestion_only->{"STATUS"} ) =~ /ACCEPTED|REJECTED/i ) { + $suggestion_only->{ lc( $suggestion_only->{"STATUS"}) . "date" } = C4::Dates->today; + $suggestion_only->{ lc( $suggestion_only->{"STATUS"}) . "by" } = C4::Context->userenv->{number}; } - $$suggestion_ref{"manageddate"} = C4::Dates->today; - $$suggestion_ref{"managedby"} = C4::Context->userenv->{number}; + $suggestion_only->{"manageddate"} = C4::Dates->today; + $suggestion_only->{"managedby"} = C4::Context->userenv->{number}; } - if ( $$suggestion_ref{'suggestionid'} > 0 ) { - &ModSuggestion($suggestion_ref); + if ( $suggestion_only->{'suggestionid'} > 0 ) { + &ModSuggestion($suggestion_only); } else { ###FIXME:Search here if suggestion already exists. my $suggestions_loop = - SearchSuggestion( $suggestion_ref ); + SearchSuggestion( $suggestion_only ); if (@$suggestions_loop>=1){ #some suggestion are answering the request Donot Add } else { ## Adding some informations related to suggestion - &NewSuggestion($suggestion_ref); + &NewSuggestion($suggestion_only); } # empty fields, to avoid filter in "SearchSuggestion" } @@ -160,16 +166,16 @@ elsif ($op=~/edit/) { elsif ($op eq "change" ) { # set accepted/rejected/managed informations if applicable # ie= if the librarian has choosen some action on the suggestions - if ($$suggestion_ref{"STATUS"} eq "ACCEPTED"){ - $$suggestion_ref{"accepteddate"}=C4::Dates->today; - $$suggestion_ref{"acceptedby"}=C4::Context->userenv->{number}; - } elsif ($$suggestion_ref{"STATUS"} eq "REJECTED"){ - $$suggestion_ref{"rejecteddate"}=C4::Dates->today; - $$suggestion_ref{"rejectedby"}=C4::Context->userenv->{number}; + if ($suggestion_only->{"STATUS"} eq "ACCEPTED"){ + $suggestion_only->{"accepteddate"}=C4::Dates->today; + $suggestion_only->{"acceptedby"}=C4::Context->userenv->{number}; + } elsif ($suggestion_only->{"STATUS"} eq "REJECTED"){ + $suggestion_only->{"rejecteddate"}=C4::Dates->today; + $suggestion_only->{"rejectedby"}=C4::Context->userenv->{number}; } - if ($$suggestion_ref{"STATUS"}){ - $$suggestion_ref{"manageddate"}=C4::Dates->today; - $$suggestion_ref{"managedby"}=C4::Context->userenv->{number}; + if ($suggestion_only->{"STATUS"}){ + $suggestion_only->{"manageddate"}=C4::Dates->today; + $suggestion_only->{"managedby"}=C4::Context->userenv->{number}; } if ( my $reason = $$suggestion_ref{"reason$tabcode"}){ if ( $reason eq "other" ) { @@ -183,8 +189,8 @@ elsif ($op eq "change" ) { } foreach my $suggestionid (@editsuggestions) { next unless $suggestionid; - $$suggestion_ref{'suggestionid'}=$suggestionid; - &ModSuggestion($suggestion_ref); + $suggestion_only->{'suggestionid'}=$suggestionid; + &ModSuggestion($suggestion_only); } my $params = ''; foreach my $key ( diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t index 299bdf6..e3bc26c 100644 --- a/t/db_dependent/Suggestions.t +++ b/t/db_dependent/Suggestions.t @@ -89,7 +89,7 @@ my $mod_suggestion1 = { publishercode => 'my modified publishercode', }; my $status = ModSuggestion($mod_suggestion1); -is( $status, '0E0', 'ModSuggestion without the suggestion id returns 0E0' ); +is( $status, undef, 'ModSuggestion without the suggestion id returns undef' ); $mod_suggestion1->{suggestionid} = $my_suggestionid; $status = ModSuggestion($mod_suggestion1); -- 1.7.9.5