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

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

Return to bug 16787