From bbc3a8c929593af2528a01fcaa45bec5cc377b64 Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Thu, 16 Jul 2020 10:26:23 +0200 Subject: [PATCH] Bug 15261: (QA follow-up) --- C4/Circulation.pm | 4 +- reserve/placerequest.pl | 2 +- t/db_dependent/Circulation.t | 5 +- t/db_dependent/Circulation/CanBookBeIssued.t | 40 +++++++--- t/db_dependent/Reserves/ReserveDate.t | 79 ++++++++++++-------- 5 files changed, 84 insertions(+), 46 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f45efa3d44..7583b5c7c0 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1069,7 +1069,7 @@ sub CanBookBeIssued { $needsconfirmation{ressurname} = $patron->surname; $needsconfirmation{rescardnumber} = $patron->cardnumber; $needsconfirmation{resborrowernumber} = $patron->borrowernumber; - $needsconfirmation{resbranchname} = $branchname; + $needsconfirmation{resbranchcode} = $patron->branchcode; $needsconfirmation{resreservedate} = $reserve->{reservedate}; $needsconfirmation{reserve_id} = $reserve->{reserve_id}; } @@ -2933,7 +2933,7 @@ sub CanBookBeRenewed { my $patron_unblessed = $patron->unblessed; $date_due = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ? dt_from_string( $issue->date_due, 'sql' ) : - DateTime->now( time_zone => C4::Context->tz()); + dt_from_string(); $date_due = CalcDateDue($date_due, $itemtype, _GetCircControlBranch($item->unblessed(), $patron_unblessed), $patron_unblessed, 'is a renewal'); } diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index de4d9552bb..cbeb93cf64 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -43,7 +43,6 @@ my ($template, $loggedinuser, $cookie) = get_template_and_user({ template_name => 'reserve/placerequest.tt', query => $input, type => 'intranet', - authnotrequired => 0, flagsrequired => { reserveforothers => 'place_holds' }, }); @@ -73,6 +72,7 @@ my $holds_to_place_count = $input->param('holds_to_place_count') || 1; my %bibinfos = (); my @biblionumbers = split '/', $biblionumbers; +my $multi_hold = @biblionumbers > 1; foreach my $bibnum (@biblionumbers) { my %bibinfo = (); $bibinfo{title} = $input->param("title_$bibnum"); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b957cec9ad..a33046bcfa 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 48; +use Test::More tests => 50; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -379,7 +379,6 @@ subtest "CanBookBeRenewed tests" => sub { my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $item_1->itemnumber } )->borrowernumber; is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}"); - t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary'); my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); is( $renewokay, 1, 'Can renew, no holds for this title or item'); @@ -1551,7 +1550,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { # Don't allow renewing on reserve period with PreventCheckoutOnSameReservePeriod enabled t::lib::Mocks::mock_preference( 'PreventCheckoutOnSameReservePeriod', 1 ); - my $start_issue_dt = DateTime->now(); + my $start_issue_dt = dt_from_string(); $start_issue_dt->subtract( days => 15); my $due_date = $start_issue_dt->clone; $due_date->add( days => 17 ); diff --git a/t/db_dependent/Circulation/CanBookBeIssued.t b/t/db_dependent/Circulation/CanBookBeIssued.t index fad19eea13..e697a14fa8 100644 --- a/t/db_dependent/Circulation/CanBookBeIssued.t +++ b/t/db_dependent/Circulation/CanBookBeIssued.t @@ -23,6 +23,7 @@ use C4::Reserves; use C4::Circulation; use Koha::DateUtils; +use t::lib::Mocks; use t::lib::TestBuilder; my $schema = Koha::Database->new->schema; @@ -31,10 +32,23 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new(); subtest 'Tests for CanBookBeIssued with overlap reserves' => sub { - plan tests => 6; + plan tests => 7; + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + holds_per_record => 1, + } + } + ); + t::lib::Mocks::mock_preference('PreventCheckoutOnSameReservePeriod', 1); my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; - my $branch = $builder->build({ source => 'Branch' }); + my $branch = $builder->build({ source => 'Branch', value => { branchcode => 'BRC'} }); my $branchcode = $branch->{branchcode}; my $borrower = $builder->build_object({ @@ -70,9 +84,17 @@ subtest 'Tests for CanBookBeIssued with overlap reserves' => sub { my $expdate = $startdate->clone(); $expdate->add_duration(DateTime::Duration->new(days => 10)); - my $reserveid = AddReserve($branchcode, $borrowernumber, - $item->{biblionumber}, undef, 1, $startdate->ymd(), $expdate->ymd, - undef, undef, undef, undef); + my $reserveid = AddReserve( + { + branchcode => $branchcode, + borrowernumber => $borrowernumber, + biblionumber => $item->{biblionumber}, + priority => 1, + reservation_date => $startdate->ymd, + expiration_date => $expdate->ymd, + } + ); + is(defined $reserveid, 1, "Hold has been placed"); my $non_overlap_duedate = dt_from_string(); $non_overlap_duedate->add_duration(DateTime::Duration->new(days => 2)); @@ -94,12 +116,12 @@ subtest 'Tests for CanBookBeIssued with overlap reserves' => sub { resfirstname => $borrower->firstname, ressurname => $borrower->surname, rescardnumber => $borrower->cardnumber, - resborrowernumber => $borrower->borrowernumber, - resbranchname => $branch->{branchname}, + resborrowernumber => $borrowernumber, + resbranchcode => $borrower->branchcode, resreservedate => $startdate->ymd, - resreserveid => $reserveid, + reserve_id => $reserveid, }; - is_deeply($question, $expected, ""); + is_deeply($question, $expected, "Need confirmation"); is_deeply($alerts, {}, ""); }; diff --git a/t/db_dependent/Reserves/ReserveDate.t b/t/db_dependent/Reserves/ReserveDate.t index 4d7f8afc4a..5cfa91cd58 100644 --- a/t/db_dependent/Reserves/ReserveDate.t +++ b/t/db_dependent/Reserves/ReserveDate.t @@ -28,6 +28,7 @@ use C4::Items; use C4::Members; use C4::Circulation; use Koha::Holds; +use t::lib::Mocks; use t::lib::TestBuilder; use Koha::DateUtils; @@ -35,15 +36,24 @@ use Koha::DateUtils; use_ok('C4::Reserves'); -my $dbh = C4::Context->dbh; # Start transaction +my $database = Koha::Database->new(); +my $schema = $database->schema(); +$schema->storage->txn_begin(); +my $dbh = C4::Context->dbh; +$dbh->do('DELETE FROM circulation_rules'); + $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; -my $builder = t::lib::TestBuilder->new(); +my $builder = t::lib::TestBuilder->new; + my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; -my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; +my $library = $builder->build({ source => 'Branch' })->{ branchcode }; +my $branchcode = Koha::Libraries->search->next->branchcode; + +my $frameworkcode = q//; my $borrower = $builder->build({ source => 'Borrower', @@ -64,45 +74,52 @@ my $borrower2 = $builder->build({ my $borrowernumber = $borrower->{borrowernumber}; my $borrowernumber2 = $borrower2->{borrowernumber}; -# Create a helper biblio -my $biblio = MARC::Record->new(); -my $title = 'Alone in the Dark'; -my $author = 'Karen Rose'; -if( C4::Context->preference('marcflavour') eq 'UNIMARC' ) { - $biblio->append_fields( - MARC::Field->new('600', '', '1', a => $author), - MARC::Field->new('200', '', '', a => $title), - ); -} -else { - $biblio->append_fields( - MARC::Field->new('100', '', '', a => $author), - MARC::Field->new('245', '', '', a => $title), - ); -} -my ($bibnum, $bibitemnum); -($bibnum, $title, $bibitemnum) = AddBiblio($biblio, ''); - -my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branchcode, holdingbranch => $branchcode, barcode => '333' } , $bibnum); +# Create a helper biblio and item +my $bibnum = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; +my $item = $builder->build_sample_item({ biblionumber => $bibnum, library => $branchcode, barcode => '333' }); +my ($item_bibnum, $bibitemnum, $itemnumber) = ($bibnum, $item->biblioitemnumber, $item->itemnumber); C4::Context->set_preference('AllowHoldDateInFuture', 1); - -AddReserve($branchcode, $borrowernumber, $bibnum, - $bibitemnum, 1, '2015-11-01', '2015-11-20', undef, - undef, undef, undef); +t::lib::Mocks::mock_preference('PreventReservesOnSamePeriod', 1); + +AddReserve( + { + branchcode => $branchcode, + borrowernumber => $borrowernumber, + biblionumber => $bibnum, + biblioitemnumber => $bibitemnum, + priority => 1, + reservation_date => '2015-11-01', + expiration_date => '2015-11-20', + } +); is(ReservesOnSamePeriod($bibnum, undef, '2015-11-25', '2015-11-30'), undef, "Period doesn't overlaps"); ok(ReservesOnSamePeriod($bibnum, undef, '2015-11-02', '2015-11-10'), "Period overlaps"); -my ($item_bibnum2, $item_bibitemnum2, $itemnumber2) = AddItem({ homebranch => $branchcode, holdingbranch => $branchcode, barcode => '444' } , $bibnum); +my $item2 = $builder->build_sample_item({ biblionumber => $bibnum, homebranch => $branchcode, holdingbranch => $branchcode, barcode => '444' }); +my ($item_bibnum2, $bibitemnum2, $itemnumber2) = ($bibnum, $item2->biblioitemnumber, $item2->itemnumber); + is(ReservesOnSamePeriod($bibnum, undef, '2015-11-02', '2015-11-10'), undef, "Period overlaps but there is 2 items"); -AddReserve($branchcode, $borrowernumber2, $bibnum, - $bibitemnum, 1, '2016-02-01', '2016-02-10', undef, - undef, $itemnumber, undef); + +AddReserve( + { + branchcode => $branchcode, + borrowernumber => $borrowernumber2, + biblionumber => $bibnum, + biblioitemnumber => $bibitemnum, + priority => 1, + reservation_date => '2016-02-01', + expiration_date => '2016-02-10', + itemnumber => $itemnumber, + } +); + is(ReservesOnSamePeriod($bibnum, $itemnumber, '02/12/2015', '10/12/2015'), undef, "Period on item does not overlap (with metric date format)"); my $reserve = ReservesOnSamePeriod($bibnum, $itemnumber, '2016-01-31', '2016-02-05'); is($reserve->[0]->{itemnumber}, $itemnumber, 'Period on item overlaps'); + $dbh->rollback; -- 2.20.1