View | Details | Raw Unified | Return to bug 23172
Collapse All | Expand All

(-)a/C4/Circulation.pm (+1 lines)
Lines 2060-2065 sub AddReturn { Link Here
2060
    my ($resfound, $resrec);
2060
    my ($resfound, $resrec);
2061
    my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
2061
    my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
2062
    ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
2062
    ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn );
2063
2063
    if ($resfound) {
2064
    if ($resfound) {
2064
          $resrec->{'ResFound'} = $resfound;
2065
          $resrec->{'ResFound'} = $resfound;
2065
        $messages->{'ResFound'} = $resrec;
2066
        $messages->{'ResFound'} = $resrec;
(-)a/C4/Reserves.pm (-4 / +1 lines)
Lines 214-220 sub AddReserve { Link Here
214
        }
214
        }
215
    )->store();
215
    )->store();
216
    $hold->set_waiting() if $found eq 'W';
216
    $hold->set_waiting() if $found eq 'W';
217
218
    logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) )
217
    logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) )
219
        if C4::Context->preference('HoldsLog');
218
        if C4::Context->preference('HoldsLog');
220
219
Lines 378-383 sub CanItemBeReserved { Link Here
378
            found          => undef, # Found holds don't count against a patron's holds limit
377
            found          => undef, # Found holds don't count against a patron's holds limit
379
        }
378
        }
380
    );
379
    );
380
381
    if ( $holds->count() >= $holds_per_record ) {
381
    if ( $holds->count() >= $holds_per_record ) {
382
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
382
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
383
    }
383
    }
Lines 758-764 sub CheckReserves { Link Here
758
758
759
        my $priority = 10000000;
759
        my $priority = 10000000;
760
        foreach my $res (@reserves) {
760
        foreach my $res (@reserves) {
761
762
            #Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves
761
            #Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves
763
            my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber );
762
            my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber );
764
            next unless ($checkreserve eq 'OK');
763
            next unless ($checkreserve eq 'OK');
Lines 810-823 sub CheckReserves { Link Here
810
            }
809
            }
811
        }
810
        }
812
    }
811
    }
813
814
    # If we get this far, then no exact match was found.
812
    # If we get this far, then no exact match was found.
815
    # We return the most important (i.e. next) reservation.
813
    # We return the most important (i.e. next) reservation.
816
    if ($highest) {
814
    if ($highest) {
817
        $highest->{'itemnumber'} = $item;
815
        $highest->{'itemnumber'} = $item;
818
        return ( "Reserved", $highest, \@reserves );
816
        return ( "Reserved", $highest, \@reserves );
819
    }
817
    }
820
821
    return ( '' );
818
    return ( '' );
822
}
819
}
823
820
(-)a/t/db_dependent/Circulation.t (-4 / +11 lines)
Lines 259-270 $dbh->do( Link Here
259
                                issuelength, lengthunit,
259
                                issuelength, lengthunit,
260
                                renewalsallowed, renewalperiod,
260
                                renewalsallowed, renewalperiod,
261
                                norenewalbefore, auto_renew,
261
                                norenewalbefore, auto_renew,
262
                                fine, chargeperiod)
262
                                fine, chargeperiod,
263
                                holds_per_day, holds_per_record
264
                                )
263
      VALUES (?, ?, ?, ?,
265
      VALUES (?, ?, ?, ?,
264
              ?, ?,
266
              ?, ?,
265
              ?, ?,
267
              ?, ?,
266
              ?, ?,
268
              ?, ?,
267
              ?, ?
269
              ?, ?,
270
              ?,?
268
             )
271
             )
269
    },
272
    },
270
    {},
273
    {},
Lines 272-278 $dbh->do( Link Here
272
    14, 'days',
275
    14, 'days',
273
    1, 7,
276
    1, 7,
274
    undef, 0,
277
    undef, 0,
275
    .10, 1
278
    .10, 1,
279
    12,1
276
);
280
);
277
281
278
my ( $reused_itemnumber_1, $reused_itemnumber_2 );
282
my ( $reused_itemnumber_1, $reused_itemnumber_2 );
Lines 1193-1199 subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { Link Here
1193
        undef, undef, undef
1197
        undef, undef, undef
1194
    );
1198
    );
1195
1199
1196
    C4::Context->dbh->do("UPDATE issuingrules SET onshelfholds = 0");
1200
    C4::Context->dbh->do("UPDATE issuingrules SET onshelfholds = 0, holds_per_day = 1");
1197
    t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 );
1201
    t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 );
1198
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1202
    ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
1199
    is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfholds are disabled' );
