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

(-)a/C4/Reserves.pm (-7 / +15 lines)
Lines 414-422 sub CanItemBeReserved { Link Here
414
            biblionumber   => $item->biblionumber,
414
            biblionumber   => $item->biblionumber,
415
        }
415
        }
416
    );
416
    );
417
    if (   defined $holds_per_record && $holds_per_record ne ''
417
    if (   defined $holds_per_record && $holds_per_record ne '' ){
418
        && $holds->count() >= $holds_per_record ) {
418
        if ( $holds_per_record == 0 ) {
419
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
419
            return { status => "noReservesAllowed" };
420
        }
421
        if ( $holds->count() >= $holds_per_record ) {
422
            return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
423
        }
420
    }
424
    }
421
425
422
    my $today_holds = Koha::Holds->search({
426
    my $today_holds = Koha::Holds->search({
Lines 457-465 sub CanItemBeReserved { Link Here
457
    }
461
    }
458
462
459
    # we check if it's ok or not
463
    # we check if it's ok or not
460
    if (   defined  $allowedreserves && $allowedreserves ne ''
464
    if (   defined  $allowedreserves && $allowedreserves ne '' ){
461
        && $reservecount >= $allowedreserves ) {
465
        if( $allowedreserves == 0 ){
462
        return { status => 'tooManyReserves', limit => $allowedreserves };
466
            return { status => 'noReservesAllowed' };
467
        }
468
        if ( $reservecount >= $allowedreserves ) {
469
            return { status => 'tooManyReserves', limit => $allowedreserves };
470
        }
463
    }
471
    }
464
472
465
    # Now we need to check hold limits by patron category
473
    # Now we need to check hold limits by patron category
Lines 483-489 sub CanItemBeReserved { Link Here
483
    my $reserves_control_branch =
491
    my $reserves_control_branch =
484
      GetReservesControlBranch( $item->unblessed(), $borrower );
492
      GetReservesControlBranch( $item->unblessed(), $borrower );
485
    my $branchitemrule =
493
    my $branchitemrule =
486
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); # FIXME Should not be item->effective_itemtype?
494
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
487
495
488
    if ( $branchitemrule->{holdallowed} == 0 ) {
496
    if ( $branchitemrule->{holdallowed} == 0 ) {
489
        return { status => 'notReservable' };
497
        return { status => 'notReservable' };
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt (-3 / +9 lines)
Lines 310-322 Link Here
310
                    </div>
310
                    </div>
311
                [% END %]
311
                [% END %]
312
312
313
                [% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
313
                [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
314
                    <div class="dialog alert">
314
                    <div class="dialog alert">
315
315
316
                        [% UNLESS ( multi_hold ) %]
316
                        [% UNLESS ( multi_hold ) %]
317
                            <h3>Cannot place hold</h3>
317
                            <h3>Cannot place hold</h3>
318
                            <ul>
318
                            <ul>
319
                                [% IF ( exceeded_maxreserves ) %]
319
                                [% IF ( no_reserves_allowed ) %]
320
                                    <li><strong>No holds allowed: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> cannot place a hold on any of these items.</li>
321
                                [% ELSIF ( exceeded_maxreserves ) %]
320
                                    <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can only place a maximum of [% maxreserves | html %] total holds.</li>
322
                                    <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can only place a maximum of [% maxreserves | html %] total holds.</li>
321
                                [% ELSIF ( exceeded_holds_per_record ) %]
323
                                [% ELSIF ( exceeded_holds_per_record ) %]
322
                                    <li><strong>Too many holds for this record: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can only place a maximum of [% max_holds_for_record | html %] hold(s) on this record.</li>
324
                                    <li><strong>Too many holds for this record: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can only place a maximum of [% max_holds_for_record | html %] hold(s) on this record.</li>
Lines 334-340 Link Here
334
                            </ul>
336
                            </ul>
335
                        [% ELSE # UNLESS multi_hold %]
337
                        [% ELSE # UNLESS multi_hold %]
336
                            <h3>Cannot place hold on some items</h3>
338
                            <h3>Cannot place hold on some items</h3>
337
                            [% IF ( exceeded_maxreserves ) %]
339
                            [% IF (no_reserves_allowed ) %]
340
                                <li><strong>No holds allowed: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> cannot place holds on some of these title's items.</li>
341
                            [% ELSIF ( exceeded_maxreserves ) %]
338
                                <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can place [% new_reserves_allowed | html %] of the requested [% new_reserves_count | html %] holds for a maximum of [% maxreserves | html %] total holds.</li>
342
                                <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | uri %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can place [% new_reserves_allowed | html %] of the requested [% new_reserves_count | html %] holds for a maximum of [% maxreserves | html %] total holds.</li>
339
                            [% ELSIF ( exceeded_holds_per_record ) %]
343
                            [% ELSIF ( exceeded_holds_per_record ) %]
340
                                [% FOREACH biblioloo IN biblioloop %]
344
                                [% FOREACH biblioloo IN biblioloop %]
Lines 588-593 Link Here
588
                                                                        Patron already has hold for this item
592
                                                                        Patron already has hold for this item
589
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
593
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
590
                                                                        Cannot be transferred to pickup library
594
                                                                        Cannot be transferred to pickup library
595
                                                                    [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %]
596
                                                                        No reserves are allowed on this item
591
                                                                    [% ELSE %]
597
                                                                    [% ELSE %]
592
                                                                        [% itemloo.not_holdable | html %]
598
                                                                        [% itemloo.not_holdable | html %]
593
                                                                    [% END %]
599
                                                                    [% END %]
(-)a/reserve/request.pl (-1 / +8 lines)
Lines 186-191 if ($borrowernumber_hold && !$action) { Link Here
186
    my $new_reserves_count = scalar( @biblionumbers );
186
    my $new_reserves_count = scalar( @biblionumbers );
187
187
188
    my $maxreserves = C4::Context->preference('maxreserves');
188
    my $maxreserves = C4::Context->preference('maxreserves');
189
    $template->param( maxreserves => $maxreserves );
190
189
    if ( $maxreserves
191
    if ( $maxreserves
190
        && ( $reserves_count + $new_reserves_count > $maxreserves ) )
192
        && ( $reserves_count + $new_reserves_count > $maxreserves ) )
191
    {
193
    {
Lines 288-293 if ($patron) { Link Here
288
my $itemdata_enumchron = 0;
290
my $itemdata_enumchron = 0;
289
my $itemdata_ccode = 0;
291
my $itemdata_ccode = 0;
290
my @biblioloop = ();
292
my @biblioloop = ();
293
my $no_reserves_allowed = 0;
291
foreach my $biblionumber (@biblionumbers) {
294
foreach my $biblionumber (@biblionumbers) {
292
    next unless $biblionumber =~ m|^\d+$|;
295
    next unless $biblionumber =~ m|^\d+$|;
293
296
Lines 303-309 foreach my $biblionumber (@biblionumbers) { Link Here
303
306
304
                #All is OK and we can continue
307
                #All is OK and we can continue
305
            }
308
            }
306
            elsif ( $canReserve->{status} eq 'tooManyReserves' ) {
309
            elsif ( $canReserve eq 'noReservesAllowed') {
310
                $no_reserves_allowed = 1;
311
            }
312
            elsif ( $canReserve eq 'tooManyReserves' ) {
307
                $exceeded_maxreserves = 1;
313
                $exceeded_maxreserves = 1;
308
                $template->param( maxreserves => $canReserve->{limit} );
314
                $template->param( maxreserves => $canReserve->{limit} );
309
            }
315
            }
Lines 736-741 foreach my $biblionumber (@biblionumbers) { Link Here
736
742
737
$template->param( biblioloop => \@biblioloop );
743
$template->param( biblioloop => \@biblioloop );
738
$template->param( biblionumbers => $biblionumbers );
744
$template->param( biblionumbers => $biblionumbers );
745
$template->param( no_reserves_allowed => $no_reserves_allowed );
739
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
746
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
740
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
747
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
741
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
748
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
(-)a/t/db_dependent/Holds.t (-28 / +212 lines)
Lines 7-13 use t::lib::TestBuilder; Link Here
7
7
8
use C4::Context;
8
use C4::Context;
9
9
10
use Test::More tests => 61;
10
use Test::More tests => 59;
11
use MARC::Record;
11
use MARC::Record;
12
12
13
use C4::Biblio;
13
use C4::Biblio;
Lines 346-378 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); Link Here
346
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
346
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
347
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
347
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
348
348
349
# Regression test for bug 9532
349
subtest 'CanItemBeReserved' => sub {
350
$biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
350
    plan tests => 2;
351
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
352
AddReserve(
353
    {
354
        branchcode     => $branch_1,
355
        borrowernumber => $borrowernumbers[0],
356
        biblionumber   => $biblio->biblionumber,
357
        priority       => 1,
358
    }
359
);
360
is(
361
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
362
    "cannot request item if policy that matches on item-level item type forbids it"
363
);
364
ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber);
365
ok(
366
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'OK',
367
    "can request item if policy that matches on item type allows it"
368
);
369
351
370
t::lib::Mocks::mock_preference('item-level_itypes', 0);
352
    my $itemtype_can         = $builder->build({source => "Itemtype"})->{itemtype};
371
ModItem({ itype => undef }, $item_bibnum, $itemnumber);
353
    my $itemtype_cant        = $builder->build({source => "Itemtype"})->{itemtype};
372
ok(
354
    my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
373
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'tooManyReserves',
355
374
    "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
356
    Koha::CirculationRules->set_rules(
375
);
357
        {
358
            categorycode => undef,
359
            branchcode   => undef,
360
            itemtype     => $itemtype_cant,
361
            rules        => {
362
                reservesallowed  => 0,
363
                holds_per_record => 99,
364
            }
365
        }
366
    );
367
    Koha::CirculationRules->set_rules(
368
        {
369
            categorycode => undef,
370
            branchcode   => undef,
371
            itemtype     => $itemtype_can,
372
            rules        => {
373
                reservesallowed  => 2,
374
                holds_per_record => 2,
375
            }
376
        }
377
    );
378
    Koha::CirculationRules->set_rules(
379
        {
380
            categorycode => undef,
381
            branchcode   => undef,
382
            itemtype     => $itemtype_cant_record,
383
            rules        => {
384
                reservesallowed  => 0,
385
                holds_per_record => 0,
386
            }
387
        }
388
    );
389
390
    Koha::CirculationRules->set_rules(
391
        {
392
            branchcode => $branch_1,
393
            itemtype   => $itemtype_cant,
394
            rules => {
395
                holdallowed => 0,
396
                returnbranch => 'homebranch',
397
            }
398
        }
399
    );
400
    Koha::CirculationRules->set_rules(
401
        {
402
            branchcode => $branch_1,
403
            itemtype   => $itemtype_can,
404
            rules => {
405
                holdallowed => 1,
406
                returnbranch => 'homebranch',
407
            }
408
        }
409
    );
410
411
    subtest 'noReservesAllowed' => sub {
412
        plan tests => 5;
413
414
        my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
415
        my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
416
        my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
417
        my ( undef, undef, $itemnumber_1_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_cannot);
418
        my ( undef, undef, $itemnumber_1_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_cannot);
419
420
        my ( undef, undef, $itemnumber_2_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_can);
421
        my ( undef, undef, $itemnumber_2_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_can);
422
423
        my ( undef, undef, $itemnumber_3_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record } , $biblionumber_record_cannot);
424
425
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
426
427
        t::lib::Mocks::mock_preference('item-level_itypes', 1);
428
        is(
429
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
430
            "With item level set, rule from item must be picked (CANNOT)"
431
        );
432
        is(
433
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
434
            "With item level set, rule from item must be picked (CAN)"
435
        );
436
        t::lib::Mocks::mock_preference('item-level_itypes', 0);
437
        is(
438
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
439
            "With biblio level set, rule from biblio must be picked (CANNOT)"
440
        );
441
        is(
442
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
443
            "With biblio level set, rule from biblio must be picked (CAN)"
444
        );
445
        is(
446
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
447
            "When no holds allowed and no holds per record allowed should return noReservesAllowed"
448
        );
449
    };
450
451
    subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
452
        plan tests => 7;
453
454
        my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
455
        my ( undef, undef, $itemnumber_11) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1);
456
        my ( undef, undef, $itemnumber_12) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1);
457
        my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
458
        my ( undef, undef, $itemnumber_21) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2);
459
        my ( undef, undef, $itemnumber_22) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2);
460
461
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
462
463
        # Biblio-level hold
464
        AddReserve({
465
            branch => $branch_1,
466
            borrowernumber => $borrowernumbers[0],
467
            biblionumber => $biblionumber_1,
468
        });
469
        for my $item_level ( 0..1 ) {
470
            t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
471
            is(
472
                # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
473
                CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
474
                "A biblio-level hold already exists - another hold can be placed on a specific item item"
475
            );
476
        }
477
478
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
479
        # Item-level hold
480
        AddReserve({
481
            branch => $branch_1,
482
            borrowernumber => $borrowernumbers[0],
483
            biblionumber => $biblionumber_1,
484
            itemnumber => $itemnumber_11,
485
        });
486
487
        $dbh->do('DELETE FROM circulation_rules');
488
        Koha::CirculationRules->set_rules(
489
            {
490
                categorycode => undef,
491
                branchcode   => undef,
492
                itemtype     => undef,
493
                rules        => {
494
                    reservesallowed  => 5,
495
                    holds_per_record => 1,
496
                }
497
            }
498
        );
499
        is(
500
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
501
            "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
502
        );
503
        Koha::CirculationRules->set_rules(
504
            {
505
                categorycode => undef,
506
                branchcode   => undef,
507
                itemtype     => undef,
508
                rules        => {
509
                    reservesallowed  => 1,
510
                    holds_per_record => 1,
511
                }
512
            }
513
        );
514
        is(
515
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
516
            "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
517
        );
518
        Koha::CirculationRules->set_rules(
519
            {
520
                categorycode => undef,
521
                branchcode   => undef,
522
                itemtype     => undef,
523
                rules        => {
524
                    reservesallowed  => 5,
525
                    holds_per_record => 2,
526
                }
527
            }
528
        );
529
        is(
530
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
531
            "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
532
        );
533
534
        AddReserve({
535
            branch => $branch_1,
536
            borrowernumber => $borrowernumbers[0],
537
            biblionumber => $biblionumber_2,
538
            itemnumber => $itemnumber_21
539
        });
540
        Koha::CirculationRules->set_rules(
541
            {
542
                categorycode => undef,
543
                branchcode   => undef,
544
                itemtype     => undef,
545
                rules        => {
546
                    reservesallowed  => 2,
547
                    holds_per_record => 2,
548
                }
549
            }
550
        );
551
        is(
552
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
553
            "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
554
        );
555
        is(
556
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
557
            "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
558
        );
559
    };
560
};
376
561
377
562
378
# Test branch item rules
563
# Test branch item rules
379
- 

Return to bug 16787