Attachment #159071 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 1-12 Link Here

(-)a/installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl (-4 / +6 lines)
1	use Modern::Perl;	1	use Modern::Perl;
2		2
3	return {	3	return {
4	bug_number => "32142",	4	bug_number => "32142",
5	description => "Add issued_or_reserved option to HoldFeeMode system preference",	5	description => "Add issued_or_reserved option to HoldFeeMode system preference",
6	up => sub {	6	up => sub {
7	my ($args) = @_;	7	my ($args) = @_;
8	my ($dbh, $out) = @$args{qw(dbh out)};	8	my ( $dbh, $out ) = @$args{qw(dbh out)};
9		9
10	$dbh->do(q{ UPDATE systempreferences SET options = 'any_time_is_placed\|not_always\|issued_or_reserved\|any_time_is_collected' where variable = 'HoldFeeMode' });	10	$dbh->do(
		11	q{ UPDATE systempreferences SET options = 'any_time_is_placed\|not_always\|issued_or_reserved\|any_time_is_collected' where variable = 'HoldFeeMode' }
		12	);
11	},	13	},
12	};	14	};

Lines 1100-1105 Circulation: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+1 lines)
1100	choices:	1100	choices:
1101	any_time_is_placed: "any time a hold is placed."	1101	any_time_is_placed: "any time a hold is placed."
1102	not_always: "only if all items are checked out and the record has at least one hold already."	1102	not_always: "only if all items are checked out and the record has at least one hold already."
		1103	issued_or_reserved: "either if all items are checked out, or the record has at least one hold already."
1103	any_time_is_collected: "any time a hold is collected."	1104	any_time_is_collected: "any time a hold is collected."
1104	-	1105	-
1105	- pref: useDefaultReplacementCost	1106	- 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=not_always, 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 the items are not checked out, even if 1 hold is already placed'
        );

        C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} );
        $dbh->do( "DELETE FROM reserves     WHERE biblionumber=?", undef, $biblio->biblionumber );

        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 815-835 SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? Link Here

(-)a/C4/Reserves.pm (-12 / +13 lines)
815	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );	815	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
816	$fee += 0;	816	$fee += 0;
817	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';	817	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';
818	if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) {	818	if( $fee and $fee > 0 and ( $hold_fee_mode eq 'not_always' or $hold_fee_mode eq 'issued_or_reserved' ) ) {
819	# This is a reconstruction of the old code:	819	# This is a reconstruction of the old code:
820	# Compare number of items with items issued, and optionally check holds	820	# Compare number of items with items issued, and optionally check holds
821	# If not all items are issued and there are no holds: charge no fee
822	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here	821	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
823	my ( $notissued, $reserved );	822
824	( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,	823	my $notissued = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) );
825	( $biblionumber ) );	824	my $reserved = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) );
826	if( $notissued == 0 ) {	825
827	# all items are issued	826	if ( $hold_fee_mode eq 'issued_or_reserved' ) {
828	( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,	827	unless ( $notissued == 0 or $reserved > 0 ) {
829	( $biblionumber, $borrowernumber ) );	828	$fee = 0;
830	$fee = 0 if $reserved == 0;	829	}
831	} else {	830	} elsif ( $hold_fee_mode eq 'not_always' ) {
832	$fee = 0;	831	unless ( $notissued == 0 and $reserved > 0 ) {
		832	$fee = 0;
		833	}
833	}	834	}
834	}	835	}
835	return $fee;	836	return $fee;

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

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