Repair Labels code to accomodate patroncards. Purged EXPR.

Major FIXME's still remain, like the use of GET instead of POST.
The code is also a bit too INCLUDE-happy to net good performance.
The entire mechanism of adding to a batch should probably be proper
AJAX instead of the GET-centric opener.location approach.

Signed-off-by: Joshua Ferraro <jmf@liblime.com>

diff --git a/C4/Labels.pm b/C4/Labels.pm
index 8f7197b..e78df94 100644 (file)
--- a/C4/Labels.pm
+++ b/C4/Labels.pm
@@ -296,25 +296,23 @@ sub get_text_fields {
  else, return the next available batch_id.
 =return
 =cut
-sub add_batch {
-    my ( $batch_type,$batch_list ) = @_;
-    my $new_batch;
+sub add_batch ($;$) {
+       my $table = (@_ and 'patroncards' eq shift) ? 'patroncards' : 'labels';
+    my $batch_list = (@_) ? shift : undef;
     my $dbh = C4::Context->dbh;
-    my $q ="SELECT MAX(DISTINCT batch_id) FROM $batch_type";
+    my $q ="SELECT MAX(DISTINCT batch_id) FROM $table";
     my $sth = $dbh->prepare($q);
     $sth->execute();
-    my ($batch_id) = $sth->fetchrow_array;
-    $sth->finish;
-       if($batch_id) {
-               $batch_id++;
-       } else {
-               $batch_id = 1;
-       }
-       # TODO: let this block use $batch_type
-       if(ref($batch_list) && ($batch_type eq 'labels') ) {
-               my $sth = $dbh->prepare("INSERT INTO labels (`batch_id`,`itemnumber`) VALUES (?,?)"); 
-               for my $item (@$batch_list) {
-                       $sth->execute($batch_id,$item);
+    my ($batch_id) = $sth->fetchrow_array || 0;
+       $batch_id++;
+       if ($batch_list) {
+               if ($table eq 'patroncards') {
+                       $sth = $dbh->prepare("INSERT INTO $table (`batch_id`,`borrowernumber`) VALUES (?,?)"); 
+               } else {
+                       $sth = $dbh->prepare("INSERT INTO $table (`batch_id`,`itemnumber`    ) VALUES (?,?)"); 
+               }
+               for (@$batch_list) {
+                       $sth->execute($batch_id,$_);
                }
        }
        return $batch_id;
@@ -323,31 +321,19 @@ sub add_batch {
 #FIXME: Needs to be ported to receive $batch_type
 # ... this looks eerily like add_batch() ...
 sub get_highest_batch {
-    my $new_batch;
-    my $dbh = C4::Context->dbh;
+       my $table = (@_ and 'patroncards' eq shift) ? 'patroncards' : 'labels';
     my $q =
-      "select distinct batch_id from labels order by batch_id desc limit 1";
-    my $sth = $dbh->prepare($q);
+      "select distinct batch_id from $table order by batch_id desc limit 1";
+    my $sth = C4::Context->dbh->prepare($q);
     $sth->execute();
-    my $data = $sth->fetchrow_hashref;
-    $sth->finish;
-
-    if ( !$data->{'batch_id'} ) {
-        $new_batch = 1;
-    }
-    else {
-        $new_batch =  $data->{'batch_id'};
-    }
-
-    return $new_batch;
+    my $data = $sth->fetchrow_hashref or return 1;
+       return ($data->{'batch_id'} || 1);
 }
 
 
-sub get_batches {
-       # my $q   = "SELECT batch_id, COUNT(*) AS num FROM " . shift . " GROUP BY batch_id";
-    # FIXEDME:  There is only ONE table with batch_id, so why try to select a different one?
-       # get_batches() was frequently being called w/ no args, crashing DBD
-    my $q   = "SELECT batch_id, COUNT(*) AS num FROM labels GROUP BY batch_id";
+sub get_batches (;$) {
+       my $table = (@_ and 'patroncards' eq shift) ? 'patroncards' : 'labels';
+    my $q   = "SELECT batch_id, COUNT(*) AS num FROM $table GROUP BY batch_id";
     my $sth = C4::Context->dbh->prepare($q);
     $sth->execute();
        my $batches = $sth->fetchall_arrayref({});

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/label-status.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/label-status.inc
index b4b86ca..3202ad4 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/label-status.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/label-status.inc
@@ -3,6 +3,6 @@
 <table>
 <tr><th>Layout:</th><td><!-- TMPL_IF NAME="active_layout_name" --><!-- TMPL_VAR NAME="active_layout_name" --><!-- TMPL_ELSE --><span class="error">No Layout Specified: <a href="/cgi-bin/koha/labels/label-home.pl">Select a Label Layout</a></span><!-- /TMPL_IF --> </td></tr>
 <tr><th>Template: </th><td><!-- TMPL_IF NAME="active_template_name" --><!-- TMPL_VAR NAME="active_template_name" --><!-- TMPL_ELSE --><span class="error">No Template Specified: <a href="/cgi-bin/koha/labels/label-templates.pl">Select a Label Template</a></span><!-- /TMPL_IF --> </td></tr>
-<tr><th>Batch: </th><td><!-- TMPL_IF NAME="batch_id" --><!-- TMPL_VAR NAME="batch_id" --><!-- TMPL_ELSE --><span class="error"><a href="/cgi-bin/koha/labels/label-manager.pl?op=add_batch&amp;type=<!-- TMPL_VAR NAME="type" -->">Create a new batch</a></span><!-- /TMPL_IF --> </td></tr>
+<tr><th>Batch: </th><td><!-- TMPL_IF NAME="batch_id" --><!-- TMPL_VAR NAME="batch_id" --><!-- TMPL_ELSE --><span class="error"><a href="/cgi-bin/koha/labels/label-manager.pl?op=add_batch&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Create a new batch</a></span><!-- /TMPL_IF --> </td></tr>
 </table>
 </div>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/tools-labels-batches-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/tools-labels-batches-toolbar.inc
index 6b4fa3c..394d5a9 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/tools-labels-batches-toolbar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/tools-labels-batches-toolbar.inc
@@ -1,4 +1,4 @@
-<!-- TMPL_IF EXPR="(type eq 'labels')" -->
+<!-- TMPL_IF NAME="batch_is_labels" -->
 <div id="toolbar">
        <script type="text/javascript">
        //<![CDATA[
@@ -24,7 +24,7 @@ return false;
                          href: "#",
               label: _("Add item(s) to batch"), 
               container: "additemsc",
-onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR NAME="type" -->")}}
+onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR NAME="batch_type" -->")}}
           });
                new YAHOO.widget.Button("deletebatch");
                new YAHOO.widget.Button("dedup");
@@ -35,12 +35,12 @@ onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR
        </script>
        <ul class="toolbar">
        <li id="additemsc"><a id="additems" href="#">Add item(s) to batch</a></li>
-       <li><a id="deletebatch" href="/cgi-bin/koha/labels/label-manager.pl?op=delete_batch&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Delete current batch</a></li>
+       <li><a id="deletebatch" href="/cgi-bin/koha/labels/label-manager.pl?op=delete_batch&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Delete current batch</a></li>
                                <!-- FIXME: should use POST to change server state, not GET -->
-       <li><a id="dedup" href="/cgi-bin/koha/labels/label-manager.pl?op=deduplicate&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Remove duplicate barcodes</a></li>
-       <li><a id="generate" href="/cgi-bin/koha/labels/label-print-pdf.pl?batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Generate PDF for Batch</a></li>
+       <li><a id="dedup" href="/cgi-bin/koha/labels/label-manager.pl?op=deduplicate&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Remove duplicates</a></li>
+       <li><a id="generate" href="/cgi-bin/koha/labels/label-print-pdf.pl?batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Generate PDF for Batch</a></li>
 </ul></div>
-<!-- TMPL_ELSIF EXPR="(type eq 'patroncards')" -->
+<!-- TMPL_ELSIF NAME="batch_is_patroncards" -->
 <div id="toolbar">
        <script type="text/javascript">
        //<![CDATA[
@@ -66,7 +66,7 @@ onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR
             href: "#",
             label: _("Add patron(s) to batch"), 
             container: "addpatronsc",
-onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR NAME="type" -->"); }}
+onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR NAME="batch_type" -->"); }}
         });
                new YAHOO.widget.Button("deletebatch");
                new YAHOO.widget.Button("dedup");
@@ -76,10 +76,10 @@ onclick: {fn:function(){Plugin(<!-- TMPL_VAR NAME="batch_id" -->,"<!-- TMPL_VAR
        //]]>
        </script>
        <ul class="toolbar">
-       <li id="addpatronsc"><a id="addpatrons" href="#">Add item(s) to batch</a></li>
-       <li><a id="deletebatch" href="/cgi-bin/koha/labels/label-manager.pl?op=delete_batch&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Delete current batch</a></li>
+       <li id="addpatronsc"><a id="addpatrons" href="#">Add patron(s) to batch</a></li>
+       <li><a id="deletebatch" href="/cgi-bin/koha/labels/label-manager.pl?op=delete_batch&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Delete current batch</a></li>
                                <!-- FIXME: should use POST to change server state, not GET -->
-       <li><a id="dedup" href="/cgi-bin/koha/labels/label-manager.pl?op=deduplicate&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Remove duplicate patrons</a></li>
-       <li><a id="generate" href="/cgi-bin/koha/labels/label-print-pdf.pl?batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="type" -->">Generate PDF for Batch</a></li>
+       <li><a id="dedup" href="/cgi-bin/koha/labels/label-manager.pl?op=deduplicate&amp;batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Remove duplicates</a></li>
+       <li><a id="generate" href="/cgi-bin/koha/labels/label-print-pdf.pl?batch_id=<!-- TMPL_VAR NAME="batch_id" -->&amp;type=<!-- TMPL_VAR NAME="batch_type" -->">Generate PDF for Batch</a></li>
 </ul></div>
 <!-- /TMPL_IF -->

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-manager.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-manager.tmpl
index c52fdac..056a639 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-manager.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-manager.tmpl
@@ -1,11 +1,11 @@
-<!-- TMPL_INCLUDE NAME="doc-head-open.inc" --><title>Koha &rsaquo; Tools &rsaquo; Labels &rsaquo; <!-- TMPL_IF EXPR="(type eq 'labels')" -->Label Batch<!-- TMPL_ELSIF EXPR="(type eq 'patroncards')" -->Patron Card Batch<!-- /TMPL_IF --></title>
+<!-- TMPL_INCLUDE NAME="doc-head-open.inc" --><title>Koha &rsaquo; Tools &rsaquo; Labels &rsaquo; <!-- TMPL_IF NAME="batch_is_labels" -->Label<!-- TMPL_ELSIF NAME="batch_is_patroncards" -->Patron Card<!-- TMPL_ELSE -->Unknown Batchtype<!-- /TMPL_IF --> Batch</title>
 <!-- TMPL_INCLUDE NAME="doc-head-close.inc" -->
 </head>
 <body>
 <!-- TMPL_INCLUDE NAME="header.inc" -->
 <!-- TMPL_INCLUDE NAME="cat-search.inc" -->
 
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo; <a href="/cgi-bin/koha/labels/label-home.pl">Labels</a> &rsaquo; <!-- TMPL_IF EXPR="(type eq 'labels')" -->Label Batch<!-- TMPL_ELSIF EXPR="(type eq 'patroncards')" -->Patron Card Batch<!-- /TMPL_IF --></div>
+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo; <a href="/cgi-bin/koha/labels/label-home.pl">Labels</a> &rsaquo; <!-- TMPL_IF NAME="batch_is_labels" -->Label<!-- TMPL_ELSIF NAME="batch_is_patroncards" -->Patron Card<!-- TMPL_ELSE -->Unknown Batchtype<!-- /TMPL_IF --> Batch</div>
  <div id="doc3" class="yui-t2">
   <div id="bd">
    <div id="yui-main">
@@ -19,9 +19,9 @@
 
 <!-- TMPL_IF NAME="batch_id" -->
 <!-- TMPL_INCLUDE NAME="tools-labels-batches-toolbar.inc" -->
-<!-- TMPL_IF EXPR="(type eq 'labels')" -->
 <div class="yui-g">
     <div class="yui-u first">
+<!-- TMPL_IF NAME="batch_is_labels" -->
     <h2>Items to be Printed for Batch <!-- TMPL_VAR NAME="batch_id" -->  (<!-- TMPL_VAR NAME="batch_count" -->  items)</h2>
     <table>
         <tr>
@@ -42,10 +42,7 @@
         </tr>
         <!-- /TMPL_LOOP -->
     </table>
-    </div>
-<!-- TMPL_ELSIF EXPR="(type eq 'patroncards')" -->
-<div class="yui-g">
-    <div class="yui-u first">
+<!-- TMPL_ELSIF NAME="batch_is_patroncards" -->
     <h2>Patron Cards to be Printed for Batch <!-- TMPL_VAR NAME="batch_id" -->  (<!-- TMPL_VAR NAME="batch_count" -->  items)</h2>
     <table>
         <tr>
@@ -66,18 +63,20 @@
         </tr>
         <!-- /TMPL_LOOP -->
     </table>
+<!-- TMPL_ELSE -->
+       <span class="error">Error: Unknown Batch Type &quot;<!-- TMPL_VAR NAME="batch_type" -->&quot;
+<!-- /TMPL_IF -->
     </div>
-<!-- /TMPL_IF --><!-- /type -->
     <div class="yui-u">
         <!-- TMPL_INCLUDE NAME="label-status.inc" -->
     </div>
 </div>
 <!-- TMPL_ELSE -->
 <!-- TMPL_INCLUDE NAME="tools-labels-toolbar.inc" -->
-<!-- TMPL_IF EXPR="(type eq 'labels')" -->
-<!-- TMPL_IF NAME="batches" -->
+<!-- TMPL_IF NAME="batch_is_labels" -->
     <div class="yui-g">
         <div class="yui-u first">
+<!-- TMPL_IF NAME="batches" -->
             <h2>Label Batches</h2>
             <table>
                 <tr>
@@ -97,30 +96,23 @@
                 </tr>
                 <!-- /TMPL_LOOP -->
             </table>
-        </div>
-        <div class="yui-u">
-        <!-- TMPL_INCLUDE NAME="label-status.inc" -->
-        </div>
-    </div>
 <!-- TMPL_ELSE -->
-    <div class="yui-g">
-        <div class="yui-u first">
             <fieldset class="brief">
             <legend>No Label Batches Currently Defined</legend>
             <div class="hint">
                 Select "New Label Batch" to create a Label batch.
             </div>
             </fieldset>
+<!-- /TMPL_IF --><!-- /batches -->
         </div>
         <div class="yui-u">
         <!-- TMPL_INCLUDE NAME="label-status.inc" -->
         </div>
     </div>
-<!-- /TMPL_IF --><!-- /batches -->
-<!-- TMPL_ELSIF EXPR="(type eq 'patroncards')" -->
-<!-- TMPL_IF NAME="batches" -->
+<!-- TMPL_ELSIF NAME="batch_is_patroncards" -->
     <div class="yui-g">
         <div class="yui-u first">
+<!-- TMPL_IF NAME="batches" -->
             <h2>Patron Card Batches</h2>
             <table>
                 <tr>
@@ -140,26 +132,19 @@
                 </tr>
                 <!-- /TMPL_LOOP -->
             </table>
-        </div>
-        <div class="yui-u">
-            <!-- TMPL_INCLUDE NAME="label-status.inc" -->
-        </div>
-    </div>
 <!-- TMPL_ELSE -->
-    <div class="yui-g">
-        <div class="yui-u first">
             <fieldset class="brief">
             <legend>No Patron Card Batches Currently Defined</legend>
             <div class="hint">
                 Select "New Patron Card Batch" to create a Label batch.
             </div>
             </fieldset>
+<!-- /TMPL_IF --><!-- /batches -->
         </div>
         <div class="yui-u">
             <!-- TMPL_INCLUDE NAME="label-status.inc" -->
         </div>
     </div>
-<!-- /TMPL_IF --><!-- /batches -->
 <!-- /TMPL_IF --><!-- /type -->
 <!-- /TMPL_IF --><!-- batch_id -->
 </div>

diff --git a/labels/label-manager.pl b/labels/label-manager.pl
index eab45b0..64f48db 100755 (executable)
--- a/labels/label-manager.pl
+++ b/labels/label-manager.pl
@@ -41,7 +41,8 @@ my $printingtype   = $query->param('printingtype');
 my $guidebox       = $query->param('guidebox');
 my $fontsize       = $query->param('fontsize');
 my $formatstring   = $query->param('formatstring');
-my $batch_type     = $query->param('type') || 'labels';
+my $batch_type     = $query->param('type');
+($batch_type and $batch_type eq 'patroncards') or $batch_type = 'labels';
 my @itemnumber;
 ($batch_type eq 'labels') ? (@itemnumber = $query->param('itemnumber')) : (@itemnumber = $query->param('borrowernumber'));
 
@@ -112,19 +113,19 @@ elsif  ( $op eq 'add_layout' ) {
 # FIXME: The trinary conditionals here really need to be replaced with a more robust form of db abstraction -fbcit
 
 elsif ( $op eq 'add' ) {   # add item
-       my $query2 = "INSERT INTO labels (itemnumber, batch_id) values ( ?,? )";
+       my $query2 = ($batch_type eq 'patroncards') ? 
+               "INSERT INTO patroncards (borrowernumber, batch_id) values (?,?)" :
+               "INSERT INTO labels      (itemnumber,     batch_id) values (?,?)" ;
        my $sth2   = $dbh->prepare($query2);
        for my $inum (@itemnumber) {
-            # warn "INSERTing " . (($batch_type eq 'labels') ? 'itemnumber' : 'borrowernumber') . ":$inum for batch $batch_id";
+               # warn "INSERTing " . (($batch_type eq 'labels') ? 'itemnumber' : 'borrowernumber') . ":$inum for batch $batch_id";
            $sth2->execute($inum, $batch_id);
        }
-       $sth2->finish;
 }
 elsif ( $op eq 'deleteall' ) {
        my $query2 = "DELETE FROM $batch_type";
        my $sth2   = $dbh->prepare($query2);
        $sth2->execute();
-       $sth2->finish;
 }
 elsif ( $op eq 'delete' ) {
        my @labelids = $query->param((($batch_type eq 'labels') ? 'labelid' : 'cardid'));
@@ -135,7 +136,6 @@ elsif ( $op eq 'delete' ) {
        $debug and push @messages, "query2: $query2 -- (@labelids)";
        my $sth2   = $dbh->prepare($query2);
        $sth2->execute(@labelids);
-       $sth2->finish;
 }
 elsif ( $op eq 'delete_batch' ) {
        delete_batch($batch_id, $batch_type);
@@ -180,8 +180,11 @@ if (scalar @messages) {
        }
        $template->param(message_loop => \@complex);
 }
+if ($batch_type eq 'labels' or $batch_type eq 'patroncards') {
+       $template->param("batch_is_$batch_type" => 1);
+}
 $template->param(
-    type                        => $batch_type,                # FIXME: type is an otherwise RESERVED template variable with 2 valid values: 'opac' and 'intranet'
+    batch_type                  => $batch_type,
     batch_id                    => $batch_id,
     batch_count                 => scalar @resultsloop,
     active_layout_name          => $active_layout_name,