Attachment 156842 Details for Bug 16223 – Bug 16223: Add ability to define liftable restrictions

[patch] Bug 16223: Add ability to define liftable restrictions

Bug-16223-Add-ability-to-define-liftable-restricti.patch (text/plain), 13.75 KB, created by Tomás Cohen Arazi (tcohen) on 2023-10-11 15:10:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2023-10-11 15:10:47 UTC

Size: 13.75 KB

patch

obsolete

>From 4e5bf9c08bed0db0deb3bb933ca9b7948b1524f5 Mon Sep 17 00:00:00 2001
>From: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
>Date: Thu, 22 Sep 2022 12:19:03 +0300
>Subject: [PATCH] Bug 16223: Add ability to define liftable restrictions
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Some libraries debar Patrons at the end of the year for having unpaid fines,
>like in Bug 15157. Currently librarians have to manually remove this type of
>debarments after Patron has paid his/her fines.
>
>This patch adds ability to create restrictions which are lifted after
>patron pays ceratain amount of fines.
>
>To test:
>1. Apply this patch.
>2. Restart your services if needed.
>3. Navigate to page restrictions.pl.
>=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
>4. Add new restriction which has "Lift after payment?" set
>as Yes and fee limit as 5.
>5. Create fees for a patron so they exceed fee limit e.g. 10
>6. Add restriction made in step 2. for the patron
>7. Pay patrons fees partially so that they go under fee limit
>=> Note that patrons restriction should now be lifted.
>
>Also prove t/db_dependent/Patron/Borrower_Debarments.t.
>
>Sponsored-by: Koha-Suomi Oy
>Signed-off-by: Anneli Ãsterman <anneli.osterman@koha-suomi.fi>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Account.pm                               |  3 +
> Koha/Patron/Debarments.pm                     | 38 ++++++++++
> admin/restrictions.pl                         | 10 ++-
> .../prog/en/modules/admin/restrictions.tt     | 34 +++++++++
> .../intranet-tmpl/prog/js/restrictiontypes.js |  3 +-
> t/db_dependent/Patron/Borrower_Debarments.t   | 76 ++++++++++++++++++-
> 6 files changed, 160 insertions(+), 4 deletions(-)
>
>diff --git a/Koha/Account.pm b/Koha/Account.pm
>index ad08a6df4b7..b6bd357b165 100644
>--- a/Koha/Account.pm
>+++ b/Koha/Account.pm
>@@ -39,6 +39,7 @@ use Koha::Account::DebitTypes;
> use Koha::Exceptions;
> use Koha::Exceptions::Account;
> use Koha::Plugins;
>+use Koha::Patron::Debarments;
> 
> =head1 NAME
> 
>@@ -117,6 +118,8 @@ sub pay {
>         }
>     );
> 
>+    Koha::Patron::Debarments::del_restrictions_after_payment({ borrowernumber => $self->{patron_id} });
>+
>     # NOTE: Pay historically always applied as much credit as it could to all
>     # existing outstanding debits, whether passed specific debits or otherwise.
>     if ( $payment->amountoutstanding ) {
>diff --git a/Koha/Patron/Debarments.pm b/Koha/Patron/Debarments.pm
>index 60c3f5504e1..ca6e27c2eef 100644
>--- a/Koha/Patron/Debarments.pm
>+++ b/Koha/Patron/Debarments.pm
>@@ -21,6 +21,8 @@ use Modern::Perl;
> 
> use C4::Context;
> 
>+use Koha::Patron::Restriction::Types;
>+
> our ( @ISA, @EXPORT_OK );
> 
> BEGIN {
>@@ -270,6 +272,42 @@ sub UpdateBorrowerDebarmentFlags {
>     return $dbh->do( "UPDATE borrowers SET debarred = ?, debarredcomment = ? WHERE borrowernumber = ?", {}, ( $expiration, $comment, $borrowernumber ) );
> }
> 
>+=head2 del_restrictions_after_payment
>+
>+my $success = del_restrictions_after_payment({
>+    borrowernumber => $borrowernumber,
>+});
>+
>+Deletes any restrictions from patron by following the rules
>+defined in "Patron restrictions".
>+
>+=cut
>+
>+sub del_restrictions_after_payment {
>+    my ($params) = @_;
>+
>+    my $borrowernumber = $params->{'borrowernumber'};
>+    return unless ( $borrowernumber );
>+
>+    my $patron_restrictions = GetDebarments( { borrowernumber => $borrowernumber } );
>+    return unless ( $patron_restrictions );
>+
>+    my $patron = Koha::Patrons->find( $borrowernumber );
>+    return unless ( $patron );
>+
>+    my $lines = Koha::Account::Lines->search({ borrowernumber  => $borrowernumber });
>+    my $total_due = $lines->total_outstanding;
>+
>+    foreach my $patron_restriction (@{ $patron_restrictions }){
>+        my $restriction = Koha::Patron::Restriction::Types->find({
>+            code => $patron_restriction->{type}
>+        });
>+        if($restriction->lift_after_payment && $total_due <= $restriction->fee_limit){
>+            DelDebarment($patron_restriction->{'borrower_debarment_id'});
>+        }
>+    }
>+}
>+
> =head2 _GetBorrowernumberByDebarmentId
> 
> my $borrowernumber = _GetBorrowernumberByDebarmentId( $borrower_debarment_id );
>diff --git a/admin/restrictions.pl b/admin/restrictions.pl
>index ab0c384212d..a1a0bc1bdfe 100755
>--- a/admin/restrictions.pl
>+++ b/admin/restrictions.pl
>@@ -54,6 +54,8 @@ if ( $op eq 'add_form') {
> } elsif ( $op eq 'add_validate' ) {
> 
>     my $display_text = $input->param('display_text');
>+    my $lift_after_payment = $input->param('lift_after_payment');
>+    my $fee_limit = $input->param('fee_limit');
>     my $is_a_modif = $input->param("is_a_modif");
> 
>     if ($is_a_modif) {
>@@ -64,13 +66,15 @@ if ( $op eq 'add_form') {
>                 display_text => $display_text,
>             }
>         );
>-        if ($dupe->count) {
>+        if ($dupe->count && $dupe->unblessed->{code} ne $code) {
>             push @messages, {
>                 type => 'error', code => 'duplicate_display_text'
>             };
>         } else {
>             my $restriction = Koha::Patron::Restriction::Types->find($code);
>             $restriction->display_text($display_text);
>+            $restriction->lift_after_payment($lift_after_payment);
>+            $restriction->fee_limit($fee_limit);
>             $restriction->store;
>             push @messages, { type => 'message', code => 'update_success' };
>         }
>@@ -84,7 +88,9 @@ if ( $op eq 'add_form') {
>         } else {
>             my $restriction = Koha::Patron::Restriction::Type->new({
>                 code => $code,
>-                display_text => $display_text
>+                display_text => $display_text,
>+                lift_after_payment => $lift_after_payment,
>+                fee_limit => $fee_limit
>             });
>             $restriction->store;
>             push @messages, { type => 'message', code => 'add_success' };
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt
>index dd530d964f8..0acd70ca688 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt
>@@ -113,6 +113,19 @@
>                         <input type="text" value="[% restriction.display_text | html %]" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
>                         <span class="required">Required</span>
>                     </li>
>+                    <li>
>+                        <label for="lift_after_payment" class="required">Lift after payment: </label>
>+                        <select type="text" name="lift_after_payment" id="lift_after_payment" class="required" required="required" />
>+                            <option value="1" selected="selected">Yes</option>
>+                            <option value="0">No</option>
>+                        </select>
>+                        <span class="required">Required</span>
>+                    </li>
>+                    <li>
>+                        <label for="fee_limit" class="required">Fee limit: </label>
>+                        <input type="text" value="[% restriction.fee_limit | html %]" inputmode="decimal" pattern="^\d+(\.\d{2})?$" min="0" name="fee_limit" id="fee_limit" class="required" required="required" />
>+                        <span class="required">Required</span>
>+                    </li>
>                 [% ELSE %]
>                     <li>
>                         <label for="code" class="required">Code: </label>
>@@ -124,6 +137,19 @@
>                         <input type="text" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
>                         <span class="required">Required</span>
>                     </li>
>+                    <li>
>+                        <label for="lift_after_payment" class="required">Lift after payment: </label>
>+                        <select type="text" name="lift_after_payment" id="lift_after_payment" class="required" required="required" />
>+                            <option value="1" selected="selected">Yes</option>
>+                            <option value="0">No</option>
>+                        </select>
>+                        <span class="required">Required</span>
>+                    </li>
>+                    <li>
>+                        <label for="fee_limit" class="required">Fee limit: </label>
>+                        <input type="text" inputmode="decimal" pattern="^\d+(\.\d{2})?$" min="0" name="fee_limit" id="fee_limit" class="required" required="required" />
>+                        <span class="required">Required</span>
>+                    </li>
>                 [% END %]
>             </ol>
>         </fieldset>
>@@ -166,6 +192,8 @@
>                         <th scope="col">Code</th>
>                         <th scope="col">Label</th>
>                         <th scope="col">Default</th>
>+                        <th scope="col">Lift after payment?</th>
>+                        <th scope="col">Fee limit</th>
>                         <th scope="col">Actions</th>
>                     </tr>
>                 </thead>
>@@ -181,6 +209,12 @@
>                             <td>
>                                 [% IF restriction.is_default %]Yes[% END %]
>                             </td>
>+                            <td>
>+                                [% IF restriction.lift_after_payment %]Yes[% END %]
>+                            </td>
>+                            <td>
>+                                [% IF restriction.fee_limit %][% restriction.fee_limit %][% END %]
>+                            </td>
>                             <td class="actions">
>                                 <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=add_form&amp;code=[% restriction.code | uri %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a>
>                                 [% IF !restriction.is_system && !restriction.is_default %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/restrictiontypes.js b/koha-tmpl/intranet-tmpl/prog/js/restrictiontypes.js
>index d1a4e5383e3..3f8d4f77289 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/restrictiontypes.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/restrictiontypes.js
>@@ -9,7 +9,8 @@ jQuery.validator.addMethod( "restrictionDisplayText", function(value){
>     var ex = Object.values(existing).map(function(el) {
>         return el.toLowerCase();
>     });
>-    return (value.length > 0 && ex.indexOf(value.toLowerCase()) > -1) ?
>+    var code = $('input[name="code"]').val();
>+    return (value.length > 0 && ex.indexOf(value.toLowerCase()) > -1) && existing[code] != value ?
>         false :
>         true;
> }, MSG_DUPLICATE_DISPLAY_TEXT);
>diff --git a/t/db_dependent/Patron/Borrower_Debarments.t b/t/db_dependent/Patron/Borrower_Debarments.t
>index 562281e3d35..86fe952f2b7 100755
>--- a/t/db_dependent/Patron/Borrower_Debarments.t
>+++ b/t/db_dependent/Patron/Borrower_Debarments.t
>@@ -5,10 +5,11 @@ use Modern::Perl;
> use C4::Context;
> use Koha::Database;
> use Koha::Patrons;
>+use Koha::Account;
> 
> use t::lib::TestBuilder;
> 
>-use Test::More tests => 34;
>+use Test::More tests => 38;
> 
> use_ok('Koha::Patron::Debarments');
> 
>@@ -221,3 +222,76 @@ is( Koha::Patrons->find($borrowernumber3)->debarred,
>     $debarreddate2, 'Koha::Patron->merge_with() transfers well debarred' );
> is( Koha::Patrons->find($borrowernumber3)->debarredcomment,
>     $debarredcomment2, 'Koha::Patron->merge_with() transfers well debarredcomment' );
>+
>+# Test removing debartments after payment
>+$builder->build(
>+    {
>+        source => 'RestrictionType',
>+        value  => {
>+            code               => 'TEST',
>+            display_text       => 'This is a test.',
>+            is_system          => 0,
>+            is_default         => 0,
>+            lift_after_payment => 1,
>+            fee_limit          => 5
>+        }
>+    }
>+);
>+
>+$builder->build(
>+    {
>+        source => 'RestrictionType',
>+        value  => {
>+            code               => 'TEST2',
>+            display_text       => 'This too is a test.',
>+            is_system          => 0,
>+            is_default         => 0,
>+            lift_after_payment => 1,
>+            fee_limit          => 0
>+        }
>+    }
>+);
>+
>+my $borrowernumber4 = Koha::Patron->new(
>+    {
>+        firstname    => 'First',
>+        surname      => 'Sur',
>+        categorycode => $patron_category->{categorycode},
>+        branchcode   => $library->{branchcode},
>+    }
>+)->store->borrowernumber;
>+
>+my $account = Koha::Account->new({ patron_id => $borrowernumber4 });
>+my $line1 = $account->add_debit({ type => 'ACCOUNT', amount => 10, interface => 'commandline' });
>+
>+Koha::Patron::Debarments::AddDebarment(
>+    {
>+        borrowernumber => $borrowernumber4,
>+        expiration     => '9999-06-10',
>+        type           => 'TEST',
>+        comment        => 'Test delete'
>+    }
>+);
>+
>+Koha::Patron::Debarments::AddDebarment(
>+    {
>+        borrowernumber => $borrowernumber4,
>+        expiration     => '9999-10-10',
>+        type           => 'TEST2',
>+        comment        => 'Test delete again',
>+    }
>+);
>+
>+$debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber4 });
>+
>+is( @$debarments, 2, "GetDebarments returns 2 debarments before payment" );
>+
>+$account->pay({amount => 5});
>+
>+$debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber4 });
>+is( @$debarments, 1, "GetDebarments returns 1 debarment after paying half of the fee" );
>+is( @$debarments[0]->{type}, "TEST2", "Debarment left has type value 'TEST2'" );
>+
>+$account->pay({amount => 5});
>+$debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber4 });
>+is( @$debarments, 0, "GetDebarments returns 0 debarments after paying all fees" );
>-- 
>2.42.0

Actions: View | Diff | Splinter Review

Attachments on bug 16223: 50078 | 50084 | 99697 | 99698 | 104399 | 104400 | 114272 | 114273 | 118526 | 118572 | 118574 | 118575 | 122768 | 122769 | 127252 | 128005 | 132284 | 132285 | 132286 | 132287 | 143388 | 143389 | 143390 | 143462 | 143670 | 143671 | 143672 | 143688 | 143689 | 143690 | 143765 | 143766 | 143767 | 143768 | 143769 | 143770 | 143936 | 146051 | 146052 | 146053 | 146054 | 146055 | 146056 | 146057 | 148307 | 148308 | 148309 | 148310 | 148311 | 148312 | 148313 | 150024 | 150025 | 150026 | 150027 | 150028 | 150029 | 153024 | 153025 | 153026 | 153027 | 153028 | 153029 | 155644 | 155645 | 155646 | 155647 | 155648 | 155649 | 155650 | 156841 | 156842 | 156843 | 156844 | 156845 | 156846 | 156847 | 156848 | 156849 | 156850