From 3722d7b77639204ab9d07a53e2dc05ec5d794282 Mon Sep 17 00:00:00 2001 From: Srdjan Jankovic Date: Wed, 21 Sep 2011 13:09:55 +1200 Subject: [PATCH] [Signed Off] bug_5533: new function C4::Circulation::LostItem($itemnumber, $mark_returned) Content-Type: text/plain; charset="utf-8" Disabled Lost Status on cataloging full item edit longoverdue.pl calls LostItem() *without* $mark_returned Item moredetail calls LostItem() *with* $mark_returned Batch mode edit calls LostItem() *with* $mark_returned Signed-off-by: Liz Rea I am happy to sign off on this new version. Editing works fine, behavior of 'lost' is as I expected. Steps to test: Check out an item with a replacement cost to a patron. Mark the item lost from moredetail.pl -> should remove the item from checked out items and apply replacement cost to the patron's fines. Check in the item -> should report "item was lost, now found" and remove the charge (apply a credit, technically) to the patron's fines. Check out several items to several patrons. Batch mod those items to lost -> should remove the item from checked out items and apply replacement cost to the patron's fines. Check in the items -> should report "item was lost, now found" and remove the charge (apply a credit, technically) to the patron's fines. Editing items: From your test item, Edit -> Edit Items Select an item to edit Edit the item as you see fit Verify that all values are saved in correct fields, and your changes are correct. (they are) Verify that you cannot set 'lost' status from the edit item interface. (You cannot) longoverdue.pl backdate an item so it will be long overdue run longoverdue.pl to set your item as long overdue. verify that the item is not removed from the patron's checked out items. Long Overdue does not charge the patron for the replacement. --- C4/Accounts.pm | 77 ++++++++------------------- C4/Circulation.pm | 40 ++++++++++++++- C4/Items.pm | 8 +++- catalogue/updateitem.pl | 3 +- cataloguing/additem.pl | 38 ++++++++----- 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, 100 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 520c116..589b5c5 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 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..345d0c8 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==1) ; print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 0db2d2d..4b99654 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 @@ -206,28 +207,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( $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} = " {authtypecode}."&index=$subfield_data{id}','$subfield_data{id}'); return false;\" title=\"Tag Editor\">... "; - # 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'}; @@ -498,7 +504,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"; @@ -608,6 +614,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 9d4431b..a7848b9 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, 'MARK RETURNED') if $item->{itemlost} == 1; + } + }; } } $i++; -- 1.7.2.5