From fa220ec9181efc914cb88034200fa29e65142d39 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Wed, 15 Mar 2023 15:27:11 +0000
Subject: [PATCH] Bug 33236: Move NewSuggestion to Koha::Suggestion->store

The NewSuggestion routine saved the suggestion to the DB
and returned the id

This patch moves the code to Koha::Suggestion->store and
handles emailing upon creation, this adds that functionality to
suggestions added via api

To test:
1 - Apply patch
2 - Test adding a suggestion on the opac and staff client
3 - Confirm the suggestions are added correctly

Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
---
 C4/Suggestions.pm            | 69 ------------------------------------
 Koha/Suggestion.pm           | 66 +++++++++++++++++++++++++++++++++-
 opac/opac-suggestions.pl     |  4 +--
 suggestion/suggestion.pl     |  3 +-
 t/db_dependent/Suggestions.t | 69 +++++++++++++++++++-----------------
 5 files changed, 106 insertions(+), 105 deletions(-)

diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
index 478192c224..80cbeb9f1a 100644
--- a/C4/Suggestions.pm
+++ b/C4/Suggestions.pm
@@ -233,75 +233,6 @@ sub GetSuggestionByStatus {
     return $results;
 }
 
-=head2 NewSuggestion
-
-
-&NewSuggestion($suggestion);
-
-Insert a new suggestion on database with value given on input arg.
-
-=cut
-
-sub NewSuggestion {
-    my ($suggestion) = @_;
-
-    $suggestion->{STATUS} = "ASKED" unless $suggestion->{STATUS};
-
-    $suggestion->{suggesteddate} = dt_from_string unless $suggestion->{suggesteddate};
-
-    delete $suggestion->{branchcode}
-      if defined $suggestion->{branchcode} and $suggestion->{branchcode} eq '';
-
-    my $suggestion_object = Koha::Suggestion->new( $suggestion )->store;
-    my $suggestion_id = $suggestion_object->suggestionid;
-
-    my $emailpurchasesuggestions = C4::Context->preference("EmailPurchaseSuggestions");
-    if ($emailpurchasesuggestions) {
-        my $full_suggestion = GetSuggestion( $suggestion_id); # We should not need to refetch it!
-        if (
-            my $letter = C4::Letters::GetPreparedLetter(
-                module      => 'suggestions',
-                letter_code => 'NEW_SUGGESTION',
-                tables      => {
-                    'branches'    => $full_suggestion->{branchcode},
-                    'borrowers'   => $full_suggestion->{suggestedby},
-                    'suggestions' => $full_suggestion,
-                },
-            )
-        ){
-
-            my $toaddress;
-            if ( $emailpurchasesuggestions eq "BranchEmailAddress" ) {
-                my $library =
-                  Koha::Libraries->find( $full_suggestion->{branchcode} );
-                $toaddress = $library->inbound_email_address;
-            }
-            elsif ( $emailpurchasesuggestions eq "KohaAdminEmailAddress" ) {
-                $toaddress = C4::Context->preference('ReplytoDefault')
-                  || C4::Context->preference('KohaAdminEmailAddress');
-            }
-            else {
-                $toaddress =
-                     C4::Context->preference($emailpurchasesuggestions)
-                  || C4::Context->preference('ReplytoDefault')
-                  || C4::Context->preference('KohaAdminEmailAddress');
-            }
-
-            C4::Letters::EnqueueLetter(
-                {
-                    letter         => $letter,
-                    borrowernumber => $full_suggestion->{suggestedby},
-                    suggestionid   => $full_suggestion->{suggestionid},
-                    to_address     => $toaddress,
-                    message_transport_type => 'email',
-                }
-            ) or warn "can't enqueue letter $letter";
-        }
-    }
-
-    return $suggestion_id;
-}
-
 =head2 ModSuggestion
 
 &ModSuggestion($suggestion)
