Bug 28514: Remove getletter
The way we handle notice templates is confusing (see bug 27660, bug 26787, bug 28487).
This patch remove C4::Letters::getletter and use either Koha::Notice::Templates->find
or the newly created methods ->find_effective_template that will do
all necessary to return the correct template.
Test plan:
- Create and modify notice templates
- Make sure you have TranslateNotices turned on and that some notices
templates have a translated version
- Use holds_reminder.pl and overdue_notices.pl cronjobs and confirm that
the generated notices are the expected ones
- Test also pos/printreceipt.pl
- And finally test some other notices (CHECKIN, RENEWAL for instance)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Amended by removing comment for $params={%$params}
JD amended patch:
* Add missing POD
* Fix spelling (dont ==> don't)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart
parent
d785a5536f
commit
b82c91d8cb
|
|
@ -205,35 +205,6 @@ sub GetLettersAvailableForALibrary {
|
|||
|
||||
}
|
||||
|
||||
sub getletter {
|
||||
my ( $module, $code, $branchcode, $message_transport_type, $lang) = @_;
|
||||
$message_transport_type //= '%';
|
||||
$lang = 'default' unless( $lang && C4::Context->preference('TranslateNotices') );
|
||||
|
||||
|
||||
my $only_my_library = C4::Context->only_my_library;
|
||||
if ( $only_my_library and $branchcode ) {
|
||||
$branchcode = C4::Context::mybranch();
|
||||
}
|
||||
$branchcode //= '';
|
||||
|
||||
my $dbh = C4::Context->dbh;
|
||||
my $sth = $dbh->prepare(q{
|
||||
SELECT *
|
||||
FROM letter
|
||||
WHERE module=? AND code=? AND (branchcode = ? OR branchcode = '')
|
||||
AND message_transport_type LIKE ?
|
||||
AND lang =?
|
||||
ORDER BY branchcode DESC LIMIT 1
|
||||
});
|
||||
$sth->execute( $module, $code, $branchcode, $message_transport_type, $lang );
|
||||
my $line = $sth->fetchrow_hashref
|
||||
or return;
|
||||
$line->{'content-type'} = 'text/html; charset="UTF-8"' if $line->{is_html};
|
||||
return { %$line };
|
||||
}
|
||||
|
||||
|
||||
=head2 DelLetter
|
||||
|
||||
DelLetter(
|
||||
|
|
@ -637,13 +608,23 @@ sub GetPreparedLetter {
|
|||
my $branchcode = $params{branchcode} || '';
|
||||
my $mtt = $params{message_transport_type} || 'email';
|
||||
|
||||
$letter = getletter( $module, $letter_code, $branchcode, $mtt, $lang );
|
||||
my $template = Koha::Notice::Templates->find_effective_template(
|
||||
{
|
||||
module => $module,
|
||||
code => $letter_code,
|
||||
branchcode => $branchcode,
|
||||
message_transport_type => $mtt,
|
||||
lang => $lang
|
||||
}
|
||||
);
|
||||
|
||||
unless ( $letter ) {
|
||||
$letter = getletter( $module, $letter_code, $branchcode, $mtt, 'default' )
|
||||
or warn( "No $module $letter_code letter transported by " . $mtt ),
|
||||
return;
|
||||
unless ( $template ) {
|
||||
warn( "No $module $letter_code letter transported by " . $mtt );
|
||||
return;
|
||||
}
|
||||
|
||||
$letter = $template->unblessed;
|
||||
$letter->{'content-type'} = 'text/html; charset="UTF-8"' if $letter->{is_html};
|
||||
}
|
||||
|
||||
my $tables = $params{tables} || {};
|
||||
|
|
|
|||
|
|
@ -35,6 +35,52 @@ Koha::Notice::Templates - Koha notice template Object set class, related to the
|
|||
|
||||
=cut
|
||||
|
||||
=head3
|
||||
|
||||
my $template = Koha::Notice::Templates->find_effective_template(
|
||||
{
|
||||
module => $module,
|
||||
code => $code,
|
||||
branchcode => $branchcode,
|
||||
lang => $lang,
|
||||
}
|
||||
);
|
||||
|
||||
Return the notice template that must be used for a given primary key (module, code, branchcode, lang).
|
||||
|
||||
For instance if lang="es-ES" but there is no "es-ES" template defined for this language,
|
||||
the default template will be returned.
|
||||
|
||||
lang will default to "default" if not passed.
|
||||
|
||||
=cut
|
||||
|
||||
sub find_effective_template {
|
||||
my ( $self, $params ) = @_;
|
||||
|
||||
$params = { %$params }; # don't modify original
|
||||
$params->{lang} = 'default' unless C4::Context->preference('TranslateNotices');
|
||||
|
||||
my $only_my_library = C4::Context->only_my_library;
|
||||
if ( $only_my_library and $params->{branchcode} ) {
|
||||
$params->{branchcode} = C4::Context::mybranch();
|
||||
}
|
||||
$params->{branchcode} //= '';
|
||||
$params->{branchcode} = [$params->{branchcode}, ''];
|
||||
|
||||
my $template = $self->SUPER::search( $params, { order_by => { -desc => 'branchcode' } } );
|
||||
|
||||
if ( !$template->count
|
||||
&& C4::Context->preference('TranslateNotices')
|
||||
&& $params->{lang} ne 'default' )
|
||||
{
|
||||
$params->{lang} = 'default';
|
||||
$template = $self->SUPER::search( $params, { order_by => { -desc => 'branchcode' } } );
|
||||
}
|
||||
|
||||
return $template->next if $template->count;
|
||||
}
|
||||
|
||||
=head3 type
|
||||
|
||||
=cut
|
||||
|
|
|
|||
|
|
@ -37,6 +37,8 @@ use C4::Log;
|
|||
use Koha::DateUtils;
|
||||
use Koha::Calendar;
|
||||
use Koha::Libraries;
|
||||
use Koha::Notice::Templates;
|
||||
use Koha::Patrons;
|
||||
use Koha::Script -cron;
|
||||
|
||||
=head1 NAME
|
||||
|
|
@ -223,8 +225,17 @@ else {
|
|||
# Loop through each branch
|
||||
foreach my $branchcode (@branchcodes) { #BEGIN BRANCH LOOP
|
||||
# Check that this branch has the letter code specified or skip this branch
|
||||
my $letter = C4::Letters::getletter( 'reserves', $lettercode , $branchcode );
|
||||
unless ($letter) {
|
||||
|
||||
# FIXME What if we don't want to default if the translated template does not exist?
|
||||
my $template_exists = Koha::Notice::Templates->search(
|
||||
{
|
||||
module => 'reserves',
|
||||
code => $lettercode,
|
||||
branchcode => $branchcode,
|
||||
lang => 'default',
|
||||
}
|
||||
)->count;
|
||||
unless ($template_exists) {
|
||||
$verbose and print qq|Message '$lettercode' content not found for $branchcode\n|;
|
||||
next;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -624,8 +624,14 @@ END_SQL
|
|||
}
|
||||
}
|
||||
|
||||
my $letter = C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode, undef, $patron->lang )
|
||||
|| C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode, undef, "default");
|
||||
my $letter = Koha::Notice::Templates->find_effective_template(
|
||||
{
|
||||
module => 'circulation',
|
||||
code => $overdue_rules->{"letter$i"},
|
||||
branchcode => $branchcode,
|
||||
lang => $patron->lang
|
||||
}
|
||||
);
|
||||
|
||||
unless ($letter) {
|
||||
$verbose and warn qq|Message '$overdue_rules->{"letter$i"}' content not found|;
|
||||
|
|
@ -716,8 +722,16 @@ END_SQL
|
|||
splice @items, $PrintNoticesMaxLines if $effective_mtt eq 'print' && $PrintNoticesMaxLines && scalar @items > $PrintNoticesMaxLines;
|
||||
#catch the case where we are sending a print to someone with an email
|
||||
|
||||
my $letter_exists = ( C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode, $effective_mtt, $patron->lang )
|
||||
|| C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode, $effective_mtt, "default") ) ? 1 : 0;
|
||||
my $letter_exists = Koha::Notice::Templates->find_effective_template(
|
||||
{
|
||||
module => 'circulation',
|
||||
code => $overdue_rules->{"letter$i"},
|
||||
message_transport_type => $effective_mtt,
|
||||
branchcode => $branchcode,
|
||||
lang => $patron->lang
|
||||
}
|
||||
);
|
||||
|
||||
my $letter = parse_overdues_letter(
|
||||
{ letter_code => $overdue_rules->{"letter$i"},
|
||||
borrowernumber => $borrowernumber,
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ use CGI qw ( -utf8 );
|
|||
use C4::Letters;
|
||||
use Koha::Account::Lines;
|
||||
use Koha::DateUtils;
|
||||
use Koha::Notice::Templates;
|
||||
|
||||
my $input = CGI->new;
|
||||
|
||||
|
|
@ -52,8 +53,15 @@ output_and_exit_if_error(
|
|||
) if $patron; # Payment could have been anonymous
|
||||
|
||||
my $lang = $patron ? $patron->lang : $template->lang;
|
||||
my $letter = C4::Letters::getletter( 'pos', 'RECEIPT',
|
||||
C4::Context::mybranch, 'print', $lang );
|
||||
my $letter = Koha::Notice::Templates->find_effective_template(
|
||||
{
|
||||
module => 'pos',
|
||||
code => 'RECEIPT',
|
||||
branchcode => C4::Context::mybranch,
|
||||
message_transport_type => 'print',
|
||||
lang => $lang
|
||||
}
|
||||
);
|
||||
|
||||
$template->param(
|
||||
letter => $letter,
|
||||
|
|
|
|||
|
|
@ -19,12 +19,13 @@
|
|||
|
||||
use Modern::Perl;
|
||||
|
||||
use Test::More tests => 3;
|
||||
use Test::More tests => 4;
|
||||
|
||||
use Koha::Notice::Templates;
|
||||
use Koha::Database;
|
||||
|
||||
use t::lib::TestBuilder;
|
||||
use t::lib::Mocks;
|
||||
|
||||
my $schema = Koha::Database->new->schema;
|
||||
$schema->storage->txn_begin;
|
||||
|
|
@ -66,5 +67,72 @@ $retrieved_template->delete;
|
|||
is( Koha::Notice::Templates->search->count,
|
||||
$nb_of_templates, 'Delete should have deleted the template' );
|
||||
|
||||
subtest 'find_effective_template' => sub {
|
||||
plan tests => 7;
|
||||
|
||||
my $default_template = $builder->build_object(
|
||||
{ class => 'Koha::Notice::Templates', value => { branchcode => '', lang => 'default' } }
|
||||
);
|
||||
my $key = {
|
||||
module => $default_template->module,
|
||||
code => $default_template->code,
|
||||
message_transport_type => $default_template->message_transport_type,
|
||||
};
|
||||
|
||||
my $library_specific_template = $builder->build_object(
|
||||
{ class => 'Koha::Notice::Templates', value => { %$key, lang => 'default' } }
|
||||
);
|
||||
|
||||
my $es_template = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Notice::Templates',
|
||||
value => { %$key, lang => 'es-ES' },
|
||||
}
|
||||
);
|
||||
|
||||
$key->{branchcode} = $es_template->branchcode;
|
||||
|
||||
t::lib::Mocks::mock_preference( 'TranslateNotices', 0 );
|
||||
|
||||
my $template = Koha::Notice::Templates->find_effective_template($key);
|
||||
is( $template->lang, 'default', 'no lang passed, default is returned' );
|
||||
$template = Koha::Notice::Templates->find_effective_template( { %$key, lang => 'es-ES' } );
|
||||
is( $template->lang, 'default',
|
||||
'TranslateNotices is off, default is returned' );
|
||||
|
||||
t::lib::Mocks::mock_preference( 'TranslateNotices', 1 );
|
||||
$template = Koha::Notice::Templates->find_effective_template($key);
|
||||
is( $template->lang, 'default', 'no lang passed, default is returned' );
|
||||
$template = Koha::Notice::Templates->find_effective_template( { %$key, lang => 'es-ES' } );
|
||||
is( $template->lang, 'es-ES',
|
||||
'TranslateNotices is on and es-ES is requested, es-ES is returned' );
|
||||
|
||||
|
||||
{ # IndependentBranches => 1
|
||||
t::lib::Mocks::mock_userenv( { branchcode => $library_specific_template->branchcode, flag => 0 } );
|
||||
t::lib::Mocks::mock_preference( 'IndependentBranches', 1 );
|
||||
$template = Koha::Notice::Templates->find_effective_template( { %$key, branchcode => $library_specific_template->branchcode } );
|
||||
is( $template->content, $library_specific_template->content,
|
||||
'IndependentBranches is on, logged in patron is not superlibrarian but asks for their specific template, it is returned'
|
||||
);
|
||||
|
||||
my $another_library = $builder->build_object( { class => 'Koha::Libraries' } );
|
||||
t::lib::Mocks::mock_userenv( { branchcode => $another_library->branchcode, flag => 0 } );
|
||||
$template = Koha::Notice::Templates->find_effective_template($key);
|
||||
is( $template->content, $default_template->content,
|
||||
'IndependentBranches is on, logged in patron is not superlibrarian, default is returned'
|
||||
);
|
||||
}
|
||||
|
||||
t::lib::Mocks::mock_preference( 'IndependentBranches', 0 );
|
||||
$es_template->delete;
|
||||
|
||||
$template = Koha::Notice::Templates->find_effective_template( { %$key, lang => 'es-ES' } );
|
||||
is( $template->lang, 'default',
|
||||
'TranslateNotices is on and es-ES is requested but does not exist, default is returned'
|
||||
);
|
||||
|
||||
};
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@
|
|||
# along with Koha; if not, see <http://www.gnu.org/licenses>.
|
||||
|
||||
use Modern::Perl;
|
||||
use Test::More tests => 86;
|
||||
use Test::More tests => 82;
|
||||
use Test::MockModule;
|
||||
use Test::Warn;
|
||||
use Test::Exception;
|
||||
|
|
@ -209,53 +209,6 @@ is( $letters->[0]->{module}, 'my module', 'GetLetters gets the module correctly'
|
|||
is( $letters->[0]->{code}, 'my code', 'GetLetters gets the code correctly' );
|
||||
is( $letters->[0]->{name}, 'my name', 'GetLetters gets the name correctly' );
|
||||
|
||||
|
||||
# getletter
|
||||
subtest 'getletter' => sub {
|
||||
plan tests => 16;
|
||||
t::lib::Mocks::mock_preference('IndependentBranches', 0);
|
||||
my $letter = C4::Letters::getletter('my module', 'my code', $library->{branchcode}, 'email');
|
||||
is( $letter->{branchcode}, $library->{branchcode}, 'GetLetters gets the branch code correctly' );
|
||||
is( $letter->{module}, 'my module', 'GetLetters gets the module correctly' );
|
||||
is( $letter->{code}, 'my code', 'GetLetters gets the code correctly' );
|
||||
is( $letter->{name}, 'my name', 'GetLetters gets the name correctly' );
|
||||
is( $letter->{is_html}, 1, 'GetLetters gets the boolean is_html correctly' );
|
||||
is( $letter->{title}, $title, 'GetLetters gets the title correctly' );
|
||||
is( $letter->{content}, $content, 'GetLetters gets the content correctly' );
|
||||
is( $letter->{message_transport_type}, 'email', 'GetLetters gets the message type correctly' );
|
||||
|
||||
t::lib::Mocks::mock_userenv({ branchcode => "anotherlib", flags => 1 });
|
||||
|
||||
t::lib::Mocks::mock_preference('IndependentBranches', 1);
|
||||
$letter = C4::Letters::getletter('my module', 'my code', $library->{branchcode}, 'email');
|
||||
is( $letter->{branchcode}, $library->{branchcode}, 'GetLetters gets the branch code correctly' );
|
||||
is( $letter->{module}, 'my module', 'GetLetters gets the module correctly' );
|
||||
is( $letter->{code}, 'my code', 'GetLetters gets the code correctly' );
|
||||
is( $letter->{name}, 'my name', 'GetLetters gets the name correctly' );
|
||||
is( $letter->{is_html}, 1, 'GetLetters gets the boolean is_html correctly' );
|
||||
is( $letter->{title}, $title, 'GetLetters gets the title correctly' );
|
||||
is( $letter->{content}, $content, 'GetLetters gets the content correctly' );
|
||||
is( $letter->{message_transport_type}, 'email', 'GetLetters gets the message type correctly' );
|
||||
};
|
||||
|
||||
|
||||
|
||||
# Regression test for Bug 14206
|
||||
$dbh->do( q|INSERT INTO letter(branchcode,module,code,name,is_html,title,content,message_transport_type) VALUES ('FFL','my module','my code','my name',1,?,?,'print')|, undef, $title, $content );
|
||||
my $letter14206_a = C4::Letters::getletter('my module', 'my code', 'FFL' );
|
||||
is( $letter14206_a->{message_transport_type}, 'print', 'Bug 14206 - message_transport_type not passed, correct mtt detected' );
|
||||
my $letter14206_b = C4::Letters::getletter('my module', 'my code', 'FFL', 'print');
|
||||
is( $letter14206_b->{message_transport_type}, 'print', 'Bug 14206 - message_transport_type passed, correct mtt detected' );
|
||||
|
||||
# test for overdue_notices.pl
|
||||
my $overdue_rules = {
|
||||
letter1 => 'my code',
|
||||
};
|
||||
my $i = 1;
|
||||
my $branchcode = 'FFL';
|
||||
my $letter14206_c = C4::Letters::getletter('my module', $overdue_rules->{"letter$i"}, $branchcode);
|
||||
is( $letter14206_c->{message_transport_type}, 'print', 'Bug 14206 - correct mtt detected for call from overdue_notices.pl' );
|
||||
|
||||
# GetPreparedLetter
|
||||
t::lib::Mocks::mock_preference('OPACBaseURL', 'http://thisisatest.com');
|
||||
t::lib::Mocks::mock_preference( 'SendAllEmailsTo', '' );
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ use C4::Output;
|
|||
use C4::Letters;
|
||||
use C4::Log;
|
||||
|
||||
use Koha::Notice::Templates;
|
||||
use Koha::Patron::Attribute::Types;
|
||||
|
||||
# $protected_letters = protected_letters()
|
||||
|
|
@ -303,33 +304,39 @@ sub add_validate {
|
|||
my $is_html = $input->param("is_html_$mtt\_$lang");
|
||||
my $title = shift @title;
|
||||
my $content = shift @content;
|
||||
my $letter = C4::Letters::getletter( $oldmodule, $code, $branchcode, $mtt, $lang );
|
||||
my $letter = Koha::Notice::Templates->find(
|
||||
{
|
||||
module => $oldmodule,
|
||||
code => $code,
|
||||
branchcode => $branchcode,
|
||||
message_transport_type => $mtt,
|
||||
lang => $lang
|
||||
}
|
||||
);
|
||||
|
||||
# getletter can return the default letter even if we pass a branchcode
|
||||
# If we got the default one and we needed the specific one, we didn't get the one we needed!
|
||||
if ( $letter and $branchcode and $branchcode ne $letter->{branchcode} ) {
|
||||
$letter = undef;
|
||||
}
|
||||
unless ( $title and $content ) {
|
||||
# Delete this mtt if no title or content given
|
||||
delete_confirmed( $branchcode, $oldmodule, $code, $mtt, $lang );
|
||||
next;
|
||||
}
|
||||
elsif ( $letter and $letter->{message_transport_type} eq $mtt and $letter->{lang} eq $lang ) {
|
||||
logaction( 'NOTICES', 'MODIFY', $letter->{id}, $content,
|
||||
elsif ( $letter ) {
|
||||
logaction( 'NOTICES', 'MODIFY', $letter->id, $content,
|
||||
'Intranet' )
|
||||
if ( C4::Context->preference("NoticesLog")
|
||||
&& $content ne $letter->{content} );
|
||||
$dbh->do(
|
||||
q{
|
||||
UPDATE letter
|
||||
SET branchcode = ?, module = ?, name = ?, is_html = ?, title = ?, content = ?, lang = ?
|
||||
WHERE branchcode = ? AND module = ? AND code = ? AND message_transport_type = ? AND lang = ?
|
||||
},
|
||||
undef,
|
||||
$branchcode || '', $module, $name, $is_html || 0, $title, $content, $lang,
|
||||
$branchcode, $oldmodule, $code, $mtt, $lang
|
||||
);
|
||||
&& $content ne $letter->content );
|
||||
|
||||
$letter->set(
|
||||
{
|
||||
branchcode => $branchcode || '',
|
||||
module => $module,
|
||||
name => $name,
|
||||
is_html => $is_html || 0,
|
||||
title => $title,
|
||||
content => $content,
|
||||
lang => $lang
|
||||
}
|
||||
)->store;
|
||||
|
||||
} else {
|
||||
my $letter = Koha::Notice::Template->new(
|
||||
{
|
||||
|
|
@ -357,10 +364,10 @@ sub add_validate {
|
|||
sub delete_confirm {
|
||||
my ($branchcode, $module, $code) = @_;
|
||||
my $dbh = C4::Context->dbh;
|
||||
my $letter = C4::Letters::getletter($module, $code, $branchcode);
|
||||
my @values = values %$letter;
|
||||
my $letter = Koha::Notice::Templates->search(
|
||||
{ module => $module, code => $code, branchcode => $branchcode } );
|
||||
$template->param(
|
||||
letter => $letter,
|
||||
letter => $letter ? $letter->next : undef,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue