@@ -, +, @@ libraries --- C4/Letters.pm | 141 ++++++++++++++- .../intranet-tmpl/prog/en/modules/tools/letter.tt | 31 ++-- svc/letters | 2 +- t/db_dependent/Letters/GetLetterTemplates.t | 125 +++++++++++++ .../Letters/GetLettersAvailableForALibrary.t | 195 +++++++++++++++++++++ tools/letter.pl | 57 ++---- tools/overduerules.pl | 7 +- 7 files changed, 501 insertions(+), 57 deletions(-) create mode 100644 t/db_dependent/Letters/GetLetterTemplates.t create mode 100644 t/db_dependent/Letters/GetLettersAvailableForALibrary.t --- a/C4/Letters.pm +++ a/C4/Letters.pm @@ -44,7 +44,7 @@ BEGIN { $VERSION = 3.07.00.049; @ISA = qw(Exporter); @EXPORT = qw( - &GetLetters &GetPreparedLetter &GetWrappedLetter &addalert &getalert &delalert &findrelatedto &SendAlerts &GetPrintMessages &GetMessageTransportTypes + &GetLetters &GetLettersAvailableForALibrary &GetLetterTemplates &DelLetter &GetPreparedLetter &GetWrappedLetter &addalert &getalert &delalert &findrelatedto &SendAlerts &GetPrintMessages &GetMessageTransportTypes ); } @@ -75,6 +75,7 @@ sub GetLetters { my ($filters) = @_; my $module = $filters->{module}; my $code = $filters->{code}; + my $branchcode = $filters->{branchcode}; my $dbh = C4::Context->dbh; my $letters = $dbh->selectall_arrayref( q| @@ -84,14 +85,120 @@ sub GetLetters { | . ( $module ? q| AND module = ?| : q|| ) . ( $code ? q| AND code = ?| : q|| ) + . ( defined $branchcode ? q| AND branchcode = ?| : q|| ) . q| GROUP BY code ORDER BY name|, { Slice => {} } , ( $module ? $module : () ) , ( $code ? $code : () ) + , ( defined $branchcode ? $branchcode : () ) ); return $letters; } +=head2 GetLetterTemplates + + my $letter_templates = GetLetterTemplates( + { + module => 'circulation', + code => 'my code', + branchcode => 'CPL', # '' for default, + } + ); + + Return a hashref of letter templates. + The key will be the message transport type. + +=cut + +sub GetLetterTemplates { + my ( $params ) = @_; + + my $module = $params->{module}; + my $code = $params->{code}; + my $branchcode = $params->{branchcode}; + my $dbh = C4::Context->dbh; + my $letters = $dbh->selectall_hashref( + q| + SELECT module, code, branchcode, name, is_html, title, content, message_transport_type + FROM letter + WHERE module = ? + AND code = ? + and branchcode = ? + | + , 'message_transport_type' + , undef + , $module, $code, $branchcode + ); + + return $letters; +} + +=head2 GetLettersAvailableForALibrary + + my $letters = GetLettersAvailableForALibrary( + { + branchcode => 'CPL', # '' for default + module => 'circulation', + } + ); + + Return an arrayref of letters, sorted by name. + If a specific letter exist for the given branchcode, it will be retrieve. + Otherwise the default letter will be. + +=cut + +sub GetLettersAvailableForALibrary { + my ($filters) = @_; + my $branchcode = $filters->{branchcode}; + my $module = $filters->{module}; + + croak "module should be provided" unless $module; + + my $dbh = C4::Context->dbh; + my $default_letters = $dbh->selectall_arrayref( + q| + SELECT module, code, branchcode, name + FROM letter + WHERE 1 + | + . q| AND branchcode = ''| + . ( $module ? q| AND module = ?| : q|| ) + . q| ORDER BY name|, { Slice => {} } + , ( $module ? $module : () ) + ); + + my $specific_letters; + if ($branchcode) { + $specific_letters = $dbh->selectall_arrayref( + q| + SELECT module, code, branchcode, name + FROM letter + WHERE 1 + | + . q| AND branchcode = ?| + . ( $module ? q| AND module = ?| : q|| ) + . q| ORDER BY name|, { Slice => {} } + , $branchcode + , ( $module ? $module : () ) + ); + } + + my %letters; + for my $l (@$default_letters) { + $letters{ $l->{code} } = $l; + } + for my $l (@$specific_letters) { + # Overwrite the default letter with the specific one. + $letters{ $l->{code} } = $l; + } + + return [ map { $letters{$_} } + sort { $letters{$a}->{name} cmp $letters{$b}->{name} } + keys %letters ]; + +} + # FIXME: using our here means that a Plack server will need to be # restarted fairly regularly when working with this routine. # A better option would be to use Koha::Cache and use a cache @@ -130,6 +237,38 @@ sub getletter { return { %$line }; } +=head2 DelLetter + + DelLetter( + { + branchcode => 'CPL', + module => 'circulation', + code => 'my code', + [ mtt => 'email', ] + } + ); + + Delete the letter. The mtt parameter is facultative. + If not given, all templates mathing the other parameters will be removed. + +=cut + +sub DelLetter { + my ($params) = @_; + my $branchcode = $params->{branchcode}; + my $module = $params->{module}; + my $code = $params->{code}; + my $mtt = $params->{mtt}; + my $dbh = C4::Context->dbh; + $dbh->do(q| + DELETE FROM letter + WHERE branchcode = ? + AND module = ? + AND code = ? + | . ( $mtt ? q| AND message_transport_type = ?| : q|| ) + , undef, $branchcode, $module, $code, ( $mtt ? $mtt : () ) ); +} + =head2 addalert ($borrowernumber, $type, $externalid) parameters : --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt @@ -53,23 +53,21 @@ $(document).ready(function() { // Test if code already exists in DB var new_lettercode = $("#code").val(); - [% IF copy_form %] - if ( new_lettercode == '[% code %]' ) { - alert( _("Please change the code.") ); - return false; - } - [% END %] - + var new_branchcode = $("#branch").val(); [% IF ( add_form and code ) # IF edit %] if ( new_lettercode != '[% code %]' ) { [% END %] $.ajax({ - data: { code: new_lettercode }, + data: { code: new_lettercode, branchcode: new_branchcode }, type: 'GET', url: '/cgi-bin/koha/svc/letters/', success: function (data) { if ( data.letters.length > 0 ) { - alert( _("This letter code is already used for another letter.") ); + if( new_branchcode == '' ) { + alert( _("A default letter with the code '%s' already exists.").format(new_lettercode) ); + } else { + alert( _("A letter with the code '%s' already exists for '%s'.").format(new_lettercode, new_branchcode) ); + } return false; } else { $("#add_notice").submit(); @@ -333,13 +331,15 @@ $(document).ready(function() {
  • - - - Required - [% IF copying %] - You must change this code to reflect the copy. + [% IF adding %] + + + Required + [% ELSE %] + + + [% code %] [% END %] -
  • @@ -448,7 +448,6 @@ $(document).ready(function() { - --- a/svc/letters +++ a/svc/letters @@ -53,7 +53,7 @@ Used to get letters with a given letter code. our ( $query, $response ) = C4::Service->init( tools => 'edit_notices' ); sub get_letters { - my $letters = GetLetters({ code => $query->param('code') }); + my $letters = GetLetters({ code => $query->param('code'), branchcode => $query->param('branchcode') }); $response->param( letters => $letters ); C4::Service->return_success( $response ); } --- a/t/db_dependent/Letters/GetLetterTemplates.t +++ a/t/db_dependent/Letters/GetLetterTemplates.t @@ -0,0 +1,125 @@ +use Modern::Perl; +use Test::More tests => 6; + +use C4::Context; +use C4::Letters qw( GetLetterTemplates ); + +my $dbh = C4::Context->dbh; +$dbh->{RaiseError} = 1; +$dbh->{AutoCommit} = 0; + +$dbh->do(q|DELETE FROM letter|); + +my $letters = [ + { + module => 'circulation', + code => 'code1', + branchcode => '', + name => 'B default name for code1 circ', + is_html => 0, + title => 'default title for code1 email', + content => 'default content for code1 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code1', + branchcode => '', + name => 'B default name for code1 circ', + is_html => 0, + title => 'default title for code1 sms', + content => 'default content for code1 sms', + message_transport_type => 'sms', + }, + { + module => 'circulation', + code => 'code2', + branchcode => '', + name => 'A default name for code2 circ', + is_html => 0, + title => 'default title for code2 email', + content => 'default content for code2 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code3', + branchcode => '', + name => 'C default name for code3 circ', + is_html => 0, + title => 'default title for code3 email', + content => 'default content for code3 email', + message_transport_type => 'email', + }, + + { + module => 'cataloguing', + code => 'code1', + branchcode => '', + name => 'default name for code1 cat', + is_html => 0, + title => 'default title for code1 cat email', + content => 'default content for code1 cat email', + message_transport_type => 'email', + }, + + { + module => 'circulation', + code => 'code1', + branchcode => 'CPL', + name => 'B CPL name for code1 circ', + is_html => 0, + title => 'CPL title for code1 email', + content => 'CPL content for code1 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code2', + branchcode => 'CPL', + name => 'A CPL name for code1 circ', + is_html => 0, + title => 'CPL title for code1 sms', + content => 'CPL content for code1 sms', + message_transport_type => 'sms', + }, + { + module => 'circulation', + code => 'code1', + branchcode => 'MPL', + name => 'B MPL name for code1 circ', + is_html => 0, + title => 'MPL title for code1 email', + content => 'MPL content for code1 email', + message_transport_type => 'email', + }, +]; + +my $sth = $dbh->prepare( +q|INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type) VALUES (?, ?, ?, ?, ?, ?, ?)| +); +for my $l (@$letters) { + $sth->execute( $l->{module}, $l->{code}, $l->{branchcode}, $l->{name}, + $l->{title}, $l->{content}, $l->{message_transport_type} ); +} + +my $letter_templates; +$letter_templates = C4::Letters::GetLetterTemplates; +is_deeply( $letter_templates, {}, + 'GetLetterTemplates should not return templates if not param is given' ); + +$letter_templates = C4::Letters::GetLetterTemplates( + { module => 'circulation', code => 'code1', branchcode => '' } ); +is( scalar( keys %$letter_templates ), + 2, '2 default templates should exist for circulation code1' ); +is( exists( $letter_templates->{email} ), + 1, 'The mtt email should exist for circulation code1' ); +is( exists( $letter_templates->{sms} ), + 1, 'The mtt sms should exist for circulation code1' ); + +$letter_templates = C4::Letters::GetLetterTemplates( + { module => 'circulation', code => 'code1', branchcode => 'CPL' } ); +is( scalar( keys %$letter_templates ), + 1, '1 template should exist for circulation CPL code1' ); +is( exists( $letter_templates->{email} ), + 1, 'The mtt should be email for circulation CPL code1' ); --- a/t/db_dependent/Letters/GetLettersAvailableForALibrary.t +++ a/t/db_dependent/Letters/GetLettersAvailableForALibrary.t @@ -0,0 +1,195 @@ +use Modern::Perl; +use Test::More tests => 19; + +use C4::Context; +use C4::Letters qw( GetLettersAvailableForALibrary DelLetter ); + +my $dbh = C4::Context->dbh; +$dbh->{RaiseError} = 1; +$dbh->{AutoCommit} = 0; + +$dbh->do(q|DELETE FROM letter|); + +my $letters = [ + { + module => 'circulation', + code => 'code1', + branchcode => '', + name => 'B default name for code1 circ', + is_html => 0, + title => 'default title for code1 email', + content => 'default content for code1 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code1', + branchcode => '', + name => 'B default name for code1 circ', + is_html => 0, + title => 'default title for code1 sms', + content => 'default content for code1 sms', + message_transport_type => 'sms', + }, + { + module => 'circulation', + code => 'code2', + branchcode => '', + name => 'A default name for code2 circ', + is_html => 0, + title => 'default title for code2 email', + content => 'default content for code2 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code3', + branchcode => '', + name => 'C default name for code3 circ', + is_html => 0, + title => 'default title for code3 email', + content => 'default content for code3 email', + message_transport_type => 'email', + }, + + { + module => 'cataloguing', + code => 'code1', + branchcode => '', + name => 'default name for code1 cat', + is_html => 0, + title => 'default title for code1 cat email', + content => 'default content for code1 cat email', + message_transport_type => 'email', + }, + + { + module => 'circulation', + code => 'code1', + branchcode => 'CPL', + name => 'B CPL name for code1 circ', + is_html => 0, + title => 'CPL title for code1 email', + content => 'CPL content for code1 email', + message_transport_type => 'email', + }, + { + module => 'circulation', + code => 'code2', + branchcode => 'CPL', + name => 'A CPL name for code1 circ', + is_html => 0, + title => 'CPL title for code1 sms', + content => 'CPL content for code1 sms', + message_transport_type => 'sms', + }, + { + module => 'circulation', + code => 'code1', + branchcode => 'MPL', + name => 'B MPL name for code1 circ', + is_html => 0, + title => 'MPL title for code1 email', + content => 'MPL content for code1 email', + message_transport_type => 'email', + }, +]; + +my $sth = $dbh->prepare( +q|INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type) VALUES (?, ?, ?, ?, ?, ?, ?)| +); +for my $l (@$letters) { + $sth->execute( $l->{module}, $l->{code}, $l->{branchcode}, $l->{name}, + $l->{title}, $l->{content}, $l->{message_transport_type} ); + + # GetLettersAvailableForALibrary does not return these fields + delete $l->{title}; + delete $l->{content}; + delete $l->{is_html}; + delete $l->{message_transport_type}; +} + +my $available_letters; +$available_letters = + C4::Letters::GetLettersAvailableForALibrary( { module => 'circulation' } ); +is( scalar(@$available_letters), + 3, 'There should be 3 default letters for circulation (3 distinct codes)' ); + +$available_letters = C4::Letters::GetLettersAvailableForALibrary( + { module => 'circulation', branchcode => '' } ); +is( scalar(@$available_letters), 3, +'There should be 3 default letters for circulation (3 distinct codes), branchcode=""' +); +is_deeply( $available_letters->[0], + $letters->[2], 'The letters should be sorted by name (A)' ); +is_deeply( $available_letters->[1], + $letters->[0], 'The letters should be sorted by name (B)' ); +is_deeply( $available_letters->[2], + $letters->[3], 'The letters should be sorted by name (C)' ); + +$available_letters = C4::Letters::GetLettersAvailableForALibrary( + { module => 'circulation', branchcode => 'CPL' } ); +is( scalar(@$available_letters), 3, +'There should be 3 default letters for circulation (3 distinct codes), branchcode="CPL"' +); +is_deeply( $available_letters->[0], + $letters->[6], 'The letters should be sorted by name (A)' ); +is_deeply( $available_letters->[1], + $letters->[5], 'The letters should be sorted by name (B)' ); +is_deeply( $available_letters->[2], + $letters->[3], 'The letters should be sorted by name (C)' ); + +$available_letters = C4::Letters::GetLettersAvailableForALibrary( + { module => 'circulation', branchcode => 'MPL' } ); +is( scalar(@$available_letters), 3, +'There should be 3 default letters for circulation (3 distinct codes), branchcode="CPL"' +); +is_deeply( $available_letters->[0], + $letters->[2], 'The letters should be sorted by name (A)' ); +is_deeply( $available_letters->[1], + $letters->[7], 'The letters should be sorted by name (B)' ); +is_deeply( $available_letters->[2], + $letters->[3], 'The letters should be sorted by name (C)' ); + +my $letters_by_module = C4::Letters::GetLetters( { module => 'circulation' } ); +is( scalar(@$letters_by_module), + 3, '3 different letter codes exist for circulation' ); + +my $letters_by_branchcode = C4::Letters::GetLetters( { branchcode => 'CPL' } ); +is( scalar(@$letters_by_branchcode), + 2, '2 different letter codes exist for CPL' ); + +# On the way, we test DelLetter +is( + C4::Letters::DelLetter( + { module => 'cataloguing', code => 'code1', branchcode => 'MPL' } + ), + '0E0', + 'No letter exist for MPL cat/code1' +); +is( + C4::Letters::DelLetter( + { module => 'circulation', code => 'code1', branchcode => '' } + ), + 2, + '2 default letters existed for circ/code1 (1 for email and 1 for sms)' +); +is( + C4::Letters::DelLetter( + { + module => 'circulation', + code => 'code1', + branchcode => 'CPL', + mtt => 'email' + } + ), + 1, + '1 letter existed for CPL circ/code1/email' +); +is( + C4::Letters::DelLetter( + { module => 'circulation', code => 'code1', branchcode => 'MPL' } + ), + 1, + '1 letter existed for MPL circ/code1' +); --- a/tools/letter.pl +++ a/tools/letter.pl @@ -50,34 +50,6 @@ use C4::Branch; # GetBranches use C4::Letters; use C4::Members::Attributes; -# _letter_from_where($branchcode,$module, $code, $mtt) -# - return FROM WHERE clause and bind args for a letter -sub _letter_from_where { - my ($branchcode, $module, $code, $mtt) = @_; - my $sql = q{FROM letter WHERE branchcode = ? AND module = ? AND code = ?}; - $sql .= q{ AND message_transport_type = ?} if $mtt ne '*'; - my @args = ( $branchcode || '', $module, $code, ($mtt ne '*' ? $mtt : ()) ); -# Mysql is retarded. cause branchcode is part of the primary key it cannot be null. How does that -# work with foreign key constraint I wonder... - -# if ($branchcode) { -# $sql .= " AND branchcode = ?"; -# push @args, $branchcode; -# } else { -# $sql .= " AND branchcode IS NULL"; -# } - - return ($sql, \@args); -} - -# get_letters($branchcode,$module, $code, $mtt) -# - return letters with the given $branchcode, $module, $code and $mtt exists -sub get_letters { - my ($sql, $args) = _letter_from_where(@_); - my $dbh = C4::Context->dbh; - my $letter = $dbh->selectall_hashref("SELECT * $sql", 'message_transport_type', undef, @$args); - return $letter; -} # $protected_letters = protected_letters() # - return a hashref of letter_codes representing letters that should never be deleted sub protected_letters { @@ -127,13 +99,11 @@ if ( $op eq 'add_validate' or $op eq 'copy_validate' ) { if ($op eq 'copy_form') { my $oldbranchcode = $input->param('oldbranchcode') || q||; my $branchcode = $input->param('branchcode') || q||; - my $oldcode = $input->param('oldcode') || $input->param('code'); add_form($oldbranchcode, $module, $code); $template->param( oldbranchcode => $oldbranchcode, branchcode => $branchcode, branchloop => _branchloop($branchcode), - oldcode => $oldcode, copying => 1, modify => 0, ); @@ -145,8 +115,7 @@ elsif ( $op eq 'delete_confirm' ) { delete_confirm($branchcode, $module, $code); } elsif ( $op eq 'delete_confirmed' ) { - my $mtt = $input->param('message_transport_type'); - delete_confirmed($branchcode, $module, $code, $mtt); + delete_confirmed($branchcode, $module, $code); $op = q{}; # next operation is to return to default screen } else { @@ -168,7 +137,13 @@ sub add_form { my $letters; # if code has been passed we can identify letter and its an update action if ($code) { - $letters = get_letters($branchcode,$module, $code, '*'); + $letters = C4::Letters::GetLetterTemplates( + { + branchcode => $branchcode, + module => $module, + code => $code, + } + ); } my $message_transport_types = GetMessageTransportTypes(); @@ -278,8 +253,9 @@ sub add_validate { my $is_html = $input->param("is_html_$mtt"); my $title = shift @title; my $content = shift @content; - my $letter = get_letters($branchcode,$oldmodule, $code, $mtt); + my $letter = C4::Letters::getletter( $oldmodule, $code, $branchcode, $mtt); unless ( $title and $content ) { + # Delete this mtt if no title or content given delete_confirmed( $branchcode, $oldmodule, $code, $mtt ); next; } @@ -310,7 +286,7 @@ sub add_validate { sub delete_confirm { my ($branchcode, $module, $code) = @_; my $dbh = C4::Context->dbh; - my $letter = get_letters($branchcode, $module, $code, '*'); + my $letter = C4::Letters::getletter($module, $code, $branchcode); my @values = values %$letter; $template->param( branchcode => $branchcode, @@ -324,9 +300,14 @@ sub delete_confirm { sub delete_confirmed { my ($branchcode, $module, $code, $mtt) = @_; - my ($sql, $args) = _letter_from_where($branchcode, $module, $code, $mtt); - my $dbh = C4::Context->dbh; - $dbh->do("DELETE $sql", undef, @$args); + C4::Letters::DelLetter( + { + branchcode => $branchcode, + module => $module, + code => $code, + mtt => $mtt + } + ); # setup default display for screen default_display($branchcode); return; --- a/tools/overduerules.pl +++ a/tools/overduerules.pl @@ -201,7 +201,12 @@ if ($op eq 'save') { } my $branchloop = GetBranchesLoop($branch); -my $letters = GetLetters({ module => "circulation" }); +my $letters = C4::Letters::GetLettersAvailableForALibrary( + { + branchcode => $branch, + module => "circulation", + } +); my @line_loop; --