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

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

Return to bug 16787