Attachment 45241 Details for Bug 7683 – [PASSED QA] Bug 7683: Catalog statistics wizard: QA fixes

[patch] [PASSED QA] Bug 7683: Catalog statistics wizard: QA fixes

PASSED-QA-Bug-7683-Catalog-statistics-wizard-QA-fi.patch (text/plain), 10.04 KB, created by Katrin Fischer on 2015-11-30 21:49:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2015-11-30 21:49:45 UTC

Size: 10.04 KB

patch

obsolete

>From 377c75ad70b4565c49fe6b489b955b630f4a92d0 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Mon, 24 Aug 2015 12:20:17 +0200
>Subject: [PATCH] [PASSED QA] Bug 7683: Catalog statistics wizard: QA fixes
>
>1/ Do not allow invalid date ranges (from > to) (datepicker only)
>2/ Relabel "From:" to "From" for consistency
>3/ Fix MIME type for CSV
>4/ Use Koha::DateUtils instead of C4::Dates
>5/ Use placeholders in SQL query
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> .../prog/en/modules/reports/catalogue_stats.tt     | 32 +++++-----
> reports/catalogue_stats.pl                         | 73 ++++++++++++++--------
> 2 files changed, 61 insertions(+), 44 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
>index cdad039..efb062a 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
>@@ -20,19 +20,17 @@
>     }
> 
>     $(document).ready(function() {
>-        var datesRO = $( "#fromRO, #toRO" ).datepicker({
>-            changeMonth: true,
>-            numberOfMonths: 1,
>-            onSelect: function( selectedDate ) {
>-                var option = this.id == "fromRO" ? "minDate" : "maxDate",
>-                    instance = $( this ).data( "datepicker" );
>-                    date = $.datepicker.parseDate(
>-                        instance.settings.dateFormat ||
>-                        $.datepicker._defaults.dateFormat,
>-                        selectedDate, instance.settings );
>-                datesRO.not( this ).datepicker( "option", option, date );
>-            }
>-        });
>+        $('#acqdateFrom, #deldateFrom')
>+          .datepicker('option', 'onSelect', function(selectedDate) {
>+            var id = $(this).attr('id').replace('From', 'To');
>+            $('#' + id).datepicker('option', 'minDate', selectedDate);
>+          });
>+        $('#acqdateTo, #deldateTo')
>+          .datepicker('option', 'onSelect', function(selectedDate) {
>+            var id = $(this).attr('id').replace('To', 'From');
>+            $('#' + id).datepicker('option', 'maxDate', selectedDate);
>+          });
>+
>         $("input[name='Cellvalue']").change(function() {
>             changeRemovedDateTrStatus();
>         });
>@@ -259,9 +257,9 @@
>                 <td><input type="radio" name="Line" value="items.dateaccessioned" /></td>
>                 <td><input type="radio" name="Column" value="items.dateaccessioned" /></td>
>                 <td>
>-                    <label for="acqdateFrom">From:</label>
>+                    <label for="acqdateFrom">From</label>
>                     <input type="text" name="Filter" id="acqdateFrom" class="datepicker" />
>-                    <label for="acqdateTo">To:</label>
>+                    <label for="acqdateTo">To</label>
>                     <input type="text" name="Filter" id="acqdateTo" class="datepicker" />
>                 </td>
>             </tr>
>@@ -270,9 +268,9 @@
>                 <td><input type="radio" name="Line" value="deleteditems.timestamp" /></td>
>                 <td><input type="radio" name="Column" value="deleteditems.timestamp" /></td>
>                 <td>
>-                    <label for="deldateFrom">From:</label>
>+                    <label for="deldateFrom">From</label>
>                     <input type="text" name="Filter" id="deldateFrom" class="datepicker" />
>-                    <label for="deldateTo">To:</label>
>+                    <label for="deldateTo">To</label>
>                     <input type="text" name="Filter" id="deldateTo" class="datepicker"/>
>                 </td>
>             </tr>
>diff --git a/reports/catalogue_stats.pl b/reports/catalogue_stats.pl
>index aa2df2e..120df88 100755
>--- a/reports/catalogue_stats.pl
>+++ b/reports/catalogue_stats.pl
>@@ -30,6 +30,8 @@ use C4::Reports;
> use C4::Circulation;
> use C4::Biblio qw/GetMarcSubfieldStructureFromKohaField/;
> 
>+use Koha::DateUtils;
>+
> =head1 NAME
> 
> plugin that shows a stats on borrowers
>@@ -81,7 +83,7 @@ if ($do_it) {
>         exit;
>     } else {
>         print $input->header(
>-            -type       => 'application/vnd.sun.xml.calc',
>+            -type       => 'text/csv',
>             -encoding   => 'utf-8',
>             -attachment => "$basename.csv",
>             -name       => "$basename.csv"
>@@ -231,10 +233,10 @@ sub calculate {
>         }
>     }
> 
>-    @$filters[13] = C4::Dates->new(@$filters[13])->output('iso') if @$filters[13];
>-    @$filters[14] = C4::Dates->new(@$filters[14])->output('iso') if @$filters[14];
>-    @$filters[15] = C4::Dates->new(@$filters[15])->output('iso') if @$filters[15];
>-    @$filters[16] = C4::Dates->new(@$filters[16])->output('iso') if @$filters[16];
>+    @$filters[13] = dt_from_string(@$filters[13])->date() if @$filters[13];
>+    @$filters[14] = dt_from_string(@$filters[14])->date() if @$filters[14];
>+    @$filters[15] = dt_from_string(@$filters[15])->date() if @$filters[15];
>+    @$filters[16] = dt_from_string(@$filters[16])->date() if @$filters[16];
> 
>     my @linefilter;
>     $linefilter[0] = @$filters[0] if ( $line =~ /items\.itemcallnumber/ );
>@@ -427,84 +429,101 @@ sub calculate {
>         LEFT JOIN biblioitems ON ($itemstable.biblioitemnumber = biblioitems.biblioitemnumber)
>         LEFT JOIN biblio ON (biblioitems.biblionumber = biblio.biblionumber)
>         WHERE 1 ";
>-    $strcalc .= "AND barcode $not like ? " if ($barcodefilter);
>+
>+    my @sqlargs;
>+
>+    if ($barcodefilter) {
>+        $strcalc .= "AND barcode $not like ? ";
>+        push @sqlargs, $barcodefilter;
>+    }
> 
>     if ( @$filters[0] ) {
>+        $strcalc .= " AND $itemstable.itemcallnumber >= ? ";
>         @$filters[0] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.itemcallnumber >=" . $dbh->quote( @$filters[0] );
>+        push @sqlargs, @$filters[0];
>     }
> 
>     if ( @$filters[1] ) {
>+        $strcalc .= " AND $itemstable.itemcallnumber <= ? ";
>         @$filters[1] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.itemcallnumber <=" . $dbh->quote( @$filters[1] );
>+        push @sqlargs, @$filters[1];
>     }
> 
>     if ( @$filters[2] ) {
>+        $strcalc .= " AND " . ( C4::Context->preference('item-level_itypes') ? "$itemstable.itype" : 'biblioitems.itemtype' ) . " LIKE ? ";
>         @$filters[2] =~ s/\*/%/g;
>-        $strcalc .= " AND " . ( C4::Context->preference('item-level_itypes') ? "$itemstable.itype" : 'biblioitems.itemtype' ) . " LIKE '" . @$filters[2] . "'";
>+        push @sqlargs, @$filters[2];
>     }
> 
>     if ( @$filters[3] ) {
>+        $strcalc .= " AND biblioitems.publishercode LIKE ? ";
>         @$filters[3] =~ s/\*/%/g;
>         @$filters[3] .= "%" unless @$filters[3] =~ /%/;
>-        $strcalc .= " AND biblioitems.publishercode LIKE \"" . @$filters[3] . "\"";
>+        push @sqlargs, @$filters[3];
>     }
>     if ( @$filters[4] ) {
>-        @$filters[4] =~ s/\*/%/g;
>         $strcalc .= " AND " .
>         (C4::Context->preference('marcflavour') eq 'UNIMARC' ? 'publicationyear' : 'copyrightdate')
>-        . ">" . @$filters[4];
>+        . "> ? ";
>+        @$filters[4] =~ s/\*/%/g;
>+        push @sqlargs, @$filters[4];
>     }
>     if ( @$filters[5] ) {
>         @$filters[5] =~ s/\*/%/g;
>         $strcalc .= " AND " .
>         (C4::Context->preference('marcflavour') eq 'UNIMARC' ? 'publicationyear' : 'copyrightdate')
>-        . "<" . @$filters[5];
>+        . "< ? ";
>+        push @sqlargs, @$filters[5];
>     }
>     if ( @$filters[6] ) {
>+        $strcalc .= " AND $itemstable.homebranch LIKE ? ";
>         @$filters[6] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.homebranch LIKE '" . @$filters[6] . "'";
>+        push @sqlargs, @$filters[6];
>     }
>     if ( @$filters[7] ) {
>+        $strcalc .= " AND $itemstable.location LIKE ? ";
>         @$filters[7] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.location LIKE '" . @$filters[7] . "'";
>+        push @sqlargs, @$filters[7];
>     }
>     if ( @$filters[8] ) {
>+        $strcalc .= " AND $itemstable.ccode  LIKE ? ";
>         @$filters[8] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.ccode  LIKE '" . @$filters[8] . "'";
>+        push @sqlargs, @$filters[8];
>     }
>     if ( defined @$filters[9] and @$filters[9] ne '' ) {
>+        $strcalc .= " AND $itemstable.notforloan  LIKE ? ";
>         @$filters[9] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.notforloan  LIKE '" . @$filters[9] . "'";
>+        push @sqlargs, @$filters[9];
>     }
>     if ( defined @$filters[10] and @$filters[10] ne '' ) {
>+        $strcalc .= " AND $itemstable.materials  LIKE ? ";
>         @$filters[10] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.materials  LIKE '" . @$filters[10] . "'";
>+        push @sqlargs, @$filters[10];
>     }
>     if ( @$filters[13] ) {
>+        $strcalc .= " AND $itemstable.dateaccessioned >= ? ";
>         @$filters[13] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.dateaccessioned >= '@$filters[13]' ";
>+        push @sqlargs, @$filters[13];
>     }
>     if ( @$filters[14] ) {
>+        $strcalc .= " AND $itemstable.dateaccessioned <= ? ";
>         @$filters[14] =~ s/\*/%/g;
>-        $strcalc .= " AND $itemstable.dateaccessioned <= '@$filters[14]' ";
>+        push @sqlargs, @$filters[14];
>     }
>     if ( $cellvalue eq 'deleteditems' and @$filters[15] ) {
>+        $strcalc .= " AND DATE(deleteditems.timestamp) >= ? ";
>         @$filters[15] =~ s/\*/%/g;
>-        $strcalc .= " AND DATE(deleteditems.timestamp) >= '@$filters[15]' ";
>+        push @sqlargs, @$filters[15];
>     }
>     if ( $cellvalue eq 'deleteditems' and @$filters[16] ) {
>         @$filters[16] =~ s/\*/%/g;
>-        $strcalc .= " AND DATE(deleteditems.timestamp) <= '@$filters[16]' ";
>+        $strcalc .= " AND DATE(deleteditems.timestamp) <= ?";
>+        push @sqlargs, @$filters[16];
>     }
>     $strcalc .= " group by $linefield, $colfield order by $linefield,$colfield";
>     $debug and warn "SQL: $strcalc";
>     my $dbcalc = $dbh->prepare($strcalc);
>-    if ($barcodefilter) {
>-        $dbcalc->execute($barcodefilter);
>-    } else {
>-        $dbcalc->execute();
>-    }
>+    $dbcalc->execute(@sqlargs);
> 
>     while ( my ( $row, $col, $value ) = $dbcalc->fetchrow ) {
> 
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 7683: 12762 | 12785 | 12940 | 14399 | 14466 | 16327 | 16328 | 16329 | 16367 | 16775 | 20241 | 20242 | 20243 | 20244 | 20245 | 21357 | 21358 | 23683 | 23684 | 23685 | 23686 | 23687 | 23688 | 23689 | 23690 | 23691 | 25072 | 25073 | 25091 | 25092 | 25093 | 25094 | 25095 | 25096 | 25097 | 25098 | 25099 | 25100 | 26678 | 26679 | 26680 | 26681 | 26682 | 28466 | 28467 | 28468 | 28469 | 28470 | 36489 | 36490 | 36491 | 36492 | 36493 | 39129 | 39130 | 39131 | 39132 | 39133 | 41821 | 41829 | 44651 | 44652 | 44653 | 44654 | 44655 | 45238 | 45239 | 45240 | 45241 | 45242