Attachment #143818 for bug #32142

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




    my $dbh = C4::Context->dbh;
    my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
    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 1124-1129 Circulation: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+1 lines)
1124	choices:	1124	choices:
1125	any_time_is_placed: "any time a hold is placed."	1125	any_time_is_placed: "any time a hold is placed."
1126	not_always: "only if all items are checked out and the record has at least one hold already."	1126	not_always: "only if all items are checked out and the record has at least one hold already."
		1127	issued_or_reserved: "either if all items are checked out, or the record has at least one hold already."
1127	any_time_is_collected: "any time a hold is collected."	1128	any_time_is_collected: "any time a hold is collected."
1128	-	1129	-
1129	- pref: useDefaultReplacementCost	1130	- pref: useDefaultReplacementCost




);

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

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

(-)a/C4/Reserves.pm (-12 / +13 lines)
764	my $dbh = C4::Context->dbh;	764	my $dbh = C4::Context->dbh;
765	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );	765	my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
766	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';	766	my $hold_fee_mode = C4::Context->preference('HoldFeeMode') \|\| 'not_always';
767	if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) {	767	if( $fee and $fee > 0 and ( $hold_fee_mode eq 'not_always' or $hold_fee_mode eq 'issued_or_reserved' ) ) {
768	# This is a reconstruction of the old code:	768	# This is a reconstruction of the old code:
769	# Compare number of items with items issued, and optionally check holds	769	# Compare number of items with items issued, and optionally check holds
770	# If not all items are issued and there are no holds: charge no fee
771	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here	770	# NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
772	my ( $notissued, $reserved );	771
773	( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,	772	my $notissued = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) );
774	( $biblionumber ) );	773	my $reserved = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) );
775	if( $notissued == 0 ) {	774
776	# all items are issued	775	if ( $hold_fee_mode eq 'issued_or_reserved' ) {
777	( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,	776	unless ( $notissued == 0 or $reserved > 0 ) {
778	( $biblionumber, $borrowernumber ) );	777	$fee = 0;
779	$fee = 0 if $reserved == 0;	778	}
780	} else {	779	} elsif ( $hold_fee_mode eq 'not_always' ) {
781	$fee = 0;	780	unless ( $notissued == 0 and $reserved > 0 ) {
		781	$fee = 0;
		782	}
782	}	783	}
783	}	784	}
784	return $fee;	785	return $fee;

Lines 84-98 my $item2 = $builder->build_sample_item( Link Here

(-)a/t/db_dependent/Reserves/GetReserveFee.t (-5 / +33 lines)
84	);	84	);
85		85
86	subtest 'GetReserveFee' => sub {	86	subtest 'GetReserveFee' => sub {
87	plan tests => 5;	87	plan tests => 7;
88		88
89	C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter	89	C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
90	C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
91	my $acc2 = acctlines( $patron2->{borrowernumber} );	90	my $acc2 = acctlines( $patron2->{borrowernumber} );
92	my $res1 = addreserve( $patron1->{borrowernumber} );	91	my $res1 = addreserve( $patron1->{borrowernumber} );
93		92
94	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');	93	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
95	my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );	94	my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
		95	is( int($fee), 2, 'HoldFeeMode=issued_or_reserved, Patron 2 should be charged' );
		96
		97	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
		98	$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
		99	is( $fee, 0, 'HoldFeeMode=issued_or_reserved, Patron 2 should not be charged' );
		100
		101	C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter
		102	$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber );
96	is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' );	103	is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' );
97	C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title );	104	C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title );
98	is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' );	105	is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' );
Lines 139-145 subtest 'Integration with AddReserve' => sub { Link Here
139	};	146	};
140		147
141	subtest 'Items are issued' => sub {	148	subtest 'Items are issued' => sub {
142	plan tests => 4;	149	plan tests => 7;
143		150
144	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber);	151	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber);
145	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber);	152	$dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber);
Lines 151-156 subtest 'Integration with AddReserve' => sub { Link Here
151	addreserve( $patron1->{borrowernumber} );	158	addreserve( $patron1->{borrowernumber} );
152	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' );	159	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' );
153		160
		161	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
		162	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
		163	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
		164	addreserve( $patron3->{borrowernumber} );
		165	addreserve( $patron1->{borrowernumber} );
		166	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' );
		167
		168	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
154	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );	169	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
155	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );	170	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
156	addreserve( $patron3->{borrowernumber} );	171	addreserve( $patron3->{borrowernumber} );
Lines 163-173 subtest 'Integration with AddReserve' => sub { Link Here
163	addreserve( $patron1->{borrowernumber} );	178	addreserve( $patron1->{borrowernumber} );
164	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' );	179	is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' );
165		180
		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( $patron1->{borrowernumber} );
		185	is( acctlines( $patron1->{borrowernumber} ), 1, 'issued_or_reserved - Patron should be charged if all items are checked out but no holds are placed' );
		186
		187	t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always');
166	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );	188	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
167	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );	189	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
168	addreserve( $patron3->{borrowernumber} );	190	addreserve( $patron3->{borrowernumber} );
169	addreserve( $patron1->{borrowernumber} );	191	addreserve( $patron1->{borrowernumber} );
170	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' );	192	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' );
		193
		194	t::lib::Mocks::mock_preference('HoldFeeMode', 'issued_or_reserved');
		195	$dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber );
		196	$dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} );
		197	addreserve( $patron3->{borrowernumber} );
		198	addreserve( $patron1->{borrowernumber} );
		199	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' );
171	};	200	};
172	};	201	};
173		202
174	-