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

(-)a/C4/Reserves.pm (-1 / +7 lines)
Lines 394-399 sub CanItemBeReserved { Link Here
394
            found          => undef, # Found holds don't count against a patron's holds limit
394
            found          => undef, # Found holds don't count against a patron's holds limit
395
        }
395
        }
396
    );
396
    );
397
    if ( $holds_per_record == 0 ) {
398
        return { status => "noReservesAllowed" };
399
    }
397
    if ( $holds->count() >= $holds_per_record ) {
400
    if ( $holds->count() >= $holds_per_record ) {
398
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
401
        return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
399
    }
402
    }
Lines 437-442 sub CanItemBeReserved { Link Here
437
    }
440
    }
438
441
439
    # we check if it's ok or not
442
    # we check if it's ok or not
443
    if( $allowedreserves == 0 ){
444
        return { status => 'noReservesAllowed' };
445
    }
440
    if ( $reservecount >= $allowedreserves ) {
446
    if ( $reservecount >= $allowedreserves ) {
441
        return { status => 'tooManyReserves', limit => $allowedreserves };
447
        return { status => 'tooManyReserves', limit => $allowedreserves };
442
    }
448
    }
Lines 462-468 sub CanItemBeReserved { Link Here
462
    my $reserves_control_branch =
468
    my $reserves_control_branch =
463
      GetReservesControlBranch( $item->unblessed(), $borrower );
469
      GetReservesControlBranch( $item->unblessed(), $borrower );
464
    my $branchitemrule =
470
    my $branchitemrule =
465
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); # FIXME Should not be item->effective_itemtype?
471
      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
466
472
467
    if ( $branchitemrule->{holdallowed} == 0 ) {
473
    if ( $branchitemrule->{holdallowed} == 0 ) {
468
        return { status => 'notReservable' };
474
        return { status => 'notReservable' };
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt (-3 / +9 lines)
Lines 309-321 Link Here
309
                    </div>
309
                    </div>
310
                [% END %]
310
                [% END %]
311
311
312
                [% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
312
                [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
313
                    <div class="dialog alert">
313
                    <div class="dialog alert">
314
314
315
                        [% UNLESS ( multi_hold ) %]
315
                        [% UNLESS ( multi_hold ) %]
316
                            <h3>Cannot place hold</h3>
316
                            <h3>Cannot place hold</h3>
317
                            <ul>
317
                            <ul>
318
                                [% IF ( exceeded_maxreserves ) %]
318
                                [% IF ( no_reserves_allowed ) %]
319
                                    <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>
320
                                [% ELSIF ( exceeded_maxreserves ) %]
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>
321
                                    <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>
320
                                [% ELSIF ( exceeded_holds_per_record ) %]
322
                                [% ELSIF ( exceeded_holds_per_record ) %]
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>
323
                                    <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 333-339 Link Here
333
                            </ul>
335
                            </ul>
334
                        [% ELSE # UNLESS multi_hold %]
336
                        [% ELSE # UNLESS multi_hold %]
335
                            <h3>Cannot place hold on some items</h3>
337
                            <h3>Cannot place hold on some items</h3>
336
                            [% IF ( exceeded_maxreserves ) %]
338
                            [% IF (no_reserves_allowed ) %]
339
                                <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>
340
                            [% ELSIF ( exceeded_maxreserves ) %]
337
                                <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>
341
                                <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>
338
                            [% ELSIF ( exceeded_holds_per_record ) %]
342
                            [% ELSIF ( exceeded_holds_per_record ) %]
339
                                [% FOREACH biblioloo IN biblioloop %]
343
                                [% FOREACH biblioloo IN biblioloop %]
Lines 580-585 Link Here
580
                                                                        Patron already has hold for this item
584
                                                                        Patron already has hold for this item
581
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
585
                                                                    [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
582
                                                                        Cannot be transferred to pickup library
586
                                                                        Cannot be transferred to pickup library
587
                                                                    [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %]
588
                                                                        No reserves are allowed on this item
583
                                                                    [% ELSE %]
589
                                                                    [% ELSE %]
584
                                                                        [% itemloo.not_holdable | html %]
590
                                                                        [% itemloo.not_holdable | html %]
585
                                                                    [% END %]
591
                                                                    [% 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 719-724 foreach my $biblionumber (@biblionumbers) { Link Here
719
725
720
$template->param( biblioloop => \@biblioloop );
726
$template->param( biblioloop => \@biblioloop );
721
$template->param( biblionumbers => $biblionumbers );
727
$template->param( biblionumbers => $biblionumbers );
728
$template->param( no_reserves_allowed => $no_reserves_allowed );
722
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
729
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
723
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
730
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
724
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
731
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
(-)a/t/db_dependent/Holds.t (-30 / +166 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 => 59;
10
use Test::More tests => 57;
11
use MARC::Record;
11
use MARC::Record;
12
12
13
use C4::Biblio;
13
use C4::Biblio;
Lines 243-249 is( $hold->priority, '6', "Test AlterPriority(), move to bottom" ); Link Here
243
my $foreign_biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
243
my $foreign_biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
244
my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber)
244
my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber)
245
  = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
245
  = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
246
$dbh->do('DELETE FROM issuingrules');
246
delete_all_rules();
247
$dbh->do(
247
$dbh->do(
248
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
248
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
249
      VALUES (?, ?, ?, ?, ?)},
249
      VALUES (?, ?, ?, ?, ?)},
Lines 321-358 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); Link Here
321
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
321
ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
322
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
322
ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
323
323
324
# Regression test for bug 9532
324
subtest 'CanItemBeReserved' => sub {
325
$biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
325
    plan tests => 2;
326
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
327
AddReserve(
328
    $branch_1,
329
    $borrowernumbers[0],
330
    $biblio->biblionumber,
331
    '',
332
    1,
333
);
334
is(
335
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
336
    "cannot request item if policy that matches on item-level item type forbids it"
337
);
338
ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber);
339
ok(
340
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'OK',
341
    "can request item if policy that matches on item type allows it"
342
);
343
326
344
t::lib::Mocks::mock_preference('item-level_itypes', 0);
327
    delete_all_rules();
345
ModItem({ itype => undef }, $item_bibnum, $itemnumber);
328
346
ok(
329
    my $itemtype_can         = $builder->build({source => "Itemtype"})->{itemtype};
347
    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'tooManyReserves',
330
    my $itemtype_cant        = $builder->build({source => "Itemtype"})->{itemtype};
348
    "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
331
    my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
349
);
332
333
    $dbh->do(
334
        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) VALUES (?, ?, ?, ?, ?)}, {},
335
        '*', '*', $itemtype_cant, 0, 99
336
    );
337
    $dbh->do(
338
        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) VALUES (?, ?, ?, ?, ?)}, {},
339
        '*', '*', $itemtype_can, 2, 2
340
    );
341
    $dbh->do(
342
        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) VALUES (?, ?, ?, ?, ?)}, {},
343
        '*', '*', $itemtype_cant_record, 0, 0
344
    );
345
    Koha::CirculationRules->set_rules(
346
        {
347
            branchcode => $branch_1,
348
            itemtype   => $itemtype_cant,
349
            categorycode => undef,
350
            rules => {
351
                holdallowed => 0,
352
                returnbranch => 'homebranch',
353
            }
354
        }
355
    );
356
    Koha::CirculationRules->set_rules(
357
        {
358
            branchcode => $branch_1,
359
            itemtype   => $itemtype_can,
360
            categorycode => undef,
361
            rules => {
362
                holdallowed => 1,
363
                returnbranch => 'homebranch',
364
            }
365
        }
366
    );
367
368
    subtest 'noReservesAllowed' => sub {
369
        plan tests => 5;
370
371
        my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
372
        my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
373
        my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
374
        my ( undef, undef, $itemnumber_1_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_cannot);
375
        my ( undef, undef, $itemnumber_1_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_cannot);
376
377
        my ( undef, undef, $itemnumber_2_can ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_can);
378
        my ( undef, undef, $itemnumber_2_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant } , $biblionumber_can);
379
380
        my ( undef, undef, $itemnumber_3_cannot ) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record } , $biblionumber_record_cannot);
