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 <bgkriegel@gmail.com>
Test pass, suggestion created on staff and opac,
suggestion edited without problems, no koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script:
Also tested:
- adding suggestion from staff and OPAC
- edit suggestion from staff
- deleting suggestion from OPAC
- changing to a normal status (email got created)
- changing to a custom status (SUGGEST_STATUS)
- display of custom status in OPAC

No problems found.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
index b543293..ddfa1b6 100644 (file)
--- 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 (executable)
--- 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 (executable)
--- 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 (file)
--- 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);