Bugzilla – Attachment 107118 Details for
Bug 24683
Holds on biblios with different item types: rules for holds allowed are not applied correctly if any item type is available
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24683: Fix for take smart rules into account in "if all unavailable"
Bug-24683-Fix-for-take-smart-rules-into-account-in.patch (text/plain), 7.47 KB, created by
Agustín Moyano
on 2020-07-20 15:28:34 UTC
(
hide
)
Description:
Bug 24683: Fix for take smart rules into account in "if all unavailable"
Filename:
MIME Type:
Creator:
Agustín Moyano
Created:
2020-07-20 15:28:34 UTC
Size:
7.47 KB
patch
obsolete
>From 1d1f559ec27f672ee04eb4311b014c67479e6c50 Mon Sep 17 00:00:00 2001 >From: Andrew Nugged <nugged@gmail.com> >Date: Mon, 20 Jul 2020 00:20:56 +0300 >Subject: [PATCH] Bug 24683: Fix for take smart rules into account in "if all > unavailable" > >Inside of ItemsAnyAvailableAndNotRestricted was no effect from main set >of smart rules (per record and other limits): i.e. call to >"CanItemBeReserved" was absent totally. > >Because of this there was a bug: for example none of two items were >allowed to be held when first was allowed by one smart rule, BUT on loan, >and second was disallowed by another smart rule (for example, >0 "Holds per record"), > >i.e. in this case both items unavailable: so on-shelf holds setting >"allow hold if all unavailable" should allow to hold first one, and not >the second one. But it was that both wasn't allowed to be held. > >Solution: call to sub "CanItemBeReserved" added so it checked for >"...->{status} ne 'OK'" so now if item restricted by smart rule it also >accounted as "unavailable" and "AnyAvailavble" not counts it. > >How to reproduce: > >1. Add 2 smart rules (/cgi-bin/koha/admin/smart-rules.pl) with "on shelf > holds": "if all unavailable" for all rules, no "item level holds", and > set "holds per record" to 2 for "books" and "0" for "computer files". > >2. Create only 2 items for one biblio, but different types, "book" > and "computer file". For example in misc4dev env: > /cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additem > >3. Check out that item of type "book" to some person, for example, > in misc4dev: > /cgi-bin/koha/circ/circulation.pl?borrowernumber=2&barcode=3999900000001 > >4. Open reserve/request, for example, for item 1 and patron 1 in misc4dev > env (/cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1) > >5. It does not allow to hold, both red crossed, but computer file says > "Exceeded max holds per record" because of "0" limit set on step 1. > >6. Apply the patch. > >7. Reload page on step 5 and see that "book" will be available for hold, > but "computer file" still will be red-crossed "Exceeded max holds > per record", now that's correct because both items unavailable: > one because on load, another because of "0" limit for computer files. > >8. Check-in book from step 3 so it will be returned to the library, > >9. Reload page on step 5 and see that again no any holds available, > but it's now also correct: "book" now returned but "on shelf holds" > set to "if all unavailable". > >Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> >--- > C4/Reserves.pm | 3 +- > .../Holds/DisallowHoldIfItemsAvailable.t | 81 ++++++++++++++++++- > 2 files changed, 82 insertions(+), 2 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 4ddd443959..d7a22d62d1 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -1340,7 +1340,8 @@ sub ItemsAnyAvailableAndNotRestricted { > && ! C4::Context->preference('AllowHoldsOnDamagedItems') ) > || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan > || $branchitemrule->{holdallowed} == 1 && $param->{patron}->branchcode ne $i->homebranch >- || $branchitemrule->{holdallowed} == 3 && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } ); >+ || $branchitemrule->{holdallowed} == 3 && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } ) >+ || CanItemBeReserved( $param->{patron}->borrowernumber, $i->id )->{status} ne 'OK'; > } > > return 0; >diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >index e54efc327c..c10d2c5b62 100755 >--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >@@ -8,7 +8,7 @@ use C4::Items; > use Koha::Items; > use Koha::CirculationRules; > >-use Test::More tests => 10; >+use Test::More tests => 11; > > use t::lib::TestBuilder; > use t::lib::Mocks; >@@ -285,6 +285,85 @@ is( $is, 1, "Items availability: 1 item is available, 1 item held in T" ); > $is = IsAvailableForItemLevelRequest( $item3, $patron1); > is( $is, 1, "Item can be held, items in transit are not available" ); > >+subtest 'Check holds availability with different item types' => sub { >+ plan tests => 6; >+ >+ # Check for holds availability when different item types have different >+ # smart rules assigned both with "if all unavailable" set, >+ # and $itemtype rule allows holds, $itemtype2 rule disallows holds. >+ # So, $item should be available for hold when checked out even if $item2 >+ # is not checked out, because anyway $item2 unavailable for holds by rule >+ # (Bug 24683): >+ >+ my $biblio2 = $builder->build_sample_biblio( { itemtype => $itemtype } ); >+ my $biblionumber1 = $biblio2->biblionumber; >+ my $item4 = $builder->build_sample_item( >+ { biblionumber => $biblionumber1, >+ itype => $itemtype, >+ homebranch => $library_A, >+ holdingbranch => $library_A >+ } >+ ); >+ my $item5 = $builder->build_sample_item( >+ { biblionumber => $biblionumber1, >+ itype => $itemtype2, >+ homebranch => $library_A, >+ holdingbranch => $library_A >+ } >+ ); >+ >+ # Test hold_fulfillment_policy >+ $dbh->do("DELETE FROM circulation_rules"); >+ Koha::CirculationRules->set_rules( >+ { categorycode => undef, >+ itemtype => $itemtype, >+ branchcode => undef, >+ rules => { >+ issuelength => 7, >+ lengthunit => 8, >+ reservesallowed => 99, >+ holds_per_record => 99, >+ onshelfholds => 2, >+ } >+ } >+ ); >+ Koha::CirculationRules->set_rules( >+ { categorycode => undef, >+ itemtype => $itemtype2, >+ branchcode => undef, >+ rules => { >+ issuelength => 7, >+ lengthunit => 8, >+ reservesallowed => 0, >+ holds_per_record => 0, >+ onshelfholds => 2, >+ } >+ } >+ ); >+ >+ $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber1, patron => $patron1 } ); >+ is( $is, 1, "Items availability: 2 items, one allowed by smart rule but not checked out, another one not allowed to be held by smart rule" ); >+ >+ $is = IsAvailableForItemLevelRequest( $item4, $patron1 ); >+ is( $is, 0, "Item4 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" ); >+ >+ $is = IsAvailableForItemLevelRequest( $item5, $patron1 ); >+ is( $is, 0, "Item5 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" ); >+ >+ AddIssue( $patron2->unblessed, $item4->barcode ); >+ >+ $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber1, patron => $patron1 } ); >+ is( $is, 0, "Items availability: 2 items, one allowed by smart rule and checked out, another one not allowed to be held by smart rule" ); >+ >+ $is = IsAvailableForItemLevelRequest( $item4, $patron1 ); >+ is( $is, 1, "Item4 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" ); >+ >+ $is = IsAvailableForItemLevelRequest( $item5, $patron1 ); >+ # Note: read IsAvailableForItemLevelRequest sub description about CanItemBeReserved/CanBookBeReserved: >+ is( $is, 1, "Item5 can be requested to hold, 2 items, Item4 checked out, Item5 restricted" ); >+}; >+ >+ > # Cleanup > $schema->storage->txn_rollback; > >-- >2.25.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 24683
:
99257
|
100966
|
105651
|
105652
|
107072
|
107073
|
107074
|
107075
|
107076
|
107116
|
107117
|
107118
|
107119
|
107120
|
108905
|
108906
|
108907
|
108908
|
108909