381
382
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
383
384
        t::lib::Mocks::mock_preference('item-level_itypes', 1);
385
        is(
386
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
387
            "With item level set, rule from item must be picked (CANNOT)"
388
        );
389
        is(
390
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
391
            "With item level set, rule from item must be picked (CAN)"
392
        );
393
        t::lib::Mocks::mock_preference('item-level_itypes', 0);
394
        is(
395
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
396
            "With biblio level set, rule from biblio must be picked (CANNOT)"
397
        );
398
        is(
399
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
400
            "With biblio level set, rule from biblio must be picked (CAN)"
401
        );
402
        is(
403
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
404
            "When no holds allowed and no holds per record allowed should return noReservesAllowed"
405
        );
406
    };
407
408
    subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
409
        plan tests => 7;
410
411
        my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
412
        my ( undef, undef, $itemnumber_11) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1);
413
        my ( undef, undef, $itemnumber_12) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_1);
414
        my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
415
        my ( undef, undef, $itemnumber_21) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2);
416
        my ( undef, undef, $itemnumber_22) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can } , $biblionumber_2);
417
418
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
419
420
        # Biblio-level hold
421
        AddReserve(
422
            $branch_1,
423
            $borrowernumbers[0],
424
            $biblionumber_1,
425
            '',
426
            1,
427
        );
428
        for my $item_level ( 0..1 ) {
429
            t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
430
            is(
431
                # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
432
                CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
433
                "A biblio-level hold already exists - another hold can be placed on a specific item item"
434
            );
435
        }
436
437
        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
438
        # Item-level hold
439
        AddReserve(
440
            $branch_1,
441
            $borrowernumbers[0],
442
            $biblionumber_1,
443
            '',
444
            1,
445
            undef, undef, undef, undef, $itemnumber_11
446
        );
447
        $dbh->do(q{UPDATE issuingrules set reservesallowed=5, holds_per_record=1});
448
        is(
449
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
450
            "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
451
        );
452
        $dbh->do(q{UPDATE issuingrules set reservesallowed=1, holds_per_record=1});
453
        is(
454
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
455
            "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
456
        );
457
        $dbh->do(q{UPDATE issuingrules set reservesallowed=5, holds_per_record=2});
458
        is(
459
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
460
            "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
461
        );
462
463
        AddReserve(
464
            $branch_1,
465
            $borrowernumbers[0],
466
            $biblionumber_2,
467
            '',
468
            1,
469
            undef, undef, undef, undef, $itemnumber_21
470
        );
471
        $dbh->do(q{UPDATE issuingrules set reservesallowed=2, holds_per_record=2});
472
        is(
473
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
474
            "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
475
        );
476
        is(
477
            CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
478
            "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
479
        );
480
    };
481
};
350
482
351
483
352
# Test branch item rules
484
# Test branch item rules
353
485
354
$dbh->do('DELETE FROM issuingrules');
486
delete_all_rules();
355
$dbh->do('DELETE FROM circulation_rules');
356
$dbh->do(
487
$dbh->do(
357
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
488
    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
358
      VALUES (?, ?, ?, ?)},
489
      VALUES (?, ?, ?, ?)},
Lines 687-689 subtest 'CanItemBeReserved / holds_per_day tests' => sub { Link Here
687
818
688
    $schema->storage->txn_rollback;
819
    $schema->storage->txn_rollback;
689
};
820
};
690
- 
821
822
sub delete_all_rules {
823
    my $dbh = C4::Context->dbh;
824
    $dbh->do('DELETE FROM issuingrules');
825
    $dbh->do('DELETE FROM circulation_rules');
826
}

Return to bug 16787