[Koha-patches] [PATCH] bug_5533: Slightly improved marking items as lost
Srdjan Jankovic
srdjan at catalyst.net.nz
Mon Oct 17 04:40:27 CEST 2011
Call LostItem() whenever item is lost.
LostItem() new arg - mark returned.
Disabled 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 | 7 ++-
9 files changed, 99 insertions(+), 78 deletions(-)
diff --git a/C4/Accounts.pm b/C4/Accounts.pm
index 2352e9e..860e187 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);
@@ -218,7 +217,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'} );
}
}
@@ -278,64 +277,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 42a8b2d..12eece4 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
@@ -2949,8 +2950,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 23eb1eb..ca67c6d 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -401,6 +401,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 = (
@@ -450,7 +452,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
@@ -499,6 +502,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 6229f0c..4442ed3 100755
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -206,28 +206,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'};
@@ -500,7 +505,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";
@@ -665,6 +670,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 04cfc87..701d421 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
@@ -174,7 +175,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, 'MARK RETURNED') if $item->{itemlost};
+ }
+ };
}
}
$i++;
--
1.6.5
More information about the Koha-patches
mailing list