Bugzilla – Attachment 91149 Details for
Bug 16787
'Too many holds' message appears inappropriately and is missing data
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16787: Rebase fixes and updates
Bug-16787-Rebase-fixes-and-updates.patch (text/plain), 12.18 KB, created by
Nick Clemens (kidclamp)
on 2019-07-01 14:02:51 UTC
(
hide
)
Description:
Bug 16787: Rebase fixes and updates
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2019-07-01 14:02:51 UTC
Size:
12.18 KB
patch
obsolete
>From e399e804bf1680f626340076231a7144ada0e048 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 %] > <h3>Cannot place hold on some items</h3> > [% IF ( no_reserves_allowed) %] >- <li><strong>No holds allowed: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> cannot place holds on some of these title's items.</li> >+ <li><strong>No holds allowed: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | uri %]">[% borrowerfirstname | html %] [% borrowersurname | html %] </a> cannot place holds on some of these title's items.</li> > [% END %] > [% IF ( exceeded_maxreserves ) %] > <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can place [% new_reserves_allowed | html %] of the requested [% new_reserves_count | html %] holds for a maximum of [% maxreserves | html %] total holds.</li> >@@ -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
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 16787
:
52652
|
68894
|
68895
|
72827
|
72828
|
72889
|
91146
|
91147
|
91148
|
91149
|
96152
|
97403
|
99949
|
99950
|
100951
|
101473
|
101474
|
101475
|
101895
|
107793
|
107794
|
107795
|
114675
|
114676
|
114677
|
119279
|
119280
|
119281
|
119335
|
119336
|
119337
|
119616
|
119617
|
119619
|
119620
|
119621
|
119622
|
119623
|
119738
|
119739
|
119740
|
119741
|
119942
|
120009