Bugzilla – Attachment 172117 Details for
Bug 32142
Add HoldFeeMode option "if all items are checked out OR the record has at least one hold already"
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32142: Implement issued_or_reserved HoldFeeMode option
Bug-32142-Implement-issuedorreserved-HoldFeeMode-o.patch (text/plain), 13.79 KB, created by
Aleisha Amohia
on 2024-09-27 14:54:34 UTC
(
hide
)
Description:
Bug 32142: Implement issued_or_reserved HoldFeeMode option
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2024-09-27 14:54:34 UTC
Size:
13.79 KB
patch
obsolete
>From 8108b201336d911f66bd1a33afc7f8f4f3d409c3 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Wed, 9 Nov 2022 01:30:55 +0000 >Subject: [PATCH] Bug 32142: Implement issued_or_reserved HoldFeeMode option > >This enhancement adds an issued_or_reserved option to the HoldFeeMode >system preference, so that a hold fee may be charged if either all items >are check out, or the record has at least one hold already. > >To test: >1) Apply patch, update database, restart services >2) Go to Administration -> Patron categories >3) Edit Category A and add a hold fee of $1.00 > >4) Go to Administration -> System preferences and search for >HoldFeeMode >5) Set HoldFeeMode to not_always - "only if all items are checked out >and the record has at least one hold already" > >6) In another tab, open the staff interface. Search for a record with >one item attached to it (Biblio A) >7) Place a hold on this record for Patron B (any category). > >not_always, reserved but not issued > >8) Place a hold on Biblio A for Patron A. (Make sure Patron A is of >Category A which the hold fee applies to) >9) Check Patron A's accounting page. They should NOT have a hold fee > >issued_or_reserved, reserved but not issued > >10) In your System preferences tab, change HoldFeeMode to >issued_or_reserved - "either if all items are checked out, or the record >has at least one hold already" >11) Delete Patron A's hold on Biblio A >12) Place a hold on Biblio A for Patron A >13) Check Patron A's accounting page. They SHOULD have a hold fee > >not_always, issued but not reserved > >14) In your System preferences tab, change HoldFeeMode to >not_always - "only if all items are checked out and the record >has at least one hold already" >15) Delete all holds on Biblio A >16) Check out Biblio A's item (Item A) to Patron B >17) Place a hold on Biblio A for Patron A >18) Check Patron A's accounting page. They should NOT have a hold fee > >issued_or_reserved, issued but not reserved > >19) In your System preferences tab, change HoldFeeMode to >issued_or_reserved - "either if all items are checked out, or the record >has at least one hold already" >20) Delete Patron A's hold on Biblio A >21) Place a hold on Biblio A for Patron A >22) Check Patron A's accounting page. They SHOULD have a hold fee > >not_always, issued and reserved > >23) In your System preferences tab, change HoldFeeMode to >not_always - "only if all items are checked out and the record >has at least one hold already" >24) Delete all holds on Biblio A >25) Place a hold on Biblio A for Patron C (any category). >26) Place a hold on Biblio A for Patron A >27) Check Patron A's accounting page. They SHOULD have a hold fee > >issued_or_reserved, issued and reserved > >28) In your System preferences tab, change HoldFeeMode to >issued_or_reserved - "either if all items are checked out, or the record >has at least one hold already" >29) Delete Patron A's hold on Biblio A. There should still be a hold for >Patron C. >30) Place a hold on Biblio A for Patron A >31) Check Patron A's accounting page. They SHOULD have a hold fee > >32) Ensure tests pass t/db_dependent/Reserves/GetReserveFee.t > >Sponsored-by: Horowhenua Libraries Trust >Signed-off-by: Sam Lau <samalau@gmail.com> >--- > C4/Reserves.pm | 27 +++++---- > ..._reserved_option_to_HoldFeeMode_syspref.pl | 10 ++-- > .../admin/preferences/circulation.pref | 1 + > t/db_dependent/Reserves/GetReserveFee.t | 56 +++++++++++++++++-- > 4 files changed, 72 insertions(+), 22 deletions(-) > mode change 100644 => 100755 installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 729d5d0eae7..35daf89778e 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -736,21 +736,24 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? > 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' ) { >+ 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, $reserved ); >- ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef, >- ( $biblionumber ) ); >- if( $notissued == 0 ) { >- # all items are issued >- ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef, >- ( $biblionumber, $borrowernumber ) ); >- $fee = 0 if $reserved == 0; >- } else { >- $fee = 0; >+ >+ 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; >diff --git a/installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl b/installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl >old mode 100644 >new mode 100755 >index e0df726935f..92903185e11 >--- a/installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl >+++ b/installer/data/mysql/atomicupdate/bug_32142_-_add_issued_or_reserved_option_to_HoldFeeMode_syspref.pl >@@ -1,12 +1,14 @@ > use Modern::Perl; > > return { >- bug_number => "32142", >+ bug_number => "32142", > description => "Add issued_or_reserved option to HoldFeeMode system preference", >- up => sub { >+ up => sub { > my ($args) = @_; >- my ($dbh, $out) = @$args{qw(dbh out)}; >+ my ( $dbh, $out ) = @$args{qw(dbh out)}; > >- $dbh->do(q{ UPDATE systempreferences SET options = 'any_time_is_placed|not_always|issued_or_reserved|any_time_is_collected' where variable = 'HoldFeeMode' }); >+ $dbh->do( >+ q{ UPDATE systempreferences SET options = 'any_time_is_placed|not_always|issued_or_reserved|any_time_is_collected' where variable = 'HoldFeeMode' } >+ ); > }, > }; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >index 317e4238ec2..22f2874d472 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >@@ -1176,6 +1176,7 @@ Circulation: > choices: > any_time_is_placed: "any time a hold is placed." > not_always: "only if all items are checked out and the record has at least one hold already." >+ issued_or_reserved: "either if all items are checked out or the record has at least one hold already." > any_time_is_collected: "any time a hold is collected." > - > - pref: useDefaultReplacementCost >diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t >index 03029d83331..437ac6c6dff 100755 >--- a/t/db_dependent/Reserves/GetReserveFee.t >+++ b/t/db_dependent/Reserves/GetReserveFee.t >@@ -97,19 +97,29 @@ my $item2 = $builder->build_sample_item( > ); > > subtest 'GetReserveFee' => sub { >- plan tests => 6; >+ 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'); >+ 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 ); >@@ -156,7 +166,7 @@ subtest 'Integration with AddReserve' => sub { > }; > > subtest 'Items are issued' => sub { >- plan tests => 4; >+ plan tests => 7; > > $dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber); > $dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber); >@@ -168,11 +178,25 @@ subtest 'Integration with AddReserve' => sub { > addreserve( $patron1->borrowernumber ); > is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged if items are not all checked out' ); > >- $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); >+ 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' ); >+ 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 ); >@@ -180,11 +204,31 @@ subtest 'Integration with AddReserve' => sub { > 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' >+ ); > }; > }; > >-- >2.39.5
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=172117">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32142
:
143452
|
143453
|
143598
|
143817
|
143818
|
153251
|
153286
|
153287
|
159070
|
159071
|
162626
|
162627
| 172117