From c0ad32b8ccafacc876f28c33a9b7568406fec706 Mon Sep 17 00:00:00 2001 From: Hammat Wele Date: Wed, 26 Apr 2023 15:02:02 +0000 Subject: [PATCH] Bug 12532: Send email to guarantee and guarantor This patch allows guarantors to receive emails sended to their guarentees. This patch is a rebase of the previous patches. I took all the content of previous commit and put it in one commit. TO TEST: Before applying: 1) Search, or create, a patron with guarantor. 2) For both guarantors and guarantees: - Add an email address - Update the 'Patron messaging preferences' section so that an email is sent for item checkouts 3) Checkout an item. An email should be sent only to the guarantee. 4) Apply the patch. 5) Run updatedatabase.pl 6) Run prove t/db_dependent/Members.t and prove t/db_dependent/Letters.t 7) Enable 'RedirectGuaranteeEmail' 8) Run misc/cronjobs/process_message_queue.pl 9) Notice that the email should be sended to both the guarantee AND the guarantor. Signed-off-by: David Nind --- C4/Letters.pm | 13 +++-- Koha/Email.pm | 12 +++-- Koha/Patron.pm | 25 +++++++++- ...ug_12532-RedirectGuaranteeEmail_syspref.pl | 14 ++++++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../en/modules/admin/preferences/patrons.pref | 6 +++ perltidy.ERR | 16 ++++++ t/db_dependent/Letters.t | 1 + t/db_dependent/Members.t | 50 ++++++++++++++++++- 9 files changed, 128 insertions(+), 10 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl create mode 100644 perltidy.ERR diff --git a/C4/Letters.pm b/C4/Letters.pm index e070697df4..4c17c29f13 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1321,9 +1321,10 @@ sub _send_message_by_email { my $patron; my $to_address = $message->{'to_address'}; - unless ($to_address) { + my $use_garantor = C4::Context->preference('RedirectGuaranteeEmail'); + if($use_garantor eq 'yes' || !$to_address) { $patron = Koha::Patrons->find( $message->{borrowernumber} ); - unless ($patron) { + unless ($patron or $to_address) { warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; _set_message_status( { @@ -1334,7 +1335,9 @@ sub _send_message_by_email { ); return; } - $to_address = $patron->notice_email_address; + if ($patron) { + $to_address = $patron->notice_email_address; + } unless ($to_address) { # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})"; # warning too verbose for this more common case? @@ -1472,6 +1475,10 @@ sub _send_message_by_email { $smtp_transports->{ $smtp_server->id // 'default' } ||= $smtp_server->transport; my $smtp_transport = $smtp_transports->{ $smtp_server->id // 'default' }; + _update_message_from_address($message->{'message_id'},$email->email->header('From') ) + if !$message->{from_address} + || $message->{from_address} ne $email->email->header('From'); + try { $email->send_or_die({ transport => $smtp_transport }); diff --git a/Koha/Email.pm b/Koha/Email.pm index 090f475938..fa87b34735 100644 --- a/Koha/Email.pm +++ b/Koha/Email.pm @@ -118,10 +118,14 @@ sub create { $args->{to} = $params->{to}; } - Koha::Exceptions::BadParameter->throw( - error => "Invalid 'to' parameter: " . $args->{to}, - parameter => 'to' - ) unless Koha::Email->is_valid( $args->{to} ); # to is mandatory + my @emails = split(',', $args->{to}); + foreach my $email (@emails) { + $email =~ s/ //g; + Koha::Exceptions::BadParameter->throw( + error => "Invalid 'to' parameter: ".$email, + parameter => 'to' + ) unless Koha::Email->is_valid($email); + } my $addresses = {}; $addresses->{reply_to} = $params->{reply_to}; diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 6937ea904a..d86692741b 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1413,14 +1413,35 @@ Returns the empty string if no email address. sub notice_email_address{ my ( $self ) = @_; + my $address; + my $guarantor_address; my $which_address = C4::Context->preference("EmailFieldPrimary"); # if syspref is set to 'first valid' (value == OFF), look up email address if ( $which_address eq 'OFF' ) { - return $self->first_valid_email_address; + $address = $self->first_valid_email_address; + } else { + $address = $self->$which_address || ''; } - return $self->$which_address || ''; + my $use_guarantor = C4::Context->preference('RedirectGuaranteeEmail'); + if ($use_guarantor) { + my @guarantors = map { $_->guarantors->as_list } $self->guarantor_relationships(); + if (@guarantors) { + foreach my $guarantor (@guarantors) { + if ( $which_address eq 'OFF' ) { + $guarantor_address = $guarantor->first_valid_email_address; + } else { + $guarantor_address = $guarantor->$which_address || ''; + } + if ($address){ + $address .= ', '; + } + $address .= $guarantor_address if $guarantor_address; + } + } + } + return $address; } =head3 first_valid_email_address diff --git a/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl b/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl new file mode 100755 index 0000000000..270c34b3ad --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_12532-RedirectGuaranteeEmail_syspref.pl @@ -0,0 +1,14 @@ +use Modern::Perl; + +return { + bug_number => "12532", + description => "Add new system preference RedirectGuaranteeEmail", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('RedirectGuaranteeEmail', '0', 'Enable the ability to redirect guarantee email messages to guarantor.', NULL, 'YesNo') }); + + say $out "Added system preference 'RedirectGuaranteeEmail'"; + }, +}; \ No newline at end of file diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 30197591a8..5d611a1bac 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -599,6 +599,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('RecallsMaxPickUpDelay','7',NULL,'Define the maximum time a recall can be awaiting pickup','Integer'), ('RecordLocalUseOnReturn','0',NULL,'If ON, statistically record returns of unissued items as local use, instead of return','YesNo'), ('Reference_NFL_Statuses','1|2',NULL,'Contains not for loan statuses considered as available for reference','Free'), +('RedirectGuaranteeEmail', '0', NULL, 'Enable the ability to redirect guarantee email messages to guarantor.', 'YesNo'), ('RefundLostOnReturnControl','CheckinLibrary','CheckinLibrary|ItemHomeBranch|ItemHoldingBranch','If a lost item is returned, choose which branch to pick rules for refunding.','Choice'), ('RenewAccruingItemWhenPaid','0','','If enabled, when the fines on an item accruing is paid off, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue','YesNo'), ('RenewAccruingItemInOpac','0','','If enabled, when the fines on an item accruing is paid off in the OPAC via a payment plugin, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 32cd34f706..4cf3e22e29 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -357,6 +357,12 @@ Patrons: 1: Allow 0: "Don't allow" - staff to set the ability for a patron's checkouts to be viewed by linked patrons in the OPAC. + - + - pref: RedirectGuaranteeEmail + choices: + yes: Enable + no: Disable + - sending emails to both guarantees and their guarantor. This does not affect patrons without guarantors. - - pref: AllowStaffToSetFinesVisibilityForGuarantor choices: diff --git a/perltidy.ERR b/perltidy.ERR new file mode 100644 index 0000000000..d0646c2347 --- /dev/null +++ b/perltidy.ERR @@ -0,0 +1,16 @@ +Perltidy version is 20210717 + +: Begin Error Output Stream +1: final indentation level: 3 + +Final nesting depth of '{'s is 1 +The most recent un-matched '{' is on line 1 +1: if (!$to_address && !$count_guarantor_address) { + ^ +1: final indentation level: 3 + +Final nesting depth of '{'s is 1 +The most recent un-matched '{' is on line 1 +1: if (!$to_address && !$count_guarantor_address) { + ^ +1: To save a full .LOG file rerun with -g diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 3d1e34a6bb..cd55ef42b4 100755 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -853,6 +853,7 @@ subtest 'Test SMS handling in SendQueuedMessages' => sub { t::lib::Mocks::mock_preference( 'SMSSendDriver', 'Email' ); t::lib::Mocks::mock_preference('EmailSMSSendDriverFromAddress', ''); + t::lib::Mocks::mock_preference('RedirectGuaranteeEmail', '0'); my $patron = Koha::Patrons->find($borrowernumber); $dbh->do(q| INSERT INTO message_queue(borrowernumber, subject, content, message_transport_type, status, letter_code) diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 7d9852af57..d66bb1284d 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 53; +use Test::More tests => 57; use Test::MockModule; use Test::Exception; @@ -119,6 +119,54 @@ C4::Context->clear_syspref_cache(); $checkcardnum=C4::Members::checkcardnumber($IMPOSSIBLE_CARDNUMBER, ""); is ($checkcardnum, "2", "Card number is too long"); +# Test notice_email_address +# Add Guarantor for testing +my $GUARANTOR_EMAIL = "Robert\@email.com"; +%data = ( + cardnumber => "2997924548", + firstname => "Robert", + surname => "Tables", + categorycode => $patron_category->{categorycode}, + branchcode => $BRANCHCODE, + dateofbirth => '', + dateexpiry => '9999-12-31', + userid => 'bobbytables', + email => $GUARANTOR_EMAIL +); + +$addmem=Koha::Patron->new(\%data)->store->borrowernumber; +ok($addmem, "Koha::Patron->store()"); + +my $patron_guarantor = Koha::Patrons->find( { cardnumber => (\%data)->{'cardnumber'} } ) + or BAIL_OUT("Cannot read member with card (\%data)->{'cardnumber'}"); +my $member_guarantor = $patron_guarantor->unblessed; + +my %data2 = ( + guarantor_id => $member_guarantor->{borrowernumber}, + guarantee_id => $member->{borrowernumber}, + relationship => "father" +); +Koha::Patron::Relationship->new(\%data2)->store; + +$member = Koha::Patrons->find( { cardnumber => $CARDNUMBER } ); +t::lib::Mocks::mock_preference( 'RedirectGuaranteeEmail', '0' ); +t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); +C4::Context->clear_syspref_cache(); + +my $notice_email = $member->notice_email_address; +is ($notice_email, $EMAIL, "notice_email_address returns correct value when EmailFieldPrimary is off"); + +t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'emailpro' ); +C4::Context->clear_syspref_cache(); + +$notice_email = $member->notice_email_address; +is ($notice_email, $EMAILPRO, "notice_email_address returns correct value when EmailFieldPrimary is emailpro"); + +t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); +t::lib::Mocks::mock_preference( 'RedirectGuaranteeEmail', '1' ); +C4::Context->clear_syspref_cache(); +$notice_email = $member->notice_email_address; +is ($notice_email, $EMAIL . ", " . $GUARANTOR_EMAIL, "notice_email_address returns correct value when RedirectGuaranteeEmail is enabled"); # Add a new borrower %data = ( -- 2.34.1