From a93d4a52bb37a333a307d9f9ce9dd39ebd3b7b2a Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Thu, 15 Oct 2020 14:57:19 +0100 Subject: [PATCH] Bug 22818: (follow-up) Respond to feedback This commit fixes this bug, it was broken in a number of ways. Fixes include: - Added necessary config block in C4::Letters to enable the TT notice syntax introduced in an earlier commit to work - Changed template variables to refer to singular objects rather than multiple e.g. borrowers -> borrower - Fixed missing / misnamed variables This commit also implements the additional syspref checks suggested by Katrin in comment #87 --- C4/Letters.pm | 6 ++++++ Koha/Illrequest.pm | 10 ++++++---- api/v1/swagger/definitions/library.json | 4 ++++ .../bug_22818_add_ill_notices.perl | 20 +++++++++---------- .../en/includes/messaging-preference-form.inc | 4 ++-- .../prog/en/modules/admin/categories.tt | 4 ++-- .../bootstrap/en/modules/opac-messaging.tt | 4 ++-- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index e4636df18d..e999a70fe4 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1615,6 +1615,12 @@ sub _get_tt_params { plural => 'patron_modifications', fk => 'verification_token', }, + illrequests => { + module => 'Koha::Illrequests', + singular => 'illrequest', + plural => 'illrequests', + pk => 'illrequest_id' + } }; foreach my $table ( keys %$tables ) { diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 6e4f64761c..52741555f9 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -1290,7 +1290,7 @@ attempts to submit the email. sub generic_confirm { my ( $self, $params ) = @_; - my $library = Koha::Libraries->find($params->{current_branchcode}) + my $branch = Koha::Libraries->find($params->{current_branchcode}) || die "Invalid current branchcode. Are you logged in as the database user?"; if ( !$params->{stage}|| $params->{stage} eq 'init' ) { # Get the message body from the notice definition @@ -1328,8 +1328,8 @@ sub generic_confirm { "No target email addresses found. Either select at least one partner or check your ILL partner library records.") if ( !$to ); # Create the from, replyto and sender headers - my $from = $branch->branchemail; - my $replyto = $branch->inbound_ill_address; + my $from = $branch->branchillemail || $branch->branchemail; + my $replyto = $branch->branchreplyto || $from; Koha::Exceptions::Ill::NoLibraryEmail->throw( "Your library has no usable email address. Please set it.") if ( !$from ); @@ -1343,7 +1343,7 @@ sub generic_confirm { $letter->{title} = $params->{subject}; $letter->{content} = $params->{body}; - # Send the email + # Queue the notice my $params = { letter => $letter, borrowernumber => $self->borrowernumber, @@ -1501,11 +1501,13 @@ sub send_staff_notice { # Try and get an address to which to send staff notices my $branch = Koha::Libraries->find($self->branchcode); my $to_address = $branch->inbound_ill_address; + my $from_address = $branch->inbound_ill_address; my $params = { letter => $letter, borrowernumber => $self->borrowernumber, message_transport_type => 'email', + from_address => $from_address }; if ($to_address) { diff --git a/api/v1/swagger/definitions/library.json b/api/v1/swagger/definitions/library.json index 28c3bf5f79..14b0b4c97d 100644 --- a/api/v1/swagger/definitions/library.json +++ b/api/v1/swagger/definitions/library.json @@ -48,6 +48,10 @@ "type": ["string", "null"], "description": "the primary email address of the library" }, + "illemail": { + "type": ["string", "null"], + "description": "the ILL staff email address of the library" + }, "reply_to_email": { "type": ["string", "null"], "description": "the email to be used as a Reply-To" diff --git a/installer/data/mysql/atomicupdate/bug_22818_add_ill_notices.perl b/installer/data/mysql/atomicupdate/bug_22818_add_ill_notices.perl index 298c72c44b..9661b0fcbc 100644 --- a/installer/data/mysql/atomicupdate/bug_22818_add_ill_notices.perl +++ b/installer/data/mysql/atomicupdate/bug_22818_add_ill_notices.perl @@ -7,16 +7,16 @@ if( CheckVersion( $DBversion ) ) { $dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('ILLDefaultStaffEmail', '', 'Fallback email address for staff ILL notices to be sent to in the absence of a branch address', NULL, 'Free'); | ); $dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('ILLSendStaffNotices', NULL, 'Send these ILL notices to staff', NULL, 'multiple'); | ); # Add new notices - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PICKUP_READY', '', 'ILL request ready for pickup', 0, "Interlibrary loan request ready for pickup", "Dear [% borrowers.firstname %] [% borrowers.surname %],\n\nThe Interlibrary loans request number [% illrequests.illrequest_id %] you placed for:\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis ready for pick up from [% branches.branchname %].\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'email', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_UNAVAIL', '', 'ILL request unavailable', 0, "Interlibrary loan request unavailable", "Dear [% borrowers.firstname %] [% borrowers.surname %],\n\nThe Interlibrary loans request number [% illrequests.illrequest_id %] you placed for\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis unfortunately unavailable.\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'email', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_CANCEL', '', 'ILL request cancelled', 0, "Interlibrary loan request cancelled", "The patron for interlibrary loans request [% illrequests.illrequest_id %], with the following details, has requested cancellation of this ILL request:\n\n[% ill_full_metadata %]", 'email', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_MODIFIED', '', 'ILL request modified', 0, "Interlibrary loan request modified", "The patron for interlibrary loans request [% illrequests.illrequest_id %], with the following details, has modified this ILL request:\n\n[% ill_full_metadata %]", 'email', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PARTNER_REQ', '', 'ILL request to partners', 0, "Interlibrary loan request to partners", "Dear Sir/Madam,\n\nWe would like to request an interlibrary loan for a title matching the following description:\n\n[% ill_full_metadata %]\n\nPlease let us know if you are able to supply this to us.\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'email', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PICKUP_READY', '', 'ILL request ready for pickup', 0, "Interlibrary loan request ready for pickup", "Dear [% borrowers.firstname %] [% borrowers.surname %],\n\nThe Interlibrary loans request number [% illrequests.illrequest_id %] you placed for:\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis ready for pick up from [% branches.branchname %].\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'sms', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_UNAVAIL', '', 'ILL request unavailable', 0, "Interlibrary loan request unavailable", "Dear [% borrowers.firstname %] [% borrowers.surname %],\n\nThe Interlibrary loans request number [% illrequests.illrequest_id %] you placed for\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis unfortunately unavailable.\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'sms', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_CANCEL', '', 'ILL request cancelled', 0, "Interlibrary loan request cancelled", "The patron for interlibrary loans request [% illrequests.illrequest_id %], with the following details, has requested cancellation of this ILL request:\n\n[% ill_full_metadata %]", 'sms', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_MODIFIED', '', 'ILL request modified', 0, "Interlibrary loan request modified", "The patron for interlibrary loans request [% illrequests.illrequest_id %], with the following details, has modified this ILL request:\n\n[% ill_full_metadata %]", 'sms', 'default'); | ); - $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PARTNER_REQ', '', 'ILL request to partners', 0, "Interlibrary loan request to partners", "Dear Sir/Madam,\n\nWe would like to request an interlibrary loan for a title matching the following description:\n\n[% ill_full_metadata %]\n\nPlease let us know if you are able to supply this to us.\n\nKind Regards\n\n[% branches.branchname %]\n[% branches.branchaddress1 %]\n[% branches.branchaddress2 %]\n[% branches.branchaddress3 %]\n[% branches.branchcity %]\n[% branches.branchstate %]\n[% branches.branchzip %]\n[% branches.branchphone %]\n[% branches.branchillemail %]\n[% branches.branchemail %]", 'sms', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PICKUP_READY', '', 'ILL request ready for pickup', 0, "Interlibrary loan request ready for pickup", "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe Interlibrary loans request number [% illrequest.illrequest_id %] you placed for:\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis ready for pick up from [% branch.branchname %].\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'email', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_UNAVAIL', '', 'ILL request unavailable', 0, "Interlibrary loan request unavailable", "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe Interlibrary loans request number [% illrequest.illrequest_id %] you placed for\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis unfortunately unavailable.\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'email', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_CANCEL', '', 'ILL request cancelled', 0, "Interlibrary loan request cancelled", "The patron for interlibrary loans request [% illrequest.illrequest_id %], with the following details, has requested cancellation of this ILL request:\n\n[% ill_full_metadata %]", 'email', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_MODIFIED', '', 'ILL request modified', 0, "Interlibrary loan request modified", "The patron for interlibrary loans request [% illrequest.illrequest_id %], with the following details, has modified this ILL request:\n\n[% ill_full_metadata %]", 'email', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PARTNER_REQ', '', 'ILL request to partners', 0, "Interlibrary loan request to partners", "Dear Sir/Madam,\n\nWe would like to request an interlibrary loan for a title matching the following description:\n\n[% ill_full_metadata %]\n\nPlease let us know if you are able to supply this to us.\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'email', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PICKUP_READY', '', 'ILL request ready for pickup', 0, "Interlibrary loan request ready for pickup", "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe Interlibrary loans request number [% illrequest.illrequest_id %] you placed for:\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis ready for pick up from [% branch.branchname %].\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'sms', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_UNAVAIL', '', 'ILL request unavailable', 0, "Interlibrary loan request unavailable", "Dear [% borrower.firstname %] [% borrower.surname %],\n\nThe Interlibrary loans request number [% illrequest.illrequest_id %] you placed for\n\n- [% ill_bib_title %] - [% ill_bib_author %]\n\nis unfortunately unavailable.\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'sms', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_CANCEL', '', 'ILL request cancelled', 0, "Interlibrary loan request cancelled", "The patron for interlibrary loans request [% illrequest.illrequest_id %], with the following details, has requested cancellation of this ILL request:\n\n[% ill_full_metadata %]", 'sms', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_REQUEST_MODIFIED', '', 'ILL request modified', 0, "Interlibrary loan request modified", "The patron for interlibrary loans request [% illrequest.illrequest_id %], with the following details, has modified this ILL request:\n\n[% ill_full_metadata %]", 'sms', 'default'); | ); + $dbh->do( q| INSERT IGNORE INTO letter(module, code, branchcode, name, is_html, title, content, message_transport_type, lang) VALUES ('ill', 'ILL_PARTNER_REQ', '', 'ILL request to partners', 0, "Interlibrary loan request to partners", "Dear Sir/Madam,\n\nWe would like to request an interlibrary loan for a title matching the following description:\n\n[% ill_full_metadata %]\n\nPlease let us know if you are able to supply this to us.\n\nKind Regards\n\n[% branch.branchname %]\n[% branch.branchaddress1 %]\n[% branch.branchaddress2 %]\n[% branch.branchaddress3 %]\n[% branch.branchcity %]\n[% branch.branchstate %]\n[% branch.branchzip %]\n[% branch.branchphone %]\n[% branch.branchillemail %]\n[% branch.branchemail %]", 'sms', 'default'); | ); # Add patron messaging preferences $dbh->do( q| INSERT IGNORE INTO message_attributes (message_name, takes_days) VALUES ('Ill_ready', 0); | ); my $ready_id = $dbh->{mysql_insertid}; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc index c8f1437423..71bf75faa0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc @@ -25,8 +25,8 @@ [% ELSE %] Item checkout [% END %] - [% ELSIF ( messaging_preference.Ill_ready ) %]Interlibrary loan ready - [% ELSIF ( messaging_preference.Ill_unavailable ) %]Interlibrary loan unavailable + [% ELSIF ( Koha.Preference('ILLModule') && messaging_preference.Ill_ready ) %]Interlibrary loan ready + [% ELSIF ( Koha.Preference('ILLModule') && messaging_preference.Ill_unavailable ) %]Interlibrary loan unavailable [% ELSE %]Unknown [% END %] [% IF ( messaging_preference.takes_days ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt index 0bacd6c126..92166554d7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt @@ -536,8 +536,8 @@ [% ELSIF ( prefs.Hold_Filled ) %]Hold filled [% ELSIF ( prefs.Item_Check_in ) %]Item check-in [% ELSIF ( prefs.Item_Checkout ) %]Item checkout - [% ELSIF ( prefs.Ill_ready ) %]Interlibrary loan ready - [% ELSIF ( prefs.Ill_unavailable ) %]Interlibrary loan unavailable + [% ELSIF ( Koha.Preference('ILLModule') && prefs.Ill_ready ) %]Interlibrary loan ready + [% ELSIF ( Koha.Preference('ILLModule') && prefs.Ill_unavailable ) %]Interlibrary loan unavailable [% ELSE %]Unknown [% END %]: [% transport.transport | html %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt index 3d376e64ff..f59fa3a157 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt @@ -66,8 +66,8 @@ [% ELSE %] Item checkout [% END %] - [% ELSIF ( messaging_preference.Ill_ready ) %]Interlibrary loan ready - [% ELSIF ( messaging_preference.Ill_unavailable ) %]Interlibrary loan unavailable + [% ELSIF ( Koha.Preference( 'ILLModule' ) && messaging_preference.Ill_ready ) %]Interlibrary loan ready + [% ELSIF ( Koha.Preference( 'ILLModule' ) && messaging_preference.Ill_unavailable ) %]Interlibrary loan unavailable [% ELSE %]Unknown [% END %] [% IF ( messaging_preference.takes_days ) %]