1203
    is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfholds are disabled' );
Lines 2661-2666 subtest 'Set waiting flag' => sub { Link Here
2661
    my $biblio = $builder->build( { source => 'Biblio' } );
2665
    my $biblio = $builder->build( { source => 'Biblio' } );
2662
    my $biblioitem = $builder->build( { source => 'Biblioitem', value => { biblionumber => $biblio->{biblionumber} } } );
2666
    my $biblioitem = $builder->build( { source => 'Biblioitem', value => { biblionumber => $biblio->{biblionumber} } } );
2663
2667
2668
    C4::Context->dbh->do("UPDATE issuingrules SET reservesallowed = 1, holds_per_day = 1, holds_per_record = 1");
2669
2664
    my $item = $builder->build(
2670
    my $item = $builder->build(
2665
        {
2671
        {
2666
            source => 'Item',
2672
            source => 'Item',
Lines 2688-2693 subtest 'Set waiting flag' => sub { Link Here
2688
    my $hold = Koha::Holds->find( $reserve_id );
2694
    my $hold = Koha::Holds->find( $reserve_id );
2689
    is( $hold->found, 'T', 'Hold is in transit' );
2695
    is( $hold->found, 'T', 'Hold is in transit' );
2690
2696
2697
2691
    my ( $status ) = CheckReserves($item->{itemnumber});
2698
    my ( $status ) = CheckReserves($item->{itemnumber});
2692
    is( $status, 'Reserved', 'Hold is not waiting yet');
2699
    is( $status, 'Reserved', 'Hold is not waiting yet');
2693
2700
(-)a/t/db_dependent/Holds.t (-6 / +7 lines)
Lines 245-260 my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber) Link Here
245
  = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
245
  = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
246
$dbh->do('DELETE FROM issuingrules');
246
$dbh->do('DELETE FROM issuingrules');
247
$dbh->do(
247
$dbh->do(
248
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
248
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record, holds_per_day)
249
      VALUES (?, ?, ?, ?, ?)},
249
      VALUES (?, ?, ?, ?, ?, ?)},
250
    {},
250
    {},
251
    '*', '*', '*', 25, 99
251
    '*', '*', '*', 25, 99, 99
252
);
252
);
253
$dbh->do(
253
$dbh->do(
254
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
254
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record, holds_per_day)
255
      VALUES (?, ?, ?, ?, ?)},
255
      VALUES (?, ?, ?, ?, ?, ?)},
256
    {},
256
    {},
257
    '*', '*', 'CANNOT', 0, 99
257
    '*', '*', 'CANNOT', 1, 99, 99
258
);
258
);
259
259
260
# make sure some basic sysprefs are set
260
# make sure some basic sysprefs are set
Lines 331-336 AddReserve( Link Here
331
    '',
331
    '',
332
    1,
332
    1,
333
);
333
);
334
334
is(
335
is(
335
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
336
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
336
    "cannot request item if policy that matches on item-level item type forbids it"
337
    "cannot request item if policy that matches on item-level item type forbids it"
(-)a/t/db_dependent/Reserves.t (-5 / +96 lines)
Lines 17-23 Link Here
17
17
18
use Modern::Perl;
18
use Modern::Perl;
19
19
20
use Test::More tests => 62;
20
use Test::More tests => 65;
21
use Test::MockModule;
21
use Test::MockModule;
22
use Test::Warn;
22
use Test::Warn;
23
23
Lines 192-201 $requesters{$branch_3} = Koha::Patron->new({ Link Here
192
192
193
$dbh->do('DELETE FROM issuingrules');
193
$dbh->do('DELETE FROM issuingrules');
194
$dbh->do(
194
$dbh->do(
195
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
195
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record, holds_per_day)
196
      VALUES (?, ?, ?, ?)},
196
      VALUES (?, ?, ?, ?, ?, ?)},
197
    {},
197
    {},
198
    '*', '*', '*', 25
198
    '*', '*', '*', 25, 1, 1
199
);
199
);
200
200
201
# CPL allows only its own patrons to request its items
201
# CPL allows only its own patrons to request its items
Lines 290-295 my $messages; Link Here
290
# the one placed by the CPL patron, as the other two patron's hold
290
# the one placed by the CPL patron, as the other two patron's hold
291
# requests cannot be filled by that item per policy.
291
# requests cannot be filled by that item per policy.
292
(undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2);
292
(undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2);
293
293
is( $messages->{ResFound}->{borrowernumber},
294
is( $messages->{ResFound}->{borrowernumber},
294
    $requesters{$branch_1},
295
    $requesters{$branch_1},
295
    'restrictive library\'s items only fill requests by own patrons (bug 10272)');
296
    'restrictive library\'s items only fill requests by own patrons (bug 10272)');
Lines 596-601 MoveReserve( $itemnumber, $borrowernumber ); Link Here
596
is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');
597
is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');
597
$dbh->do('DELETE FROM reserves', undef, ($bibnum));
598
$dbh->do('DELETE FROM reserves', undef, ($bibnum));
598
599
600
601
602
603
604
605
#Test bug 23172. Check when returning an item that any patron category/item type combinations that forbid reservation are skipped over
606
#when allocating the item to a bib-level hold
607
$dbh->do('DELETE FROM issuingrules');
608
609
#Create two new patrons
610
my $patronnumcat1 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_1})->store->borrowernumber;
611
my $patronnumcat2 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_2})->store->borrowernumber;
612
613
#Create a new biblio record
614
my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber;
615
616
#Create a second item type
617
my $itemtype2 = $builder->build({ source => 'Itemtype', value => { notforloan => undef } } )->{itemtype};
618
619
# Create an item from new itemtype
620
my ( $item_bibnum2, $item_bibitemnum2, $itemnumber2 ) = AddItem(
621
    {   homebranch    => $branch_1,
622
        holdingbranch => $branch_1,
623
        itype         => $itemtype2,
624
        barcode       => 'bug23172_CPL'
625
    },
626
    $bibnum3
627
);
628
629
#Make a general All patron category/all item type rule
630
$dbh->do(
631
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
632
      VALUES (?, ?, ?, ?, ?, ?)},
