From c6c2cc75e82d2d152c280fb1de1875d36f625077 Mon Sep 17 00:00:00 2001 From: Fridolyn SOMERS Date: Fri, 31 May 2013 16:08:15 +0200 Subject: [PATCH] Bug 9351 - item type not recorded correctly in statistics for returns and some local use When returning a loan, or when returning an item for local use, the corresponding entry in the statistics table takes the item type from the bib level. This is incorrect when item-level item types is enabled. This patch unfies the way the informations are retrieved in AddIssue, AddRenewal, AddIssue, CanBookBeIssued and CanBookBeRenewed : - item informations : my $item = GetItem() - biblio+biblioitems informations : my $biblio = GetBiblioItemData() - itemtype value used everywhere : my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblio->{'itemtype'} Test plan : ----------- On a catalogue with itemtype on item level : - Select an item with itemtype not for loan - Check-out this item => You get a message the item is not for loan - Select an item with itemtype allows check-out - Check-out this item => Issue is well done and statistics table has a new line with correct itemtype - Select an item with itemtype that can not be renewed (from issuing rules) - Check-out this item and try to renew => You get a message the item can not be renewed - Select an item with itemtype that be renewed - Check-out this item and try to renew => Renewal is well done and statistics table has a new line with correct itemtype - Check-in this item => Return is well done and statistics table has a new line with correct itemtype Idem on a catalogue with itemtype on biblio level. --- C4/Circulation.pm | 68 +++++++++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 30cbe2f..43890a9 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -693,10 +693,7 @@ sub CanBookBeIssued { my %issuingimpossible; # filled with problems that causes the issue to be IMPOSSIBLE my %alerts; # filled with messages that shouldn't stop issuing, but the librarian should be aware of. - my $item = GetItem(GetItemnumberFromBarcode( $barcode )); - my $issue = GetItemIssue($item->{itemnumber}); - my $biblioitem = GetBiblioItemData($item->{biblioitemnumber}); - $item->{'itemtype'}=$item->{'itype'}; + my $item = GetItem(undef, $barcode); my $dbh = C4::Context->dbh; # MANDATORY CHECKS - unless item exists, nothing else matters @@ -705,6 +702,10 @@ sub CanBookBeIssued { } return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible; + my $issue = GetItemIssue($item->{'itemnumber'}); + my $biblio = GetBiblioItemData($item->{'biblioitemnumber'}); + my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblio->{'itemtype'}; + # # DUE DATE is OK ? -- should already have checked. # @@ -716,7 +717,6 @@ sub CanBookBeIssued { my $issuedate = $now->clone(); my $branch = _GetCircControlBranch($item,$borrower); - my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblioitem->{'itemtype'}; $duedate = CalcDateDue( $issuedate, $itype, $branch, $borrower ); # Offline circ calls AddIssue directly, doesn't run through here @@ -737,7 +737,7 @@ sub CanBookBeIssued { # if ( $borrower->{'category_type'} eq 'X' && ( $item->{barcode} )) { # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1 . - &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'}, undef, $item->{'ccode'}); + &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$itype,$borrower->{'borrowernumber'}, undef, $item->{'ccode'}); ModDateLastSeen( $item->{'itemnumber'} ); return( { STATS => 1 }, {}); } @@ -863,13 +863,13 @@ sub CanBookBeIssued { } } } - elsif ($biblioitem->{'notforloan'} == 1){ + elsif ($biblio->{'notforloan'} == 1){ if (!C4::Context->preference("AllowNotForLoanOverride")) { $issuingimpossible{NOT_FOR_LOAN} = 1; - $issuingimpossible{itemtype_notforloan} = $biblioitem->{'itemtype'}; + $issuingimpossible{itemtype_notforloan} = $biblio->{'itemtype'}; } else { $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1; - $needsconfirmation{itemtype_notforloan} = $biblioitem->{'itemtype'}; + $needsconfirmation{itemtype_notforloan} = $biblio->{'itemtype'}; } } } @@ -968,7 +968,7 @@ sub CanBookBeIssued { # get $marker from preferences. Could be something like "FSK|PEGI|Alter|Age:" my $markers = C4::Context->preference('AgeRestrictionMarker' ); - my $bibvalues = $biblioitem->{'agerestriction'}; + my $bibvalues = $biblio->{'agerestriction'}; if (($markers)&&($bibvalues)) { # Split $bibvalues to something like FSK 16 or PEGI 6 @@ -1194,7 +1194,8 @@ sub AddIssue { my $actualissue = GetItemIssue( $item->{itemnumber}); # get biblioinformation for this item - my $biblio = GetBiblioFromItemNumber($item->{itemnumber}); + my $biblio = GetBiblioItemData($item->{'biblioitemnumber'}); + my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblio->{'itemtype'}; # # check if we just renew the issue. @@ -1243,9 +1244,7 @@ sub AddIssue { VALUES (?,?,?,?,?)" ); unless ($datedue) { - my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'}; $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower ); - } $datedue->truncate( to => 'minute'); $sth->execute( @@ -1279,7 +1278,7 @@ sub AddIssue { ModDateLastSeen( $item->{'itemnumber'} ); # If it costs to borrow this book, charge it to the patron's account. - my ( $charge, $itemtype ) = GetIssuingCharges( + my ( $charge, $charge_itemtype ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} ); @@ -1296,7 +1295,7 @@ sub AddIssue { C4::Context->userenv->{'branch'}, 'issue', $charge, ($sipmode ? "SIP-$sipmode" : ''), $item->{'itemnumber'}, - $item->{'itype'}, $borrower->{'borrowernumber'}, undef, $item->{'ccode'} + $itype, $borrower->{'borrowernumber'}, undef, $item->{'ccode'} ); # Send a checkout slip. @@ -1304,7 +1303,7 @@ sub AddIssue { my %conditions = ( branchcode => $branch, categorycode => $borrower->{categorycode}, - item_type => $item->{itype}, + item_type => $itype, notification => 'CHECKOUT', ); if ($circulation_alert->is_enabled_for(\%conditions)) { @@ -1317,7 +1316,7 @@ sub AddIssue { } } - logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'itemnumber'}) + logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $item->{'itemnumber'}) if C4::Context->preference("IssueLog"); } return ($datedue); # not necessarily the same as when it came in! @@ -1708,7 +1707,6 @@ sub AddReturn { $branch = C4::Context->userenv->{'branch'} unless $branch; # we trust userenv to be a safe fallback/default my $messages; my $borrower; - my $biblio; my $doreturn = 1; my $validTransfert = 0; my $stat_type = 'return'; @@ -1739,7 +1737,12 @@ sub AddReturn { my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed"; # full item data, but no borrowernumber or checkout info (no issue) # we know GetItem should work because GetItemnumberFromBarcode worked - my $hbr = GetBranchItemRule($item->{'homebranch'}, $item->{'itype'})->{'returnbranch'} || "homebranch"; + + # get biblioinformation for this item + my $biblio = GetBiblioItemData($item->{'biblioitemnumber'}); + my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblio->{'itemtype'}; + + my $hbr = GetBranchItemRule($item->{'homebranch'}, $itype)->{'returnbranch'} || "homebranch"; # get the proper branch to which to return the item $hbr = $item->{$hbr} || $branch ; # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) @@ -1887,7 +1890,7 @@ sub AddReturn { UpdateStats( $branch, $stat_type, '0', '', $item->{'itemnumber'}, - $biblio->{'itemtype'}, + $itype, $borrowernumber, undef, $item->{'ccode'} ); @@ -1896,7 +1899,7 @@ sub AddReturn { my %conditions = ( branchcode => $branch, categorycode => $borrower->{categorycode}, - item_type => $item->{itype}, + item_type => $itype, notification => 'CHECKIN', ); if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) { @@ -2466,13 +2469,15 @@ sub CanBookBeRenewed { my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 ) or return; my $item = GetItem($itemnumber) or return; - my $itemissue = GetItemIssue($itemnumber) or return; + my $issue = GetItemIssue($itemnumber) or return; + my $biblio = GetBiblioItemData($item->{'biblioitemnumber'}); + my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblio->{'itemtype'}; my $branchcode = _GetCircControlBranch($item, $borrower); - my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode); + my $issuingrule = GetIssuingRule($borrower->{categorycode}, $itype, $branchcode); - if ( ( $issuingrule->{renewalsallowed} > $itemissue->{renewals} ) || $override_limit ) { + if ( ( $issuingrule->{renewalsallowed} > $issue->{renewals} ) || $override_limit ) { $renewokay = 1; } else { $error = "too_many"; @@ -2517,8 +2522,10 @@ sub AddRenewal { my $branch = shift; my $datedue = shift; my $lastreneweddate = shift || DateTime->now(time_zone => C4::Context->tz)->ymd(); + my $item = GetItem($itemnumber) or return; - my $biblio = GetBiblioFromItemNumber($itemnumber) or return; + my $biblio = GetBiblioItemData($item->{'biblioitemnumber'}) or return; + my $itype = (C4::Context->preference('item-level_itypes')) ? $item->{'itype'} : $biblio->{'itemtype'}; my $dbh = C4::Context->dbh; # Find the issues record for this book @@ -2540,12 +2547,11 @@ sub AddRenewal { unless ($datedue) { my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return; - my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'}; $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ? dt_from_string( $issuedata->{date_due} ) : DateTime->now( time_zone => C4::Context->tz()); - $datedue = CalcDateDue($datedue, $itemtype, $issuedata->{'branchcode'}, $borrower, 'is a renewal'); + $datedue = CalcDateDue($datedue, $itype, $issuedata->{'branchcode'}, $borrower, 'is a renewal'); } # Update the issues record to have the new due date, and a new count @@ -2560,8 +2566,8 @@ sub AddRenewal { $sth->finish; # Update the renewal count on the item, and tell zebra to reindex - $renews = $biblio->{'renewals'} + 1; - ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $biblio->{'biblionumber'}, $itemnumber); + $renews = $item->{'renewals'} + 1; + ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{'biblionumber'}, $itemnumber); # Charge a new rental fee, if applicable? my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); @@ -2588,7 +2594,7 @@ sub AddRenewal { my %conditions = ( branchcode => $branch, categorycode => $borrower->{categorycode}, - item_type => $item->{itype}, + item_type => $itype, notification => 'CHECKOUT', ); if ($circulation_alert->is_enabled_for(\%conditions)) { @@ -2602,7 +2608,7 @@ sub AddRenewal { } # Log the renewal - UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $item->{itype}, $borrowernumber, undef, $item->{'ccode'}); + UpdateStats( $branch, 'renew', $charge, '', $itemnumber, $itype, $borrowernumber, undef, $item->{'ccode'}); return $datedue; } -- 1.7.10.4