From 9511513f54f7e35d72ff8d788cae302468f90977 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Thu, 9 Nov 2023 02:25:34 +0000 Subject: [PATCH] Bug 31391: Test fixes Signed-off-by: David Nind --- 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 %]
- Patron has placed one or more record-level holds. Convert a hold to a recall. + Patron has placed one or more record-level holds. Convert a hold to a recall.
[% 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