Attachment #96440 for bug #22284

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




         { status => ageRestricted },   if the Item is age restricted for this borrower.
         { status => damaged },         if the Item is damaged.
         { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK.
         { status => branchNotInHoldGroup }, if borrower home library is not in hold group, and holds are only allowed from hold groups.
         { status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount.
         { status => notReservable },   if holds on this item are not allowed
         { status => libraryNotFound },   if given branchcode is not an existing library
         { status => libraryNotPickupLocation },   if given branchcode is not configured to be a pickup location
         { status => cannotBeTransferred }, if branch transfer limit applies on given item and branchcode
         { status => pickupNotInHoldGroup }, pickup location is not in hold group, and pickup locations are only allowed from hold groups.

=cut


        return { status => 'cannotReserveFromOtherBranches' };
    }

    my $branch_control = C4::Context->preference('HomeOrHoldingBranch');
    my $itembranchcode = $branch_control eq 'holdingbranch' ? $item->holdingbranch : $item->homebranch;
    my $item_library = Koha::Libraries->find( {branchcode => $itembranchcode} );
    if ( $branchitemrule->{holdallowed} == 3) {
        if($borrower->{branchcode} ne $itembranchcode && !$item_library->validate_hold_sibling( {branchcode => $borrower->{branchcode}} )) {
            return { status => 'branchNotInHoldGroup' };
        }
    }

    # If reservecount is ok, we check item branch if IndependentBranches is ON
    # and canreservefromotherbranches is OFF
    if ( C4::Context->preference('IndependentBranches')

        unless ($item->can_be_transferred({ to => $destination })) {
            return { status => 'cannotBeTransferred' };
        }
        unless ($branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' || $item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) {
            return { status => 'pickupNotInHoldGroup' };
        }
    }

    return { status => 'OK' };

                    my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);
                    next if ($branchitemrule->{'holdallowed'} == 0);
                    next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode));
                    my $library = Koha::Libraries->find({branchcode=>$branch});
                    next if (($branchitemrule->{'holdallowed'} == 3) && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) ));
                    my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy};
                    next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) );
                    next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
                    next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
                           $hold_fulfillment_policy eq ''
                        || ( $res->{branchcode} ne
                            $item->$hold_fulfillment_policy )
                      );
                    next unless $item->can_be_transferred( { to => scalar Koha::Libraries->find( $res->{branchcode} ) } );
                    $priority = $res->{'priority'};
                    $highest  = $res;

        return 0 unless $destination;
        return 0 unless $destination->pickup_location;
        return 0 unless $item->can_be_transferred( { to => $destination } );
        my $reserves_control_branch =
            GetReservesControlBranch( $item->unblessed(), $patron->unblessed() );
        my $branchitemrule =
            C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype );
        my $home_library = Koka::Libraries->find( {branchcode => $item->homebranch} );
        return 0 unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' || $home_library->validate_hold_sibling( {branchcode => $pickup_branchcode} );
    }

    if ( $on_shelf_holds == 1 ) {

        foreach my $i (@items) {
            my $reserves_control_branch = GetReservesControlBranch( $i->unblessed(), $patron->unblessed );
            my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype );
            my $branch_control = C4::Context->preference('HomeOrHoldingBranch');
            my $itembranchcode = $branch_control eq 'holdingbranch' ? $item->holdingbranch : $item->homebranch;
            my $item_library = Koha::Libraries->find( {branchcode => $itembranchcode} );


            $any_available = 1
              unless $i->itemlost

              || ( $i->damaged
                && !C4::Context->preference('AllowHoldsOnDamagedItems') )
              || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan
              || $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch
              || $branchitemrule->{holdallowed} == 3 && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} );
        }

        return $any_available ? 0 : 1;





use C4::Context;

use Test::More tests => 60;
use MARC::Record;

use C4::Biblio;

use Koha::Item::Transfer::Limits;
use Koha::Items;
use Koha::Libraries;
use Koha::Library::Groups;
use Koha::Patrons;

BEGIN {


# if IndependentBranches is OFF, a $branch_1 patron can reserve an $branch_2 item
t::lib::Mocks::mock_preference('IndependentBranches', 0);

ok(
    CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status} eq 'OK',
    '$branch_1 patron allowed to reserve $branch_2 item with IndependentBranches OFF (bug 2394)'


    $schema->storage->txn_rollback;
};

subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
    plan tests => 9;

    $schema->storage->txn_begin;

    # Cleanup database
    Koha::Holds->search->delete;
    $dbh->do('DELETE FROM issues');
    $dbh->do('DELETE FROM issuingrules');
    $dbh->do(
        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
        VALUES (?, ?, ?, ?)},
        {},
        '*', '*', '*', 25
    );
    $dbh->do('DELETE FROM branch_item_rules');
    $dbh->do('DELETE FROM default_branch_circ_rules');
    $dbh->do('DELETE FROM default_branch_item_rules');
    $dbh->do('DELETE FROM default_circ_rules');

    Koha::Items->search->delete;
    Koha::Biblios->search->delete;

    # Create item types
    my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
    my $itemtype2 = $builder->build_object( { class => 'Koha::ItemTypes' } );

    # Create libraries
    my $library1  = $builder->build_object( { class => 'Koha::Libraries' } );
    my $library2  = $builder->build_object( { class => 'Koha::Libraries' } );
    my $library3  = $builder->build_object( { class => 'Koha::Libraries' } );

    # Create library groups hierarchy
    my $rootgroup  = $builder->build_object( { class => 'Koha::Library::Groups', value => {ft_local_hold_group => 1} } );
    my $group1  = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library1->branchcode}} );
    my $group2  = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library2->branchcode} } );

    # Create 2 patrons
    my $patron1   = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library1->branchcode} } );
    my $patron3   = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library3->branchcode} } );

    # Create 3 biblios with items
    my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
    my ( undef, undef, $itemnumber_1 ) = AddItem(
        {   homebranch    => $library1->branchcode,
            holdingbranch => $library1->branchcode
        },
        $biblio_1->biblionumber
    );
    my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
    my ( undef, undef, $itemnumber_2 ) = AddItem(
        {   homebranch    => $library2->branchcode,
            holdingbranch => $library2->branchcode
        },
        $biblio_2->biblionumber
    );
    my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
    my ( undef, undef, $itemnumber_3 ) = AddItem(
        {   homebranch    => $library1->branchcode,
            holdingbranch => $library1->branchcode
        },
        $biblio_3->biblionumber
    );

    # Test 1: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'OK' },
        'Patron can place hold if no circ_rules where defined'
    );

    # Insert default circ rule of holds allowed only from local hold group for all libraries
    $dbh->do(
        q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?)},
        {},
        3, 'any', 'any'
    );

    # Test 2: Patron 1 can place hold
    is_deeply(
        CanItemBeReserved( $patron1->borrowernumber, $itemnumber_2 ),
        { status => 'OK' },
        'Patron can place hold because patron\'s home library is part of hold group'
    );

    # Test 3: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'branchNotInHoldGroup' },
        'Patron cannot place hold because patron\'s home library is not part of hold group'
    );

    # Cleanup default_cirt_rules
    $dbh->do('DELETE FROM default_circ_rules');

    # Insert default circ rule to "any" for library 2
    $dbh->do(
        q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?)},
        {},
        $library2->branchcode, 2, 'any', 'any'
    );

    # Test 4: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'OK' },
        'Patron can place hold if holdallowed is set to "any" for library 2'
    );

    # Update default circ rule to "hold group" for library 2
    $dbh->do(
        q{UPDATE default_branch_circ_rules set holdallowed = ?
        WHERE branchcode = ?},
        {},
        3, $library2->branchcode
    );

    # Test 5: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'branchNotInHoldGroup' },
        'Patron cannot place hold if holdallowed is set to "hold group" for library 2'
    );

    # Cleanup default_branch_cirt_rules
    $dbh->do('DELETE FROM default_branch_circ_rules');

    # Insert default item rule to "any" for itemtype 2
    $dbh->do(
        q{INSERT INTO default_branch_item_rules (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?)},
        {},
        $itemtype2->itemtype, 2, 'any', 'any'
    );

    # Test 6: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'OK' },
        'Patron can place hold if holdallowed is set to "any" for itemtype 2'
    );

    # Update default item rule to "hold group" for itemtype 2
    $dbh->do(
        q{UPDATE default_branch_item_rules set holdallowed = ?
        WHERE itemtype = ?},
        {},
        3, $itemtype2->itemtype
    );

    # Test 7: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'branchNotInHoldGroup' },
        'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2'
    );

    # Cleanup default_branch_item_rules
    $dbh->do('DELETE FROM default_branch_item_rules');

    # Insert branch item rule to "any" for itemtype 2 and library 2
    $dbh->do(
        q{INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?,?)},
        {},
        $library2->branchcode, $itemtype2->itemtype, 2, 'any', 'any'
    );

    # Test 8: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'OK' },
        'Patron can place hold if holdallowed is set to "any" for itemtype 2 and library 2'
    );

    # Update branch item rule to "hold group" for itemtype 2 and library 2
    $dbh->do(
        q{UPDATE branch_item_rules set holdallowed = ?
        WHERE branchcode = ? and itemtype = ?},
        {},
        3, $library2->branchcode, $itemtype2->itemtype
    );

    # Test 9: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
        { status => 'branchNotInHoldGroup' },
        'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2 and library 2'
    );

    $schema->storage->txn_rollback;

};

subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
    plan tests => 9;

    $schema->storage->txn_begin;

    # Cleanup database
    Koha::Holds->search->delete;
    $dbh->do('DELETE FROM issues');
    $dbh->do('DELETE FROM issuingrules');
    $dbh->do(
        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
        VALUES (?, ?, ?, ?)},
        {},
        '*', '*', '*', 25
    );
    $dbh->do('DELETE FROM branch_item_rules');
    $dbh->do('DELETE FROM default_branch_circ_rules');
    $dbh->do('DELETE FROM default_branch_item_rules');
    $dbh->do('DELETE FROM default_circ_rules');

    Koha::Items->search->delete;
    Koha::Biblios->search->delete;

    # Create item types
    my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
    my $itemtype2 = $builder->build_object( { class => 'Koha::ItemTypes' } );

    # Create libraries
    my $library1  = $builder->build_object( { class => 'Koha::Libraries' } );
    my $library2  = $builder->build_object( { class => 'Koha::Libraries' } );
    my $library3  = $builder->build_object( { class => 'Koha::Libraries' } );

    # Create library groups hierarchy
    my $rootgroup  = $builder->build_object( { class => 'Koha::Library::Groups', value => {ft_local_hold_group => 1} } );
    my $group1  = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library1->branchcode}} );
    my $group2  = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library2->branchcode} } );

    # Create 2 patrons
    my $patron1   = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library1->branchcode} } );
    my $patron3   = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library3->branchcode} } );

    # Create 3 biblios with items
    my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
    my ( undef, undef, $itemnumber_1 ) = AddItem(
        {   homebranch    => $library1->branchcode,
            holdingbranch => $library1->branchcode
        },
        $biblio_1->biblionumber
    );
    my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
    my ( undef, undef, $itemnumber_2 ) = AddItem(
        {   homebranch    => $library2->branchcode,
            holdingbranch => $library2->branchcode
        },
        $biblio_2->biblionumber
    );
    my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
    my ( undef, undef, $itemnumber_3 ) = AddItem(
        {   homebranch    => $library1->branchcode,
            holdingbranch => $library1->branchcode
        },
        $biblio_3->biblionumber
    );

    # Test 1: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'OK' },
        'Patron can place hold if no circ_rules where defined'
    );

    # Insert default circ rule of holds allowed only from local hold group for all libraries
    $dbh->do(
        q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?)},
        {},
        2, 'holdgroup', 'any'
    );

    # Test 2: Patron 1 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library1->branchcode ),
        { status => 'OK' },
        'Patron can place hold because pickup location is part of hold group'
    );

    # Test 3: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'pickupNotInHoldGroup' },
        'Patron cannot place hold because pickup location is not part of hold group'
    );

    # Cleanup default_cirt_rules
    $dbh->do('DELETE FROM default_circ_rules');

    # Insert default circ rule to "any" for library 2
    $dbh->do(
        q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?)},
        {},
        $library2->branchcode, 2, 'any', 'any'
    );

    # Test 4: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'OK' },
        'Patron can place hold if default_branch_circ_rules is set to "any" for library 2'
    );

    # Update default circ rule to "hold group" for library 2
    $dbh->do(
        q{UPDATE default_branch_circ_rules set hold_fulfillment_policy = ?
        WHERE branchcode = ?},
        {},
        'holdgroup', $library2->branchcode
    );

    # Test 5: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'pickupNotInHoldGroup' },
        'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for library 2'
    );

    # Cleanup default_branch_cirt_rules
    $dbh->do('DELETE FROM default_branch_circ_rules');

    # Insert default item rule to "any" for itemtype 2
    $dbh->do(
        q{INSERT INTO default_branch_item_rules (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?)},
        {},
        $itemtype2->itemtype, 2, 'any', 'any'
    );

    # Test 6: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'OK' },
        'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2'
    );

    # Update default item rule to "hold group" for itemtype 2
    $dbh->do(
        q{UPDATE default_branch_item_rules set hold_fulfillment_policy = ?
        WHERE itemtype = ?},
        {},
        'holdgroup', $itemtype2->itemtype
    );

    # Test 7: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'pickupNotInHoldGroup' },
        'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2'
    );

    # Cleanup default_branch_item_rules
    $dbh->do('DELETE FROM default_branch_item_rules');

    # Insert branch item rule to "any" for itemtype 2 and library 2
    $dbh->do(
        q{INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
        VALUES (?,?,?,?,?)},
        {},
        $library2->branchcode, $itemtype2->itemtype, 2, 'any', 'any'
    );

    # Test 8: Patron 3 can place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'OK' },
        'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2 and library 2'
    );

    # Update branch item rule to "hold group" for itemtype 2 and library 2
    $dbh->do(
        q{UPDATE branch_item_rules set hold_fulfillment_policy = ?
        WHERE branchcode = ? and itemtype = ?},
        {},
        'holdgroup', $library2->branchcode, $itemtype2->itemtype
    );

    # Test 9: Patron 3 cannot place hold
    is_deeply(
        CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
        { status => 'pickupNotInHoldGroup' },
        'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2 and library 2'
    );

    $schema->storage->txn_rollback;
};

