From 20fb798f51ed707bcd4fb7b882313f6cfaac5123 Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Thu, 8 Apr 2010 12:46:06 +0100
Subject: [PATCH] Remove unused or unnecessary variables in claims processing

Also cleaned the interface to the claims related functions
in C4::Serials so they do not return an extra count variable
moved generation of dropdown to template instead of inline code
---
 C4/Serials.pm                                 | 40 ++++++-------------
 .../prog/en/modules/serials/claims.tmpl       | 10 ++---
 serials/claims.pl                             | 40 +++++++++----------
 serials/lateissues-excel.pl                   | 13 +++---
 4 files changed, 43 insertions(+), 60 deletions(-)

diff --git a/C4/Serials.pm b/C4/Serials.pm
index ad6f1e711e..c9b8246308 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -84,9 +84,8 @@ Give all XYZ functions
 this function get all suppliers with late issues.
 
 return :
-the supplierlist into a hash. this hash containts id & name of the supplier
-Only valid suppliers are returned. Late subscriptions lacking a supplier are
-ignored.
+an array_ref of suppliers each entry is a hash_ref containing id and name
+the array is in name order
 
 =back
 
@@ -103,14 +102,7 @@ sub GetSuppliersWithLateIssues {
         AND             (planneddate < now() OR serial.STATUS = 3 OR serial.STATUS = 4)
         ORDER BY name
     |;
-    my $sth = $dbh->prepare($query);
-    $sth->execute;
-    my %supplierlist;
-    while ( my ( $id, $name ) = $sth->fetchrow ) {
-        next if !defined $id;
-        $supplierlist{$id} = $name;
-    }
-    return %supplierlist;
+    return $dbh->selectall_arrayref($query, { Slice => {} });
 }
 
 =head2 GetLateIssues
@@ -161,16 +153,14 @@ sub GetLateIssues {
     my @issuelist;
     my $last_title;
     my $odd   = 0;
-    my $count = 0;
     while ( my $line = $sth->fetchrow_hashref ) {
         $odd++ unless $line->{title} eq $last_title;
         $line->{title} = "" if $line->{title} eq $last_title;
         $last_title = $line->{title} if ( $line->{title} );
         $line->{planneddate} = format_date( $line->{planneddate} );
-        $count++;
         push @issuelist, $line;
     }
-    return $count, @issuelist;
+    return @issuelist;
 }
 
 =head2 GetSubscriptionHistoryFromSubscriptionId
@@ -1845,12 +1835,11 @@ sub DelIssue {
 
 =over 4
 
-($count,@issuelist) = &GetLateMissingIssues($supplierid,$serialid)
+@issuelist = &GetLateMissingIssues($supplierid,$serialid)
 
 this function select missing issues on database - where serial.status = 4 or serial.status=3 or planneddate<now
 
 return :
-a count of the number of missing issues
 the issuelist into a table. Each line of this table containts a ref to a hash which it containts
 name,title,planneddate,serialseq,serial.subscriptionid from tables : subscription, serial & biblio
 
@@ -1905,20 +1894,17 @@ sub GetLateOrMissingIssues {
     }
     $sth->execute;
     my @issuelist;
-    my $last_title;
-    my $odd   = 0;
-    my $count = 0;
     while ( my $line = $sth->fetchrow_hashref ) {
-        $odd++ unless $line->{title} eq $last_title;
-        $last_title = $line->{title} if ( $line->{title} );
-        $line->{planneddate}                  = format_date( $line->{planneddate} );
-        $line->{claimdate}                    = format_date( $line->{claimdate} );
-        $line->{ "status" . $line->{status} } = 1;
-        $line->{'odd'} = 1 if $odd % 2;
-        $count++;
+        if ($line->{planneddate}) {
+            $line->{planneddate} = format_date( $line->{planneddate} );
+        }
+        if ($line->{claimdate}) {
+            $line->{claimdate}   = format_date( $line->{claimdate} );
+        }
+        $line->{"status".$line->{status}}   = 1;
         push @issuelist, $line;
     }
-    return $count, @issuelist;
+    return @issuelist;
 }
 
 =head2 removeMissingIssue
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tmpl
index 16511acb1e..ae09e48c74 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tmpl
@@ -167,10 +167,10 @@
     <form id="claims" name="claims" action="claims.pl" method="post">
     <fieldset><label for="supplierid">View: </label>
         <select id="supplierid" size="1" name="supplierid">
-        <!-- TMPL_LOOP name="supplier_loop" -->
-            <option value="<!--TMPL_VAR name="id"-->" <!-- TMPL_IF NAME="selected" -->selected="1"<!-- /TMPL_IF -->>
-                 <!--TMPL_VAR name="name"--></option>
-        <!--/TMPL_LOOP-->
+        <!-- TMPL_LOOP NAME="supplier_loop" -->
+          <option value="<!-- TMPL_VAR NAME="id" -->"<!--  TMPL_IF NAME="selected" --> selected="yes" <!-- /TMPL_IF -->><!-- TMPL_VAR NAME="name" -->       
+          </option>
+        <!-- /TMPL_LOOP -->
         </select>
         <input type="submit" value="OK" />
         <!-- TMPL_IF name="phone" -->Phone: <!-- TMPL_VAR name="phone" --><!-- /TMPL_IF -->
@@ -306,9 +306,9 @@
 			</select>
 	    <input type="hidden" name="op" value="send_alert" /><input type="submit" name="submit" class="button" value="Send notification" /></fieldset>
             <!--/TMPL_IF-->
+        </form>
 <!-- /TMPL_IF -->
 
-        </form>
 <!-- TMPL_ELSE -->
 
 <div id="doc" class="yui-t7">
