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 307-319 Link Here
307
                    </div>
307
                    </div>
308
                [% END %]
308
                [% END %]
309
309
310
                [% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
310
                [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
311
                    <div class="dialog alert">
311
                    <div class="dialog alert">
312
312
313
                        [% UNLESS ( multi_hold ) %]
313
                        [% UNLESS ( multi_hold ) %]
314
                            <h3>Cannot place hold</h3>
314
                            <h3>Cannot place hold</h3>
315
                            <ul>
315
                            <ul>
316
                                [% IF ( exceeded_maxreserves ) %]
316
                                [% IF ( no_reserves_allowed ) %]
317
                                    <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>
318
                                [% ELSIF ( exceeded_maxreserves ) %]
317
                                    <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>
319
                                    <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>
318
                                [% ELSIF ( exceeded_holds_per_record ) %]
320
                                [% ELSIF ( exceeded_holds_per_record ) %]
319
                                    <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>
321
                                    <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 331-337 Link Here
331
                            </ul>
333
                            </ul>
332
                        [% ELSE # UNLESS multi_hold %]
334
                        [% ELSE # UNLESS multi_hold %]
333
                            <h3>Cannot place hold on some items</h3>
335
                            <h3>Cannot place hold on some items</h3>
334
                            [% IF ( exceeded_maxreserves ) %]
336
                            [% IF (no_reserves_allowed ) %]
337
                                <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>
338
                            [% ELSIF ( exceeded_maxreserves ) %]
335
                                <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
                                <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>
336
                            [% ELSIF ( exceeded_holds_per_record ) %]
340
                            [% ELSIF ( exceeded_holds_per_record ) %]
337
                                [% FOREACH biblioloo IN biblioloop %]
341
                                [% FOREACH biblioloo IN biblioloop %]
Lines 585-590 Link Here
585
                                                                        Patron already has hold for this item
589
                                                                        Patron already has hold for this item
586
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
590
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
587
                                                                        Cannot be transferred to pickup library
591
                                                                        Cannot be transferred to pickup library
592
                                                                    [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %]
593
                                                                        No reserves are allowed on this item
588
                                                                    [% ELSE %]
594
                                                                    [% ELSE %]
589
                                                                        [% itemloo.not_holdable | html %]
595
                                                                        [% itemloo.not_holdable | html %]
590
                                                                    [% END %]
596
                                                                    [% 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 / +210 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 344-377 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); Link Here
344
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
344
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
345
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
345
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
346
346
347
# Regression test for bug 9532
347
subtest 'CanItemBeReserved' => sub {
348
$biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
348
    plan tests => 2;
349
$item = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber});
350
AddReserve(
351
    {
352
        branchcode     => $branch_1,
353
        borrowernumber => $borrowernumbers[0],
354
        biblionumber   => $biblio->biblionumber,
355
        priority       => 1,
356
    }
357
);
358
is(
359
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
360
    "cannot request item if policy that matches on item-level item type forbids it"
361
);
362
349
363
$item->itype('CAN')->store;
350
    my $itemtype_can         = $builder->build({source => "Itemtype"})->{itemtype};
364
ok(
351
    my $itemtype_cant        = $builder->build({source => "Itemtype"})->{itemtype};
365
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'OK',
352
    my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
366
    "can request item if policy that matches on item type allows it"
367
);
368
353
369
t::lib::Mocks::mock_preference('item-level_itypes', 0);
354
    Koha::CirculationRules->set_rules(
370
$item->itype(undef)->store;
355
        {
371
ok(
356
            categorycode => undef,
372
    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'tooManyReserves',
357
            branchcode   => undef,
373
    "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
358
            itemtype     => $itemtype_cant,
374
);
359
            rules        => {
360
                reservesallowed  => 0,
361
                holds_per_record => 99,
362
            }
363
        }
364
    );
365
    Koha::CirculationRules->set_rules(
366
        {
367
            categorycode => undef,
368
            branchcode   => undef,
369
            itemtype     => $itemtype_can,
370
            rules        => {
371
                reservesallowed  => 2,
372
                holds_per_record => 2,
373
            }
374
        }
375
    );
376
    Koha::CirculationRules->set_rules(
377
        {
378
            categorycode => undef,
379
            branchcode   => undef,
380
            itemtype     => $itemtype_cant_record,
381
            rules        => {
382
                reservesallowed  => 0,
383
                holds_per_record => 0,
384
            }
385
        }
386
    );
387
388
    Koha::CirculationRules->set_rules(
389
        {
390
            branchcode => $branch_1,
391
            itemtype   => $itemtype_cant,
392
            rules => {
393
                holdallowed => 0,
394
                returnbranch => 'homebranch',
395
            }
396
        }
397
    );
398
    Koha::CirculationRules->set_rules(
399
        {
400
            branchcode => $branch_1,
401
            itemtype   => $itemtype_can,
402
            rules => {
403
                holdallowed => 1,
404
                returnbranch => 'homebranch',
405
            }
406
        }
407
    );
408
409
    subtest 'noReservesAllowed' => sub {
410
        plan tests => 5;
411
412
        my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
413
        my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
414
        my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
415
416
        my $itemnumber_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot })->itemnumber;
417
        my $itemnumber_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot })->itemnumber;
