[Koha-patches] [PATCH] bug_5533: Improved marking items as lost
Srdjan Jankovic
srdjan at catalyst.net.nz
Mon Sep 12 04:49:17 CEST 2011
---
C4/Accounts.pm | 77 ++++++++-------------------
C4/Circulation.pm | 40 ++++++++++++++-
C4/Items.pm | 8 +++-
catalogue/updateitem.pl | 3 +-
cataloguing/additem.pl | 5 ++-
misc/cronjobs/longoverdue.pl | 3 +-
t/db_dependent/lib/KohaTest/Accounts.pm | 1 -
t/db_dependent/lib/KohaTest/Circulation.pm | 1 +
tools/batchMod.pl | 7 ++-
9 files changed, 81 insertions(+), 64 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 520c116..23769b4 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) = @_;
+
+ 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();
+
+ # if a borrower lost the item, add a replacement cost to the their record
+ if ( $issues->{borrowernumber} ){
+
+ C4::Accounts::chargelostitem($issues->{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($issues->{borrowernumber},$itemnumber);
+ }
+ $sth->finish;
+}
+
+
+1;
__END__
diff --git a/C4/Items.pm b/C4/Items.pm
index bc36dd1..9e7167d 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..8bdc74f 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) if ($itemlost==1) ;
print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber");
diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
index 0db2d2d..0d591c7 100755
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -26,6 +26,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
@@ -498,7 +499,9 @@ if ($op eq "additem") {
if ($exist_itemnumber && $exist_itemnumber != $itemnumber) {
push @errors,"barcode_not_unique";
} else {
- my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = ModItemFromMarc($itemtosave,$biblionumber,$itemnumber);
+ if ( my $item = ModItemFromMarc($itemtosave,$biblionumber,$itemnumber) ) {
+ LostItem($itemnumber) if $item->{itemlost} == 1;
+ }
$itemnumber="";
}
$nextop="additem";
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 9d4431b..f6d58a8 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
@@ -173,7 +174,11 @@ if ($op eq "action") {
if ($values_to_modify || $values_to_blank) {
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) if $item->{itemlost} == 1;
+ }
+ };
}
}
$i++;
--
1.6.5
More information about the Koha-patches
mailing list