diff --git a/Koha/Suggestion.pm b/Koha/Suggestion.pm
index 9d9e460a65..3b88c8aefb 100644
--- a/Koha/Suggestion.pm
+++ b/Koha/Suggestion.pm
@@ -46,11 +46,75 @@ a suggesteddate of today
 sub store {
     my ($self) = @_;
 
+    $self->STATUS("ASKED") unless $self->STATUS;
+
+    $self->branchcode(undef) if $self->branchcode eq '';
     unless ( $self->suggesteddate() ) {
         $self->suggesteddate( dt_from_string()->ymd );
     }
 
-    return $self->SUPER::store();
+    my $emailpurchasesuggestions = C4::Context->preference("EmailPurchaseSuggestions");
+
+    my $result = $self->SUPER::store();
+
+    if( $emailpurchasesuggestions ){
+
+        if (
+            my $letter = C4::Letters::GetPreparedLetter(
+                module      => 'suggestions',
+                letter_code => 'NEW_SUGGESTION',
+                tables      => {
+                    'branches'    => $result->branchcode,
+                    'borrowers'   => $result->suggestedby,
+                    'suggestions' => $result->unblessed,
+                },
+            )
+        ){
+
+            my $toaddress;
+            if ( $emailpurchasesuggestions eq "BranchEmailAddress" ) {
+                my $library = $result->library;
+                $toaddress = $library->inbound_email_address;
+            }
+            elsif ( $emailpurchasesuggestions eq "KohaAdminEmailAddress" ) {
+                $toaddress = C4::Context->preference('ReplytoDefault')
+                  || C4::Context->preference('KohaAdminEmailAddress');
+            }
+            else {
+                $toaddress =
+                     C4::Context->preference($emailpurchasesuggestions)
+                  || C4::Context->preference('ReplytoDefault')
+                  || C4::Context->preference('KohaAdminEmailAddress');
+            }
+
+            C4::Letters::EnqueueLetter(
+                {
+                    letter         => $letter,
+                    borrowernumber => $result->suggestedby,
+                    suggestionid   => $result->id,
+                    to_address     => $toaddress,
+                    message_transport_type => 'email',
+                }
+            ) or warn "can't enqueue letter $letter";
+        }
+    }
+
+    return $result;
+}
+
+=head3 library
+
+my $library = $suggestion->library;
+
+Returns the library of the suggestion (Koha::Library for branchcode field)
+
+=cut
+
+sub library {
+    my ($self) = @_;
+    my $library_rs = $self->_result->branchcode;
+    return unless $library_rs;
+    return Koha::Library->_new_from_dbic($library_rs);
 }
 
 =head3 suggester
diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl
index dceae6c766..61e742a343 100755
--- a/opac/opac-suggestions.pl
+++ b/opac/opac-suggestions.pl
@@ -27,7 +27,6 @@ use C4::Output qw( output_html_with_http_headers );
 use C4::Suggestions qw(
     DelSuggestion
     MarcRecordFromNewSuggestion
-    NewSuggestion
 );
 use C4::Koha qw( GetAuthorisedValues );
 use C4::Scrubber;
@@ -36,6 +35,7 @@ use C4::Search qw( FindDuplicate );
 use Koha::AuthorisedValues;
 use Koha::Libraries;
 use Koha::Patrons;
+use Koha::Suggestions;
 
 use Koha::DateUtils qw( dt_from_string );
 
@@ -200,7 +200,7 @@ if ( $op eq "add_confirm" ) {
         $suggestion->{branchcode} = $input->param('branchcode') || C4::Context->userenv->{"branch"};
         $suggestion->{STATUS} = 'ASKED';
 
-        &NewSuggestion($suggestion);
+        Koha::Suggestion->new($suggestion)->store();
         $patrons_pending_suggestions_count++;
         $patrons_total_suggestions_count++;
 
diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl
index f85b4366ea..dbc48c94ca 100755
--- a/suggestion/suggestion.pl
+++ b/suggestion/suggestion.pl
@@ -32,6 +32,7 @@ use Koha::AuthorisedValues;
 use Koha::Acquisition::Currencies;
 use Koha::Libraries;
 use Koha::Patrons;
+use Koha::Suggestions;
 
 use URI::Escape qw( uri_escape );
 
@@ -232,7 +233,7 @@ if ( $op =~ /save/i ) {
             }
             else {
                 ## Adding some informations related to suggestion
-                &NewSuggestion($suggestion_only);
+                Koha::Suggestion->new($suggestion_only)->store();
             }
             # empty fields, to avoid filter in "SearchSuggestion"
         }
diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t
index 481dfaa3d7..b5d4b5390f 100755
--- a/t/db_dependent/Suggestions.t
+++ b/t/db_dependent/Suggestions.t
@@ -34,7 +34,7 @@ use Koha::Patrons;
 use Koha::Suggestions;
 
 BEGIN {
-    use_ok('C4::Suggestions', qw( NewSuggestion GetSuggestion ModSuggestion GetSuggestionInfo GetSuggestionFromBiblionumber GetSuggestionInfoFromBiblionumber GetSuggestionByStatus ConnectSuggestionAndBiblio DelSuggestion MarcRecordFromNewSuggestion GetUnprocessedSuggestions DelSuggestionsOlderThan ));
+    use_ok('C4::Suggestions', qw( GetSuggestion ModSuggestion GetSuggestionInfo GetSuggestionFromBiblionumber GetSuggestionInfoFromBiblionumber GetSuggestionByStatus ConnectSuggestionAndBiblio DelSuggestion MarcRecordFromNewSuggestion GetUnprocessedSuggestions DelSuggestionsOlderThan ));
 }
 
 my $schema  = Koha::Database->new->schema;
@@ -159,21 +159,23 @@ my $my_suggestion_without_suggestedby = {
     quantity      => '', # Insert an empty string into int to catch strict SQL modes errors
 };
 
-my $my_suggestionid = NewSuggestion($my_suggestion);
-isnt( $my_suggestionid, 0, 'NewSuggestion returns an not null id' );
-my $my_suggestionid_with_budget = NewSuggestion($my_suggestion_with_budget);
+my $my_suggestion_object = Koha::Suggestion->new($my_suggestion)->store;
+my $my_suggestionid = $my_suggestion_object->id;
+isnt( $my_suggestionid, 0, 'Suggestion is correctly saved' );
+my $my_suggestion_with_budget_object = Koha::Suggestion->new($my_suggestion_with_budget)->store;
+my $my_suggestionid_with_budget = $my_suggestion_with_budget_object->id;
 
 is( GetSuggestion(), undef, 'GetSuggestion without the suggestion id returns undef' );
 my $suggestion = GetSuggestion($my_suggestionid);
