Bugzilla – Attachment 64925 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), 19.74 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-07-07 19:46:07 UTC
(
hide
)
Description:
Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-07-07 19:46:07 UTC
Size:
19.74 KB
patch
obsolete
>From bb7b48de1b2c58380213dcd7dc5ae74ce7268a66 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 >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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 > >Followed test plan, works as expected >Signed-off-by: Marc Véron <veron@veron.ch> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > C4/Circulation.pm | 40 ++++++++++------------ > C4/ILSDI/Services.pm | 8 +++-- > C4/SIP/ILS/Item.pm | 7 ++-- > C4/SIP/ILS/Transaction.pm | 8 ++--- > offline_circ/enqueue_koc.pl | 8 +++-- > offline_circ/process_koc.pl | 13 +++---- > opac/opac-reserve.pl | 7 ++-- > reserve/request.pl | 7 ++-- > t/db_dependent/Circulation.t | 5 +-- > t/db_dependent/Circulation/SwitchOnSiteCheckouts.t | 7 ++-- > t/db_dependent/Circulation/issue.t | 8 +---- > 11 files changed, 60 insertions(+), 58 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 1cfa69b..6725d4b 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::Checkouts->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..... >@@ -674,7 +674,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::Checkouts->find( { itemnumber => $item->{itemnumber} } ); > my $biblioitem = GetBiblioItemData($item->{biblioitemnumber}); > $item->{'itemtype'}=$item->{'itype'}; > my $dbh = C4::Context->dbh; >@@ -832,9 +832,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 ) { >@@ -941,13 +941,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 { >@@ -968,10 +968,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} ); >@@ -1304,13 +1304,13 @@ sub AddIssue { > my $branch = _GetCircControlBranch( $item, $borrower ); > > # get actual issuing if there is one >- my $actualissue = GetItemIssue( $item->{itemnumber} ); >+ my $actualissue = Koha::Checkouts->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'}, >@@ -1322,8 +1322,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} ); >@@ -3045,9 +3044,9 @@ sub GetSoonestRenewDate { > my $dbh = C4::Context->dbh; > > my $item = GetItem($itemnumber) or return; >- my $itemissue = GetItemIssue($itemnumber) or return; >+ my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return; > >- $borrowernumber ||= $itemissue->{borrowernumber}; >+ $borrowernumber ||= $itemissue->borrowernumber; > my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) > or return; > >@@ -3066,8 +3065,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' >@@ -3105,9 +3103,9 @@ sub GetLatestAutoRenewDate { > my $dbh = C4::Context->dbh; > > my $item = GetItem($itemnumber) or return; >- my $itemissue = GetItemIssue($itemnumber) or return; >+ my $itemissue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return; > >- $borrowernumber ||= $itemissue->{borrowernumber}; >+ $borrowernumber ||= $itemissue->borrowernumber; > my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) > or return; > >@@ -3128,7 +3126,7 @@ sub GetLatestAutoRenewDate { > > my $maximum_renewal_date; > if ( $issuing_rule->no_auto_renewal_after ) { >- $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); >+ $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 c1632d5..d4d8a96 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -34,8 +34,10 @@ use CGI qw ( -utf8 ); > use DateTime; > use C4::Auth; > use C4::Members::Attributes qw(GetBorrowerAttributes); >+use Koha::DateUtils; > > use Koha::Biblios; >+use Koha::Checkouts; > use Koha::Libraries; > use Koha::Patrons; > >@@ -587,12 +589,12 @@ sub RenewLoan { > my @renewal = CanBookBeRenewed( $borrowernumber, $itemnumber ); > if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber ); } > >- my $issue = GetItemIssue($itemnumber); >+ my $issue = Koha::Checkouts->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 a6c735c..922c24c 100644 >--- a/C4/SIP/ILS/Item.pm >+++ b/C4/SIP/ILS/Item.pm >@@ -24,6 +24,7 @@ use C4::Members; > use C4::Reserves; > use Koha::Database; > use Koha::Biblios; >+use Koha::Checkouts; > use Koha::Patrons; > > =encoding UTF-8 >@@ -89,11 +90,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::Checkouts->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 $biblio = Koha::Biblios->find( $item->{biblionumber } ); > my $holds = $biblio->current_holds->unblessed; >diff --git a/C4/SIP/ILS/Transaction.pm b/C4/SIP/ILS/Transaction.pm >index c751125..011aa98 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::Checkouts; > > 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::Checkouts->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 73a5053..e7e8746 100755 >--- a/offline_circ/enqueue_koc.pl >+++ b/offline_circ/enqueue_koc.pl >@@ -33,6 +33,8 @@ use C4::Items; > use C4::Members; > use C4::Stats; > use Koha::UploadedFiles; >+use Koha::Checkouts; >+use Koha::Upload; > > use Date::Calc qw( Add_Delta_Days Date_to_Days ); > >@@ -192,7 +194,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::Checkouts->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 dbe8c40..7567dd3 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::UploadedFiles; > use Koha::Account; >+use Koha::Checkouts; > use Koha::Patrons; > > use Date::Calc qw( Add_Delta_Days Date_to_Days ); >@@ -250,11 +251,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::Checkouts->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."; >@@ -398,7 +399,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::Checkouts->find( { itemnumber => $item->{itemnumber} } ); >+ return unless $issue; >+ return $issue->borrowernumber; > } >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 9ddbb26..7bb6e7a 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -36,6 +36,7 @@ use Koha::AuthorisedValues; > use Koha::DateUtils; > use Koha::Items; > use Koha::ItemTypes; >+use Koha::Checkouts; > use Koha::Libraries; > use Koha::Patrons; > use Date::Calc qw/Today Date_to_Days/; >@@ -465,9 +466,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::Checkouts->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 db35e16..791a2bd 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::Checkouts; > use Koha::Holds; > use Koha::Items; > use Koha::ItemTypes; >@@ -383,9 +384,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::Checkouts->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 9d0f77c..7bbb25b 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -34,6 +34,7 @@ use C4::Overdues qw(UpdateFine CalcFine); > use Koha::DateUtils; > use Koha::Database; > use Koha::IssuingRules; >+use Koha::Checkouts; > use Koha::Subscriptions; > > my $schema = Koha::Database->schema; >@@ -313,7 +314,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::Checkouts->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); >@@ -1396,7 +1397,7 @@ subtest 'MultipleReserves' => sub { > my $issue = AddIssue( $renewing_borrower, $barcode1); > my $datedue = dt_from_string( $issue->date_due() ); > is (defined $issue->date_due(), 1, "item 1 checked out"); >- my $borrowing_borrowernumber = GetItemIssue($itemnumber1)->{borrowernumber}; >+ my $borrowing_borrowernumber = Koha::Checkouts->find({ itemnumber => $itemnumber1 })->borrowernumber; > > my %reserving_borrower_data1 = ( > firstname => 'Katrin', >diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >index 7b77e9c..48c2a60 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::Checkouts; > > use t::lib::TestBuilder; > use t::lib::Mocks; >@@ -106,10 +107,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::Checkouts->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 22f4d0a..312f6a9 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 => 33; >+use Test::More tests => 32; > use DateTime::Duration; > > use t::lib::Mocks; >@@ -45,7 +45,6 @@ can_ok( > AddReturn > GetBiblioIssues > GetIssuingCharges >- GetItemIssue > GetOpenIssue > GetRenewCount > GetUpcomingDueIssues >@@ -234,11 +233,6 @@ ok( $stat, "Bug 17781 - 'Improper branchcode set during renewal' still fixed" ); > #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.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=64925">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