Bug 23273: Fix CSV export for overdues

We should construct the URI parameters string manually to avoid
filtering problems.
We cannot send the full query_string to the template and expect that the
string will be escaped correctly.

Test plan:
- go to overdues.pl
- construct a search limiting by date due and library
- note number of overdues in results
- click "Download file of displayed overdues"
=> note that downloaded file contains just those in your search

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/circ/overdue.pl b/circ/overdue.pl
index 0b6f0f8..f5683bd 100755 (executable)
--- a/circ/overdue.pl
+++ b/circ/overdue.pl
@@ -39,16 +39,30 @@ my $borflagsfilter  = $input->param('borflag') || '';
 my $branchfilter    = $input->param('branch') || '';
 my $homebranchfilter    = $input->param('homebranch') || '';
 my $holdingbranchfilter = $input->param('holdingbranch') || '';
+my $dateduefrom     = $input->param('dateduefrom');
+my $datedueto       = $input->param('datedueto');
 my $op              = $input->param('op') || '';
 
-my ($dateduefrom, $datedueto);
-if ( $dateduefrom = $input->param('dateduefrom') ) {
+if ( $dateduefrom ) {
     $dateduefrom = dt_from_string( $dateduefrom );
 }
-if ( $datedueto = $input->param('datedueto') ) {
+if ( $datedueto ) {
     $datedueto = dt_from_string( $datedueto )->set_hour(23)->set_minute(59);
 }
 
+my $filters = {
+    itemtype      => $itemtypefilter,
+    borname       => $bornamefilter,
+    borcat        => $borcatfilter,
+    itemtype      => $itemtypefilter,
+    borflag       => $borflagsfilter,
+    branch        => $branchfilter,
+    homebranch    => $homebranchfilter,
+    holdingbranch => $holdingbranchfilter,
+    dateduefrom   => $dateduefrom,
+    datedueto     => $datedueto,
+};
+
 my $isfiltered      = $op =~ /apply/i && $op =~ /filter/i;
 my $noreport        = C4::Context->preference('FilterBeforeOverdueReport') && ! $isfiltered && $op ne "csv";
 
@@ -179,18 +193,14 @@ if (@patron_attr_filter_loop) {
 }
 
 
+use Data::Printer colored => 1; warn p $filters;
 $template->param(
     patron_attr_header_loop => [ map { { header => $_->{description} } } grep { ! $_->{isclone} } @patron_attr_filter_loop ],
-    branchfilter => $branchfilter,
-    homebranchfilter => $homebranchfilter,
-    holdingbranchfilter => $holdingbranchfilter,
+    filters => $filters,
     borcatloop=> \@borcatloop,
     itemtypeloop => \@itemtypeloop,
     patron_attr_filter_loop => \@patron_attr_filter_loop,
-    borname => $bornamefilter,
     showall => $showall,
-    dateduefrom => $dateduefrom,
-    datedueto   => $datedueto,
 );
 
 if ($noreport) {
@@ -346,10 +356,8 @@ if ($noreport) {
     # generate parameter list for CSV download link
     my $new_cgi = CGI->new($input);
     $new_cgi->delete('op');
-    my $csv_param_string = $new_cgi->query_string();
 
     $template->param(
-        csv_param_string        => $csv_param_string,
         todaysdate              => output_pref($today_dt),
         overdueloop             => \@overduedata,
         nnoverdue               => scalar(@overduedata),

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
index efa42ea..d644d45 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
@@ -34,7 +34,11 @@
 
   <p>
     [% IF ( isfiltered ) %]
-      <a href="overdue.pl?op=csv&amp;[% csv_param_string | uri %]">Download file of displayed overdues</a>
+        [% SET url_params = '' %]
+        [% FOR var IN filters.keys %]
+            [% url_params = BLOCK %][% url_params %]&amp;[% var | uri %]=[% filters.$var | uri %][% END %]
+        [% END %]
+      <a href="overdue.pl?op=csv[% url_params | $raw %]">Download file of displayed overdues</a>
     [% ELSE %]
       <a href="overdue.pl?op=csv">Download file of all overdues</a>
     [% END %]
@@ -102,15 +106,15 @@
        <fieldset><legend>Date due:</legend>
        <ol>
     <li><label for="from">From:</label>
-    <input type="text" id="from" name="dateduefrom" size="10" value="[% dateduefrom | $KohaDates %]" class="datepickerfrom" />
+    <input type="text" id="from" name="dateduefrom" size="10" value="[% filters.dateduefrom | $KohaDates %]" class="datepickerfrom" />
        </li>
        <li>
     <label for="to">To:</label>
-    <input type="text" id="to" name="datedueto" size="10" value="[% datedueto | $KohaDates %]" class="datepickerto" />
+    <input type="text" id="to" name="datedueto" size="10" value="[% filters.datedueto | $KohaDates %]" class="datepickerto" />
     </li>
     </ol></fieldset>
     <ol>
-    <li><label>Name or cardnumber:</label><input type="text" name="borname" value="[% borname | html %]" /></li>
+    <li><label>Name or cardnumber:</label><input type="text" name="borname" value="[% filters.borname | html %]" /></li>
     <li><label>Patron category:</label><select name="borcat" id="borcat"><option value="">Any</option>
       [% FOREACH borcatloo IN borcatloop %]
         [% IF ( borcatloo.selected ) %]<option value="[% borcatloo.value | html %]" selected="selected">[% borcatloo.catname | html %]</option>[% ELSE %]<option value="[% borcatloo.value | html %]">[% borcatloo.catname | html %]</option>[% END %]
@@ -167,7 +171,7 @@
         <label>Item home library:</label>
         <select name="homebranch" id="homebranch">
             <option value="">Any</option>
-            [% PROCESS options_for_libraries libraries => Branches.all( selected => homebranchfilter ) %]
+            [% PROCESS options_for_libraries libraries => Branches.all( selected => filters.homebranch ) %]
         </select>
     </li>
 
@@ -175,7 +179,7 @@
         <label>Item holding library:</label>
         <select name="holdingbranch" id="holdingbranch">
             <option value="">Any</option>
-            [% PROCESS options_for_libraries libraries => Branches.all( selected => holdingbranchfilter ) %]
+            [% PROCESS options_for_libraries libraries => Branches.all( selected => filters.holdingbranch ) %]
         </select>
     </li>
 
@@ -183,7 +187,7 @@
         <label>Library of the patron:</label>
         <select name="branch" id="branch">
             <option value="">Any</option>
-            [% PROCESS options_for_libraries libraries => Branches.all( selected => branchfilter, only_from_group => 1 ) %]
+            [% PROCESS options_for_libraries libraries => Branches.all( selected => filters.branch, only_from_group => 1 ) %]
         </select>
     </li>