From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Tue, 16 Jul 2013 12:32:55 +0000 (-0400)
Subject: Bug 10558 [QA Follow-up]
X-Git-Tag: v3.16.00-beta~677
X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=commitdiff_plain;h=0713f3bf032b92535e87595e95a44898fb70c72b;p=koha_fer

Bug 10558 [QA Follow-up]

This patch addresses a number of issues with the main patch:

- regression on bug 2060 (i.e., displaying authority import batches
  correctly)
- regression on bug 10170 (translation of import record states)A
- use of datatables.inc
- lack of clarity as to the licensing of tools/batch_records_ajax.pl
- insufficent sanitizing of input used to generate an SQL statement

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
---

diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm
index f320c7a0b4..6ddab77fdd 100644
--- a/C4/ImportBatch.pm
+++ b/C4/ImportBatch.pm
@@ -1032,8 +1032,10 @@ sub GetImportRecordsRange {
 
     my $dbh = C4::Context->dbh;
 
-    my $order_by =
-      $dbh->quote_identifier( $parameters->{order_by} || 'import_record_id' );
+    my $order_by = $parameters->{order_by} || 'import_record_id';
+    ( $order_by ) = grep( /^$order_by$/, qw( import_record_id title status overlay_status ) ) ? $order_by : 'import_record_id';
+    $order_by .= ",authorized_heading" if $order_by eq 'title';
+
     my $order_by_direction =
       uc( $parameters->{order_by_direction} ) eq 'DESC' ? 'DESC' : 'ASC';
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt
index 1a2680343d..40f916d984 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt
@@ -7,10 +7,8 @@
 [% INCLUDE 'doc-head-close.inc' %]
 <script type="text/javascript" src="[% themelang %]/js/background-job-progressbar.js"></script>
 <link rel="stylesheet" type="text/css" href="[% themelang %]/css/datatables.css" />
-<script type="text/javascript" src="[% themelang %]/lib/jquery/plugins/jquery.dataTables.min.js"></script>
+[% INCLUDE 'datatables.inc' %]
 <script type="text/javascript" src="[% themelang %]/lib/jquery/plugins/jquery.dataTables.columnFilter.js"></script>
-[% INCLUDE 'datatables-strings.inc' %]
-<script type="text/javascript" src="[% themelang %]/js/datatables.js"></script>
 <script type="text/JavaScript" language="JavaScript">
 //<![CDATA[
 var MSG_CONFIRM_CLEAN = _("Clear all reservoir records staged in this batch?  This cannot be undone.");
@@ -27,55 +25,85 @@ $(document).ready(function(){
       $(this).parent().hide();
   });
 
-  $("#records-table").dataTable({
-      "aLengthMenu": [[10, 15, 20, 25, 50, 100], [10, 15, 20, 25, 50, 100]],
-      "iDisplayLength" : 20,
-      "bAutoWidth": false,
-      "bFilter": false,
-      "bProcessing": true,
-      "bServerSide": true,
-      "sAjaxSource": 'batch_records_ajax.pl',
-      "sPaginationType": "full_numbers",
-      "sDom": '<"top pager"iflp>rt<"bottom pager"flp><"clear">',
-      "aoColumns": [
-          { "mDataProp": "import_record_id" },
-          { "mDataProp": "citation" },
-          { "mDataProp": "status" },
-          { "mDataProp": "overlay_status" },
-          { "mDataProp": "match_citation" },
-          { "mDataProp": "matched" },
-      ],
-      "fnServerData": function ( sSource, aoData, fnCallback ) {
-          aoData.push( { "name": "import_batch_id", "value": [% import_batch_id %] } );
+  [% IF import_batch_id %]
+      $("#records-table").dataTable({
+          "aLengthMenu": [[10, 15, 20, 25, 50, 100, -1], [10, 15, 20, 25, 50, 100, _("All")]],
+          "iDisplayLength" : 20,
+          "bAutoWidth": false,
+          "bFilter": false,
+          "bProcessing": true,
+          "bServerSide": true,
+          "sAjaxSource": 'batch_records_ajax.pl',
+          "sPaginationType": "full_numbers",
+          "sDom": '<"top pager"iflp>rt<"bottom pager"flp><"clear">',
+          "aoColumns": [
+              { "mDataProp": "import_record_id" },
+              { "mDataProp": "citation" },
+              { "mDataProp": "status" },
+              { "mDataProp": "overlay_status" },
+              { "mDataProp": "match_citation" },
+              { "mDataProp": "matched" },
+          ],
+          "fnServerData": function ( sSource, aoData, fnCallback ) {
+              aoData.push( { "name": "import_batch_id", "value": [% import_batch_id %] } );
 
-          $.getJSON( sSource, aoData, function (json) {
-              fnCallback(json)
-          } );
-      },
-      "fnRowCallback": function( nRow, aData, iDisplayIndex, iDisplayIndexFull ) {
-        $('td:eq(1)', nRow).html(
-            '<a href="javascript:void()" onclick="show_marc('
-            + aData['import_record_id']
-            + ')">' + aData['citation'] + '</a>'
-        );
+              $.ajax({
+                  'dataType': 'json',
+                  'type': 'POST',
+                  'url': sSource,
+                  'data': aoData,
+                  'success': function(json){
+                      fnCallback(json);
+                  }
+              });
+          },
+          "fnRowCallback": function( nRow, aData, iDisplayIndex, iDisplayIndexFull ) {
+            [% IF ( record_type == 'auth' ) %]
+                var record_details_url = "/cgi-bin/koha/authorities/detail.pl?authid=";
+            [% ELSE %]
+                var record_details_url = "/cgi-bin/koha/catalogue/detail.pl?biblionumber=";
+            [% END %]
 
-        if ( aData['match_id'] ) {
-            $('td:eq(4)', nRow).html(
-                _("Matches biblio ")
-                + aData['match_id']
-                + " (" + _("score") + "="
-                + aData['score']
-                + '):' + '<a target="_blank" href="/cgi-bin/koha/catalogue/detail.pl?biblionumber='
-                + aData['match_id'] + '">' + aData['match_citation'] + '</a>'
+            $('td:eq(1)', nRow).html(
+                '<a href="javascript:void(0)" onclick="show_marc('
+                + aData['import_record_id']
+                + ')">' + aData['citation'] + '</a>'
             );
-        }
 
-        $('td:eq(5)', nRow).html(
-            '<a target="_blank" href="/cgi-bin/koha/catalogue/detail.pl?biblionumber='
-            + aData['matched'] + '">' + aData['matched'] + '</a>'
-        );
-      },
-  });
+            $('td:eq(2)', nRow).html(
+                aData['status'] == 'imported' ? _("Imported") :
+                aData['status'] == 'ignored'  ? _("Ignored")  :
+                aData['status'] == 'reverted' ? _("Reverted") :
+                aData['status'] == 'staged'   ? _("Staged")   :
+                aData['status'] == 'error'    ? _("Error")    :
+                aData['status']
+            );
+
+            $('td:eq(3)', nRow).html(
+                aData['overlay_status'] == 'no_match'      ? _("No match")       :
+                aData['overlay_status'] == 'match_applied' ? _("Match applied")  :
+                aData['overlay_status'] == 'auto_match'    ? _("Match found")    :
+                aData['overlay_status']
+            );
+
+            if ( aData['match_id'] ) {
+                $('td:eq(4)', nRow).html(
+                    _("Matches biblio ")
+                    + aData['match_id']
+                    + " (" + _("score") + "="
+                    + aData['score']
+                    + '):' + '<a target="_blank" href="' + record_details_url
+                    + aData['match_id'] + '">' + aData['match_citation'] + '</a>'
+                );
+            }
+
+            $('td:eq(5)', nRow).html(
+                '<a target="_blank" href="' + record_details_url
+                    + aData['matched'] + '">' + aData['matched'] + '</a>'
+            );
+          },
+      });
+    [% END %]
 });
 
 function show_marc( id ) {
@@ -412,18 +440,20 @@ Page
   [% END %]
 [% END %]
 
-<table id="records-table">
-    <thead>
-        <tr>
-            <th>#</th>
-            <th>Citation</th>
-            <th>Status</th>
-            <th>Match type</th>
-            <th>Match details</th>
-            <th>Record</th>
-        </tr>
-    </thead>
-</table>
+[% IF import_batch_id %]
+    <table id="records-table">
+        <thead>
+            <tr>
+                <th>#</th>
+                <th>Citation</th>
+                <th>Status</th>
+                <th>Match type</th>
+                <th>Match details</th>
+                <th>Record</th>
+            </tr>
+        </thead>
+    </table>
+[% END %]
 
 </div>
 </div>
diff --git a/tools/batch_records_ajax.pl b/tools/batch_records_ajax.pl
index f4a1542694..bc0fe8284f 100755
--- a/tools/batch_records_ajax.pl
+++ b/tools/batch_records_ajax.pl
@@ -1,14 +1,13 @@
 #!/usr/bin/perl
 
-# This software is placed under the gnu General Public License, v2 (http://www.gnu.org/licenses/gpl.html)
-
-# Copyright 2007 Tamil s.a.r.l.
+# Copyright 2013 ByWater Solutions
+# Based on circ/ysearch.pl: Copyright 2007 Tamil s.a.r.l.
 #
 # This file is part of Koha.
 #
 # Koha is free software; you can redistribute it and/or modify it under the
 # terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
+# Foundation; either version 3 of the License, or (at your option) any later
 # version.
 #
 # Koha is distributed in the hope that it will be useful, but WITHOUT ANY
@@ -19,8 +18,14 @@
 # with Koha; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-=head1 ysearch.pl
+=head1 NAME
+
+batch_records_ajax.pl - A script for searching batch imported records via ajax
 
+=head1 SYNOPSIS
+
+This script is to be used as a data source for DataTables that load and display
+the records from an import batch.
 
 =cut
 
@@ -45,11 +50,13 @@ my $results_per_page  = $input->param('iDisplayLength');
 my $sorting_column    = $sort_columns[ $input->param('iSortCol_0') ];
 my $sorting_direction = $input->param('sSortDir_0');
 
+$results_per_page = undef if ( $results_per_page == -1 );
+
 binmode STDOUT, ":encoding(UTF-8)";
 print $input->header( -type => 'text/plain', -charset => 'UTF-8' );
 
 my ( $auth_status, $sessionID ) =
-  check_cookie_auth( $input->cookie('CGISESSID'), { editcatalogue => '*' } );
+  check_cookie_auth( $input->cookie('CGISESSID'), { tools => 'manage_staged_marc' } );
 if ( $auth_status ne "ok" ) {
     exit 0;
 }
diff --git a/tools/manage-marc-import.pl b/tools/manage-marc-import.pl
index 86dc975b09..deaab7d1c7 100755
--- a/tools/manage-marc-import.pl
+++ b/tools/manage-marc-import.pl
@@ -352,65 +352,20 @@ sub import_records_list {
     my ($template, $import_batch_id, $offset, $results_per_page) = @_;
 
     my $batch = GetImportBatch($import_batch_id);
-#    my $records = GetImportRecordsRange($import_batch_id);
-#    my @list = ();
-#    foreach my $record (@$records) {
-#        my $citation = $record->{'title'} || $record->{'authorized_heading'};
-#        $citation .= " $record->{'author'}" if $record->{'author'};
-#        $citation .= " (" if $record->{'issn'} or $record->{'isbn'};
-#        $citation .= $record->{'isbn'} if $record->{'isbn'};
-#        $citation .= ", " if $record->{'issn'} and $record->{'isbn'};
-#        $citation .= $record->{'issn'} if $record->{'issn'};
-#        $citation .= ")" if $record->{'issn'} or $record->{'isbn'};
-#
-#        my $match = GetImportRecordMatches($record->{'import_record_id'}, 1);
-#        my $match_citation = '';
-#        my $match_id;
-#        if ($#$match > -1) {
-#            if ($match->[0]->{'record_type'} eq 'biblio') {
-#                $match_citation .= $match->[0]->{'title'} if defined($match->[0]->{'title'});
-#                $match_citation .= ' ' . $match->[0]->{'author'} if defined($match->[0]->{'author'});
-#                $match_id = $match->[0]->{'biblionumber'};
-#            } elsif ($match->[0]->{'record_type'} eq 'auth') {
-#                if (defined($match->[0]->{'authorized_heading'})) {
-#                    $match_citation .= $match->[0]->{'authorized_heading'};
-#                    $match_id = $match->[0]->{'candidate_match_id'};
-#                }
-#            }
-#        }
-#
-#        push @list,
-#          { import_record_id         => $record->{'import_record_id'},
-#            final_match_id           => $record->{'matched_biblionumber'} || $record->{'matched_authid'},
-#            citation                 => $citation,
-#            status                   => $record->{'status'},
-#            record_sequence          => $record->{'record_sequence'},
-#            overlay_status           => $record->{'overlay_status'},
-#            # Sorry about the match_id being from the "biblionumber" field;
-#            # as it turns out, any match id will go in biblionumber
-#            match_id                 => $match_id,
-#            match_citation           => $match_citation,
-#            match_score              => $#$match > -1 ? $match->[0]->{'score'} : 0,
-#            record_type              => $record->{'record_type'},
-#          };
-#    }
-#    my $num_records = $batch->{'num_records'};
-#    $template->param(record_list => \@list);
-#    add_page_numbers($template, $offset, $results_per_page, $num_records);
-#    $template->param(offset => $offset);
-#    $template->param(range_top => $offset + $results_per_page - 1);
-#    $template->param(num_results => $num_records);
-#    $template->param(results_per_page => $results_per_page);
     $template->param(import_batch_id => $import_batch_id);
+
     my $overlay_action = GetImportBatchOverlayAction($import_batch_id);
     $template->param("overlay_action_${overlay_action}" => 1);
     $template->param(overlay_action => $overlay_action);
+
     my $nomatch_action = GetImportBatchNoMatchAction($import_batch_id);
     $template->param("nomatch_action_${nomatch_action}" => 1);
     $template->param(nomatch_action => $nomatch_action);
+
     my $item_action = GetImportBatchItemAction($import_batch_id);
     $template->param("item_action_${item_action}" => 1);
     $template->param(item_action => $item_action);
+
     batch_info($template, $batch);
     
 }