Bugzilla – Attachment 57787 Details for
Bug 17680
C4::Circulation - Replace GetItemIssue with Koha::Checkouts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls
Bug-17680-C4Circulation---Remove-GetItemIssue-simp.patch (text/plain), 18.82 KB, created by
Jonathan Druart
on 2016-11-28 10:37:00 UTC
(
hide
)
Description:
Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2016-11-28 10:37:00 UTC
Size:
18.82 KB
patch
obsolete
>From fb6d367732eac7cbe15ff8735b6f959109272d54 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 25 Nov 2016 12:02:01 +0000 >Subject: [PATCH] Bug 17680: C4::Circulation - Remove GetItemIssue, simple > calls > >C4::Circulation::GetItemIssue returned all the issue and item >informations for a given issue. Moveover it also did some date >manipulations. Most of the time this subroutine was called, there >additional information were useless as the caller usually just needed >the basic issue's infos 'from the issue table). > >This first patch updates the simple calls, ie. the ones that just need >the issue's infomations. > >Test plan: >The following operations should success: >- transfer a book >- create a rule for on-site checkouts and confirm that a patron cannot >check more items out that it's defined in the rule. >- Renew an issue using ILSDI >- Using SIP confirm that you are able to see your issues >--- > C4/Circulation.pm | 40 ++++++++++------------ > C4/ILSDI/Services.pm | 8 +++-- > C4/SIP/ILS/Item.pm | 8 ++--- > C4/SIP/ILS/Transaction.pm | 8 ++--- > offline_circ/enqueue_koc.pl | 7 ++-- > offline_circ/process_koc.pl | 13 +++---- > opac/opac-reserve.pl | 7 ++-- > reserve/request.pl | 7 ++-- > t/db_dependent/Circulation.t | 3 +- > t/db_dependent/Circulation/SwitchOnSiteCheckouts.t | 7 ++-- > t/db_dependent/Circulation/issue.t | 8 +---- > 11 files changed, 58 insertions(+), 58 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 8abc9c2..c3ebfe9 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -307,7 +307,7 @@ sub transferbook { > my $messages; > my $dotransfer = 1; > my $itemnumber = GetItemnumberFromBarcode( $barcode ); >- my $issue = GetItemIssue($itemnumber); >+ my $issue = Koha::Issues->find({ itemnumber => $itemnumber }); > my $biblio = GetBiblioFromItemNumber($itemnumber); > > # bad barcode.. >@@ -349,9 +349,9 @@ sub transferbook { > } > > # check if it is still issued to someone, return it... >- if ($issue->{borrowernumber}) { >+ if ( $issue ) { > AddReturn( $barcode, $fbr ); >- $messages->{'WasReturned'} = $issue->{borrowernumber}; >+ $messages->{'WasReturned'} = $issue->borrowernumber; > } > > # find reserves..... >@@ -670,7 +670,7 @@ sub CanBookBeIssued { > my $override_high_holds = $params->{override_high_holds} || 0; > > my $item = GetItem(GetItemnumberFromBarcode( $barcode )); >- my $issue = GetItemIssue($item->{itemnumber}); >+ my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); > my $biblioitem = GetBiblioItemData($item->{biblioitemnumber}); > $item->{'itemtype'}=$item->{'itype'}; > my $dbh = C4::Context->dbh; >@@ -826,9 +826,9 @@ sub CanBookBeIssued { > # > my $switch_onsite_checkout = > C4::Context->preference('SwitchOnSiteCheckouts') >- and $issue->{onsite_checkout} > and $issue >- and $issue->{borrowernumber} == $borrower->{'borrowernumber'} ? 1 : 0; >+ and $issue->onsite_checkout >+ and $issue->borrowernumber == $borrower->{'borrowernumber'} ? 1 : 0; > my $toomany = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } ); > # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book > if ( $toomany ) { >@@ -936,13 +936,13 @@ sub CanBookBeIssued { > # > # CHECK IF BOOK ALREADY ISSUED TO THIS BORROWER > # >- if ( $issue->{borrowernumber} && $issue->{borrowernumber} eq $borrower->{'borrowernumber'} ){ >+ if ( $issue && $issue->borrowernumber eq $borrower->{'borrowernumber'} ){ > > # Already issued to current borrower. > # If it is an on-site checkout if it can be switched to a normal checkout > # or ask whether the loan should be renewed > >- if ( $issue->{onsite_checkout} >+ if ( $issue->onsite_checkout > and C4::Context->preference('SwitchOnSiteCheckouts') ) { > $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1; > } else { >@@ -963,10 +963,10 @@ sub CanBookBeIssued { > } > } > } >- elsif ($issue->{borrowernumber}) { >+ elsif ( $issue ) { > > # issued to someone else >- my $currborinfo = C4::Members::GetMember( borrowernumber => $issue->{borrowernumber} ); >+ my $currborinfo = C4::Members::GetMember( borrowernumber => $issue->borrowernumber ); > > > my ( $can_be_returned, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} ); >@@ -1292,13 +1292,13 @@ sub AddIssue { > my $branch = _GetCircControlBranch( $item, $borrower ); > > # get actual issuing if there is one >- my $actualissue = GetItemIssue( $item->{itemnumber} ); >+ my $actualissue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); > > # get biblioinformation for this item > my $biblio = GetBiblioFromItemNumber( $item->{itemnumber} ); > > # check if we just renew the issue. >- if ( $actualissue->{borrowernumber} eq $borrower->{'borrowernumber'} >+ if ( $actualissue and $actualissue->borrowernumber eq $borrower->{'borrowernumber'} > and not $switch_onsite_checkout ) { > $datedue = AddRenewal( > $borrower->{'borrowernumber'}, >@@ -1310,8 +1310,7 @@ sub AddIssue { > } > else { > # it's NOT a renewal >- if ( $actualissue->{borrowernumber} >- and not $switch_onsite_checkout ) { >+ if ( $actualissue and not $switch_onsite_checkout ) { > # This book is currently on loan, but not to the person > # who wants to borrow it now. mark it returned before issuing to the new borrower > my ( $allowed, $message ) = CanBookBeReturned( $item, C4::Context->userenv->{branch} ); >@@ -3040,9 +3039,9 @@ sub GetSoonestRenewDate { > my $dbh = C4::Context->dbh; > > my $item = GetItem($itemnumber) or return; >- my $itemissue = GetItemIssue($itemnumber) or return; >+ my $itemissue = Koha::Issues->find( { itemnumber => $itemnumber } ) or return; > >- $borrowernumber ||= $itemissue->{borrowernumber}; >+ $borrowernumber ||= $itemissue->borrowernumber; > my $borrower = C4::Members::GetMemberDetails($borrowernumber) > or return; > >@@ -3061,8 +3060,7 @@ sub GetSoonestRenewDate { > and $issuing_rule->norenewalbefore ne "" ) > { > my $soonestrenewal = >- $itemissue->{date_due}->clone() >- ->subtract( >+ dt_from_string( $itemissue->date_due )->subtract( > $issuing_rule->lengthunit => $issuing_rule->norenewalbefore ); > > if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date' >@@ -3100,9 +3098,9 @@ sub GetLatestAutoRenewDate { > my $dbh = C4::Context->dbh; > > my $item = GetItem($itemnumber) or return; >- my $itemissue = GetItemIssue($itemnumber) or return; >+ my $itemissue = Koha::Issues->find( { itemnumber => $itemnumber } ) or return; > >- $borrowernumber ||= $itemissue->{borrowernumber}; >+ $borrowernumber ||= $itemissue->borrowernumber; > my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) > or return; > >@@ -3118,7 +3116,7 @@ sub GetLatestAutoRenewDate { > return if not $issuing_rule->no_auto_renewal_after > or $issuing_rule->no_auto_renewal_after eq ''; > >- my $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); >+ my $maximum_renewal_date = dt_from_string($itemissue->issuedate); > $maximum_renewal_date->add( > $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after > ); >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index 7c5e5f3..d527574 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -34,6 +34,8 @@ use CGI qw ( -utf8 ); > use DateTime; > use C4::Auth; > use C4::Members::Attributes qw(GetBorrowerAttributes); >+use Koha::DateUtils; >+use Koha::Issues; > > use Koha::Libraries; > >@@ -569,12 +571,12 @@ sub RenewLoan { > my @renewal = CanBookBeRenewed( $borrowernumber, $itemnumber ); > if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber ); } > >- my $issue = GetItemIssue($itemnumber); >+ my $issue = Koha::Issues->find( { itemnumber => $itemnumber } ) or return; # FIXME should be handled > > # Hashref building > my $out; >- $out->{'renewals'} = $issue->{'renewals'}; >- $out->{date_due} = $issue->{date_due}->strftime('%Y-%m-%d %H:%S'); >+ $out->{'renewals'} = $issue->renewals; >+ $out->{date_due} = dt_from_string($issue->date_due)->strftime('%Y-%m-%d %H:%S'); > $out->{'success'} = $renewal[0]; > $out->{'error'} = $renewal[1]; > >diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm >index 578f382..a8b32a6 100644 >--- a/C4/SIP/ILS/Item.pm >+++ b/C4/SIP/ILS/Item.pm >@@ -22,7 +22,7 @@ use C4::Circulation; > use C4::Members; > use C4::Reserves; > use Koha::Database; >- >+use Koha::Issues; > > =encoding UTF-8 > >@@ -93,11 +93,11 @@ sub new { > $item->{sip_media_type} = $itemtype->sip_media_type() if $itemtype; > > # check if its on issue and if so get the borrower >- my $issue = GetItemIssue($item->{'itemnumber'}); >+ my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); > if ($issue) { >- $item->{due_date} = $issue->{date_due}; >+ $item->{due_date} = dt_from_string( $issue->date_due, 'sql' )->truncate( to => 'minute' ); > } >- my $borrower = GetMember(borrowernumber=>$issue->{'borrowernumber'}); >+ my $borrower = $issue ? GetMember( borrowernumber => $issue->borrowernumber ) : {}; > $item->{patron} = $borrower->{'cardnumber'}; > my $reserves = GetReservesFromBiblionumber({ biblionumber => $item->{biblionumber} }); > $item->{hold_queue} = [ sort priority_sort @$reserves ]; >diff --git a/C4/SIP/ILS/Transaction.pm b/C4/SIP/ILS/Transaction.pm >index c751125..1d2e3ce 100644 >--- a/C4/SIP/ILS/Transaction.pm >+++ b/C4/SIP/ILS/Transaction.pm >@@ -8,8 +8,8 @@ use Carp; > use strict; > use warnings; > use C4::Context; >-use C4::Circulation qw( GetItemIssue ); > use Koha::DateUtils; >+use Koha::Issues; > > my %fields = ( > ok => 0, >@@ -45,9 +45,9 @@ sub duedatefromissue { > } # renew from AddIssue ?? > else { > # need to reread the issue to get due date >- $iss = GetItemIssue($itemnum); >- if ($iss && $iss->{date_due} ) { >- $due_dt = dt_from_string( $iss->{date_due} ); >+ $iss = Koha::Issues->find( { itemnumber => $itemnum } ); >+ if ($iss && $iss->date_due ) { >+ $due_dt = dt_from_string( $iss->date_due, 'sql' ); > } > } > return $due_dt; >diff --git a/offline_circ/enqueue_koc.pl b/offline_circ/enqueue_koc.pl >index a81aecb..546243b 100755 >--- a/offline_circ/enqueue_koc.pl >+++ b/offline_circ/enqueue_koc.pl >@@ -32,6 +32,7 @@ use C4::Circulation; > use C4::Items; > use C4::Members; > use C4::Stats; >+use Koha::Issues; > use Koha::Upload; > > use Date::Calc qw( Add_Delta_Days Date_to_Days ); >@@ -191,7 +192,7 @@ sub _get_borrowernumber_from_barcode { > my $item = GetBiblioFromItemNumber( undef, $barcode ); > return unless $item->{'itemnumber'}; > >- my $issue = C4::Circulation::GetItemIssue( $item->{'itemnumber'} ); >- return unless $issue->{'borrowernumber'}; >- return $issue->{'borrowernumber'}; >+ my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); >+ return unless $issue; >+ return $issue->borrowernumber; > } >diff --git a/offline_circ/process_koc.pl b/offline_circ/process_koc.pl >index f959d6f..55a8798 100755 >--- a/offline_circ/process_koc.pl >+++ b/offline_circ/process_koc.pl >@@ -37,6 +37,7 @@ use C4::Stats; > use C4::BackgroundJob; > use Koha::Upload; > use Koha::Account; >+use Koha::Issues; > use Koha::Patrons; > > use Date::Calc qw( Add_Delta_Days Date_to_Days ); >@@ -249,11 +250,11 @@ sub kocIssueItem { > my $branchcode = C4::Context->userenv->{branch}; > my $borrower = GetMember( 'cardnumber'=>$circ->{ 'cardnumber' } ); > my $item = GetBiblioFromItemNumber( undef, $circ->{ 'barcode' } ); >- my $issue = GetItemIssue( $item->{'itemnumber'} ); >+ my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); > >- if ( $issue->{ 'date_due' } ) { ## Item is currently checked out to another person. >+ if ( $issue ) { ## Item is currently checked out to another person. > #warn "Item Currently Issued."; >- my $issue = GetOpenIssue( $item->{'itemnumber'} ); >+ my $issue = GetOpenIssue( $item->{'itemnumber'} ); # FIXME Hum? That does not make sense, if it's in the issue table, the issue is open (i.e. returndate is null) > > if ( $issue->{'borrowernumber'} eq $borrower->{'borrowernumber'} ) { ## Issued to this person already, renew it. > #warn "Item issued to this member already, renewing."; >@@ -397,7 +398,7 @@ sub _get_borrowernumber_from_barcode { > my $item = GetBiblioFromItemNumber( undef, $barcode ); > return unless $item->{'itemnumber'}; > >- my $issue = C4::Circulation::GetItemIssue( $item->{'itemnumber'} ); >- return unless $issue->{'borrowernumber'}; >- return $issue->{'borrowernumber'}; >+ my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); >+ return unless $issue; >+ return $issue->borrowernumber; > } >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index d02e96c..1ea42cd 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -33,6 +33,7 @@ use C4::Members; > use C4::Overdues; > use C4::Debug; > use Koha::DateUtils; >+use Koha::Issues; > use Koha::Libraries; > use Koha::Patrons; > use Date::Calc qw/Today Date_to_Days/; >@@ -438,9 +439,9 @@ foreach my $biblioNum (@biblionumbers) { > > # If the item is currently on loan, we display its return date and > # change the background color. >- my $issues= GetItemIssue($itemNum); >- if ( $issues->{'date_due'} ) { >- $itemLoopIter->{dateDue} = output_pref({ dt => dt_from_string($issues->{date_due}, 'sql'), as_due_date => 1 }); >+ my $issue = Koha::Issues->find( { itemnumber => $itemNum } ); >+ if ( $issue ) { >+ $itemLoopIter->{dateDue} = output_pref({ dt => dt_from_string($issue->date_due, 'sql'), as_due_date => 1 }); > $itemLoopIter->{backgroundcolor} = 'onloan'; > } > >diff --git a/reserve/request.pl b/reserve/request.pl >index bc55fdf..681e052 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -43,6 +43,7 @@ use C4::Utils::DataTables::Members; > use C4::Members; > use C4::Search; # enabled_staff_search_views > use Koha::DateUtils; >+use Koha::Issues; > use Koha::Holds; > use Koha::Libraries; > >@@ -376,9 +377,9 @@ foreach my $biblionumber (@biblionumbers) { > > # if the item is currently on loan, we display its return date and > # change the background color >- my $issues= GetItemIssue($itemnumber); >- if ( $issues->{'date_due'} ) { >- $item->{date_due} = $issues->{date_due_sql}; >+ my $issue = Koha::Issues->find( { itemnumber => $itemnumber } ); >+ if ( $issue ) { >+ $item->{date_due} = $issue->date_due; > $item->{backgroundcolor} = 'onloan'; > } > >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index f29dcd0..4a96295 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -36,6 +36,7 @@ use C4::Reserves; > use C4::Overdues qw(UpdateFine CalcFine); > use Koha::DateUtils; > use Koha::Database; >+use Koha::Issues; > > my $schema = Koha::Database->schema; > $schema->storage->txn_begin; >@@ -312,7 +313,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > is (defined $issue2, 1, "Item 2 checked out, due date: " . $issue2->date_due()); > > >- my $borrowing_borrowernumber = GetItemIssue($itemnumber)->{borrowernumber}; >+ my $borrowing_borrowernumber = Koha::Issues->find( { itemnumber => $itemnumber } )->borrowernumber; > is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}"); > > my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber, 1); >diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >index 024801e..0525744 100644 >--- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >+++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >@@ -26,6 +26,7 @@ use C4::Context; > > use Koha::DateUtils qw( dt_from_string ); > use Koha::Database; >+use Koha::Issues; > > use t::lib::TestBuilder; > use t::lib::Mocks; >@@ -100,10 +101,10 @@ t::lib::Mocks::mock_preference('SwitchOnSiteCheckouts', 1); > is( $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED}, 1, 'If SwitchOnSiteCheckouts, switch the on-site checkout' ); > is( exists $impossible->{TOO_MANY}, '', 'If SwitchOnSiteCheckouts, switch the on-site checkout' ); > C4::Circulation::AddIssue( $patron, $item->{barcode}, undef, undef, undef, undef, { switch_onsite_checkout => 1 } ); >-my $issue = C4::Circulation::GetItemIssue( $item->{itemnumber} ); >-is( $issue->{onsite_checkout}, 0, 'The issue should have been switched to a regular checkout' ); >+my $issue = Koha::Issues->find( { itemnumber => $item->{itemnumber} } ); >+is( $issue->onsite_checkout, 0, 'The issue should have been switched to a regular checkout' ); > my $five_days_after = dt_from_string->add( days => 5 )->set( hour => 23, minute => 59, second => 0 ); >-is( $issue->{date_due}, $five_days_after, 'The date_due should have been set depending on the circ rules when the on-site checkout has been switched' ); >+is( dt_from_string($issue->date_due, 'sql'), $five_days_after, 'The date_due should have been set depending on the circ rules when the on-site checkout has been switched' ); > > # Specific case > t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1); >diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t >index c1a157d..56d0b8e 100644 >--- a/t/db_dependent/Circulation/issue.t >+++ b/t/db_dependent/Circulation/issue.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 32; >+use Test::More tests => 31; > use DateTime::Duration; > > use t::lib::Mocks; >@@ -45,7 +45,6 @@ can_ok( > AddReturn > GetBiblioIssues > GetIssuingCharges >- GetItemIssue > GetOpenIssue > GetRenewCount > GetUpcomingDueIssues >@@ -218,11 +217,6 @@ like( > #Test GetBiblioIssues > is( GetBiblioIssues(), undef, "GetBiblio Issues without parameters" ); > >-#Test GetItemIssue >-#FIXME : As the issues are not correctly added in the database, these tests don't work correctly >-is(GetItemIssue,undef,"Without parameter GetItemIssue returns undef"); >-#is(GetItemIssue($item_id1),{},"Item1's issues"); >- > #Test GetOpenIssue > is( GetOpenIssue(), undef, "Without parameter GetOpenIssue returns undef" ); > is( GetOpenIssue(-1), undef, >-- >2.9.3
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=57787">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17680
:
57787
|
57788
|
57789
|
57847
|
58431
|
58432
|
58433
|
58501
|
58918
|
58919
|
58920
|
61761
|
61762
|
61763
|
62034
|
62086
|
62102
|
62103
|
62104
|
64257
|
64258
|
64259
|
64284
|
64337
|
64805
|
64806
|
64807
|
64833
|
64866
|
64867
|
64868
|
64869
|
64870
|
64871
|
64925
|
64926
|
64927
|
64928
|
64929
|
64930
|
64931
|
65302
|
65310