Bugzilla – Attachment 5907 Details for
Bug 5533
marking item lost diff in two places
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch
0001-bug_5533-Slightly-improved-marking-items-as-lost.patch (text/plain), 15.44 KB, created by
Srdjan Jankovic
on 2011-10-17 02:41:44 UTC
(
hide
)
Description:
patch
Filename:
MIME Type:
Creator:
Srdjan Jankovic
Created:
2011-10-17 02:41:44 UTC
Size:
15.44 KB
patch
obsolete
>From c943b5efc668184ae0ca75efdec35e190d308e91 Mon Sep 17 00:00:00 2001 >From: Srdjan Jankovic <srdjan@catalyst.net.nz> >Date: Mon, 17 Oct 2011 15:31:29 +1300 >Subject: [PATCH] bug_5533: Slightly improved marking items as lost > >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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 5533
:
5398
|
5473
|
5486
|
5515
|
5907
|
5936
|
5937
|
6001
|
6810
|
6896