From e399e804bf1680f626340076231a7144ada0e048 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 1 Jul 2019 13:59:01 +0000 Subject: [PATCH] Bug 16787: Rebase fixes and updates --- C4/Reserves.pm | 4 +- .../prog/en/modules/reserve/request.tt | 4 +- t/db_dependent/Holds.t | 49 +++++++++++----------- 3 files changed, 30 insertions(+), 27 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 12ea26e835..a618c3a079 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -379,7 +379,7 @@ sub CanItemBeReserved { } ); if ( $holds_per_record == 0 ) { - return "noReservesAllowed"; + return { status => "noReservesAllowed" }; } if ( $holds->count() >= $holds_per_record ) { return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record }; @@ -425,7 +425,7 @@ sub CanItemBeReserved { # we check if it's ok or not if( $allowedreserves == 0 ){ - return 'noReservesAllowed'; + return { status => 'noReservesAllowed' }; } if ( $reservecount >= $allowedreserves ) { return { status => 'tooManyReserves', limit => $allowedreserves }; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index b75135deaa..8807a8d3c7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -129,7 +129,7 @@ [% ELSE %]

Cannot place hold on some items

[% IF ( no_reserves_allowed) %] -
  • No holds allowed: [% borrowerfirstname %] [% borrowersurname %] cannot place holds on some of these title's items.
  • +
  • No holds allowed: [% borrowerfirstname | html %] [% borrowersurname | html %] cannot place holds on some of these title's items.
  • [% END %] [% IF ( exceeded_maxreserves ) %]
  • Too many holds: [% patron.firstname | html %] [% patron.surname | html %] can place [% new_reserves_allowed | html %] of the requested [% new_reserves_count | html %] holds for a maximum of [% maxreserves | html %] total holds.
  • @@ -359,6 +359,8 @@ Patron already has hold for this item [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %] Cannot be transferred to pickup library + [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %] + No reserves are allowed on this item [% ELSE %] [% itemloo.not_holdable | html %] [% END %] diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index ef1a8aa92d..5770218bb1 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -325,8 +325,9 @@ subtest 'CanItemBeReserved' => sub { delete_all_rules(); - my $itemtype_can = $builder->build({source => "Itemtype"})->{itemtype}; - my $itemtype_cant = $builder->build({source => "Itemtype"})->{itemtype}; + my $itemtype_can = $builder->build({source => "Itemtype"})->{itemtype}; + my $itemtype_cant = $builder->build({source => "Itemtype"})->{itemtype}; + my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype}; $dbh->do( q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) VALUES (?, ?, ?, ?, ?)}, {}, @@ -338,7 +339,7 @@ subtest 'CanItemBeReserved' => sub { ); $dbh->do( q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) VALUES (?, ?, ?, ?, ?)}, {}, - '*', '*', 'CANNOT_RECORD', 0, 0 + '*', '*', $itemtype_cant_record, 0, 0 ); subtest 'noReservesAllowed' => sub { @@ -346,37 +347,37 @@ subtest 'CanItemBeReserved' => sub { my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber; my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; - my ( undef, undef, $itemnumber_1_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_cannot); - my ( undef, undef, $itemnumber_1_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblionumber_cannot); + my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber; + my ( undef, undef, $itemnumber_1_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_cannot); + my ( undef, undef, $itemnumber_1_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_cannot); - my ( undef, undef, $itemnumber_2_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_can); - my ( undef, undef, $itemnumber_2_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblionumber_can); + my ( undef, undef, $itemnumber_2_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_can); + my ( undef, undef, $itemnumber_2_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_can); - my ($biblionumber_record_cannot) = create_helper_biblio('CANNOT_RECORD'); - my ( undef, undef, $itemnumber_3_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT_RECORD' } , $biblionumber_record_cannot); + my ( undef, undef, $itemnumber_3_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record } , $biblionumber_record_cannot); Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete; t::lib::Mocks::mock_preference('item-level_itypes', 1); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot), 'noReservesAllowed', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed', "With item level set, rule from item must be picked (CANNOT)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can), 'OK', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK', "With item level set, rule from item must be picked (CAN)" ); t::lib::Mocks::mock_preference('item-level_itypes', 0); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can), 'noReservesAllowed', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed', "With biblio level set, rule from biblio must be picked (CANNOT)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot), 'OK', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK', "With biblio level set, rule from biblio must be picked (CAN)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot), 'noReservesAllowed', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed', "When no holds allowed and no holds per record allowed should return noReservesAllowed" ); }; @@ -385,11 +386,11 @@ subtest 'CanItemBeReserved' => sub { plan tests => 7; my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; - my ( undef, undef, $itemnumber_11) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_1); - my ( undef, undef, $itemnumber_12) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_1); + my ( undef, undef, $itemnumber_11) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1); + my ( undef, undef, $itemnumber_12) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1); my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; - my ( undef, undef, $itemnumber_21) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_2); - my ( undef, undef, $itemnumber_22) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblionumber_2); + my ( undef, undef, $itemnumber_21) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2); + my ( undef, undef, $itemnumber_22) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2); Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete; @@ -405,7 +406,7 @@ subtest 'CanItemBeReserved' => sub { t::lib::Mocks::mock_preference('item-level_itypes', $item_level); is( # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist - CanItemBeReserved( $borrowernumbers[0], $itemnumber_11), 'OK', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK', "A biblio-level hold already exists - another hold can be placed on a specific item item" ); } @@ -422,17 +423,17 @@ subtest 'CanItemBeReserved' => sub { ); $dbh->do(q{UPDATE issuingrules set reservesallowed=5, holds_per_record=1}); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12), 'tooManyHoldsForThisRecord', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord', "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record" ); $dbh->do(q{UPDATE issuingrules set reservesallowed=1, holds_per_record=1}); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12), 'tooManyHoldsForThisRecord', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord', "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves" ); $dbh->do(q{UPDATE issuingrules set reservesallowed=5, holds_per_record=2}); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12), 'OK', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK', "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record" ); @@ -446,11 +447,11 @@ subtest 'CanItemBeReserved' => sub { ); $dbh->do(q{UPDATE issuingrules set reservesallowed=2, holds_per_record=2}); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_21), 'itemAlreadyOnHold', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold', "A item-level holds already exists on this item, itemAlreadyOnHold should be raised" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_22), 'tooManyReserves', + CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves', "This patron has already placed reservesallowed holds, tooManyReserves should be raised" ); }; -- 2.11.0