Attachment 125794 Details for Bug 15261 – Bug 15261: (QA follow-up)

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

Bug-15261-QA-follow-up.patch (text/plain), 11.15 KB, created by Alex Arnaud on 2021-10-06 12:32:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Arnaud

Created: 2021-10-06 12:32:24 UTC

Size: 11.15 KB

patch

obsolete

>From 17a68be8344442052f0591dc4504b6cb74f719e9 Mon Sep 17 00:00:00 2001
>From: Arthur Suzuki <arthur.suzuki@biblibre.com>
>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 b86a7d0..71a80bf 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1164,7 +1164,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};
>         }
>@@ -2947,7 +2947,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 6602e74..515b05a 100755
>--- a/reserve/placerequest.pl
>+++ b/reserve/placerequest.pl
>@@ -39,7 +39,6 @@ my ($template, $loggedinuser, $cookie) = get_template_and_user({
>     template_name   => 'reserve/placerequest.tt',
>     query           => $input,
>     type            => 'intranet',
>-    authnotrequired => 0,
>     flagsrequired   => { reserveforothers => 'place_holds' },
> });
> 
>@@ -70,6 +69,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 6790cf1..30d4c8c 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 => 55;
>+use Test::More tests => 57;
> use Test::Exception;
> use Test::MockModule;
> use Test::Deep qw( cmp_deeply );
>@@ -521,7 +521,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');
> 
>@@ -1739,7 +1738,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 fad19ee..e697a14 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 4d7f8af..5cfa91c 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.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 15261: 45216 | 45833 | 45841 | 46432 | 46433 | 46441 | 46442 | 49001 | 49002 | 49003 | 52704 | 59643 | 61534 | 61539 | 61544 | 67088 | 67094 | 67143 | 68660 | 68661 | 68662 | 68663 | 68664 | 68852 | 68853 | 72956 | 72957 | 76221 | 76222 | 88292 | 88293 | 88297 | 88298 | 92376 | 92377 | 92584 | 92585 | 92664 | 93136 | 94871 | 98383 | 98424 | 98672 | 106944 | 106949 | 120822 | 123405 | 125793 | 125794 | 125795 | 125796 | 131569 | 131570 | 131571 | 131572 | 131573