Attachment 111774 Details for Bug 22806 – Bug 22806: (QA follow-up)

[patch] Bug 22806: (QA follow-up)

Bug-22806-QA-follow-up.patch (text/plain), 8.21 KB, created by Nick Clemens (kidclamp) on 2020-10-15 16:45:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2020-10-15 16:45:38 UTC

Size: 8.21 KB

patch

obsolete

>From 608ed7782c7c933460a135b5022eda5a540f5f4f Mon Sep 17 00:00:00 2001
>From: Arthur Suzuki <arthur.suzuki@biblibre.com>
>Date: Wed, 30 Sep 2020 10:33:12 +0200
>Subject: [PATCH] Bug 22806: (QA follow-up)
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> C4/Reserves.pm                                           |  4 ++--
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 12 ++++++------
> opac/opac-reserve.pl                                     |  9 +++------
> reserve/request.pl                                       | 14 ++++----------
> t/db_dependent/ILSDI_Services.t                          |  2 +-
> t/db_dependent/Reserves.t                                |  6 +++---
> 6 files changed, 19 insertions(+), 28 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 4827928c24..0a38aa389e 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -321,7 +321,7 @@ sub CanBookBeReserved{
>     # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
>     if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
>         && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) {
>-        return { status =>'itemAlreadyOnLoan' };
>+        return { status =>'alreadypossession' };
>     }
> 
>     my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber");
>@@ -395,7 +395,7 @@ sub CanItemBeReserved {
>     # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
>     if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
>         && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $biblio->biblionumber ) ) {
>-        return { status =>'itemAlreadyOnLoan' };
>+        return { status =>'alreadypossession' };
>     }
> 
>     my $controlbranch = C4::Context->preference('ReservesControlBranch');
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>index 1b9dd3bdee..412b21a8c0 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>@@ -200,15 +200,15 @@
>                                     [% IF ( bibitemloo.already_reserved ) %]
>                                         <div class="alert alert-warning">You have already requested this title.</div>
>                                     [% ELSE %]
>-                                        [% UNLESS ( bibitemloo.bib_available ) %]
>-                                            <div class="alert alert-warning">There are no items that can be placed on hold.</div>
>+                                        [% IF ( bibitemloo.already_patron_possession ) %]
>+                                            <div class="alert alert-warning">This title cannot be requested because it's already in your possession.</div>
>                                         [% ELSE %]
>-                                            [% IF ( bibitemloo.already_patron_possession ) %]
>-                                                <div class="alert alert-warning">This title cannot be requested because it's already in your possession.</div>
>+                                            [% UNLESS ( bibitemloo.bib_available ) %]
>+                                                <div class="alert">There are no items that can be placed on hold.</div>
>                                             [% ELSE %]
>-                                                <div class="alert alert-warning">This title cannot be requested.</div>
>+                                                <div class="alert">This title cannot be requested.</div>
>                                             [% END %]
>-                                        [% END # / UNLESS bibitemloo.bib_available %]
>+                                        [% END # / UNLESS bibitemloo.already_patron_possession %]
>                                     [% END # / IF bibitemloo.already_reserved %]
>                                 [% END # / UNLESS bibitemloo.holdable %]
> 
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index 2e3a8352ab..b0ed088689 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -601,11 +601,6 @@ foreach my $biblioNum (@biblionumbers) {
>         $biblioLoopIter{holdable} = undef;
>         $biblioLoopIter{itemholdable} = undef;
>     }
>-    if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) {
>-        $biblioLoopIter{holdable} = undef;
>-        $biblioLoopIter{already_patron_possession} = 1;
>-    }
>-
>     if ( $biblioLoopIter{holdable} ) {
>         @not_available_at = uniq @not_available_at;
>         $biblioLoopIter{not_available_at} = \@not_available_at ;
>@@ -620,7 +615,9 @@ foreach my $biblioNum (@biblionumbers) {
>         }
>     }
> 
>-    $biblioLoopIter{holdable} &&= CanBookBeReserved( $borrowernumber, $biblioNum )->{status} eq 'OK';
>+    my $status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status};
>+    $biblioLoopIter{holdable} &&= $status eq 'OK';
>+    $biblioLoopIter{already_patron_possession} = $status eq 'alreadypossession';
> 
>     # For multiple holds per record, if a patron has previously placed a hold,
>     # the patron can only place more holds of the same type. That is, if the
>diff --git a/reserve/request.pl b/reserve/request.pl
>index 503a6764c9..bb7d91e68b 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -314,6 +314,10 @@ foreach my $biblionumber (@biblionumbers) {
>                 $template->param( $canReserve->{status} => 1 );
>                 $biblioloopiter{ $canReserve->{status} } = 1;
>             }
>+            elsif ( $canReserve->{status} eq 'alreadypossession' ) {
>+                $template->param( $canReserve->{status} => 1);
>+                $biblioloopiter{ $canReserve->{status} } = 1;
>+            }
>             else {
>                 $biblioloopiter{ $canReserve->{status} } = 1;
>             }
>@@ -342,16 +346,6 @@ foreach my $biblionumber (@biblionumbers) {
>         $biblioloopiter{remaining_holds_for_record} = $max_holds_for_record;
>         $template->param( max_holds_for_record => $max_holds_for_record );
>         $template->param( remaining_holds_for_record => $remaining_holds_for_record );
>-
>-        { # alreadypossession
>-            # Check to see if patron is allowed to place holds on records where the
>-            # patron already has an item from that record checked out
>-            if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
>-                && CheckIfIssuedToPatron( $patron->borrowernumber, $biblionumber ) )
>-            {
>-                $template->param( alreadypossession => 1, );
>-            }
>-        }
>     }
> 
> 
>diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t
>index adc1ade944..c721a2496d 100755
>--- a/t/db_dependent/ILSDI_Services.t
>+++ b/t/db_dependent/ILSDI_Services.t
>@@ -424,7 +424,7 @@ subtest 'Holds test' => sub {
>     $query->param( 'pickup_location', $origin_branch->{branchcode});
>     $reply = C4::ILSDI::Services::HoldItem( $query );
> 
>-    is( $reply->{code}, 'itemAlreadyOnLoan', "Patron has issued same book" );
>+    is( $reply->{code}, 'alreadypossession', "Patron has issued same book" );
>     is( $reply->{pickup_location}, undef, "No reserve placed");
> 
>     $schema->storage->txn_rollback;
>diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
>index 52c779510b..6ddb3f4f56 100755
>--- a/t/db_dependent/Reserves.t
>+++ b/t/db_dependent/Reserves.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 64;
>+use Test::More tests => 65;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -1169,12 +1169,12 @@ subtest 'AllowHoldOnPatronPossession test' => sub {
> 
>     is(C4::Reserves::CanBookBeReserved($patron->borrowernumber,
>                                        $item->biblionumber)->{status},
>-       'itemAlreadyOnLoan',
>+       'alreadypossession',
>        'Patron cannot place hold on a book loaned to itself');
> 
>     is(C4::Reserves::CanItemBeReserved($patron->borrowernumber,
>                                        $item->itemnumber)->{status},
>-       'itemAlreadyOnLoan',
>+       'alreadypossession',
>        'Patron cannot place hold on an item loaned to itself');
> 
>     t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 1);
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 22806: 89039 | 89724 | 91784 | 94388 | 94389 | 97947 | 99881 | 99882 | 99883 | 99884 | 99885 | 100091 | 100092 | 100093 | 100173 | 100174 | 100175 | 106564 | 106778 | 106779 | 106780 | 107760 | 107806 | 110100 | 110101 | 110102 | 110973 | 110974 | 110975 | 110976 | 111771 | 111772 | 111773 | 111774 | 112824