Bugzilla – Attachment 149714 Details for
Bug 33236
Move C4::Suggestions::NewSuggestion to Koha namespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33236: Move NewSuggestion to Koha::Suggestion->store
Bug-33236-Move-NewSuggestion-to-KohaSuggestion-sto.patch (text/plain), 18.06 KB, created by
Katrin Fischer
on 2023-04-15 22:21:54 UTC
(
hide
)
Description:
Bug 33236: Move NewSuggestion to Koha::Suggestion->store
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2023-04-15 22:21:54 UTC
Size:
18.06 KB
patch
obsolete
>From fc686e28e2610f1604632766d1f3dd4c9cda2112 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> >Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> >--- > 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 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=149714">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33236
:
148221
|
149103
|
149679
|
149680
| 149714 |
149715
|
149716
|
150510