Bug 9016: Define different transport types for overdues
authorJonathan Druart <jonathan.druart@biblibre.com>
Fri, 6 Sep 2013 08:28:39 +0000 (10:28 +0200)
committerGalen Charlton <gmc@esilibrary.com>
Fri, 2 May 2014 20:29:16 +0000 (20:29 +0000)
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.

Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
C4/Overdues.pm
koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt
t/db_dependent/Overdues.t [new file with mode: 0644]
tools/overduerules.pl

index af83e0f..f9212f4 100644 (file)
@@ -50,6 +50,7 @@ BEGIN {
         &GetOverduesForBranch
         &RemoveNotifyLine
         &AddNotifyLine
+        &GetOverdueMessageTransportTypes
        );
        # subs to remove
        push @EXPORT, qw(
@@ -906,6 +907,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__
 
index 12b53c3..a9ba122 100644 (file)
@@ -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>&nbsp;</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>&nbsp;</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>&nbsp;</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>&nbsp;</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 (file)
index 0000000..2725565
--- /dev/null
@@ -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;
index cc90d0b..810b1fd 100755 (executable)
@@ -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;