633
    {},
634
    '*', '*', '*', 25, 1, 1
635
);
636
637
#When patron category = $category_1 and itemtype = $itemtype1 then reservesallowed, holds_per_day and holds_per_record are 0
638
$dbh->do(
639
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
640
     VALUES (?, ?, ?, ?, ?, ?)},
641
    {},
642
    $category_1, $branch_1, $itemtype2, 0, 1, 1
643
);
644
645
#When patron category = $category_2 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 1
646
$dbh->do(
647
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
648
     VALUES (?, ?, ?, ?, ?, ?)},
649
    {},
650
    $category_2, $branch_1, $itemtype2, 1, 1, 1
651
);
652
653
#Remove existing reserves
654
$dbh->do("DELETE FROM reserves");
655
656
#Create Bib-level hold for borrower who has a patron category of $category_2
657
my $reserveid2 = AddReserve($branch_1,  $patronnumcat2, $bibnum3);
658
659
#Bib-level hold for borrower who has a patron category of $category_1
660
my $reserveid1 = AddReserve($branch_1,  $patronnumcat1, $bibnum3 );
661
662
#Check bib-level hold priority and confirm hold made by patronnumcat1 (who fits into issuingrule with reserveallowed = 0) has first priority
663
my $reserve1 = Koha::Holds->find($reserveid1);
664
is( $reserve1->priority, 1, 'patronnumcat1 reserve is first priority');
665
666
my $reserve2 = Koha::Holds->find($reserveid2);
667
is( $reserve2->priority, 2, 'patronnumcat2 reserve is second priority');
668
669
#Return the item
670
(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);
671
672
#First priority hold is skipped as issuingrule is checked and the first hold does not respect issuingrules (as one of the three fields: reservesallowed, holds_per_day is 0) therefore not allocating item to this bib-level hold
673
is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');
674
675
#Remove and re-add the issuing rule that prevented patronnumcat1 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0
676
$dbh->do(q{DELETE FROM issuingrules WHERE categorycode = ? AND branchcode = ? AND itemtype = ?},
677
        {},
678
        $category_1, $branch_1, $itemtype2
679
);
680
$dbh->do(
681
   q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record)
682
    VALUES (?, ?, ?, ?, ?, ?)},
683
   {},
684
   $category_1, $branch_1, $itemtype2, 1, 1, 1
685
);
686
687
#Return the item again and notice this time it is allocated to the bib-level hold made by patronnumcat1
688
(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1);
689
is( $messages->{ResFound}->{borrowernumber}, $patronnumcat1,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)');
690
599
$cache->clear_from_cache("MarcStructure-0-$frameworkcode");
691
$cache->clear_from_cache("MarcStructure-0-$frameworkcode");
600
$cache->clear_from_cache("MarcStructure-1-$frameworkcode");
692
$cache->clear_from_cache("MarcStructure-1-$frameworkcode");
601
$cache->clear_from_cache("default_value_for_mod_marc-");
693
$cache->clear_from_cache("default_value_for_mod_marc-");
602
- 

Return to bug 23172