[Patches] [PATCH] bug_5533: Improved marking items as lost

koha-patchbot at kohaaloha.com koha-patchbot at kohaaloha.com
Fri Nov 4 19:52:17 NZDT 2011


From: Srdjan Jankovic <srdjan at catalyst.net.nz>
Date: Thu, 20 Oct 2011 14:21:26 +1300
Subject: [PATCH] bug_5533: Improved marking items as lost

Call LostItem() whenever an item is lost
Disable lost status on catalogue item edit
---
 C4/Accounts.pm                             |   77 ++++++++-------------------
 C4/Circulation.pm                          |   40 ++++++++++++++-
 C4/Items.pm                                |    8 +++-
 catalogue/updateitem.pl                    |    3 +-
 cataloguing/additem.pl                     |   37 ++++++++-----
 misc/cronjobs/longoverdue.pl               |    3 +-
 t/db_dependent/lib/KohaTest/Accounts.pm    |    1 -
 t/db_dependent/lib/KohaTest/Circulation.pm |    1 +
 tools/batchMod.pl                          |   16 +++++-
 9 files changed, 106 insertions(+), 80 deletions(-)

diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index eea142c..70b3944 100644
--- a/C4/Accounts.pm
+++ b/C4/Accounts.pm
@@ -23,8 +23,7 @@ use strict;
 use C4::Context;
 use C4::Stats;
 use C4::Members;
-use C4::Items;
-use C4::Circulation qw(MarkIssueReturned);
+use C4::Circulation qw(ReturnLostItem);
 
 use vars qw($VERSION @ISA @EXPORT);
 
@@ -216,7 +215,7 @@ sub makepayment {
 
     #check to see what accounttype
     if ( $data->{'accounttype'} eq 'Rep' || $data->{'accounttype'} eq 'L' ) {
-        returnlost( $borrowernumber, $data->{'itemnumber'} );
+        ReturnLostItem( $borrowernumber, $data->{'itemnumber'} );
     }
 }
 
@@ -276,64 +275,34 @@ EOT
 
 =cut
 
