Attachment 158832 Details for Bug 31391 – Bug 31391: Test fixes

[patch] Bug 31391: Test fixes

Bug-31391-Test-fixes.patch (text/plain), 21.55 KB, created by David Nind on 2023-11-11 01:49:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2023-11-11 01:49:15 UTC

Size: 21.55 KB

patch

obsolete

>From 9511513f54f7e35d72ff8d788cae302468f90977 Mon Sep 17 00:00:00 2001
>From: Aleisha Amohia <aleishaamohia@hotmail.com>
>Date: Thu, 9 Nov 2023 02:25:34 +0000
>Subject: [PATCH] Bug 31391: Test fixes
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> C4/Reserves.pm                                |  51 ++++---
> ..._CANCELLATION-RECALLED_authorised_value.pl |  10 +-
> ...ug_31391_-_add_RecallsInterface_syspref.pl |  10 +-
> .../prog/en/modules/recalls/request.tt        |   2 +-
> recalls/request.pl                            | 137 ++++++++++--------
> reserve/modrequest.pl                         |  16 +-
> reserve/request.pl                            |   2 +-
> t/db_dependent/Holds.t                        |  65 +++++----
> 8 files changed, 162 insertions(+), 131 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 7c6281c5a0..45073418a2 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -1130,39 +1130,42 @@ sub ModReserve {
>     Koha::Exceptions::ObjectNotFound->throw( 'No hold with id ' . $reserve_id ) unless $hold;
> 
>     if ( $rank eq "del" ) {
>-        $hold->cancel({ cancellation_reason => $cancellation_reason });
>-    }
>-    elsif ( $rank eq "recall" ) {
>+        $hold->cancel( { cancellation_reason => $cancellation_reason } );
>+    } elsif ( $rank eq "recall" ) {
>         if ( !$biblionumber ) {
>             $biblionumber = $hold->biblionumber;
>         }
>-        my $biblio = Koha::Biblios->find( $biblionumber );
>+        my $biblio = Koha::Biblios->find($biblionumber);
>         if ( !$borrowernumber ) {
>             $borrowernumber = $hold->borrowernumber;
>         }
>-        my $patron = Koha::Patrons->find( $borrowernumber );
>+        my $patron = Koha::Patrons->find($borrowernumber);
>         if ( $hold->item_level_hold ) {
>-            if ( $hold->item->can_be_recalled({ patron => $patron, hold_convert => 1 }) ) {
>-                my ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall({
>-                    patron => $patron,
>-                    biblio => $biblio,
>-                    branchcode => $branchcode,
>-                    expirationdate => $date,
>-                    interface => 'intranet',
>-                    item => $hold->item,
>-                });
>-                $hold->cancel({ cancellation_reason => 'RECALLED' });
>+            if ( $hold->item->can_be_recalled( { patron => $patron, hold_convert => 1 } ) ) {
>+                my ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall(
>+                    {
>+                        patron         => $patron,
>+                        biblio         => $biblio,
>+                        branchcode     => $branchcode,
>+                        expirationdate => $date,
>+                        interface      => 'intranet',
>+                        item           => $hold->item,
>+                    }
>+                );
>+                $hold->cancel( { cancellation_reason => 'RECALLED' } );
>             }
>         } else {
>-            if ( $biblio->can_be_recalled({ patron => $patron, hold_convert => 1 }) ) {
>-                my ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall({
>-                    patron => $patron,
>-                    biblio => $biblio,
>-                    branchcode => $branchcode,
>-                    expirationdate => $date,
>-                    interface => 'intranet',
>-                });
>-                $hold->cancel({ cancellation_reason => 'RECALLED' });
>+            if ( $biblio->can_be_recalled( { patron => $patron, hold_convert => 1 } ) ) {
>+                my ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall(
>+                    {
>+                        patron         => $patron,
>+                        biblio         => $biblio,
>+                        branchcode     => $branchcode,
>+                        expirationdate => $date,
>+                        interface      => 'intranet',
>+                    }
>+                );
>+                $hold->cancel( { cancellation_reason => 'RECALLED' } );
>             }
>         }
>     }
>diff --git a/installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value.pl b/installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value.pl
>index 57967eedb8..c00dcadefb 100755
>--- a/installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value.pl
>+++ b/installer/data/mysql/atomicupdate/bug_31391_-_add_HOLD_CANCELLATION-RECALLED_authorised_value.pl
>@@ -1,13 +1,15 @@
> use Modern::Perl;
> 
> return {
>-    bug_number => "31391",
>+    bug_number  => "31391",
>     description => "Staff-side recalls",
>-    up => sub {
>+    up          => sub {
>         my ($args) = @_;
>-        my ($dbh, $out) = @$args{qw(dbh out)};
>+        my ( $dbh, $out ) = @$args{qw(dbh out)};
> 
>-        $dbh->do(q{ INSERT IGNORE INTO authorised_values (category, authorised_value, lib) VALUES ('HOLD_CANCELLATION','RECALLED','Hold was converted to a recall') });
>+        $dbh->do(
>+            q{ INSERT IGNORE INTO authorised_values (category, authorised_value, lib) VALUES ('HOLD_CANCELLATION','RECALLED','Hold was converted to a recall') }
>+        );
> 
>         say $out "Added RECALLED value to HOLD_CANCELLATION authorised value category";
>     },
>diff --git a/installer/data/mysql/atomicupdate/bug_31391_-_add_RecallsInterface_syspref.pl b/installer/data/mysql/atomicupdate/bug_31391_-_add_RecallsInterface_syspref.pl
>index 15c301fa82..17c0896d33 100755
>--- a/installer/data/mysql/atomicupdate/bug_31391_-_add_RecallsInterface_syspref.pl
>+++ b/installer/data/mysql/atomicupdate/bug_31391_-_add_RecallsInterface_syspref.pl
>@@ -1,13 +1,15 @@
> use Modern::Perl;
> 
> return {
>-    bug_number => "31391",
>+    bug_number  => "31391",
>     description => "Staff-side recalls",
>-    up => sub {
>+    up          => sub {
>         my ($args) = @_;
>-        my ($dbh, $out) = @$args{qw(dbh out)};
>+        my ( $dbh, $out ) = @$args{qw(dbh out)};
> 
>-        $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('RecallsInterface','opac','opac|staff|both','The interface that recalls can be placed through','Choice') });
>+        $dbh->do(
>+            q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('RecallsInterface','opac','opac|staff|both','The interface that recalls can be placed through','Choice') }
>+        );
> 
>         say $out "Added system preference 'RecallsInterface'";
>     },
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt
>index 7316306425..d4db78c459 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt
>@@ -59,7 +59,7 @@
> 
>         [% IF patron_holds_count %]
>             <div class="dialog message">
>-                Patron has placed one or more record-level holds. <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% biblio.biblionumber | html %]">Convert a hold to a recall</a>.
>+                Patron has placed one or more record-level holds. <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% biblio.biblionumber | uri %]">Convert a hold to a recall</a>.
>             </div>
>         [% END %]
> 
>diff --git a/recalls/request.pl b/recalls/request.pl
>index 5839ad76c1..e6e3a2373f 100755
>--- a/recalls/request.pl
>+++ b/recalls/request.pl
>@@ -35,72 +35,84 @@ my ($template, $loggedinuser, $cookie)= get_template_and_user(
>     }
> );
> 
>-my $op = $input->param('op') || 'list';
>-my @recall_ids = $input->multi_param('recall_ids');
>-my $biblionumber = $input->param('biblionumber');
>-my $biblio = Koha::Biblios->find( $biblionumber );
>+my $op             = $input->param('op') || 'list';
>+my @recall_ids     = $input->multi_param('recall_ids');
>+my $biblionumber   = $input->param('biblionumber');
>+my $biblio         = Koha::Biblios->find($biblionumber);
> my $borrowernumber = $input->param('borrowernumber');
>-my $error = $input->param('error');
>+my $error          = $input->param('error');
> 
> if ( $op eq 'cancel_multiple_recalls' ) {
>-    foreach my $id ( @recall_ids ) {
>-        Koha::Recalls->find( $id )->set_cancelled;
>+    foreach my $id (@recall_ids) {
>+        Koha::Recalls->find($id)->set_cancelled;
>     }
>-    print $input->redirect('/cgi-bin/koha/recalls/request.pl?biblionumber='.$biblionumber);
>+    print $input->redirect( '/cgi-bin/koha/recalls/request.pl?biblionumber=' . $biblionumber );
> }
> elsif ( $op eq 'request' ) {
> 
>-    if ( C4::Context->preference('UseRecalls') and C4::Context->preference('RecallsInterface') ne 'opac' and $borrowernumber ) {
>-        my $patron = Koha::Patrons->find( $borrowernumber );
>+    if (    C4::Context->preference('UseRecalls')
>+        and C4::Context->preference('RecallsInterface') ne 'opac'
>+        and $borrowernumber )
>+    {
>+        my $patron = Koha::Patrons->find($borrowernumber);
> 
>-        unless ( $biblio->can_be_recalled({ patron => $patron }) ) { $error = 'unavailable'; }
>-        my $items = Koha::Items->search({ biblionumber => $biblionumber })->as_list;
>+        unless ( $biblio->can_be_recalled( { patron => $patron } ) ) { $error = 'unavailable'; }
>+        my $items = Koha::Items->search( { biblionumber => $biblionumber } )->as_list;
> 
>         # check if already recalled
>-        my $recalled = $biblio->recalls->filter_by_current->search({ patron_id => $borrowernumber })->count;
>+        my $recalled = $biblio->recalls->filter_by_current->search( { patron_id => $borrowernumber } )->count;
>         if ( defined $recalled and $recalled > 0 ) {
>-            my $recalls_per_record = Koha::CirculationRules->get_effective_rule({
>-                categorycode => $patron->categorycode,
>-                branchcode => undef,
>-                itemtype => undef,
>-                rule_name => 'recalls_per_record'
>-            });
>-            if ( defined $recalls_per_record and $recalls_per_record->rule_value and $recalled >= $recalls_per_record->rule_value ){
>+            my $recalls_per_record = Koha::CirculationRules->get_effective_rule(
>+                {
>+                    categorycode => $patron->categorycode,
>+                    branchcode   => undef,
>+                    itemtype     => undef,
>+                    rule_name    => 'recalls_per_record'
>+                }
>+            );
>+            if (    defined $recalls_per_record
>+                and $recalls_per_record->rule_value
>+                and $recalled >= $recalls_per_record->rule_value )
>+            {
>                 $error = 'duplicate';
>             }
>         }
> 
>-        if ( !defined $error ){
>-            my $pickuploc = $input->param('pickup');
>-            my $expdate = $input->param('expirationdate');
>-            my $level = $input->param('type');
>+        if ( !defined $error ) {
>+            my $pickuploc  = $input->param('pickup');
>+            my $expdate    = $input->param('expirationdate');
>+            my $level      = $input->param('type');
>             my $itemnumber = $input->param('itemnumber');
> 
>             my ( $recall, $due_interval, $due_date );
> 
>             if ( !defined $level and defined $itemnumber ) {
>-                my $item = Koha::Items->find( $itemnumber );
>-                if ( $item->can_be_recalled({ patron => $patron }) ) {
>-                    ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall({
>-                        patron => $patron,
>-                        biblio => $biblio,
>-                        branchcode => $pickuploc,
>-                        item => $item,
>-                        expirationdate => $expdate,
>-                        interface => 'staff',
>-                    });
>+                my $item = Koha::Items->find($itemnumber);
>+                if ( $item->can_be_recalled( { patron => $patron } ) ) {
>+                    ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall(
>+                        {
>+                            patron         => $patron,
>+                            biblio         => $biblio,
>+                            branchcode     => $pickuploc,
>+                            item           => $item,
>+                            expirationdate => $expdate,
>+                            interface      => 'staff',
>+                        }
>+                    );
>                 } else {
>                     $error = 'cannot';
>                 }
>             } else {
>-                if ( $biblio->can_be_recalled({ patron => $patron }) ) {
>-                    ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall({
>-                        patron => $patron,
>-                        biblio => $biblio,
>-                        branchcode => $pickuploc,
>-                        expirationdate => $expdate,
>-                        interface => 'staff',
>-                    });
>+                if ( $biblio->can_be_recalled( { patron => $patron } ) ) {
>+                    ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall(
>+                        {
>+                            patron         => $patron,
>+                            biblio         => $biblio,
>+                            branchcode     => $pickuploc,
>+                            expirationdate => $expdate,
>+                            interface      => 'staff',
>+                        }
>+                    );
>                 } else {
>                     $error = 'cannot';
>                 }
>@@ -108,6 +120,7 @@ elsif ( $op eq 'request' ) {
>             if ( !defined $recall ) {
>                 $error = 'failed';
>             } else {
>+
>                 # successful recall, go back to Recalls tab
>                 print $input->redirect("/cgi-bin/koha/recalls/request.pl?biblionumber=$biblionumber");
>             }
>@@ -115,38 +128,44 @@ elsif ( $op eq 'request' ) {
>     }
> }
> 
>-if ( $borrowernumber ) {
>-    my $patron = Koha::Patrons->find( $borrowernumber );
>+if ($borrowernumber) {
>+    my $patron = Koha::Patrons->find($borrowernumber);
> 
>-    if ( C4::Context->preference('UseRecalls') and C4::Context->preference('RecallsInterface') ne 'opac' and $patron and $patron->borrowernumber ) {
>-        if ( !$biblio->can_be_recalled({ patron => $patron }) ) {
>+    if (    C4::Context->preference('UseRecalls')
>+        and C4::Context->preference('RecallsInterface') ne 'opac'
>+        and $patron
>+        and $patron->borrowernumber )
>+    {
>+        if ( !$biblio->can_be_recalled( { patron => $patron } ) ) {
>             $error = 1;
>         }
>-        my $patron_holds_count = Koha::Holds->search({ borrowernumber => $borrowernumber, biblionumber => $biblio->biblionumber, item_level_hold => 0 })->count;
>+        my $patron_holds_count = Koha::Holds->search(
>+            { borrowernumber => $borrowernumber, biblionumber => $biblio->biblionumber, item_level_hold => 0 } )->count;
> 
>         $template->param(
>-            patron => $patron,
>+            patron             => $patron,
>             patron_holds_count => $patron_holds_count,
>         );
>     }
> }
> 
>-my $recalls = Koha::Recalls->search({ biblio_id => $biblionumber, completed => 0 });
>-my $branches = Koha::Libraries->search;
>+my $recalls            = Koha::Recalls->search( { biblio_id => $biblionumber, completed => 0 } );
>+my $branches           = Koha::Libraries->search;
> my $single_branch_mode = $branches->count == 1;
>-my $items = Koha::Items->search({ biblionumber => $biblionumber });
>+my $items              = Koha::Items->search( { biblionumber => $biblionumber } );
> 
> $template->param(
>-    recalls     => $recalls,
>-    recallsview => 1,
>-    biblio      => $biblio,
>-    checkboxes  => 1,
>-    branches    => $branches,
>-    items       => $items,
>-    error       => $error,
>+    recalls            => $recalls,
>+    recallsview        => 1,
>+    biblio             => $biblio,
>+    checkboxes         => 1,
>+    branches           => $branches,
>+    items              => $items,
>+    error              => $error,
>     single_branch_mode => $single_branch_mode,
>     C4::Search::enabled_staff_search_views,
>-    attribute_type_codes => ( C4::Context->preference('ExtendedPatronAttributes')
>+    attribute_type_codes => (
>+        C4::Context->preference('ExtendedPatronAttributes')
>         ? [ Koha::Patron::Attribute::Types->search( { staff_searchable => 1 } )->get_column('code') ]
>         : []
>     ),
>diff --git a/reserve/modrequest.pl b/reserve/modrequest.pl
>index 5be38d35e5..8cffc152fe 100755
>--- a/reserve/modrequest.pl
>+++ b/reserve/modrequest.pl
>@@ -83,7 +83,7 @@ else {
> 
>         if ( $rank[$i] eq 'recall' and !$recallerror ) {
>             my $hold = Koha::Holds->find( $reserve_id[$i] );
>-            if ( !$hold->biblio->can_be_recalled({ patron => $hold->patron }) ) {
>+            if ( !$hold->biblio->can_be_recalled( { patron => $hold->patron } ) ) {
>                 $recallerror = 1;
>             }
>         }
>@@ -127,11 +127,11 @@ if ( $from eq 'borrower'){
> } elsif ( $from eq 'circ'){
>     print $query->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrower[0]");
> } else {
>-     my $url = URI->new("/cgi-bin/koha/reserve/request.pl");
>-     if ( $recallerror ) {
>-         $url->query_form( biblionumber => [@biblionumber], recallerror => 1 );
>-     } else {
>-         $url->query_form( biblionumber => [@biblionumber] );
>-     }
>-     print $query->redirect($url);
>+    my $url = URI->new("/cgi-bin/koha/reserve/request.pl");
>+    if ($recallerror) {
>+        $url->query_form( biblionumber => [@biblionumber], recallerror => 1 );
>+    } else {
>+        $url->query_form( biblionumber => [@biblionumber] );
>+    }
>+    print $query->redirect($url);
> }
>diff --git a/reserve/request.pl b/reserve/request.pl
>index 6cd144b5c4..1e5f176b28 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -710,7 +710,7 @@ $template->param( pickup => $pickup || C4::Context->userenv->{branch} );
> $template->param(borrowernumber => $borrowernumber_hold);
> 
> my $recallerror = $input->param('recallerror');
>-if ( $recallerror ) {
>+if ($recallerror) {
>     $template->param( recallerror => 1 );
> }
> 
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index d33b3a41d0..201ab70a41 100755
>--- a/t/db_dependent/Holds.t
>+++ b/t/db_dependent/Holds.t
>@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
> 
> use C4::Context;
> 
>-use Test::More tests => 74;
>+use Test::More tests => 75;
> use Test::Exception;
> 
> use MARC::Record;
>@@ -1893,11 +1893,13 @@ subtest 'ModReserve to convert a hold to a recall' => sub {
> 
>     $schema->storage->txn_begin;
> 
>-    my $category = $builder->build({ source => 'Category' });
>-    my $branch = $builder->build({ source => 'Branch' })->{ branchcode };
>+    my $category = $builder->build( { source => 'Category' } );
>+    my $branch   = $builder->build( { source => 'Branch' } )->{branchcode};
>     my $biblio = $builder->build_sample_biblio( { itemtype => 'DUMMY' } );
>-    my $item = $builder->build_sample_item(
>-        { library => $branch, biblionumber => $biblio->biblionumber } );
>+    my $itype  = $builder->build( { source => 'Itemtype' } );
>+    my $item =
>+        $builder->build_sample_item(
>+        { library => $branch, biblionumber => $biblio->biblionumber, itype => $itype->{itemtype} } );
> 
>     my $patron = Koha::Patron->new(
>         {
>@@ -1920,44 +1922,47 @@ subtest 'ModReserve to convert a hold to a recall' => sub {
>         }
>     )->store;
> 
>-    t::lib::Mocks::mock_preference("UseRecalls", 1);
>+    t::lib::Mocks::mock_preference( "UseRecalls", 1 );
>     t::lib::Mocks::mock_userenv( { branchcode => $branch } );
> 
>-    C4::Circulation::AddIssue( $patron2->unblessed, $item->barcode );
>+    C4::Circulation::AddIssue( $patron2, $item->barcode );
> 
>-    Koha::CirculationRules->set_rules({
>-        branchcode => undef,
>-        categorycode => undef,
>-        itemtype => undef,
>-        rules => {
>-            recalls_allowed => 5,
>-            recalls_per_record => 1,
>-            on_shelf_recalls => 'any',
>-        },
>-    });
>+    Koha::CirculationRules->set_rules(
>+        {
>+            branchcode   => undef,
>+            categorycode => undef,
>+            itemtype     => undef,
>+            rules        => {
>+                recalls_allowed    => 5,
>+                recalls_per_record => 1,
>+                on_shelf_recalls   => 'any',
>+            },
>+        }
>+    );
> 
>-    my $notice = Koha::Notice::Template->new({
>-        name                   => 'Hold cancellation',
>-        module                 => 'reserves',
>-        code                   => 'HOLD_CANCELLATION',
>-        title                  => 'Hold cancelled',
>-        content                => 'Your hold was cancelled.',
>-        message_transport_type => 'email',
>-        branchcode             => q{},
>-    })->store();
>+    my $notice = Koha::Notice::Template->new(
>+        {
>+            name                   => 'Hold cancellation',
>+            module                 => 'reserves',
>+            code                   => 'HOLD_CANCELLATION',
>+            title                  => 'Hold cancelled',
>+            content                => 'Your hold was cancelled.',
>+            message_transport_type => 'email',
>+            branchcode             => q{},
>+        }
>+    )->store();
> 
>     my $reserve_id = AddReserve(
>         {
>             branchcode     => $branch,
>             borrowernumber => $borrowernumber,
>             biblionumber   => $biblio->biblionumber,
>-            priority       =>
>-              C4::Reserves::CalculatePriority( $biblio->biblionumber ),
>-            itemnumber => $item->id,
>+            priority       => C4::Reserves::CalculatePriority( $biblio->biblionumber ),
>+            itemnumber     => $item->id,
>         }
>     );
> 
>-    my $hold = Koha::Holds->find($reserve_id);
>+    my $hold           = Koha::Holds->find($reserve_id);
>     my $before_recalls = Koha::Recalls->search->filter_by_current->count;
> 
>     ModReserve(
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 31391: 139442 | 139443 | 139444 | 139446 | 139642 | 139643 | 139644 | 139645 | 141474 | 142594 | 143155 | 144104 | 144105 | 144131 | 144132 | 144133 | 144134 | 144135 | 144136 | 147581 | 147582 | 150485 | 150486 | 150487 | 150488 | 150489 | 150490 | 150491 | 156995 | 156996 | 156997 | 156998 | 156999 | 157000 | 157001 | 158691 | 158692 | 158693 | 158694 | 158695 | 158825 | 158826 | 158827 | 158828 | 158829 | 158830 | 158831 | 158832 | 172166 | 172167 | 172168 | 172169 | 172170 | 172171 | 172172 | 172173 | 172174 | 172175 | 173839 | 173840 | 173841 | 173842 | 173843 | 173844 | 173845 | 173846 | 173847 | 174396 | 174822 | 174823 | 174824 | 174825 | 174826 | 174827 | 174828 | 174829 | 174830 | 174831