From a000e1ef738e84a322f24234dd6f3e85cc97b7d5 Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@biblibre.com> Date: Fri, 6 Sep 2013 10:28:39 +0200 Subject: [PATCH] Bug 9016: Define different transport types for overdues This patch adds: - a new routine C4::Overdues::GetOverdueMessageTransportTypes. - the ability to define one or more transport types for an overdue level. Test plan: - go on tools/overduerules.pl. - verify that previous rules still exist and that the 'email' checkboxes is checked if a rule is defined. - redefined rules at different levels and check the transport type needed. Currently, only email, sms and print are relevant - Save the changes. --- C4/Overdues.pm | 24 +++ .../prog/en/modules/tools/overduerules.tt | 202 +++++++------------- t/db_dependent/Overdues.t | 66 +++++++ tools/overduerules.pl | 126 +++++++++--- 4 files changed, 268 insertions(+), 150 deletions(-) create mode 100644 t/db_dependent/Overdues.t diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 4fd0b84..bb07713 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -50,6 +50,7 @@ BEGIN { &GetOverduesForBranch &RemoveNotifyLine &AddNotifyLine + &GetOverdueMessageTransportTypes ); # subs to remove push @EXPORT, qw( @@ -934,6 +935,29 @@ sub RemoveNotifyLine { return 1; } +=head2 GetOverdueMessageTransportTypes + + my $message_transport_types = GetOverdueMessageTransportTypes( $branchcode, $categorycode, $letternumber); + + return a arrayref with message_transport_type for given branchcode, categorycode and letternumber(1,2 or 3) + +=cut +sub GetOverdueMessageTransportTypes { + my ( $branchcode, $categorycode, $letternumber ) = @_; + return unless $categorycode and $letternumber; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare(" + SELECT message_transport_type FROM overduerules_transport_types + WHERE branchcode = ? AND categorycode = ? AND letternumber = ? + "); + $sth->execute( $branchcode, $categorycode, $letternumber ); + my @mtts; + while ( my $mtt = $sth->fetchrow ) { + push @mtts, $mtt; + } + return \@mtts; +} + 1; __END__ diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt index 12b53c3..a9ba122 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt @@ -4,12 +4,17 @@ <script type="text/javascript"> //<![CDATA[ +var tab_map = { "1" : _("First"), "2" : _("Second"), "3" : _("Third")}; $(document).ready(function() { - $('#selectlibrary').find("input:submit").hide(); - $('#branch').change(function() { - $('#selectlibrary').submit(); - }); - $('#rulestabs').tabs(); + $('#selectlibrary').find("input:submit").hide(); + $('#branch').change(function() { + $('#selectlibrary').submit(); + }); + $("li>a.tab").each( function(){ + var id = $(this).attr("data-number"); + $(this).html(tab_map[id]); + } ); + $('#rulestabs').tabs(); }); //]]> </script> @@ -72,131 +77,72 @@ $(document).ready(function() { [% IF ( datasaved ) %]<div class="dialog message">INPUT SAVED</div> [% END %] <div id="rulestabs" class="toptabs"> - <ul class="ui-tabs-nav"> - <li><a href="#first">First</a></li> - <li><a href="#second">Second</a></li> - <li><a href="#third">Third</a></li> - </ul> + <ul class="ui-tabs-nav"> + [% FOR tab IN tabs %] + <li><a href="#[% tab.id %]" class="tab [% tab.id %]" data-number="[% tab.number %]"></a></li> + [% END %] + </ul> + [% FOR tab IN tabs %] + <div id="[% tab.id %]"> + <table> + <thead> + <tr> + <th> </th> + <th scope="col">Delay</th> + <th scope="col">Letter</th> + <th scope="col">Restrict</th> + [% FOREACH mtt IN message_transport_types %] + <th scpre="col">[% mtt %]</th> + [% END %] + </tr> + </thead> + <tbody> + [% FOREACH value IN tab.values %] + <tr> + <th scope="row">[% value.line %]</th> + <td> + <input type="text" name="delay[% tab.number %]-[% value.overduename %]" size="5" value="[% value.delay %]" /> + </td> + <td> + [% IF ( value.noletter ) %] + <input type="text" name="letter[% tab.number %]-[% value.overduename %]" value="[% value.letter %]" /> + [% ELSE %] + <select name="letter[% tab.number %]-[% value.overduename %]"> + <option value="">No notice</option> + [% FOREACH letterloop IN value.letterloop %] + [% IF ( letterloop.selected ) %] + <option value="[% letterloop.value %]" selected="selected">[% letterloop.lettername %]</option> + [% ELSE %] + <option value="[% letterloop.value %]">[% letterloop.lettername %]</option> + [% END %] + [% END %] + </select> + [% END %] + </td> + <td> + [% IF ( value.debarred ) %] + <input type="checkbox" name="debarred[% tab.number %]-[% value.overduename %]" checked="checked" value="1" /> + [% ELSE %] + <input type="checkbox" name="debarred[% tab.number %]-[% value.overduename %]" value="1" /> + [% END %] + </td> + [% FOREACH mtt IN value.message_transport_types %] + <td> + [% IF mtt.selected %] + <input type="checkbox" name="mtt[% tab.number %]-[% value.overduename %]" value="[% mtt.value %]" checked="checked" /> + [% ELSE %] + <input type="checkbox" name="mtt[% tab.number %]-[% value.overduename %]" value="[% mtt.value %]" /> + [% END %] + </td> + [% END %] + </tr> + [% END %] + </tbody> + </table> + </div> + [% END %] -<div id="first"><table> - <tr><th> </th><th scope="col">Delay</th><th scope="col">First letter</th><th scope="col">Restrict</th></tr> - [% FOREACH tabl IN table %] - [% UNLESS ( loop.odd ) %] - <tr class="highlight"> - [% ELSE %] - <tr> - [% END %] - <th scope="row">[% tabl.line %]</th> -<td> - <input type="text" name="delay1-[% tabl.overduename %]" size="5" value="[% tabl.delay1 %]" /> - </td> -<td> - [% IF ( tabl.noletter ) %] - <input type="text" name="letter1-[% tabl.overduename %]" value="[% tabl.letter1 %]" /> - [% ELSE %] - <select name="letter1-[% tabl.overduename %]"> - <option value="">No notice</option> - [% FOREACH letterloop IN tabl.letterloop1 %] - [% IF ( letterloop.selected ) %] - <option value="[% letterloop.value %]" selected="selected">[% letterloop.lettername %]</option> - [% ELSE %] - <option value="[% letterloop.value %]">[% letterloop.lettername %]</option> - [% END %] - [% END %] - </select> - [% END %] - </td> -<td> - [% IF ( tabl.debarred1 ) %] - <input type="checkbox" name="debarred1-[% tabl.overduename %]" checked="checked" value="1" /> - [% ELSE %] - <input type="checkbox" name="debarred1-[% tabl.overduename %]" value="1" /> - [% END %] - </td> - </tr> - [% END %] - </table> -</div> -<div id="second"><table> - <tr><th> </th><th scope="col">Delay</th><th scope="col">Second letter</th><th scope="col">Restrict</th></tr> - [% FOREACH tabl IN table %] - [% UNLESS ( loop.odd ) %] - <tr class="highlight"> - [% ELSE %] - <tr> - [% END %] - <th scope="row">[% tabl.line %]</th> -<td> - <input type="text" name="delay2-[% tabl.overduename %]" size="5" value="[% tabl.delay2 %]" /> - </td> -<td> - [% IF ( tabl.noletter ) %] - <input type="text" name="letter2-[% tabl.overduename %]" value="[% tabl.letter2 %]" /> - [% ELSE %] - <select name="letter2-[% tabl.overduename %]"> - <option value="">No notice</option> - [% FOREACH letterloop IN tabl.letterloop2 %] - [% IF ( letterloop.selected ) %] - <option value="[% letterloop.value %]" selected="selected">[% letterloop.lettername %]</option> - [% ELSE %] - <option value="[% letterloop.value %]">[% letterloop.lettername %]</option> - [% END %] - [% END %] - </select> - [% END %] - </td> -<td> - [% IF ( tabl.debarred2 ) %] - <input type="checkbox" name="debarred2-[% tabl.overduename %]" checked="checked" value="1" /> - [% ELSE %] - <input type="checkbox" name="debarred2-[% tabl.overduename %]" value="1" /> - [% END %] - </td> - </tr> - [% END %] - </table> -</div> - -<div id="third"><table> - <tr><th> </th><th scope="col">Delay</th><th scope="col">Third letter</th><th scope="col">Restrict</th></tr> - [% FOREACH tabl IN table %] - [% UNLESS ( loop.odd ) %] - <tr class="highlight"> - [% ELSE %] - <tr> - [% END %] - <th scope="row">[% tabl.line %]</th> -<td> - <input type="text" name="delay3-[% tabl.overduename %]" size="5" value="[% tabl.delay3 %]" /> - </td> -<td> - [% IF ( tabl.noletter ) %] - <input type="text" name="letter3-[% tabl.overduename %]" value="[% tabl.letter3 %]" /> - [% ELSE %] - <select name="letter3-[% tabl.overduename %]"> - <option value="">No notice</option> - [% FOREACH letterloop IN tabl.letterloop3 %] - [% IF ( letterloop.selected ) %] - <option value="[% letterloop.value %]" selected="selected">[% letterloop.lettername %]</option> - [% ELSE %] - <option value="[% letterloop.value %]">[% letterloop.lettername %]</option> - [% END %] - [% END %] - </select> - [% END %] - </td> -<td> - [% IF ( tabl.debarred3 ) %] - <input type="checkbox" name="debarred3-[% tabl.overduename %]" checked="checked" value="1" /> - [% ELSE %] - <input type="checkbox" name="debarred3-[% tabl.overduename %]" value="1" /> - [% END %] - </td> - </tr> - [% END %] - </table> -</div> </div> <fieldset class="action"><input type="submit" value="Save changes" /></fieldset> diff --git a/t/db_dependent/Overdues.t b/t/db_dependent/Overdues.t new file mode 100644 index 0000000..2725565 --- /dev/null +++ b/t/db_dependent/Overdues.t @@ -0,0 +1,66 @@ +#!/usr/bin/perl; + +use Modern::Perl; +use Test::More;# tests => 3; + +use C4::Context; +use_ok('C4::Overdues'); +can_ok('C4::Overdues', 'GetOverdueMessageTransportTypes'); + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +$dbh->do(q|DELETE FROM letter|); +$dbh->do(q|DELETE FROM message_queue|); +$dbh->do(q|DELETE FROM message_transport_types|); +$dbh->do(q|DELETE FROM overduerules_transport_types|); + +$dbh->do(q| + INSERT INTO message_transport_types( message_transport_type ) VALUES ('email'), ('phone'), ('print'), ('sms') +|); + +$dbh->do(q| + INSERT INTO overduerules_transport_types( branchcode, categorycode, letternumber, message_transport_type ) VALUES + ('CPL', 'PT', 1, 'email'), + ('CPL', 'PT', 2, 'sms'), + ('CPL', 'PT', 3, 'email'), + ('CPL', 'YA', 3, 'print'), + ('', 'PT', 1, 'email'), + ('', 'PT', 2, 'email'), + ('', 'PT', 2, 'sms'), + ('', 'PT', 3, 'print'), + ('', 'YA', 2, 'sms') +|); + +my $mtts; + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL', 'PT'); +is( $mtts, undef, 'GetOverdueMessageTransportTypes: returns undef if no letternumber given' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL', undef, 1); +is( $mtts, undef, 'GetOverdueMessageTransportTypes: returns undef if no categorycode given' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL'); +is( $mtts, undef, 'GetOverdueMessageTransportTypes: returns undef if no letternumber and categorycode given' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL', 'PT', 1); +is_deeply( $mtts, ['email'], 'GetOverdueMessageTransportTypes: first overdue is by email for PT (CPL)' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL', 'PT', 2); +is_deeply( $mtts, ['sms'], 'GetOverdueMessageTransportTypes: second overdue is by sms for PT (CPL)' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('CPL', 'PT', 3); +is_deeply( $mtts, ['email'], 'GetOverdueMessageTransportTypes: third overdue is by email for PT (CPL)' ); + +my $mtts = C4::Overdues::GetOverdueMessageTransportTypes('', 'PT', 1); +is_deeply( $mtts, ['email'], 'GetOverdueMessageTransportTypes: first overdue is by email for PT (default)' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('', 'PT', 2); +is_deeply( $mtts, ['email', 'sms'], 'GetOverdueMessageTransportTypes: second overdue is by email and sms for PT (default)' ); + +$mtts = C4::Overdues::GetOverdueMessageTransportTypes('', 'PT', 3); +is_deeply( $mtts, ['print'], 'GetOverdueMessageTransportTypes: third overdue is by print for PT (default)' ); + + +done_testing; diff --git a/tools/overduerules.pl b/tools/overduerules.pl index cc90d0b..810b1fd 100755 --- a/tools/overduerules.pl +++ b/tools/overduerules.pl @@ -27,6 +27,7 @@ use C4::Koha; use C4::Branch; use C4::Letters; use C4::Members; +use C4::Overdues; our $input = new CGI; my $dbh = C4::Context->dbh; @@ -44,11 +45,11 @@ sub blank_row { for my $rp (@rule_params) { for my $n (1 .. 3) { my $key = "${rp}${n}-$category_code"; - + if (utf8::is_utf8($key)) { utf8::encode($key); } - + my $value = $input->param($key); if ($value) { return 0; @@ -84,6 +85,18 @@ if ($op eq 'save') { my $sth_insert = $dbh->prepare("INSERT INTO overduerules (branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3) VALUES (?,?,?,?,?,?,?,?,?,?,?)"); my $sth_update=$dbh->prepare("UPDATE overduerules SET delay1=?, letter1=?, debarred1=?, delay2=?, letter2=?, debarred2=?, delay3=?, letter3=?, debarred3=? WHERE branchcode=? AND categorycode=?"); my $sth_delete=$dbh->prepare("DELETE FROM overduerules WHERE branchcode=? AND categorycode=?"); + my $sth_insert_mtt = $dbh->prepare(" + INSERT INTO overduerules_transport_types( + branchcode, categorycode, letternumber, message_transport_type + ) VALUES ( + ?, ?, ?, ? + ) + "); + my $sth_delete_mtt = $dbh->prepare(" + DELETE FROM overduerules_transport_types + WHERE branchcode = ? AND categorycode = ? + "); + foreach my $key (@names){ # ISSUES if ($key =~ /(delay|letter|debarred)([1-3])-(.*)/) { @@ -166,6 +179,15 @@ if ($op eq 'save') { ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0) ); } + + $sth_delete_mtt->execute( $branch, $bor ); + for my $letternumber ( 1..3 ) { + my @mtt = $input->param( "mtt${letternumber}-$bor" ); + next unless @mtt; + for my $mtt ( @mtt ) { + $sth_insert_mtt->execute( $branch, $bor, $letternumber, $mtt); + } + } } } } @@ -185,19 +207,21 @@ my $countletters = keys %{$letters}; my @line_loop; +my $message_transport_types = C4::Letters::GetMessageTransportTypes(); +my ( @first, @second, @third ); for my $data (@categories) { - my %row = ( - overduename => $data->{'categorycode'}, - line => $data->{'description'} - ); if (%temphash and not $input_saved){ # if we managed to save the form submission, don't # reuse %temphash, but take the values from the # database - this makes it easier to identify # bugs where the form submission was not correctly saved - for (my $i=1;$i<=3;$i++){ - $row{"delay$i"}=$temphash{$data->{'categorycode'}}->{"delay$i"}; - $row{"debarred$i"}=$temphash{$data->{'categorycode'}}->{"debarred$i"}; + for my $i ( 1..3 ){ + my %row = ( + overduename => $data->{'categorycode'}, + line => $data->{'description'} + ); + $row{delay}=$temphash{$data->{'categorycode'}}->{"delay$i"}; + $row{debarred}=$temphash{$data->{'categorycode'}}->{"debarred$i"}; if ($countletters){ my @letterloop; foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) { @@ -212,10 +236,26 @@ for my $data (@categories) { ); push @letterloop, \%letterrow; } - $row{"letterloop$i"}=\@letterloop; + $row{letterloop}=\@letterloop; } else { - $row{"noletter"}=1; - $row{"letter$i"}=$temphash{$data->{'categorycode'}}->{"letter$i"}; + $row{noletter}=1; + $row{letter}=$temphash{$data->{'categorycode'}}->{"letter$i"}; + } + my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; + my @mtts; + for my $mtt ( @$message_transport_types ) { + push @mtts, { + value => $mtt, + selected => ( grep {/$mtt/} @selected_mtts ) ? 1 : 0 , + } + } + $row{message_transport_types} = \@mtts; + if ( $i == 1 ) { + push @first, \%row; + } elsif ( $i == 2 ) { + push @second, \%row; + } else { + push @third, \%row; } } } else { @@ -223,7 +263,11 @@ for my $data (@categories) { my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?"); $sth2->execute($branch,$data->{'categorycode'}); my $dat=$sth2->fetchrow_hashref; - for (my $i=1;$i<=3;$i++){ + for my $i ( 1..3 ){ + my %row = ( + overduename => $data->{'categorycode'}, + line => $data->{'description'} + ); if ($countletters){ my @letterloop; foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) { @@ -237,19 +281,57 @@ for my $data (@categories) { ); push @letterloop, \%letterrow; } - $row{"letterloop$i"}=\@letterloop; + $row{letterloop}=\@letterloop; } else { - $row{"noletter"}=1; - if ($dat->{"letter$i"}){$row{"letter$i"}=$dat->{"letter$i"};} + $row{noletter}=1; + if ($dat->{"letter$i"}){$row{letter}=$dat->{"letter$i"};} + } + if ($dat->{"delay$i"}){$row{delay}=$dat->{"delay$i"};} + if ($dat->{"debarred$i"}){$row{debarred}=$dat->{"debarred$i"};} + my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; + my @mtts; + for my $mtt ( @$message_transport_types ) { + push @mtts, { + value => $mtt, + selected => ( grep {/$mtt/} @selected_mtts ) ? 1 : 0 , + } } - if ($dat->{"delay$i"}){$row{"delay$i"}=$dat->{"delay$i"};} - if ($dat->{"debarred$i"}){$row{"debarred$i"}=$dat->{"debarred$i"};} + $row{message_transport_types} = \@mtts; + if ( $i == 1 ) { + push @first, \%row; + } elsif ( $i == 2 ) { + push @second, \%row; + } else { + push @third, \%row; + } + } } - push @line_loop,\%row; } -$template->param(table=> \@line_loop, - branchloop => $branchloop, - branch => $branch); +my @tabs = ( + { + id => 'first', + number => 1, + values => \@first, + }, + { + id => 'second', + number => 2, + values => \@second, + }, + { + id => 'third', + number => 3, + values => \@third, + }, +); + +$template->param( + table => ( @first or @second or @third ? 1 : 0 ), + branchloop => $branchloop, + branch => $branch, + tabs => \@tabs, + message_transport_types => $message_transport_types, +); output_html_with_http_headers $input, $cookie, $template->output; -- 1.7.10.4