-sub returnlost{
-    my ( $borrowernumber, $itemnum ) = @_;
-    C4::Circulation::MarkIssueReturned( $borrowernumber, $itemnum );
-    my $borrower = C4::Members::GetMember( 'borrowernumber'=>$borrowernumber );
-    my @datearr = localtime(time);
-    my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
-    my $bor = "$borrower->{'firstname'} $borrower->{'surname'} $borrower->{'cardnumber'}";
-    ModItem({ paidfor =>  "Paid for by $bor $date" }, undef, $itemnum);
-}
-
-
 sub chargelostitem{
 # lost ==1 Lost, lost==2 longoverdue, lost==3 lost and paid for
 # FIXME: itemlost should be set to 3 after payment is made, should be a warning to the interface that
 # a charge has been added
 # FIXME : if no replacement price, borrower just doesn't get charged?
-   
     my $dbh = C4::Context->dbh();
-    my ($itemnumber) = @_;
-    my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title 
-                           FROM issues 
-                           JOIN items USING (itemnumber) 
-                           JOIN biblio USING (biblionumber)
-                           WHERE issues.itemnumber=?");
-    $sth->execute($itemnumber);
-    my $issues=$sth->fetchrow_hashref();
-
-    # if a borrower lost the item, add a replacement cost to the their record
-    if ( $issues->{borrowernumber} ){
-
-        # first make sure the borrower hasn't already been charged for this item
-        my $sth1=$dbh->prepare("SELECT * from accountlines
-        WHERE borrowernumber=? AND itemnumber=? and accounttype='L'");
-        $sth1->execute($issues->{'borrowernumber'},$itemnumber);
-        my $existing_charge_hashref=$sth1->fetchrow_hashref();
-
-        # OK, they haven't
-        unless ($existing_charge_hashref) {
-            # This item is on issue ... add replacement cost to the borrower's record and mark it returned
-            #  Note that we add this to the account even if there's no replacement price, allowing some other
-            #  process (or person) to update it, since we don't handle any defaults for replacement prices.
-            my $accountno = getnextacctno($issues->{'borrowernumber'});
-            my $sth2=$dbh->prepare("INSERT INTO accountlines
-            (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber)
-            VALUES (?,?,now(),?,?,'L',?,?)");
-            $sth2->execute($issues->{'borrowernumber'},$accountno,$issues->{'replacementprice'},
-            "Lost Item $issues->{'title'} $issues->{'barcode'}",
-            $issues->{'replacementprice'},$itemnumber);
-            $sth2->finish;
-        # FIXME: Log this ?
-        }
-        #FIXME : Should probably have a way to distinguish this from an item that really was returned.
-        #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
-        C4::Circulation::MarkIssueReturned($issues->{borrowernumber},$itemnumber);
-	#  Shouldn't MarkIssueReturned do this?
-        C4::Items::ModItem({ onloan => undef }, undef, $itemnumber);
+    my ($borrowernumber, $itemnumber, $amount, $description) = @_;
+
+    # first make sure the borrower hasn't already been charged for this item
+    my $sth1=$dbh->prepare("SELECT * from accountlines
+    WHERE borrowernumber=? AND itemnumber=? and accounttype='L'");
+    $sth1->execute($borrowernumber,$itemnumber);
+    my $existing_charge_hashref=$sth1->fetchrow_hashref();
+
+    # OK, they haven't
+    unless ($existing_charge_hashref) {
+        # This item is on issue ... add replacement cost to the borrower's record and mark it returned
+        #  Note that we add this to the account even if there's no replacement price, allowing some other
+        #  process (or person) to update it, since we don't handle any defaults for replacement prices.
+        my $accountno = getnextacctno($borrowernumber);
+        my $sth2=$dbh->prepare("INSERT INTO accountlines
+        (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber)
+        VALUES (?,?,now(),?,?,'L',?,?)");
+        $sth2->execute($borrowernumber,$accountno,$amount,
+        $description,$amount,$itemnumber);
+        $sth2->finish;
+    # FIXME: Log this ?
     }
-    $sth->finish;
 }
 
 =head2 manualinvoice
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 47068f9..da212ed 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -59,8 +59,9 @@ BEGIN {
 
 	# FIXME subs that should probably be elsewhere
 	push @EXPORT, qw(
-		&FixOverduesOnReturn
 		&barcodedecode
+        &LostItem
+        &ReturnLostItem
 	);
 
 	# subs to deal with issuing a book
@@ -2943,8 +2944,43 @@ sub DeleteBranchTransferLimits {
    $sth->execute();
 }
 
+sub ReturnLostItem{
+    my ( $borrowernumber, $itemnum ) = @_;
 
-  1;
+    MarkIssueReturned( $borrowernumber, $itemnum );
+    my $borrower = C4::Members::GetMember( 'borrowernumber'=>$borrowernumber );
+    my @datearr = localtime(time);
+    my $date = ( 1900 + $datearr[5] ) . "-" . ( $datearr[4] + 1 ) . "-" . $datearr[3];
+    my $bor = "$borrower->{'firstname'} $borrower->{'surname'} $borrower->{'cardnumber'}";
+    ModItem({ paidfor =>  "Paid for by $bor $date" }, undef, $itemnum);
+}
+
+
+sub LostItem{
+    my ($itemnumber, $mark_returned) = @_;
+
+    my $dbh = C4::Context->dbh();
+    my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title 
+                           FROM issues 
+                           JOIN items USING (itemnumber) 
+                           JOIN biblio USING (biblionumber)
+                           WHERE issues.itemnumber=?");
+    $sth->execute($itemnumber);
+    my $issues=$sth->fetchrow_hashref();
+    $sth->finish;
+
+    # if a borrower lost the item, add a replacement cost to the their record
+    if ( my $borrowernumber = $issues->{borrowernumber} ){
+
+        C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'}");
+        #FIXME : Should probably have a way to distinguish this from an item that really was returned.
+        #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
+        MarkIssueReturned($borrowernumber,$itemnumber) if $mark_returned;
+    }
+}
+
+
+1;
 
 __END__
 
diff --git a/C4/Items.pm b/C4/Items.pm
index 9bbc607..ffc2f1e 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -397,6 +397,8 @@ Note that only columns that can be directly
 changed from the cataloging and serials
 item editors are included in this hash.
 
+Returns item record
+
 =cut
 
 my %default_values_for_mod_from_marc = (
@@ -446,7 +448,8 @@ sub ModItemFromMarc {
     }
     my $unlinked_item_subfields = _get_unlinked_item_subfields( $localitemmarc, $frameworkcode );
 
-    return ModItem($item, $biblionumber, $itemnumber, $dbh, $frameworkcode, $unlinked_item_subfields); 
+    ModItem($item, $biblionumber, $itemnumber, $dbh, $frameworkcode, $unlinked_item_subfields); 
+    return $item;
 }
 
 =head2 ModItem
@@ -495,6 +498,9 @@ sub ModItem {
     };
 
     $item->{'itemnumber'} = $itemnumber or return undef;
+
+    $item->{onloan} = undef if $item->{itemlost};
+
     _set_derived_columns_for_mod($item);
     _do_column_fixes_for_mod($item);
     # FIXME add checks
diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl
index e8ce20d..379c12c 100755
--- a/catalogue/updateitem.pl
+++ b/catalogue/updateitem.pl
@@ -26,7 +26,6 @@ use C4::Biblio;
 use C4::Items;
 use C4::Output;
 use C4::Circulation;
-use C4::Accounts;
 use C4::Reserves;
 
 my $cgi= new CGI;
@@ -75,6 +74,6 @@ if (defined $itemnotes) { # i.e., itemnotes parameter passed from form
 
 ModItem($item_changes, $biblionumber, $itemnumber);
 
-C4::Accounts::chargelostitem($itemnumber) if ($itemlost==1) ;
+LostItem($itemnumber, 'MARK RETURNED') if $itemlost;
 
 print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber");
diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
index ff09e77..3df2549 100755
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -200,28 +200,33 @@ sub generate_subfield_form {
                   }
             }
 
-            $subfield_data{marc_value} =CGI::scrolling_list(      # FIXME: factor out scrolling_list
-                  -name     => "field_value",
-                  -values   => \@authorised_values,
-                  -default  => $value,
-                  -labels   => \%authorised_lib,
-                  -override => 1,
-                  -size     => 1,
-                  -multiple => 0,
-                  -tabindex => 1,
-                  -id       => "tag_".$tag."_subfield_".$subfieldtag."_".$index_subfield,
-                  -class    => "input_marceditor",
-            );
+            if ($subfieldlib->{'hidden'}) {
+                $subfield_data{marc_value} = qq(<input type="hidden" $attributes /> $authorised_lib{$value});
+            }
+            else {
+                $subfield_data{marc_value} =CGI::scrolling_list(      # FIXME: factor out scrolling_list
+                    -name     => "field_value",
+                    -values   => \@authorised_values,
+                    -default  => $value,
+                    -labels   => \%authorised_lib,
+                    -override => 1,
+                    -size     => 1,
+                    -multiple => 0,
+                    -tabindex => 1,
+                    -id       => "tag_".$tag."_subfield_".$subfieldtag."_".$index_subfield,
+                    -class    => "input_marceditor",
+                );
+            }
 
-            # it's a thesaurus / authority field
         }
+            # it's a thesaurus / authority field
         elsif ( $subfieldlib->{authtypecode} ) {
                 $subfield_data{marc_value} = "<input type=\"text\" $attributes />
                     <a href=\"#\" class=\"buttonDot\"
                         onclick=\"Dopop('/cgi-bin/koha/authorities/auth_finder.pl?authtypecode=".$subfieldlib->{authtypecode}."&index=$subfield_data{id}','$subfield_data{id}'); return false;\" title=\"Tag Editor\">...</a>
             ";
-            # it's a plugin field
         }
+            # it's a plugin field
         elsif ( $subfieldlib->{value_builder} ) {
                 # opening plugin
                 my $plugin = C4::Context->intranetdir . "/cataloguing/value_builder/" . $subfieldlib->{'value_builder'};
@@ -484,7 +489,7 @@ if ($op eq "additem") {
     if ($exist_itemnumber && $exist_itemnumber != $itemnumber) {
         push @errors,"barcode_not_unique";
     } else {
-        my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = ModItemFromMarc($itemtosave,$biblionumber,$itemnumber);
+        ModItemFromMarc($itemtosave,$biblionumber,$itemnumber);
         $itemnumber="";
     }
     $nextop="additem";
@@ -590,6 +595,8 @@ if($itemrecord){
             next if subfield_is_koha_internal_p($subfieldtag);
             next if ($tagslib->{$tag}->{$subfieldtag}->{'tab'} ne "10");
 
+            $subfieldlib->{hidden} = 1
+              if $tagslib->{$tag}->{$subfieldtag}->{authorised_value} eq 'LOST';
             my $subfield_data = generate_subfield_form($tag, $subfieldtag, $value, $tagslib, $subfieldlib, $branches, $today_iso, $biblionumber, $temp, \@loop_data, $i);        
 
             push @fields, "$tag$subfieldtag";
diff --git a/misc/cronjobs/longoverdue.pl b/misc/cronjobs/longoverdue.pl
index 651b9d2..2179d10 100755
--- a/misc/cronjobs/longoverdue.pl
+++ b/misc/cronjobs/longoverdue.pl
@@ -35,7 +35,6 @@ BEGIN {
 }
 use C4::Context;
 use C4::Items;
-use C4::Accounts;
 use Getopt::Long;
 
 my  $lost;  #  key=lost value,  value=num days.
@@ -155,7 +154,7 @@ foreach my $startrange (sort keys %$lost) {
             printf ("Due %s: item %5s from borrower %5s to lost: %s\n", $row->{date_due}, $row->{itemnumber}, $row->{borrowernumber}, $lostvalue) if($verbose);
             if($confirm) {
                 ModItem({ itemlost => $lostvalue }, $row->{'biblionumber'}, $row->{'itemnumber'});
-                chargelostitem($row->{'itemnumber'}) if( $charge && $charge eq $lostvalue);
+                LostItem($row->{'itemnumber'}) if( $charge && $charge eq $lostvalue);
             }
             $count++;
         }
diff --git a/t/db_dependent/lib/KohaTest/Accounts.pm b/t/db_dependent/lib/KohaTest/Accounts.pm
index 703d478..ac3a78e 100644
--- a/t/db_dependent/lib/KohaTest/Accounts.pm
+++ b/t/db_dependent/lib/KohaTest/Accounts.pm
@@ -15,7 +15,6 @@ sub methods : Test( 1 ) {
     my @methods = qw( recordpayment
                       makepayment
                       getnextacctno
-                      returnlost
                       manualinvoice
                       fixcredit
                       refund
diff --git a/t/db_dependent/lib/KohaTest/Circulation.pm b/t/db_dependent/lib/KohaTest/Circulation.pm
index 7d5e69d..b3a1ff8 100644
--- a/t/db_dependent/lib/KohaTest/Circulation.pm
+++ b/t/db_dependent/lib/KohaTest/Circulation.pm
@@ -47,6 +47,7 @@ sub methods : Test( 1 ) {
                       CheckSpecialHolidays
                       CheckRepeatableSpecialHolidays
                       CheckValidBarcode
+                      ReturnLostItem
                 );
     
     can_ok( $self->testing_class, @methods );    
diff --git a/tools/batchMod.pl b/tools/batchMod.pl
index d9a0b76..c26def4 100755
--- a/tools/batchMod.pl
+++ b/tools/batchMod.pl
@@ -25,6 +25,7 @@ use C4::Auth;
 use C4::Output;
 use C4::Biblio;
 use C4::Items;
+use C4::Circulation;
 use C4::Context;
 use C4::Koha; # XXX subfield_is_koha_internal_p
 use C4::Branch; # XXX subfield_is_koha_internal_p
@@ -140,16 +141,25 @@ if ($op eq "action") {
 			    $deleted_items++;
 			} else {
 			    $not_deleted_items++;
-			    push @not_deleted, { biblionumber => $itemdata->{'biblionumber'}, itemnumber => $itemdata->{'itemnumber'}, barcode => $itemdata->{'barcode'}, title => $itemdata->{'title'}, $return => 1 };
+			    push @not_deleted, {
+                    biblionumber => $itemdata->{'biblionumber'},
+                    itemnumber => $itemdata->{'itemnumber'},
+                    barcode => $itemdata->{'barcode'},
+                    title => $itemdata->{'title'},
+                    $return => 1
+                };
 			}
 		} else {
 		    if ($something_to_modify) {
 			my $xml = TransformHtmlToXml(\@tags,\@subfields,\@values,\@indicator,\@ind_tag, 'ITEM');
 			my $marcitem = MARC::Record::new_from_xml($xml, 'UTF-8');
-			my $localitem = TransformMarcToKoha( $dbh, $marcitem, "", 'items' );
 			my $localmarcitem=Item2Marc($itemdata);
 			UpdateMarcWith($marcitem,$localmarcitem);
-			eval{my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = ModItemFromMarc($localmarcitem,$itemdata->{biblionumber},$itemnumber)};
+			eval{
+                if ( my $item = ModItemFromMarc( $localmarcitem, $itemdata->{biblionumber}, $itemnumber ) ) {
+                    LostItem($itemnumber, 'MARK RETURNED') if $item->{itemlost};
+                }
+            };
 		    }
 		}
 		$i++;
-- 
1.6.5




More information about the Patches mailing list