Return to bug 22284

Lines 315-325 sub CanBookBeReserved{ Link Here

(-)a/C4/Reserves.pm (-8 / +31 lines)
315	{ status => ageRestricted }, if the Item is age restricted for this borrower.	315	{ status => ageRestricted }, if the Item is age restricted for this borrower.
316	{ status => damaged }, if the Item is damaged.	316	{ status => damaged }, if the Item is damaged.
317	{ status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK.	317	{ status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK.
		318	{ status => branchNotInHoldGroup }, if borrower home library is not in hold group, and holds are only allowed from hold groups.
318	{ status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount.	319	{ status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount.
319	{ status => notReservable }, if holds on this item are not allowed	320	{ status => notReservable }, if holds on this item are not allowed
320	{ status => libraryNotFound }, if given branchcode is not an existing library	321	{ status => libraryNotFound }, if given branchcode is not an existing library
321	{ status => libraryNotPickupLocation }, if given branchcode is not configured to be a pickup location	322	{ status => libraryNotPickupLocation }, if given branchcode is not configured to be a pickup location
322	{ status => cannotBeTransferred }, if branch transfer limit applies on given item and branchcode	323	{ status => cannotBeTransferred }, if branch transfer limit applies on given item and branchcode
		324	{ status => pickupNotInHoldGroup }, pickup location is not in hold group, and pickup locations are only allowed from hold groups.
323		325
324	=cut	326	=cut
325		327
Lines 474-479 sub CanItemBeReserved { Link Here
474	return { status => 'cannotReserveFromOtherBranches' };	476	return { status => 'cannotReserveFromOtherBranches' };
475	}	477	}
476		478
		479	my $branch_control = C4::Context->preference('HomeOrHoldingBranch');
		480	my $itembranchcode = $branch_control eq 'holdingbranch' ? $item->holdingbranch : $item->homebranch;
		481	my $item_library = Koha::Libraries->find( {branchcode => $itembranchcode} );
		482	if ( $branchitemrule->{holdallowed} == 3) {
		483	if($borrower->{branchcode} ne $itembranchcode && !$item_library->validate_hold_sibling( {branchcode => $borrower->{branchcode}} )) {
		484	return { status => 'branchNotInHoldGroup' };
		485	}
		486	}
		487
477	# If reservecount is ok, we check item branch if IndependentBranches is ON	488	# If reservecount is ok, we check item branch if IndependentBranches is ON
478	# and canreservefromotherbranches is OFF	489	# and canreservefromotherbranches is OFF
479	if ( C4::Context->preference('IndependentBranches')	490	if ( C4::Context->preference('IndependentBranches')
Lines 498-503 sub CanItemBeReserved { Link Here
498	unless ($item->can_be_transferred({ to => $destination })) {	509	unless ($item->can_be_transferred({ to => $destination })) {
499	return { status => 'cannotBeTransferred' };	510	return { status => 'cannotBeTransferred' };
500	}	511	}
		512	unless ($branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' \|\| $item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) {
		513	return { status => 'pickupNotInHoldGroup' };
		514	}
501	}	515	}
502		516
503	return { status => 'OK' };	517	return { status => 'OK' };
Lines 811-824 sub CheckReserves { Link Here
811	my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);	825	my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);
812	next if ($branchitemrule->{'holdallowed'} == 0);	826	next if ($branchitemrule->{'holdallowed'} == 0);
813	next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode));	827	next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode));
		828	my $library = Koha::Libraries->find({branchcode=>$branch});
		829	next if (($branchitemrule->{'holdallowed'} == 3) && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) ));
