Attachment 162627 Details for Bug 32142 – Bug 32142: Implement issued_or_reserved HoldFeeMode option

[patch] Bug 32142: Implement issued_or_reserved HoldFeeMode option

Bug-32142-Implement-issuedorreserved-HoldFeeMode-o.patch (text/plain), 13.25 KB, created by Aleisha Amohia on 2024-02-29 22:27:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aleisha Amohia

Created: 2024-02-29 22:27:09 UTC

Size: 13.25 KB

patch

obsolete

>From face044aab7c6abcf5768338abe7abe7aa113a6c 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                                | 25 ++++-----
> ..._reserved_option_to_HoldFeeMode_syspref.pl | 10 ++--
> .../admin/preferences/circulation.pref        |  1 +
> t/db_dependent/Reserves/GetReserveFee.t       | 53 +++++++++++++++++--
> 4 files changed, 68 insertions(+), 21 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 c95a3d7ad42..32095091941 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -815,21 +815,22 @@ 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 ) {
>+                $fee = 0;
>+            }
>+        } elsif ( $hold_fee_mode eq 'not_always' ) {
>+            unless ( $notissued == 0 and $reserved > 0 ) {
>+                $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 080e8fa4948..953ac5b89f9 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
>@@ -1108,6 +1108,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..fd93b6e039b 100755
>--- a/t/db_dependent/Reserves/GetReserveFee.t
>+++ b/t/db_dependent/Reserves/GetReserveFee.t
>@@ -97,15 +97,24 @@ 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', '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' );
>@@ -156,7 +165,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 +177,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 +203,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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 32142: 143452 | 143453 | 143598 | 143817 | 143818 | 153251 | 153286 | 153287 | 159070 | 159071 | 162626 | 162627 | 172117