From 4880f2d91df87541c3d0bbbca94dbfdf77f8a444 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- 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 2a540ba147..04c800f883 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 ); @@ -211,7 +211,7 @@ if ( $op eq "add_confirm" ) { $suggestion->{place} = $biblio->biblioitem->place; } - &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 44fc5527d2..b2cbf3a639 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.39.5