diff --git a/serials/claims.pl b/serials/claims.pl
index 8ff672f5d6..5b73d8bd3a 100755
--- a/serials/claims.pl
+++ b/serials/claims.pl
@@ -18,7 +18,7 @@ my $claimletter = $input->param('claimletter');
 my $supplierid = $input->param('supplierid');
 my $suppliername = $input->param('suppliername');
 my $order = $input->param('order');
-my %supplierlist = GetSuppliersWithLateIssues();
+my $supplierlist = GetSuppliersWithLateIssues;
 
 # open template first (security & userenv set here)
 my ($template, $loggedinuser, $cookie)
@@ -29,14 +29,14 @@ my ($template, $loggedinuser, $cookie)
             flagsrequired => {serials => 1},
             debug => 1,
             });
-my $supplier_loop = [];
-foreach my $s_id (sort {$supplierlist{$a} cmp $supplierlist{$b} } keys %supplierlist){
-        my ($count) = GetLateOrMissingIssues($s_id,q{},$order);
-        push @{$supplier_loop}, {
-            id   => $s_id,
-            name => $supplierlist{$s_id} . "($count)",
-            selected => ( $supplierid && $supplierid == $s_id ),
-        };
+
+for my $supplier ( @{$supplierlist} ) {
+        my @dummy = GetLateOrMissingIssues($supplier->{id},q{},$order);
+        my $counting = scalar @dummy;
+        $supplier->{name} .= " ($counting)";
+        if ($supplierid && $supplierid == $supplier->{id}) {
+            $supplier->{selected} = 1;
+        }
 }
 
 my $letters = GetLetters('claimissues');
@@ -46,9 +46,9 @@ foreach (keys %{$letters}){
 }
 
 my $letter=((scalar(@letters)>1) || ($letters[0]->{name}||$letters[0]->{code}));
-my ($count2, @missingissues);
+my  @missingissues;
 if ($supplierid) {
-    ($count2, @missingissues) = GetLateOrMissingIssues($supplierid,$serialid,$order);
+    @missingissues = GetLateOrMissingIssues($supplierid,$serialid,$order);
 }
 
 my ($singlesupplier,@supplierinfo);
@@ -63,20 +63,18 @@ if($supplierid){
 my $preview=0;
 if($op && $op eq 'preview'){
     $preview = 1;
+} else {
+    my @serialnums=$input->param('serialid');
+    if (@serialnums) { # i.e. they have been flagged to generate claims
+        SendAlerts('claimissues',\@serialnums,$input->param("letter_code"));
+        my $cntupdate=UpdateClaimdateIssues(\@serialnums);
+        ### $cntupdate SHOULD be equal to scalar(@$serialnums)
+    }
 }
-if ($op eq "send_alert"){
-  my @serialnums=$input->param("serialid");
-  SendAlerts('claimissues',\@serialnums,$input->param("letter_code"));
-  my $cntupdate=UpdateClaimdateIssues(\@serialnums);
-  ### $cntupdate SHOULD be equal to scalar(@$serialnums)
-  $template->param('SHOWCONFIRMATION' => 1);
-  $template->param('suppliername' => $suppliername);
-}
-
 $template->param('letters'=>\@letters,'letter'=>$letter);
 $template->param(
         order =>$order,
-        supplier_loop => $supplier_loop,
+        supplier_loop => $supplierlist,
         phone => $supplierinfo[0]->{phone},
         booksellerfax => $supplierinfo[0]->{booksellerfax},
         bookselleremail => $supplierinfo[0]->{bookselleremail},
diff --git a/serials/lateissues-excel.pl b/serials/lateissues-excel.pl
index ea0f503311..47ce51a478 100755
--- a/serials/lateissues-excel.pl
+++ b/serials/lateissues-excel.pl
@@ -32,12 +32,10 @@ my @serialid = $query->param('serialid');
 my $op = $query->param('op') || q{};
 my $serialidcount = @serialid;
 
-my %supplierlist = GetSuppliersWithLateIssues;
-
 my @loop1;
-my ($count, @lateissues);
+my @lateissues;
 if($op ne 'claims'){
-    ($count, @lateissues) = GetLateIssues($supplierid);
+    @lateissues = GetLateIssues($supplierid);
     for my $issue (@lateissues){
         push @loop1,
       [ $issue->{'name'}, $issue->{'title'}, $issue->{'serialseq'}, $issue->{'planneddate'},];
@@ -45,9 +43,9 @@ if($op ne 'claims'){
 }
 my $totalcount2 = 0;
 my @loop2;
-my ($count2, @missingissues);
+my @missingissues;
 for (my $k=0;$k<@serialid;$k++){
-    ($count2, @missingissues) = GetLateOrMissingIssues($supplierid, $serialid[$k]);
+    @missingissues = GetLateOrMissingIssues($supplierid, $serialid[$k]);
 
     for (my $j=0;$j<@missingissues;$j++){
 	my @rows2 = ($missingissues[$j]->{'name'},          # lets build up a row
@@ -57,7 +55,7 @@ for (my $k=0;$k<@serialid;$k++){
                      );
         push (@loop2, \@rows2);
     }
-    $totalcount2 = $totalcount2 + $count2;
+    $totalcount2 += scalar @missingissues;
     # update claim date to let one know they have looked at this missing item
     updateClaim($serialid[$k]);
 }
@@ -107,6 +105,7 @@ for my $row ( @loop2 ) {
 print ",,,,,,,\n";
 print ",,,,,,,\n";
 if($op ne 'claims'){
+    my $count = scalar @lateissues;
     print ",,Total Number Late, $count\n";
 }
 if($serialidcount == 1){
-- 
2.39.5