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>
}
-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(
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} || {};
=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
use Koha::DateUtils;
use Koha::Calendar;
use Koha::Libraries;
+use Koha::Notice::Templates;
+use Koha::Patrons;
use Koha::Script -cron;
=head1 NAME
# 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;
}
}
}
- 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|;
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,
use C4::Letters;
use Koha::Account::Lines;
use Koha::DateUtils;
+use Koha::Notice::Templates;
my $input = CGI->new;
) 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,
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;
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;
# 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;
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', '' );
use C4::Letters;
use C4::Log;
+use Koha::Notice::Templates;
use Koha::Patron::Attribute::Types;
# $protected_letters = protected_letters()
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(
{
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;
}