Bugzilla – Attachment 67124 Details for
Bug 19302
Pass objects to IsAvailableForItemLevelRequest
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19302: Send koha::objects to C4::Reserves::IsAvailableForItemLevelRequest
Bug-19302-Send-kohaobjects-to-C4ReservesIsAvailabl.patch (text/plain), 12.71 KB, created by
Jonathan Druart
on 2017-09-13 19:25:02 UTC
(
hide
)
Description:
Bug 19302: Send koha::objects to C4::Reserves::IsAvailableForItemLevelRequest
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-09-13 19:25:02 UTC
Size:
12.71 KB
patch
obsolete
>From 58783040a6991228635f779447b3b6397579ba23 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 12 Sep 2017 15:26:27 -0300 >Subject: [PATCH] Bug 19302: Send koha::objects to > C4::Reserves::IsAvailableForItemLevelRequest > >Almost everywhere we call IsAvailableForItemLevelRequest we already have >a Koha::Patron and Koha::Item object. It makes sense to use them to >avoid a refetch > >Test plan: >It would be good to test this patch on top of 19300 and 19301 and make >sure everything works as expected >--- > C4/Circulation.pm | 20 +++++----- > C4/ILSDI/Services.pm | 11 +++--- > C4/Reserves.pm | 21 +++++------ > opac/opac-reserve.pl | 2 +- > reserve/request.pl | 2 +- > .../Holds/DisallowHoldIfItemsAvailable.t | 43 ++++++---------------- > t/db_dependent/Reserves.t | 7 ++-- > 7 files changed, 43 insertions(+), 63 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index abb4c50a66..35179b7ea4 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2659,16 +2659,16 @@ sub CanBookBeRenewed { > # can be filled with available items. We can get the union of the sets simply > # by pushing all the elements onto an array and removing the duplicates. > my @reservable; >- my %borrowers; >- ITEM: foreach my $i (@itemnumbers) { >- my $item = GetItem($i); >- next if IsItemOnHoldAndFound($i); >- for my $b (@borrowernumbers) { >- my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed; >- next unless IsAvailableForItemLevelRequest($item, $borr); >- next unless CanItemBeReserved($b,$i); >- >- push @reservable, $i; >+ my %patrons; >+ ITEM: foreach my $itemnumber (@itemnumbers) { >+ my $item = Koha::Items->find( $itemnumber ); >+ next if IsItemOnHoldAndFound( $itemnumber ); >+ for my $borrowernumber (@borrowernumbers) { >+ my $patron = $patrons{$borrowernumber} //= Koha::Patrons->find( $borrowernumber ); >+ next unless IsAvailableForItemLevelRequest($item, $patron); >+ next unless CanItemBeReserved($b,$itemnumber); >+ >+ push @reservable, $itemnumber; > if (@reservable >= @borrowernumbers) { > $resfound = 0; > last ITEM; >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index a0aec9987d..f341a593af 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -38,6 +38,7 @@ use Koha::DateUtils; > > use Koha::Biblios; > use Koha::Checkouts; >+use Koha::Items; > use Koha::Libraries; > use Koha::Patrons; > >@@ -514,17 +515,17 @@ sub GetServices { > my $borrower = $patron->unblessed; > # Get the item, or return an error code if not found > my $itemnumber = $cgi->param('item_id'); >- my $item = GetItem( $itemnumber ); >- return { code => 'RecordNotFound' } unless $$item{itemnumber}; >+ my $item = Koha::Items->find( $itemnumber ); >+ return { code => 'RecordNotFound' } unless $item; > > my @availablefor; > > # Reserve level management >- my $biblionumber = $item->{'biblionumber'}; >+ my $biblionumber = $item->biblionumber; > my $canbookbereserved = CanBookBeReserved( $borrower, $biblionumber ); > if ($canbookbereserved eq 'OK') { > push @availablefor, 'title level hold'; >- my $canitembereserved = IsAvailableForItemLevelRequest($item, $borrower); >+ my $canitembereserved = IsAvailableForItemLevelRequest($item, $patron); > if ($canitembereserved) { > push @availablefor, 'item level hold'; > } >@@ -547,7 +548,7 @@ sub GetServices { > } > > # Issuing management >- my $barcode = $item->{'barcode'} || ''; >+ my $barcode = $item->barcode || ''; > $barcode = barcodedecode($barcode) if ( $barcode && C4::Context->preference('itemBarcodeInputFilter') ); > if ($barcode) { > my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $borrower, $barcode ); >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index b97a472fb0..dad16daf3b 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -1091,35 +1091,32 @@ and canreservefromotherbranches. > =cut > > sub IsAvailableForItemLevelRequest { >- my $item = shift; >- my $borrower = shift; >+ my ( $item, $patron ) = @_; > > my $dbh = C4::Context->dbh; > # must check the notforloan setting of the itemtype > # FIXME - a lot of places in the code do this > # or something similar - need to be > # consolidated >- my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); >- my $item_object = Koha::Items->find( $item->{itemnumber } ); >- my $itemtype = $item_object->effective_itemtype; >+ my $itemtype = $item->effective_itemtype; > my $notforloan_per_itemtype > = $dbh->selectrow_array("SELECT notforloan FROM itemtypes WHERE itemtype = ?", > undef, $itemtype); > > return 0 if > $notforloan_per_itemtype || >- $item->{itemlost} || >- $item->{notforloan} > 0 || >- $item->{withdrawn} || >- ($item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems')); >+ $item->itemlost || >+ $item->notforloan > 0 || >+ $item->withdrawn || >+ ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems')); > >- my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item_object, patron => $patron } ); >+ my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); > > if ( $on_shelf_holds == 1 ) { > return 1; > } elsif ( $on_shelf_holds == 2 ) { > my @items = >- Koha::Items->search( { biblionumber => $item->{biblionumber} } ); >+ Koha::Items->search( { biblionumber => $item->biblionumber } ); > > my $any_available = 0; > >@@ -1138,7 +1135,7 @@ sub IsAvailableForItemLevelRequest { > return $any_available ? 0 : 1; > } > >- return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting"; >+ return $item->onloan || GetReserveStatus( $item->itemnumber ) eq "Waiting"; > } > > sub _get_itype { >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index dcaec358af..bfd1dc655d 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -536,7 +536,7 @@ foreach my $biblioNum (@biblionumbers) { > > my $policy_holdallowed = !$itemLoopIter->{already_reserved}; > $policy_holdallowed &&= >- IsAvailableForItemLevelRequest($itemInfo,$patron_unblessed) && >+ IsAvailableForItemLevelRequest($item, $patron) && > CanItemBeReserved($borrowernumber,$itemNum) eq 'OK'; > > if ($policy_holdallowed) { >diff --git a/reserve/request.pl b/reserve/request.pl >index 3bafa067d0..7958acc2c3 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -475,7 +475,7 @@ foreach my $biblionumber (@biblionumbers) { > if ( > !$item->{cantreserve} > && !$exceeded_maxreserves >- && IsAvailableForItemLevelRequest($item, $patron_unblessed) >+ && IsAvailableForItemLevelRequest($item_object, $patron) > && $can_item_be_reserved eq 'OK' > ) > { >diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >index ec76403286..edace55114 100755 >--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >@@ -6,6 +6,7 @@ use C4::Context; > use C4::Items; > use C4::Circulation; > use Koha::IssuingRule; >+use Koha::Items; > > use Test::More tests => 4; > >@@ -31,16 +32,16 @@ C4::Context->set_userenv(0,0,0,'firstname','surname', $library1->{branchcode}, ' > > my $bib_title = "Test Title"; > >-my $borrower1 = $builder->build({ >- source => 'Borrower', >+my $patron1 = $builder->build_object({ >+ class => 'Koha::Patrons', > value => { > branchcode => $library1->{branchcode}, > dateexpiry => '3000-01-01', > } > }); > >-my $borrower2 = $builder->build({ >- source => 'Borrower', >+my $patron2 = $builder->build_object({ >+ class => 'Koha::Patrons', > value => { > branchcode => $library1->{branchcode}, > dateexpiry => '3000-01-01', >@@ -49,8 +50,6 @@ my $borrower2 = $builder->build({ > > # Test hold_fulfillment_policy > my ( $itemtype ) = @{ $dbh->selectrow_arrayref("SELECT itemtype FROM itemtypes LIMIT 1") }; >-my $borrowernumber1 = $borrower1->{borrowernumber}; >-my $borrowernumber2 = $borrower2->{borrowernumber}; > my $library_A = $library1->{branchcode}; > > $dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')"); >@@ -64,27 +63,9 @@ my $biblioitemnumber = > $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber") > or BAIL_OUT("Cannot find newly created biblioitems record"); > >-$dbh->do(" >- INSERT INTO items (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) >- VALUES ('AllowHoldIf1', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') >-"); >+my $item1 = $builder->build_object({ class => 'Koha::Items', value => { barcode => 'AllowHoldIf1', biblionumber => $biblionumber, biblioitemnumber => $biblioitemnumber, homebranch => $library_A, holdingbranch => $library_A, notforloan => 0, damaged => 0, itemlost => 0, withdrawn => 0, onloan => undef, itype => $itemtype } }); > >-my $itemnumber1 = >- $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") >- or BAIL_OUT("Cannot find newly created item"); >- >-my $item1 = GetItem( $itemnumber1 ); >- >-$dbh->do(" >- INSERT INTO items (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) >- VALUES ('AllowHoldIf2', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') >-"); >- >-my $itemnumber2 = >- $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber ORDER BY itemnumber DESC") >- or BAIL_OUT("Cannot find newly created item"); >- >-my $item2 = GetItem( $itemnumber2 ); >+my $item2 = $builder->build_object({ class => 'Koha::Items', value => { barcode => 'AllowHoldIf2', biblionumber => $biblionumber, biblioitemnumber => $biblioitemnumber, homebranch => $library_A, holdingbranch => $library_A, notforloan => 0, damaged => 0, itemlost => 0, withdrawn => 0, onloan => undef, itype => $itemtype } }); > > $dbh->do("DELETE FROM issuingrules"); > my $rule = Koha::IssuingRule->new( >@@ -101,17 +82,17 @@ my $rule = Koha::IssuingRule->new( > ); > $rule->store(); > >-my $is = IsAvailableForItemLevelRequest( $item1, $borrower1); >+my $is = IsAvailableForItemLevelRequest( $item1, $patron1); > is( $is, 0, "Item cannot be held, 2 items available" ); > >-AddIssue( $borrower2, $item1->{barcode} ); >+AddIssue( $patron2, $item1->barcode ); > >-$is = IsAvailableForItemLevelRequest( $item1, $borrower1); >+$is = IsAvailableForItemLevelRequest( $item1, $patron1); > is( $is, 0, "Item cannot be held, 1 item available" ); > >-AddIssue( $borrower2, $item2->{barcode} ); >+AddIssue( $patron2, $item2->barcode ); > >-$is = IsAvailableForItemLevelRequest( $item1, $borrower1); >+$is = IsAvailableForItemLevelRequest( $item1, $patron1); > is( $is, 1, "Item can be held, no items available" ); > > # Cleanup >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 4db6b8a63b..cabc5a0065 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -118,7 +118,8 @@ my %data = ( > ); > Koha::Patron::Categories->find($category_1)->set({ enrolmentfee => 0})->store; > my $borrowernumber = AddMember(%data); >-my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; >+my $patron = Koha::Patrons->find( $borrowernumber ); >+my $borrower = $patron->unblessed; > my $biblionumber = $bibnum; > my $barcode = $testbarcode; > >@@ -536,9 +537,9 @@ is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Res > ####### EO Bug 13113 <<< > #### > >-$item = GetItem($itemnumber); >+$item = Koha::Items->find($itemnumber); > >-ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $borrower), "Reserving a book on item level" ); >+ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $patron), "Reserving a book on item level" ); > > # tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526) > # hold from A pos 1, today, no fut holds: MoveReserve should fill it >-- >2.11.0
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 19302
:
67124
|
68845
|
86119
|
88660
|
88987
|
89197
|
89369