Attachment #172117 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 ) {
                # items still available to be checked out OR record has no reserves
                $fee = 0;
            }
        } elsif ( $hold_fee_mode eq 'not_always' ) {
            unless ( $notissued == 0 and $reserved > 0 ) {
                # items still available to be checked out AND record has no reserves
                $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 1176-1181 Circulation: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+1 lines)
1176	choices:	1176	choices:
1177	any_time_is_placed: "any time a hold is placed."	1177	any_time_is_placed: "any time a hold is placed."
1178	not_always: "only if all items are checked out and the record has at least one hold already."	1178	not_always: "only if all items are checked out and the record has at least one hold already."
		1179	issued_or_reserved: "either if all items are checked out or the record has at least one hold already."
1179	any_time_is_collected: "any time a hold is collected."	1180	any_time_is_collected: "any time a hold is collected."
1180	-	1181	-
1181	- pref: useDefaultReplacementCost	1182	- 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', 'not_always' );
    my $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber );
    is( $fee, 0, 'HoldFeeMode=not_always, Patron 2 should not be charged' );

    my $res1 = addreserve( $patron1->borrowernumber );

    t::lib::Mocks::mock_preference( 'HoldFeeMode', 'issued_or_reserved' );
    $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber );
    is( int($fee), 2, 'HoldFeeMode=issued_or_reserved, Patron 2 should 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' );

    t::lib::Mocks::mock_preference( 'HoldFeeMode', 'not_always' );
    # If we delete the reserve, there should be no charge
    $dbh->do( "DELETE FROM reserves WHERE borrowernumber = ?", undef, ( $patron1->borrowernumber) );
    $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber );

    };

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

(-)a/C4/Reserves.pm (-12 / +15 lines)
736	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );	736	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
737	$fee += 0;	737	$fee += 0;
738	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';	738	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';
739	if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) {	739	if( $fee and $fee > 0 and ( $hold_fee_mode eq 'not_always' or $hold_fee_mode eq 'issued_or_reserved' ) ) {
740	# This is a reconstruction of the old code:	740	# This is a reconstruction of the old code:
741	# Compare number of items with items issued, and optionally check holds	741	# Compare number of items with items issued, and optionally check holds
742	# If not all items are issued and there are no holds: charge no fee
743	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here	742	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
744	my ( $notissued, $reserved );	743
745	( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,	744	my $notissued = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) );
746	( $biblionumber ) );	745	my $reserved = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) );
747	if( $notissued == 0 ) {	746
748	# all items are issued	747	if ( $hold_fee_mode eq 'issued_or_reserved' ) {
749	( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,	748	unless ( $notissued == 0 or $reserved > 0 ) {
750	( $biblionumber, $borrowernumber ) );	749	# items still available to be checked out OR record has no reserves
751	$fee = 0 if $reserved == 0;	750	$fee = 0;
752	} else {	751	}
753	$fee = 0;	752	} elsif ( $hold_fee_mode eq 'not_always' ) {
		753	unless ( $notissued == 0 and $reserved > 0 ) {
		754	# items still available to be checked out AND record has no reserves
		755	$fee = 0;
		756	}
754	}	757	}
755	}	758	}
756	return $fee;	759	return $fee;

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

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