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

(-)a/C4/Reserves.pm (-7 / +15 lines)
Lines 457-465 sub CanItemBeReserved { Link Here
457
    $search_params->{found} = undef if $params->{ignore_found_holds};
457
    $search_params->{found} = undef if $params->{ignore_found_holds};
458
458
459
    my $holds = Koha::Holds->search($search_params);
459
    my $holds = Koha::Holds->search($search_params);
460
    if (!$params->{ignore_hold_counts} && defined $holds_per_record && $holds_per_record ne ''
460
    if (   defined $holds_per_record && $holds_per_record ne '' ){
461
        && $holds->count() >= $holds_per_record ) {
461
        if ( $holds_per_record == 0 ) {
462
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
462
            return { status => "noReservesAllowed" };
463
        }
464
        if ( !$params->{ignore_hold_counts} && $holds->count() >= $holds_per_record ) {
465
            return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
466
        }
463
    }
467
    }
464
468
465
    my $today_holds = Koha::Holds->search({
469
    my $today_holds = Koha::Holds->search({
Lines 500-508 sub CanItemBeReserved { Link Here
500
    }
504
    }
501
505
502
    # we check if it's ok or not
506
    # we check if it's ok or not
503
    if ( defined  $allowedreserves && $allowedreserves ne ''
507
    if ( defined $allowedreserves && $allowedreserves ne '' ){
504
        && $reservecount >= $allowedreserves && (!$params->{ignore_hold_counts} || $allowedreserves == 0 ) ) {
508
        if( $allowedreserves == 0 ){
505
        return { status => 'tooManyReserves', limit => $allowedreserves };
509
            return { status => 'noReservesAllowed' };
510
        }
511
        if ( !$params->{ignore_hold_counts} && $reservecount >= $allowedreserves ) {
512
            return { status => 'tooManyReserves', limit => $allowedreserves };
513
        }
506
    }
514
    }
507
515
508
    # Now we need to check hold limits by patron category
516
    # Now we need to check hold limits by patron category
Lines 526-532 sub CanItemBeReserved { Link Here
526
    my $reserves_control_branch =
534
    my $reserves_control_branch =
527
      GetReservesControlBranch( $item->unblessed(), $borrower );
535
      GetReservesControlBranch( $item->unblessed(), $borrower );
528
    my $branchitemrule =
536
    my $branchitemrule =
529
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); # FIXME Should not be item->effective_itemtype?
537
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
530
538
531
    if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) {
539
    if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) {
532
        return { status => 'notReservable' };
540
        return { status => 'notReservable' };
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt (-3 / +9 lines)
Lines 324-336 Link Here
324
                    </div>
324
                    </div>
325
                [% END %]
325
                [% END %]
326
326
327
                [% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
327
                [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
328
                    <div class="dialog alert">
328
                    <div class="dialog alert">
329
329
330
                        [% UNLESS ( multi_hold ) %]
330
                        [% UNLESS ( multi_hold ) %]
331
                            <h3>Cannot place hold</h3>
331
                            <h3>Cannot place hold</h3>
332
                            <ul>
332
                            <ul>
333
                                [% IF ( exceeded_maxreserves ) %]
333
                                [% IF ( no_reserves_allowed ) %]
334
                                    <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>
335
                                [% ELSIF ( exceeded_maxreserves ) %]
334
                                    <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>
336
                                    <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>
335
                                [% ELSIF ( exceeded_holds_per_record ) %]
337
                                [% ELSIF ( exceeded_holds_per_record ) %]
336
                                    <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>
338
                                    <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 348-354 Link Here
348
                            </ul>
350
                            </ul>
349
                        [% ELSE # UNLESS multi_hold %]
351
                        [% ELSE # UNLESS multi_hold %]
350
                            <h3>Cannot place hold on some items</h3>
352
                            <h3>Cannot place hold on some items</h3>
351
                            [% IF ( exceeded_maxreserves ) %]
353
                            [% IF (no_reserves_allowed ) %]
354
                                <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>
355
                            [% ELSIF ( exceeded_maxreserves ) %]
352
                                <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>
356
                                <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>
353
                            [% ELSIF ( exceeded_holds_per_record ) %]
357
                            [% ELSIF ( exceeded_holds_per_record ) %]
354
                                [% FOREACH biblioloo IN biblioloop %]
358
                                [% FOREACH biblioloo IN biblioloop %]
Lines 609-614 Link Here
609
                                                                        Cannot be transferred to pickup library
613
                                                                        Cannot be transferred to pickup library
610
                                                                    [% ELSIF itemloo.not_holdable == 'pickupNotInHoldGroup' %]
614
                                                                    [% ELSIF itemloo.not_holdable == 'pickupNotInHoldGroup' %]
611
                                                                        Only pickup locations within the same hold group are allowed
615
                                                                        Only pickup locations within the same hold group are allowed
616
                                                                    [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %]
617
                                                                        No reserves are allowed on this item
612
                                                                    [% ELSE %]
618
                                                                    [% ELSE %]
613
                                                                        [% itemloo.not_holdable | html %]
619
                                                                        [% itemloo.not_holdable | html %]
614
                                                                    [% END %]
620
                                                                    [% END %]
(-)a/reserve/request.pl (-1 / +8 lines)
Lines 185-190 if ($borrowernumber_hold && !$action) { Link Here
185
    my $new_reserves_count = scalar( @biblionumbers );
185
    my $new_reserves_count = scalar( @biblionumbers );
186
186
187
    my $maxreserves = C4::Context->preference('maxreserves');
187
    my $maxreserves = C4::Context->preference('maxreserves');
188
    $template->param( maxreserves => $maxreserves );
189
188
    if ( $maxreserves
190
    if ( $maxreserves
189
        && ( $reserves_count + $new_reserves_count > $maxreserves ) )
191
        && ( $reserves_count + $new_reserves_count > $maxreserves ) )
190
    {
192
    {
Lines 287-292 if ($patron) { Link Here
287
my $itemdata_enumchron = 0;
289
my $itemdata_enumchron = 0;
288
my $itemdata_ccode = 0;
290
my $itemdata_ccode = 0;
289
my @biblioloop = ();
291
my @biblioloop = ();
292
my $no_reserves_allowed = 0;
290
foreach my $biblionumber (@biblionumbers) {
293
foreach my $biblionumber (@biblionumbers) {
291
    next unless $biblionumber =~ m|^\d+$|;
294
    next unless $biblionumber =~ m|^\d+$|;
292
295
Lines 302-308 foreach my $biblionumber (@biblionumbers) { Link Here
302
305
303
                #All is OK and we can continue
306
                #All is OK and we can continue
304
            }
307
            }
305
            elsif ( $canReserve->{status} eq 'tooManyReserves' ) {
308
            elsif ( $canReserve eq 'noReservesAllowed') {
309
                $no_reserves_allowed = 1;
310
            }
311
            elsif ( $canReserve eq 'tooManyReserves' ) {
306
                $exceeded_maxreserves = 1;
312
                $exceeded_maxreserves = 1;
307
                $template->param( maxreserves => $canReserve->{limit} );
313
                $template->param( maxreserves => $canReserve->{limit} );
308
            }
314
            }
Lines 729-734 foreach my $biblionumber (@biblionumbers) { Link Here
729
735
730
$template->param( biblioloop => \@biblioloop );
736
$template->param( biblioloop => \@biblioloop );
731
$template->param( biblionumbers => $biblionumbers );
737
$template->param( biblionumbers => $biblionumbers );
738
$template->param( no_reserves_allowed => $no_reserves_allowed );
732
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
739
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
733
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
740
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
734
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
741
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
(-)a/t/db_dependent/Holds.t (-15 / +213 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 => 72;
10
use Test::More tests => 71;
11
use MARC::Record;
11
use MARC::Record;
12
12
13
use C4::Biblio;
13
use C4::Biblio;
Lines 391-416 AddReserve( Link Here
391
    }
391
    }
392
);
392
);
393
is(
393
is(
394
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
394
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'noReservesAllowed',
395
    "cannot request item if policy that matches on item-level item type forbids it"
395
    "cannot request item if policy that matches on item-level item type forbids it"
396
);
396
);
397
is(
397
is(
398
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'tooManyReserves',
398
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'noReservesAllowed',
399
    "cannot request item if policy that matches on item-level item type forbids it even if ignoring counts"
399
    "cannot request item if policy that matches on item-level item type forbids it even if ignoring counts"
400
);
400
);
401
401
402
$item->itype('CAN')->store;
402
subtest 'CanItemBeReserved' => sub {
403
ok(
403
    plan tests => 2;
404
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'OK',
405
    "can request item if policy that matches on item type allows it"
406
);
407
404
408
t::lib::Mocks::mock_preference('item-level_itypes', 0);
405
    my $itemtype_can         = $builder->build({source => "Itemtype"})->{itemtype};
409
$item->itype(undef)->store;
406
    my $itemtype_cant        = $builder->build({source => "Itemtype"})->{itemtype};
410
ok(
407
    my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
411
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'tooManyReserves',
408
412
    "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
409
    Koha::CirculationRules->set_rules(
413
);
410
        {
411
            categorycode => undef,
412
            branchcode   => undef,
413
            itemtype     => $itemtype_cant,
414
            rules        => {
415
                reservesallowed  => 0,
416
                holds_per_record => 99,
417
            }
418
        }
419
    );
420
    Koha::CirculationRules->set_rules(
421
        {
422
            categorycode => undef,
423
            branchcode   => undef,
424
            itemtype     => $itemtype_can,
425
            rules        => {
426
                reservesallowed  => 2,
427
                holds_per_record => 2,
428
            }
429
        }
430
    );
431
    Koha::CirculationRules->set_rules(
432
        {
433
            categorycode => undef,
434
            branchcode   => undef,
435
            itemtype     => $itemtype_cant_record,
436
            rules        => {
437
                reservesallowed  => 0,
438
                holds_per_record => 0,
439
            }
440
        }
441
    );
442
443
    Koha::CirculationRules->set_rules(
444
        {
445
            branchcode => $branch_1,
446
            itemtype   => $itemtype_cant,
447
            rules => {
448
                holdallowed => 0,
449
                returnbranch => 'homebranch',
450
            }
451
        }
452
    );
453
    Koha::CirculationRules->set_rules(
454
        {
455
            branchcode => $branch_1,
456
            itemtype   => $itemtype_can,
457
            rules => {
458
                holdallowed => 1,
459
                returnbranch => 'homebranch',
460
            }
461
        }
462
    );
463
464
    subtest 'noReservesAllowed' => sub {
465
        plan tests => 5;
466
467
        my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
468
        my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
469
        my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
470
471
        my $itemnumber_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot })->itemnumber;
472
        my $itemnumber_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot })->itemnumber;
473
        my $itemnumber_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can })->itemnumber;
474
        my $itemnumber_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can })->itemnumber;
475
        my $itemnumber_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot })->itemnumber;
476
477
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
478
479
        t::lib::Mocks::mock_preference('item-level_itypes', 1);
480
        is(
481
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
482
            "With item level set, rule from item must be picked (CANNOT)"
483
        );
484
        is(
485
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
486
            "With item level set, rule from item must be picked (CAN)"
487
        );
488
        t::lib::Mocks::mock_preference('item-level_itypes', 0);
489
        is(
490
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
491
            "With biblio level set, rule from biblio must be picked (CANNOT)"
492
        );
493
        is(
494
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
495
            "With biblio level set, rule from biblio must be picked (CAN)"
496
        );
497
        is(
498
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
499
            "When no holds allowed and no holds per record allowed should return noReservesAllowed"
500
        );
501
    };
502
503
    subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
504
        plan tests => 7;
505
506
        my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
507
        my $itemnumber_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
508
        my $itemnumber_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
509
        my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
510
        my $itemnumber_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
511
        my $itemnumber_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
512
513
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
514
515
        # Biblio-level hold
516
        AddReserve({
517
            branch => $branch_1,
518
            borrowernumber => $borrowernumbers[0],
519
            biblionumber => $biblionumber_1,
520
        });
521
        for my $item_level ( 0..1 ) {
522
            t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
523
            is(
524
                # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
525
                CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
526
                "A biblio-level hold already exists - another hold can be placed on a specific item item"
527
            );
528
        }
529
530
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
531
        # Item-level hold
532
        AddReserve({
533
            branch => $branch_1,
534
            borrowernumber => $borrowernumbers[0],
535
            biblionumber => $biblionumber_1,
536
            itemnumber => $itemnumber_11,
537
        });
538
539
        $dbh->do('DELETE FROM circulation_rules');
540
        Koha::CirculationRules->set_rules(
541
            {
542
                categorycode => undef,
543
                branchcode   => undef,
544
                itemtype     => undef,
545
                rules        => {
546
                    reservesallowed  => 5,
547
                    holds_per_record => 1,
548
                }
549
            }
550
        );
551
        is(
552
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
553
            "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
554
        );
555
        Koha::CirculationRules->set_rules(
556
            {
557
                categorycode => undef,
558
                branchcode   => undef,
559
                itemtype     => undef,
560
                rules        => {
561
                    reservesallowed  => 1,
562
                    holds_per_record => 1,
563
                }
564
            }
565
        );
566
        is(
567
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
568
            "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
569
        );
570
        Koha::CirculationRules->set_rules(
571
            {
572
                categorycode => undef,
573
                branchcode   => undef,
574
                itemtype     => undef,
575
                rules        => {
576
                    reservesallowed  => 5,
577
                    holds_per_record => 2,
578
                }
579
            }
580
        );
581
        is(
582
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
583
            "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
584
        );
585
586
        AddReserve({
587
            branch => $branch_1,
588
            borrowernumber => $borrowernumbers[0],
589
            biblionumber => $biblionumber_2,
590
            itemnumber => $itemnumber_21
591
        });
592
        Koha::CirculationRules->set_rules(
593
            {
594
                categorycode => undef,
595
                branchcode   => undef,
596
                itemtype     => undef,
597
                rules        => {
598
                    reservesallowed  => 2,
599
                    holds_per_record => 2,
600
                }
601
            }
602
        );
603
        is(
604
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
605
            "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
606
        );
607
        is(
608
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
609
            "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
610
        );
611
    };
612
};
414
613
415
614
416
# Test branch item rules
615
# Test branch item rules
417
- 

Return to bug 16787