418
        my $itemnumber_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can })->itemnumber;
419
        my $itemnumber_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can })->itemnumber;
420
        my $itemnumber_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot })->itemnumber;
421
422
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
423
424
        t::lib::Mocks::mock_preference('item-level_itypes', 1);
425
        is(
426
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
427
            "With item level set, rule from item must be picked (CANNOT)"
428
        );
429
        is(
430
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
431
            "With item level set, rule from item must be picked (CAN)"
432
        );
433
        t::lib::Mocks::mock_preference('item-level_itypes', 0);
434
        is(
435
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
436
            "With biblio level set, rule from biblio must be picked (CANNOT)"
437
        );
438
        is(
439
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
440
            "With biblio level set, rule from biblio must be picked (CAN)"
441
        );
442
        is(
443
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
444
            "When no holds allowed and no holds per record allowed should return noReservesAllowed"
445
        );
446
    };
447
448
    subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
449
        plan tests => 7;
450
451
        my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
452
        my $itemnumber_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
453
        my $itemnumber_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
454
        my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
455
        my $itemnumber_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
456
        my $itemnumber_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
457
458
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
459
460
        # Biblio-level hold
461
        AddReserve({
462
            branch => $branch_1,
463
            borrowernumber => $borrowernumbers[0],
464
            biblionumber => $biblionumber_1,
465
        });
466
        for my $item_level ( 0..1 ) {
467
            t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
468
            is(
469
                # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
470
                CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
471
                "A biblio-level hold already exists - another hold can be placed on a specific item item"
472
            );
473
        }
474
475
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
476
        # Item-level hold
477
        AddReserve({
478
            branch => $branch_1,
479
            borrowernumber => $borrowernumbers[0],
480
            biblionumber => $biblionumber_1,
481
            itemnumber => $itemnumber_11,
482
        });
483
484
        $dbh->do('DELETE FROM circulation_rules');
485
        Koha::CirculationRules->set_rules(
486
            {
487
                categorycode => undef,
488
                branchcode   => undef,
489
                itemtype     => undef,
490
                rules        => {
491
                    reservesallowed  => 5,
492
                    holds_per_record => 1,
493
                }
494
            }
495
        );
496
        is(
497
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
498
            "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
499
        );
500
        Koha::CirculationRules->set_rules(
501
            {
502
                categorycode => undef,
503
                branchcode   => undef,
504
                itemtype     => undef,
505
                rules        => {
506
                    reservesallowed  => 1,
507
                    holds_per_record => 1,
508
                }
509
            }
510
        );
511
        is(
512
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
513
            "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
514
        );
515
        Koha::CirculationRules->set_rules(
516
            {
517
                categorycode => undef,
518
                branchcode   => undef,
519
                itemtype     => undef,
520
                rules        => {
521
                    reservesallowed  => 5,
522
                    holds_per_record => 2,
523
                }
524
            }
525
        );
526
        is(
527
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
528
            "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
529
        );
530
531
        AddReserve({
532
            branch => $branch_1,
533
            borrowernumber => $borrowernumbers[0],
534
            biblionumber => $biblionumber_2,
535
            itemnumber => $itemnumber_21
536
        });
537
        Koha::CirculationRules->set_rules(
538
            {
539
                categorycode => undef,
540
                branchcode   => undef,
541
                itemtype     => undef,
542
                rules        => {
543
                    reservesallowed  => 2,
544
                    holds_per_record => 2,
545
                }
546
            }
547
        );
548
        is(
549
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
550
            "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
551
        );
552
        is(
553
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
554
            "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
555
        );
556
    };
557
};
375
558
376
559
377
# Test branch item rules
560
# Test branch item rules
378
- 

Return to bug 16787