814	my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy};	830	my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy};
815	next	831	next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) );
816	if $hold_fulfillment_policy ne 'any'	832	next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
817	&& (	833	next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
818	$hold_fulfillment_policy eq ''
819	\|\| ( $res->{branchcode} ne
820	$item->$hold_fulfillment_policy )
821	);
822	next unless $item->can_be_transferred( { to => scalar Koha::Libraries->find( $res->{branchcode} ) } );	834	next unless $item->can_be_transferred( { to => scalar Koha::Libraries->find( $res->{branchcode} ) } );
823	$priority = $res->{'priority'};	835	$priority = $res->{'priority'};
824	$highest = $res;	836	$highest = $res;
Lines 1212-1217 sub IsAvailableForItemLevelRequest { Link Here
1212	return 0 unless $destination;	1224	return 0 unless $destination;
1213	return 0 unless $destination->pickup_location;	1225	return 0 unless $destination->pickup_location;
1214	return 0 unless $item->can_be_transferred( { to => $destination } );	1226	return 0 unless $item->can_be_transferred( { to => $destination } );
		1227	my $reserves_control_branch =
		1228	GetReservesControlBranch( $item->unblessed(), $patron->unblessed() );
		1229	my $branchitemrule =
		1230	C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype );
		1231	my $home_library = Koka::Libraries->find( {branchcode => $item->homebranch} );
		1232	return 0 unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' \|\| $home_library->validate_hold_sibling( {branchcode => $pickup_branchcode} );
1215	}	1233	}
1216		1234
1217	if ( $on_shelf_holds == 1 ) {	1235	if ( $on_shelf_holds == 1 ) {
Lines 1225-1230 sub IsAvailableForItemLevelRequest { Link Here
1225	foreach my $i (@items) {	1243	foreach my $i (@items) {
1226	my $reserves_control_branch = GetReservesControlBranch( $i->unblessed(), $patron->unblessed );	1244	my $reserves_control_branch = GetReservesControlBranch( $i->unblessed(), $patron->unblessed );
1227	my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype );	1245	my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype );
		1246	my $branch_control = C4::Context->preference('HomeOrHoldingBranch');
		1247	my $itembranchcode = $branch_control eq 'holdingbranch' ? $item->holdingbranch : $item->homebranch;
		1248	my $item_library = Koha::Libraries->find( {branchcode => $itembranchcode} );
		1249
1228		1250
1229	$any_available = 1	1251	$any_available = 1
1230	unless $i->itemlost	1252	unless $i->itemlost
Lines 1235-1241 sub IsAvailableForItemLevelRequest { Link Here
1235	\|\| ( $i->damaged	1257	\|\| ( $i->damaged
1236	&& !C4::Context->preference('AllowHoldsOnDamagedItems') )	1258	&& !C4::Context->preference('AllowHoldsOnDamagedItems') )
1237	\|\| Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan	1259	\|\| Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan
1238	\|\| $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch;	1260	\|\| $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch
		1261	\|\| $branchitemrule->{holdallowed} == 3 && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} );
1239	}	1262	}
1240		1263
1241	return $any_available ? 0 : 1;	1264	return $any_available ? 0 : 1;

Lines 7-13 use t::lib::TestBuilder; Link Here

(-)a/t/db_dependent/Holds.t (-2 / +394 lines)
7		7
8	use C4::Context;	8	use C4::Context;
9		9
10	use Test::More tests => 59;	10	use Test::More tests => 60;
11	use MARC::Record;	11	use MARC::Record;
12		12
13	use C4::Biblio;	13	use C4::Biblio;
Lines 24-29 use Koha::IssuingRules; Link Here
24	use Koha::Item::Transfer::Limits;	24	use Koha::Item::Transfer::Limits;
25	use Koha::Items;	25	use Koha::Items;
26	use Koha::Libraries;	26	use Koha::Libraries;
		27	use Koha::Library::Groups;
27	use Koha::Patrons;	28	use Koha::Patrons;
28		29
29	BEGIN {	30	BEGIN {
Lines 263-268 t::lib::Mocks::mock_preference('item-level_itypes', 1); Link Here
263		264
264	# if IndependentBranches is OFF, a $branch_1 patron can reserve an $branch_2 item	265	# if IndependentBranches is OFF, a $branch_1 patron can reserve an $branch_2 item
265	t::lib::Mocks::mock_preference('IndependentBranches', 0);	266	t::lib::Mocks::mock_preference('IndependentBranches', 0);
		267
266	ok(	268	ok(
267	CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status} eq 'OK',	269	CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status} eq 'OK',
268	'$branch_1 patron allowed to reserve $branch_2 item with IndependentBranches OFF (bug 2394)'	270	'$branch_1 patron allowed to reserve $branch_2 item with IndependentBranches OFF (bug 2394)'
Lines 687-689 subtest 'CanItemBeReserved / holds_per_day tests' => sub { Link Here
687		689
688	$schema->storage->txn_rollback;	690	$schema->storage->txn_rollback;
689	};	691	};
690	-	692
		693	subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
		694	plan tests => 9;
		695
		696	$schema->storage->txn_begin;
		697
		698	# Cleanup database
		699	Koha::Holds->search->delete;
		700	$dbh->do('DELETE FROM issues');
		701	$dbh->do('DELETE FROM issuingrules');
		702	$dbh->do(
		703	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
		704	VALUES (?, ?, ?, ?)},
		705	{},
		706	'', '', '*', 25
		707	);
		708	$dbh->do('DELETE FROM branch_item_rules');
		709	$dbh->do('DELETE FROM default_branch_circ_rules');
		710	$dbh->do('DELETE FROM default_branch_item_rules');
		711	$dbh->do('DELETE FROM default_circ_rules');
		712
		713	Koha::Items->search->delete;
		714	Koha::Biblios->search->delete;
		715
		716	# Create item types
		717	my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
		718	my $itemtype2 = $builder->build_object( { class => 'Koha::ItemTypes' } );
		719
		720	# Create libraries
		721	my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
		722	my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
		723	my $library3 = $builder->build_object( { class => 'Koha::Libraries' } );
		724
		725	# Create library groups hierarchy
		726	my $rootgroup = $builder->build_object( { class => 'Koha::Library::Groups', value => {ft_local_hold_group => 1} } );
		727	my $group1 = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library1->branchcode}} );
		728	my $group2 = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library2->branchcode} } );
		729
		730	# Create 2 patrons
		731	my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library1->branchcode} } );
		732	my $patron3 = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library3->branchcode} } );
		733
		734	# Create 3 biblios with items
		735	my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
		736	my ( undef, undef, $itemnumber_1 ) = AddItem(
		737	{ homebranch => $library1->branchcode,
		738	holdingbranch => $library1->branchcode
		739	},
		740	$biblio_1->biblionumber
		741	);
		742	my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
		743	my ( undef, undef, $itemnumber_2 ) = AddItem(
		744	{ homebranch => $library2->branchcode,
		745	holdingbranch => $library2->branchcode
		746	},
		747	$biblio_2->biblionumber
		748	);
		749	my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
		750	my ( undef, undef, $itemnumber_3 ) = AddItem(
		751	{ homebranch => $library1->branchcode,
		752	holdingbranch => $library1->branchcode
		753	},
		754	$biblio_3->biblionumber
		755	);
		756
757	# Test 1: Patron 3 can place hold
758	is_deeply(
759	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
760	{ status => 'OK' },
761	'Patron can place hold if no circ_rules where defined'
762	);
763
764	# Insert default circ rule of holds allowed only from local hold group for all libraries
765	$dbh->do(
766	q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
767	VALUES (?,?,?)},
768	{},
769	3, 'any', 'any'
770	);
771
772	# Test 2: Patron 1 can place hold
773	is_deeply(
774	CanItemBeReserved( $patron1->borrowernumber, $itemnumber_2 ),
775	{ status => 'OK' },
776	'Patron can place hold because patron\'s home library is part of hold group'
777	);
778
779	# Test 3: Patron 3 cannot place hold
780	is_deeply(
781	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
782	{ status => 'branchNotInHoldGroup' },
783	'Patron cannot place hold because patron\'s home library is not part of hold group'
784	);
785
786	# Cleanup default_cirt_rules
787	$dbh->do('DELETE FROM default_circ_rules');
788
789	# Insert default circ rule to "any" for library 2
790	$dbh->do(
791	q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
792	VALUES (?,?,?,?)},
793	{},
794	$library2->branchcode, 2, 'any', 'any'
795	);
796
797	# Test 4: Patron 3 can place hold
798	is_deeply(
799	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
800	{ status => 'OK' },
801	'Patron can place hold if holdallowed is set to "any" for library 2'
802	);
803
804	# Update default circ rule to "hold group" for library 2
805	$dbh->do(
806	q{UPDATE default_branch_circ_rules set holdallowed = ?
807	WHERE branchcode = ?},
808	{},
809	3, $library2->branchcode
810	);
811
812	# Test 5: Patron 3 cannot place hold
813	is_deeply(
814	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
815	{ status => 'branchNotInHoldGroup' },
816	'Patron cannot place hold if holdallowed is set to "hold group" for library 2'
817	);
818
819	# Cleanup default_branch_cirt_rules
820	$dbh->do('DELETE FROM default_branch_circ_rules');
821
822	# Insert default item rule to "any" for itemtype 2
823	$dbh->do(
824	q{INSERT INTO default_branch_item_rules (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
825	VALUES (?,?,?,?)},
826	{},
827	$itemtype2->itemtype, 2, 'any', 'any'
828	);
829
830	# Test 6: Patron 3 can place hold
831	is_deeply(
832	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
833	{ status => 'OK' },
834	'Patron can place hold if holdallowed is set to "any" for itemtype 2'
835	);
836
837	# Update default item rule to "hold group" for itemtype 2
838	$dbh->do(
839	q{UPDATE default_branch_item_rules set holdallowed = ?
840	WHERE itemtype = ?},
841	{},
842	3, $itemtype2->itemtype
843	);
844
845	# Test 7: Patron 3 cannot place hold
846	is_deeply(
847	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
848	{ status => 'branchNotInHoldGroup' },
849	'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2'
850	);
851
852	# Cleanup default_branch_item_rules
853	$dbh->do('DELETE FROM default_branch_item_rules');
854
855	# Insert branch item rule to "any" for itemtype 2 and library 2
856	$dbh->do(
857	q{INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
858	VALUES (?,?,?,?,?)},
859	{},
860	$library2->branchcode, $itemtype2->itemtype, 2, 'any', 'any'
861	);
862
863	# Test 8: Patron 3 can place hold
864	is_deeply(
865	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
866	{ status => 'OK' },
867	'Patron can place hold if holdallowed is set to "any" for itemtype 2 and library 2'
868	);
869
870	# Update branch item rule to "hold group" for itemtype 2 and library 2
871	$dbh->do(
872	q{UPDATE branch_item_rules set holdallowed = ?
873	WHERE branchcode = ? and itemtype = ?},
874	{},
875	3, $library2->branchcode, $itemtype2->itemtype
876	);
877
878	# Test 9: Patron 3 cannot place hold
879	is_deeply(
880	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ),
881	{ status => 'branchNotInHoldGroup' },
882	'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2 and library 2'
883	);
884
885	$schema->storage->txn_rollback;
886
887	};
888
889	subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
890	plan tests => 9;
891
892	$schema->storage->txn_begin;
893
894	# Cleanup database
895	Koha::Holds->search->delete;
896	$dbh->do('DELETE FROM issues');
897	$dbh->do('DELETE FROM issuingrules');
898	$dbh->do(
899	q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
900	VALUES (?, ?, ?, ?)},
901	{},
902	'', '', '*', 25
903	);
904	$dbh->do('DELETE FROM branch_item_rules');
905	$dbh->do('DELETE FROM default_branch_circ_rules');
906	$dbh->do('DELETE FROM default_branch_item_rules');
907	$dbh->do('DELETE FROM default_circ_rules');
908
909	Koha::Items->search->delete;
910	Koha::Biblios->search->delete;
911
912	# Create item types
913	my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
914	my $itemtype2 = $builder->build_object( { class => 'Koha::ItemTypes' } );
915
916	# Create libraries
917	my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
918	my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
919	my $library3 = $builder->build_object( { class => 'Koha::Libraries' } );
920
921	# Create library groups hierarchy
922	my $rootgroup = $builder->build_object( { class => 'Koha::Library::Groups', value => {ft_local_hold_group => 1} } );
923	my $group1 = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library1->branchcode}} );
924	my $group2 = $builder->build_object( { class => 'Koha::Library::Groups', value => {parent_id => $rootgroup->id, branchcode => $library2->branchcode} } );
925
926	# Create 2 patrons
927	my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library1->branchcode} } );
928	my $patron3 = $builder->build_object( { class => 'Koha::Patrons', value => {branchcode => $library3->branchcode} } );
929
930	# Create 3 biblios with items
931	my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
932	my ( undef, undef, $itemnumber_1 ) = AddItem(
933	{ homebranch => $library1->branchcode,
934	holdingbranch => $library1->branchcode
935	},
936	$biblio_1->biblionumber
937	);
938	my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
939	my ( undef, undef, $itemnumber_2 ) = AddItem(
940	{ homebranch => $library2->branchcode,
941	holdingbranch => $library2->branchcode
942	},
943	$biblio_2->biblionumber
944	);
945	my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
946	my ( undef, undef, $itemnumber_3 ) = AddItem(
947	{ homebranch => $library1->branchcode,
948	holdingbranch => $library1->branchcode
949	},
950	$biblio_3->biblionumber
951	);
952
953	# Test 1: Patron 3 can place hold
954	is_deeply(
955	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
956	{ status => 'OK' },
957	'Patron can place hold if no circ_rules where defined'
958	);
959
960	# Insert default circ rule of holds allowed only from local hold group for all libraries
961	$dbh->do(
962	q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
963	VALUES (?,?,?)},
964	{},
965	2, 'holdgroup', 'any'
966	);
967
968	# Test 2: Patron 1 can place hold
969	is_deeply(
970	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library1->branchcode ),
971	{ status => 'OK' },
972	'Patron can place hold because pickup location is part of hold group'
973	);
974
975	# Test 3: Patron 3 cannot place hold
976	is_deeply(
977	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
978	{ status => 'pickupNotInHoldGroup' },
979	'Patron cannot place hold because pickup location is not part of hold group'
980	);
981
982	# Cleanup default_cirt_rules
983	$dbh->do('DELETE FROM default_circ_rules');
984
985	# Insert default circ rule to "any" for library 2
986	$dbh->do(
987	q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
988	VALUES (?,?,?,?)},
989	{},
990	$library2->branchcode, 2, 'any', 'any'
991	);
992
993	# Test 4: Patron 3 can place hold
994	is_deeply(
995	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
996	{ status => 'OK' },
997	'Patron can place hold if default_branch_circ_rules is set to "any" for library 2'
998	);
999
1000	# Update default circ rule to "hold group" for library 2
1001	$dbh->do(
1002	q{UPDATE default_branch_circ_rules set hold_fulfillment_policy = ?
1003	WHERE branchcode = ?},
1004	{},
1005	'holdgroup', $library2->branchcode
1006	);
1007
1008	# Test 5: Patron 3 cannot place hold
1009	is_deeply(
1010	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
1011	{ status => 'pickupNotInHoldGroup' },
1012	'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for library 2'
1013	);
1014
1015	# Cleanup default_branch_cirt_rules
1016	$dbh->do('DELETE FROM default_branch_circ_rules');
1017
1018	# Insert default item rule to "any" for itemtype 2
1019	$dbh->do(
1020	q{INSERT INTO default_branch_item_rules (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
1021	VALUES (?,?,?,?)},
1022	{},
1023	$itemtype2->itemtype, 2, 'any', 'any'
1024	);
1025
1026	# Test 6: Patron 3 can place hold
1027	is_deeply(
1028	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
1029	{ status => 'OK' },
1030	'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2'
1031	);
1032
1033	# Update default item rule to "hold group" for itemtype 2
1034	$dbh->do(
1035	q{UPDATE default_branch_item_rules set hold_fulfillment_policy = ?
1036	WHERE itemtype = ?},
1037	{},
1038	'holdgroup', $itemtype2->itemtype
1039	);
1040
1041	# Test 7: Patron 3 cannot place hold
1042	is_deeply(
1043	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
1044	{ status => 'pickupNotInHoldGroup' },
1045	'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2'
1046	);
1047
1048	# Cleanup default_branch_item_rules
1049	$dbh->do('DELETE FROM default_branch_item_rules');
1050
1051	# Insert branch item rule to "any" for itemtype 2 and library 2
1052	$dbh->do(
1053	q{INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
1054	VALUES (?,?,?,?,?)},
1055	{},
1056	$library2->branchcode, $itemtype2->itemtype, 2, 'any', 'any'
1057	);
1058
1059	# Test 8: Patron 3 can place hold
1060	is_deeply(
1061	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
1062	{ status => 'OK' },
1063	'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2 and library 2'
1064	);
1065
1066	# Update branch item rule to "hold group" for itemtype 2 and library 2
1067	$dbh->do(
1068	q{UPDATE branch_item_rules set hold_fulfillment_policy = ?
1069	WHERE branchcode = ? and itemtype = ?},
1070	{},
1071	'holdgroup', $library2->branchcode, $itemtype2->itemtype
1072	);
1073
1074	# Test 9: Patron 3 cannot place hold
1075	is_deeply(
1076	CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ),
1077	{ status => 'pickupNotInHoldGroup' },
1078	'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2 and library 2'
1079	);
1080
1081	$schema->storage->txn_rollback;
1082	};