-is( $suggestion->{title}, $my_suggestion->{title}, 'NewSuggestion stores the title correctly' );
-is( $suggestion->{author}, $my_suggestion->{author}, 'NewSuggestion stores the author correctly' );
-is( $suggestion->{publishercode}, $my_suggestion->{publishercode}, 'NewSuggestion stores the publishercode correctly' );
-is( $suggestion->{suggestedby}, $my_suggestion->{suggestedby}, 'NewSuggestion stores the borrower number correctly' );
-is( $suggestion->{biblionumber}, $my_suggestion->{biblionumber}, 'NewSuggestion stores the biblio number correctly' );
-is( $suggestion->{STATUS}, 'ASKED', 'NewSuggestion stores a suggestion with the status ASKED by default' );
-is( $suggestion->{managedby}, undef, 'NewSuggestion stores empty string as undef for non existent foreign key (integer)' );
-is( $suggestion->{manageddate}, undef, 'NewSuggestion stores empty string as undef for date' );
-is( $suggestion->{budgetid}, undef, 'NewSuggestion should set budgetid to NULL if not given' );
+is( $suggestion->{title}, $my_suggestion->{title}, 'Suggestion stores the title correctly' );
+is( $suggestion->{author}, $my_suggestion->{author}, 'Suggestion stores the author correctly' );
+is( $suggestion->{publishercode}, $my_suggestion->{publishercode}, 'Suggestion stores the publishercode correctly' );
+is( $suggestion->{suggestedby}, $my_suggestion->{suggestedby}, 'Suggestion stores the borrower number correctly' );
+is( $suggestion->{biblionumber}, $my_suggestion->{biblionumber}, 'Suggestion stores the biblio number correctly' );
+is( $suggestion->{STATUS}, 'ASKED', 'Suggestion stores a suggestion with the status ASKED by default' );
+is( $suggestion->{managedby}, undef, 'Suggestion stores empty string as undef for non existent foreign key (integer)' );
+is( $suggestion->{manageddate}, undef, 'Suggestion stores empty string as undef for date' );
+is( $suggestion->{budgetid}, undef, 'Suggestion should set budgetid to NULL if not given' );
 
 is( ModSuggestion(), undef, 'ModSuggestion without the suggestion returns undef' );
 my $mod_suggestion1 = {
@@ -240,7 +242,8 @@ $messages = C4::Letters::GetQueuedMessages({
 is ($messages->[1]->{message_transport_type}, 'sms', 'When FallbackToSMSIfNoEmail syspref is enabled the suggestion message_transport_type is sms if the borrower has no email');
 
 #Make a new suggestion for a borrower with defined email and no smsalertnumber
-my $my_suggestion_2_id = NewSuggestion($my_suggestion_with_budget2);
+my $my_suggestion_2_object = Koha::Suggestion->new($my_suggestion_with_budget2)->store();
+my $my_suggestion_2_id = $my_suggestion_2_object->id;
 
 #Check the message_transport_type when the 'FallbackToSMSIfNoEmail' syspref is enabled and the borrower has a defined email and no smsalertnumber
 t::lib::Mocks::mock_preference( 'FallbackToSMSIfNoEmail', 1 );
@@ -334,7 +337,8 @@ my $del_suggestion = {
     STATUS => 'CHECKED',
     suggestedby => $borrowernumber,
 };
-my $del_suggestionid = NewSuggestion($del_suggestion);
+my $del_suggestion_object = Koha::Suggestion->new($del_suggestion)->store();
+my $del_suggestionid = $del_suggestion_object->id;
 
 is( DelSuggestion(), '0E0', 'DelSuggestion without arguments returns 0E0' );
 is( DelSuggestion($borrowernumber), '', 'DelSuggestion without the suggestion id returns an empty string' );
@@ -348,14 +352,15 @@ is( $suggestions->[1]->{title}, $del_suggestion->{title}, 'DelSuggestion deletes
 
 # Test budgetid fk
 $my_suggestion->{budgetid} = ''; # If budgetid == '', NULL should be set in DB
-my $my_suggestionid_test_budgetid = NewSuggestion($my_suggestion);
+my $my_suggestionid_test_budget_object = Koha::Suggestion->new($my_suggestion)->store;
+my $my_suggestionid_test_budgetid = $my_suggestionid_test_budget_object->id;
 $suggestion = GetSuggestion($my_suggestionid_test_budgetid);
-is( $suggestion->{budgetid}, undef, 'NewSuggestion Should set budgetid to NULL if equals an empty string' );
+is( $suggestion->{budgetid}, undef, 'Suggestion Should set budgetid to NULL if equals an empty string' );
 
 $my_suggestion->{budgetid} = ''; # If budgetid == '', NULL should be set in DB
 ModSuggestion( $my_suggestion );
 $suggestion = GetSuggestion($my_suggestionid_test_budgetid);
-is( $suggestion->{budgetid}, undef, 'NewSuggestion Should set budgetid to NULL if equals an empty string' );
+is( $suggestion->{budgetid}, undef, 'Suggestion Should set budgetid to NULL if equals an empty string' );
 
 my $suggestion2 = {
     title => "Cuisine d'automne",
@@ -378,7 +383,7 @@ is($record->field( $author_tag )->subfield( $author_subfield ), "Catherine", "Re
 subtest 'GetUnprocessedSuggestions' => sub {
     plan tests => 11;
     $dbh->do(q|DELETE FROM suggestions|);
-    my $my_suggestionid         = NewSuggestion($my_suggestion);
+    my $my_suggestionid         = Koha::Suggestion->new($my_suggestion)->store->id;
     my $unprocessed_suggestions = C4::Suggestions::GetUnprocessedSuggestions;
     is( scalar(@$unprocessed_suggestions), 0, 'GetUnprocessedSuggestions should return 0 if a suggestion has been processed but not linked to a fund' );
     my $status     = ModSuggestion($mod_suggestion1);
@@ -452,35 +457,35 @@ subtest 'EmailPurchaseSuggestions' => sub {
 
     # EmailPurchaseSuggestions set to disabled
     t::lib::Mocks::mock_preference( "EmailPurchaseSuggestions", "0" );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
     my $newsuggestions_messages = C4::Letters::GetQueuedMessages(
         {
             borrowernumber => $borrowernumber
         }
     );
     is( @$newsuggestions_messages, 0,
-        'NewSuggestions does not send an email when disabled' );
+        'New suggestion does not send an email when EmailPurchaseSuggestions disabled' );
 
     # EmailPurchaseSuggestions set to BranchEmailAddress
     t::lib::Mocks::mock_preference( "EmailPurchaseSuggestions",
         "BranchEmailAddress" );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     t::lib::Mocks::mock_preference( "ReplytoDefault", 'library@b.c' );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     Koha::Libraries->find('CPL')->update( { branchemail => 'branchemail@hosting.com' } );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     Koha::Libraries->find('CPL')->update( { branchreplyto => 'branchemail@b.c' } );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     $newsuggestions_messages = C4::Letters::GetQueuedMessages(
         {
             borrowernumber => $borrowernumber
         }
     );
-    isnt( @$newsuggestions_messages, 0, 'NewSuggestions sends an email' );
+    isnt( @$newsuggestions_messages, 0, 'New suggestions sends an email wne EmailPurchaseSuggestions enabled' );
     my $message1 =
       C4::Letters::GetMessage( $newsuggestions_messages->[0]->{message_id} );
     is( $message1->{to_address}, 'noreply@hosting.com',
@@ -507,10 +512,10 @@ subtest 'EmailPurchaseSuggestions' => sub {
         "KohaAdminEmailAddress" );
 
     t::lib::Mocks::mock_preference( "ReplytoDefault", undef );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     t::lib::Mocks::mock_preference( "ReplytoDefault", 'library@b.c' );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     $newsuggestions_messages = C4::Letters::GetQueuedMessages(
         {
@@ -531,14 +536,14 @@ subtest 'EmailPurchaseSuggestions' => sub {
         "EmailAddressForSuggestions" );
 
     t::lib::Mocks::mock_preference( "ReplytoDefault", undef );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     t::lib::Mocks::mock_preference( "ReplytoDefault", 'library@b.c' );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     t::lib::Mocks::mock_preference( "EmailAddressForSuggestions",
         'suggestions@b.c' );
-    NewSuggestion($my_suggestion);
+    Koha::Suggestion->new($my_suggestion)->store;
 
     $newsuggestions_messages = C4::Letters::GetQueuedMessages(
         {
@@ -565,7 +570,7 @@ subtest 'ModSuggestion should work on suggestions without a suggester' => sub {
     plan tests => 2;
 
     $dbh->do(q|DELETE FROM suggestions|);
-    my $my_suggestionid = NewSuggestion($my_suggestion_without_suggestedby);
+    my $my_suggestionid = Koha::Suggestion->new($my_suggestion_without_suggestedby)->store()->id;
     $suggestion = GetSuggestion($my_suggestionid);
     is( $suggestion->{suggestedby}, undef, "Suggestedby is undef" );
 
-- 
2.30.2