Bug 22818: (QA follow-up) Email addressing corrections
This patch updates the patchset to use recnetly introduced methods for obtaining the correct inbound and reply email addresses for branches. We also move get_staff_to_address into Koha::Library alongside inbound_email_address and rename it to inbound_ill_address. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Martin Renvoize
Jonathan Druart
parent
ed7e74307e
commit
78d2b65313
|
|
@ -29,7 +29,6 @@ use C4::Letters;
|
|||
use C4::Members;
|
||||
use Koha::Database;
|
||||
use Koha::DateUtils qw/ dt_from_string /;
|
||||
use Koha::Email;
|
||||
use Koha::Exceptions::Ill;
|
||||
use Koha::Illcomments;
|
||||
use Koha::Illrequestattributes;
|
||||
|
|
@ -1329,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->branchillemail || $branch->branchemail;
|
||||
my $replyto = $branch->branchreplyto || $from;
|
||||
my $from = $branch->branchemail;
|
||||
my $replyto = $branch->inbound_ill_address;
|
||||
Koha::Exceptions::Ill::NoLibraryEmail->throw(
|
||||
"Your library has no usable email address. Please set it.")
|
||||
if ( !$from );
|
||||
|
|
@ -1350,7 +1349,8 @@ sub generic_confirm {
|
|||
borrowernumber => $self->borrowernumber,
|
||||
message_transport_type => 'email',
|
||||
to_address => $to,
|
||||
from_address => $from
|
||||
from_address => $from,
|
||||
reply_address => $replyto
|
||||
};
|
||||
|
||||
if ($letter) {
|
||||
|
|
@ -1386,45 +1386,6 @@ sub generic_confirm {
|
|||
}
|
||||
}
|
||||
|
||||
=head3 get_staff_to_address
|
||||
|
||||
my $email = $request->get_staff_to_address();
|
||||
|
||||
Get the email address to which staff notices should be sent
|
||||
|
||||
=cut
|
||||
|
||||
sub get_staff_to_address {
|
||||
my ( $self ) = @_;
|
||||
|
||||
# The various places we can get an ILL staff email address from
|
||||
# (In order of preference)
|
||||
#
|
||||
# Dedicated branch address
|
||||
my $library = Koha::Libraries->find( $self->branchcode );
|
||||
my $branch_ill_to = $library->branchillemail;
|
||||
# General purpose ILL address from syspref
|
||||
my $syspref = C4::Context->preference("ILLDefaultStaffEmail");
|
||||
# Branch general email address
|
||||
my $branch_to = $library->branchemail;
|
||||
# Last resort
|
||||
my $koha_admin = C4::Context->preference('KohaAdminEmailAddress');
|
||||
|
||||
my $to;
|
||||
if ($branch_ill_to) {
|
||||
$to = $branch_ill_to;
|
||||
} elsif ($syspref) {
|
||||
$to = $syspref;
|
||||
} elsif ($branch_to) {
|
||||
$to = $branch_to;
|
||||
} elsif ($koha_admin) {
|
||||
$to = $koha_admin;
|
||||
}
|
||||
|
||||
# $to will not be defined if we didn't find a usable address
|
||||
return $to;
|
||||
}
|
||||
|
||||
=head3 send_patron_notice
|
||||
|
||||
my $result = $request->send_patron_notice($notice_code);
|
||||
|
|
@ -1456,6 +1417,12 @@ sub send_patron_notice {
|
|||
});
|
||||
my @transports = keys %{ $borrower_preferences->{transports} };
|
||||
|
||||
# Notice should come from the library where the request was placed,
|
||||
# not the patrons home library
|
||||
my $branch = Koha::Libraries->find($self->branchcode);
|
||||
my $from_address = $branch->branchemail;
|
||||
my $reply_address = $branch->inbound_ill_address;
|
||||
|
||||
# Send the notice to the patron via the chosen transport methods
|
||||
# and record the results
|
||||
my @success = ();
|
||||
|
|
@ -1470,6 +1437,8 @@ sub send_patron_notice {
|
|||
letter => $letter,
|
||||
borrowernumber => $self->borrowernumber,
|
||||
message_transport_type => $transport,
|
||||
from_address => $from_address,
|
||||
reply_address => $reply_address
|
||||
});
|
||||
if ($result) {
|
||||
push @success, $transport;
|
||||
|
|
@ -1515,7 +1484,7 @@ sub send_staff_notice {
|
|||
|
||||
# Get the staff notices that have been assigned for sending in
|
||||
# the syspref
|
||||
my $staff_to_send = C4::Context->preference('ILLSendStaffNotices');
|
||||
my $staff_to_send = C4::Context->preference('ILLSendStaffNotices') // q{};
|
||||
|
||||
# If it hasn't been enabled in the syspref, we don't want to send it
|
||||
if ($staff_to_send !~ /\b$notice_code\b/) {
|
||||
|
|
@ -1530,7 +1499,8 @@ sub send_staff_notice {
|
|||
});
|
||||
|
||||
# Try and get an address to which to send staff notices
|
||||
my $to_address = scalar $self->get_staff_to_address;
|
||||
my $branch = Koha::Libraries->find($self->branchcode);
|
||||
my $to_address = $branch->inbound_ill_address;
|
||||
|
||||
my $params = {
|
||||
letter => $letter,
|
||||
|
|
@ -1540,7 +1510,6 @@ sub send_staff_notice {
|
|||
|
||||
if ($to_address) {
|
||||
$params->{to_address} = $to_address;
|
||||
$params->{from_address} = $to_address;
|
||||
} else {
|
||||
return {
|
||||
error => 'notice_no_create'
|
||||
|
|
|
|||
|
|
@ -134,6 +134,24 @@ sub inbound_email_address {
|
|||
|| undef;
|
||||
}
|
||||
|
||||
=head3 inbound_ill_address
|
||||
|
||||
my $to_email = Koha::Library->inbound_ill_address;
|
||||
|
||||
Returns an effective email address which should be accessible to librarians at the branch
|
||||
for inter library loans communication.
|
||||
|
||||
=cut
|
||||
|
||||
sub inbound_ill_address {
|
||||
my ($self) = @_;
|
||||
|
||||
return
|
||||
$self->branchillemail
|
||||
|| C4::Context->preference('ILLDefaultStaffEmail')
|
||||
|| $self->inbound_email_address;
|
||||
}
|
||||
|
||||
=head3 library_groups
|
||||
|
||||
Return the Library groups of this library
|
||||
|
|
|
|||
|
|
@ -715,7 +715,7 @@ subtest 'Backend core methods' => sub {
|
|||
|
||||
subtest 'Helpers' => sub {
|
||||
|
||||
plan tests => 21;
|
||||
plan tests => 20;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
|
|
@ -817,62 +817,6 @@ subtest 'Helpers' => sub {
|
|||
"Correct error when missing type"
|
||||
);
|
||||
|
||||
#get_staff_to_address
|
||||
# Mock a KohaAdminEmailAddress syspref
|
||||
t::lib::Mocks::mock_preference(
|
||||
'KohaAdminEmailAddress',
|
||||
'kohaadmin@nowhere.com'
|
||||
);
|
||||
# No branch addresses defined and no ILLDefaultStaffEmail, so should
|
||||
# fall back to Koha admin address
|
||||
my $email_kohaadmin = $illrq_obj->get_staff_to_address;
|
||||
ok(
|
||||
$email_kohaadmin eq 'kohaadmin@nowhere.com',
|
||||
'get_staff_to_address falls back to Koha admin in the absence of other alternatives'
|
||||
);
|
||||
# General branch address defined, should fall back to that
|
||||
$builder->delete({ source => 'Branch', records => $illbrn });
|
||||
$illbrn = $builder->build({
|
||||
source => 'Branch',
|
||||
value => {
|
||||
branchcode => 'HDE',
|
||||
branchemail => 'branch@nowhere.com',
|
||||
branchillemail => "",
|
||||
branchreplyto => ""
|
||||
}
|
||||
});
|
||||
my $email_gen_branch = $illrq_obj->get_staff_to_address;
|
||||
ok(
|
||||
$email_gen_branch eq 'branch@nowhere.com',
|
||||
'get_staff_to_address falls back to general branch address when defined'
|
||||
);
|
||||
# ILL staff syspref address defined, should fall back to that
|
||||
t::lib::Mocks::mock_preference(
|
||||
'ILLDefaultStaffEmail',
|
||||
'illstaff@nowhere.com'
|
||||
);
|
||||
my $email_syspref = $illrq_obj->get_staff_to_address;
|
||||
ok(
|
||||
$email_syspref eq 'illstaff@nowhere.com',
|
||||
'get_staff_to_address falls back to ILLDefaultStaffEmail when defined'
|
||||
);
|
||||
# Branch ILL address defined, should use that
|
||||
$builder->delete({ source => 'Branch', records => $illbrn });
|
||||
$illbrn = $builder->build({
|
||||
source => 'Branch',
|
||||
value => {
|
||||
branchcode => 'HDE',
|
||||
branchemail => 'branch@nowhere.com',
|
||||
branchillemail => 'branchill@nowhere.com',
|
||||
branchreplyto => ""
|
||||
}
|
||||
});
|
||||
my $email_branch = $illrq_obj->get_staff_to_address;
|
||||
ok(
|
||||
$email_branch eq 'branchill@nowhere.com',
|
||||
'get_staff_to_address uses branch ILL address when defined'
|
||||
);
|
||||
|
||||
#send_staff_notice
|
||||
# Specify that no staff notices should be send
|
||||
t::lib::Mocks::mock_preference('ILLSendStaffNotices', '');
|
||||
|
|
@ -883,21 +827,25 @@ subtest 'Helpers' => sub {
|
|||
{ error => 'notice_not_enabled' },
|
||||
"Does not send notices that are not enabled"
|
||||
);
|
||||
my $queue = $schema->resultset('MessageQueue')->search({
|
||||
letter_code => 'ILL_REQUEST_CANCEL'
|
||||
});
|
||||
is($queue->count, 0, "Notice is not queued");
|
||||
|
||||
# Specify that the cancel notice can be sent
|
||||
t::lib::Mocks::mock_preference('ILLSendStaffNotices', 'ILL_REQUEST_CANCEL');
|
||||
my $return_staff_cancel = $illrq_obj->send_staff_notice(
|
||||
'ILL_REQUEST_CANCEL'
|
||||
);
|
||||
my $cancel = $schema->resultset('MessageQueue')->search({
|
||||
letter_code => 'ILL_REQUEST_CANCEL',
|
||||
message_transport_type => 'email',
|
||||
to_address => 'branchill@nowhere.com'
|
||||
})->next()->letter_code;
|
||||
is_deeply(
|
||||
$return_staff_cancel,
|
||||
{ success => 'notice_queued' },
|
||||
"Correct return when staff notice created"
|
||||
);
|
||||
$queue = $schema->resultset('MessageQueue')->search({
|
||||
letter_code => 'ILL_REQUEST_CANCEL'
|
||||
});
|
||||
is($queue->count, 1, "Notice queued as expected");
|
||||
|
||||
my $return_staff_fail = $illrq_obj->send_staff_notice();
|
||||
is_deeply(
|
||||
|
|
@ -905,6 +853,10 @@ subtest 'Helpers' => sub {
|
|||
{ error => 'notice_no_type' },
|
||||
"Correct error when missing type"
|
||||
);
|
||||
$queue = $schema->resultset('MessageQueue')->search({
|
||||
letter_code => 'ILL_REQUEST_CANCEL'
|
||||
});
|
||||
is($queue->count, 1, "Notice is not queued");
|
||||
|
||||
#get_notice
|
||||
my $not = $illrq_obj->get_notice({
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@
|
|||
|
||||
use Modern::Perl;
|
||||
|
||||
use Test::More tests => 9;
|
||||
use Test::More tests => 10;
|
||||
|
||||
use C4::Biblio;
|
||||
use C4::Context;
|
||||
|
|
@ -159,6 +159,57 @@ subtest '->inbound_email_address' => sub {
|
|||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
subtest '->inbound_ill_address' => sub {
|
||||
|
||||
plan tests => 7;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
my $library_1 = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Libraries',
|
||||
value => {
|
||||
branchemail => 'from@mylibrary.com',
|
||||
branchreplyto => 'reply@mylibrary.com',
|
||||
branchillemail => 'ill@mylibrary.com'
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
t::lib::Mocks::mock_preference( 'KohaAdminEmailAddress', 'admin@mylibrary.com' );
|
||||
t::lib::Mocks::mock_preference( 'ReplytoDefault', 'reply@mylibrary.com' );
|
||||
t::lib::Mocks::mock_preference( 'ILLDefaultStaffEmail', 'illdefault@mylibrary.com' );
|
||||
|
||||
is( $library_1->inbound_ill_address, $library_1->branchillemail,
|
||||
'If defined, use library branchillemail address');
|
||||
|
||||
$library_1->branchillemail(undef)->store();
|
||||
is( $library_1->inbound_ill_address, 'illdefault@mylibrary.com',
|
||||
'Fallback to ILLDefaultStaffEmail preference when branchillemail is undefined');
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ILLDefaultStaffEmail', undef );
|
||||
is( $library_1->inbound_ill_address, $library_1->branchreplyto,
|
||||
'Fallback to library replyto address when ILLDefaultStaffEmail is undefined');
|
||||
|
||||
$library_1->branchreplyto(undef)->store();
|
||||
is( $library_1->inbound_ill_address, $library_1->branchemail,
|
||||
'Fallback to branches email address when branchreplyto is undefined');
|
||||
|
||||
$library_1->branchemail(undef)->store();
|
||||
is( $library_1->inbound_ill_address, 'reply@mylibrary.com',
|
||||
'Fallback to ReplytoDefault email address when branchreplyto and branchemail are undefined');
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ReplytoDefault', '' );
|
||||
is( $library_1->inbound_ill_address, 'admin@mylibrary.com',
|
||||
'Fallback to KohaAdminEmailAddress email address when branchreplyto, branchemail and ReplytoDefault are undefined');
|
||||
|
||||
t::lib::Mocks::mock_preference( 'KohaAdminEmailAddress', '' );
|
||||
is( $library_1->inbound_ill_address, undef,
|
||||
'Return undef when email address when branchreplyto, branchemail, ReplytoDefault and KohaAdminEmailAddress are undefined');
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
subtest 'cash_registers' => sub {
|
||||
plan tests => 3;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue