Attachment #153287 for bug #32142

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




    my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
    $fee += 0;
    my $hold_fee_mode = C4::Context->preference('HoldFeeMode') || 'not_always';
    if( $fee and $fee > 0 and ( $hold_fee_mode eq 'not_always' or $hold_fee_mode eq 'issued_or_reserved' ) ) {
        # This is a reconstruction of the old code:
        # Compare number of items with items issued, and optionally check holds
        # If not all items are issued and there are no holds: charge no fee
        # NOTE: Lost, damaged, not-for-loan, etc. are just ignored here

        my $notissued = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) );
        my $reserved = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) );

        if ( $hold_fee_mode eq 'issued_or_reserved' ) {
            unless ( $notissued == 0 or $reserved > 0 ) {
                $fee = 0;
            }
        } elsif ( $hold_fee_mode eq 'not_always' ) {
            unless ( $notissued == 0 and $reserved > 0 ) {
                $fee = 0;
            }
        }
    }
    return $fee;

Lines 1067-1072 Circulation: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+1 lines)
1067	choices:	1067	choices:
1068	any_time_is_placed: "any time a hold is placed."	1068	any_time_is_placed: "any time a hold is placed."
1069	not_always: "only if all items are checked out and the record has at least one hold already."	1069	not_always: "only if all items are checked out and the record has at least one hold already."
		1070	issued_or_reserved: "either if all items are checked out, or the record has at least one hold already."
1070	any_time_is_collected: "any time a hold is collected."	1071	any_time_is_collected: "any time a hold is collected."
1071	-	1072	-
1072	- pref: useDefaultReplacementCost	1073	- pref: useDefaultReplacementCost




);

subtest 'GetReserveFee' => sub {
    plan tests => 8;

    C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
    C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
    my $acc2 = acctlines( $patron2->{borrowernumber} );
    my $res1 = addreserve( $patron1->{borrowernumber} );

    t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
    my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
    is( int($fee), 2, 'HoldFeeMode=issued_or_reserved, Patron 2 should be charged' );

    t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
    $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
    is( $fee, 0, 'HoldFeeMode=issued_or_reserved, Patron 2 should not be charged' );

    C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
    $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
    is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' );
    C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title );
    is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' );

    };

    subtest 'Items are issued' => sub {
        plan tests => 7;

        $dbh->do( "DELETE FROM issues       WHERE itemnumber=?", undef, $item1->itemnumber);
        $dbh->do( "DELETE FROM issues       WHERE itemnumber=?", undef, $item2->itemnumber);

        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' );

        t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );
        $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
        addreserve( $patron3->{borrowernumber} );
        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all the items are not checked out because 1 hold is placed' );

        t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );
        $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
        addreserve( $patron3->{borrowernumber} );

        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' );

        t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );
        $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all items are checked out but no holds are placed' );

        t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );
        $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
        addreserve( $patron3->{borrowernumber} );
        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' );

        t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );
        $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
        addreserve( $patron3->{borrowernumber} );
        addreserve( $patron1->{borrowernumber} );
        is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all items are checked out and at least 1 hold is already placed' );
    };
};

- 

Return to bug 32142

Lines 781-801 SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? Link Here

(-)a/C4/Reserves.pm (-12 / +13 lines)
781	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );	781	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
782	$fee += 0;	782	$fee += 0;
783	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';	783	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';
784	if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) {	784	if( $fee and $fee > 0 and ( $hold_fee_mode eq 'not_always' or $hold_fee_mode eq 'issued_or_reserved' ) ) {
785	# This is a reconstruction of the old code:	785	# This is a reconstruction of the old code:
786	# Compare number of items with items issued, and optionally check holds	786	# Compare number of items with items issued, and optionally check holds
787	# If not all items are issued and there are no holds: charge no fee
788	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here	787	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
789	my ( $notissued, $reserved );	788
790	( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,	789	my $notissued = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) );
791	( $biblionumber ) );	790	my $reserved = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) );
792	if( $notissued == 0 ) {	791
793	# all items are issued	792	if ( $hold_fee_mode eq 'issued_or_reserved' ) {
794	( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,	793	unless ( $notissued == 0 or $reserved > 0 ) {
795	( $biblionumber, $borrowernumber ) );	794	$fee = 0;
796	$fee = 0 if $reserved == 0;	795	}
797	} else {	796	} elsif ( $hold_fee_mode eq 'not_always' ) {
798	$fee = 0;	797	unless ( $notissued == 0 and $reserved > 0 ) {
		798	$fee = 0;
		799	}
799	}	800	}
800	}	801	}
801	return $fee;	802	return $fee;

Lines 97-111 my $item2 = $builder->build_sample_item( Link Here

(-)a/t/db_dependent/Reserves/GetReserveFee.t (-5 / +33 lines)
97	);	97	);
98		98
99	subtest 'GetReserveFee' => sub {	99	subtest 'GetReserveFee' => sub {
100	plan tests => 6;	100	plan tests => 8;
101		101
102	C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter	102	C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
103	C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
104	my $acc2 = acctlines( $patron2->{borrowernumber} );	103	my $acc2 = acctlines( $patron2->{borrowernumber} );
105	my $res1 = addreserve( $patron1->{borrowernumber} );	104	my $res1 = addreserve( $patron1->{borrowernumber} );
106		105
107	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');	106	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
108	my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );	107	my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
		108	is( int($fee), 2, 'HoldFeeMode=issued_or_reserved, Patron 2 should be charged' );
		109
		110	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
		111	$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
		112	is( $fee, 0, 'HoldFeeMode=issued_or_reserved, Patron 2 should not be charged' );
		113
		114	C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
		115	$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
109	is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' );	116	is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' );
110	C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title );	117	C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title );
111	is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' );	118	is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' );
Lines 156-162 subtest 'Integration with AddReserve' => sub { Link Here
156	};	163	};
157		164
158	subtest 'Items are issued' => sub {	165	subtest 'Items are issued' => sub {
159	plan tests => 4;	166	plan tests => 7;
160		167
161	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber);	168	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber);
162	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber);	169	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber);
Lines 168-173 subtest 'Integration with AddReserve' => sub { Link Here
168	addreserve( $patron1->{borrowernumber} );	175	addreserve( $patron1->{borrowernumber} );
169	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' );	176	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' );
170		177
		178	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
		179	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
		180	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
		181	addreserve( $patron3->{borrowernumber} );
		182	addreserve( $patron1->{borrowernumber} );
		183	is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all the items are not checked out because 1 hold is placed' );
		184
		185	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
171	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );	186	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
172	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );	187	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
173	addreserve( $patron3->{borrowernumber} );	188	addreserve( $patron3->{borrowernumber} );
Lines 180-190 subtest 'Integration with AddReserve' => sub { Link Here
180	addreserve( $patron1->{borrowernumber} );	195	addreserve( $patron1->{borrowernumber} );
181	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' );	196	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' );
182		197
		198	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
		199	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
		200	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
		201	addreserve( $patron1->{borrowernumber} );
		202	is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all items are checked out but no holds are placed' );
		203
		204	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
183	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );	205	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
184	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );	206	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
185	addreserve( $patron3->{borrowernumber} );	207	addreserve( $patron3->{borrowernumber} );
186	addreserve( $patron1->{borrowernumber} );	208	addreserve( $patron1->{borrowernumber} );
187	is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' );	209	is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' );
		210
		211	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
		212	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
		213	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
		214	addreserve( $patron3->{borrowernumber} );
		215	addreserve( $patron1->{borrowernumber} );
		216	is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all items are checked out and at least 1 hold is already placed' );
188	};	217	};
189	};	218	};
190		219
191	-