Bugzilla – Attachment 171272 Details for
Bug 14250
Don't allow to generate discharges for patrons with fines
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14250: (QA follow-up) Deduplicate checks
Bug-14250-QA-follow-up-Deduplicate-checks.patch (text/plain), 12.22 KB, created by
Sukhmandeep
on 2024-09-10 16:50:04 UTC
(
hide
)
Description:
Bug 14250: (QA follow-up) Deduplicate checks
Filename:
MIME Type:
Creator:
Sukhmandeep
Created:
2024-09-10 16:50:04 UTC
Size:
12.22 KB
patch
obsolete
>From 97127246ebe3edb49df1d4a3afae0b2fad5da574 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 23 Aug 2024 13:40:09 -0400 >Subject: [PATCH] Bug 14250: (QA follow-up) Deduplicate checks > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Sukhmandeep Benipal <sukhmandeep.benipal@inLibro.com> >--- > Koha/Patron/Discharge.pm | 42 ++++++++++++------- > .../prog/en/modules/members/discharge.tt | 17 ++++---- > .../bootstrap/en/modules/opac-discharge.tt | 19 +++++---- > members/discharge.pl | 13 ++---- > opac/opac-discharge.pl | 12 ++++-- > t/db_dependent/Patron/Borrower_Discharge.t | 24 ++++++++--- > 6 files changed, 79 insertions(+), 48 deletions(-) > >diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm >index a72f7f2b7b..7a3a7923ec 100644 >--- a/Koha/Patron/Discharge.pm >+++ b/Koha/Patron/Discharge.pm >@@ -39,13 +39,22 @@ sub can_be_discharged { > my $patron = Koha::Patrons->find( $params->{borrowernumber} ); > return unless $patron; > >- my $has_pending_checkouts = $patron->checkouts->count; >- return 0 if $has_pending_checkouts; >+ my $can_be_discharged = 1; >+ my $problems = {}; > >- my $has_debt = ( $patron->account->outstanding_debits->total_outstanding > 0 ); >- return 0 if $has_debt; >+ my $checkouts = $patron->checkouts->count; >+ if ($checkouts) { >+ $can_be_discharged = 0; >+ $problems->{checkouts} = $checkouts; >+ } >+ >+ my $debt = $patron->account->outstanding_debits->total_outstanding; >+ if ( $debt > 0 ) { >+ $can_be_discharged = 0; >+ $problems->{debt} = $debt; >+ } > >- return 1; >+ return ( $can_be_discharged, $problems ); > } > > sub is_discharged { >@@ -53,10 +62,10 @@ sub is_discharged { > return unless $params->{borrowernumber}; > my $borrowernumber = $params->{borrowernumber}; > >- my $restricted = Koha::Patrons->find( $borrowernumber )->is_debarred; >- my @validated = get_validated({borrowernumber => $borrowernumber}); >+ my $restricted = Koha::Patrons->find($borrowernumber)->is_debarred; >+ my @validated = get_validated( { borrowernumber => $borrowernumber } ); > >- if ($restricted && @validated) { >+ if ( $restricted && @validated ) { > return 1; > } else { > return 0; >@@ -67,19 +76,24 @@ sub request { > my ($params) = @_; > my $borrowernumber = $params->{borrowernumber}; > return unless $borrowernumber; >- return unless can_be_discharged({ borrowernumber => $borrowernumber }); >+ my ($can) = can_be_discharged( { borrowernumber => $borrowernumber } ); >+ return unless $can; > > my $rs = Koha::Database->new->schema->resultset('Discharge'); >- return $rs->create({ >- borrower => $borrowernumber, >- needed => dt_from_string, >- }); >+ return $rs->create( >+ { >+ borrower => $borrowernumber, >+ needed => dt_from_string, >+ } >+ ); > } > > sub discharge { > my ($params) = @_; > my $borrowernumber = $params->{borrowernumber}; >- return unless $borrowernumber and can_be_discharged( { borrowernumber => $borrowernumber } ); >+ >+ my ( $can ) = can_be_discharged({ borrowernumber => $borrowernumber }); >+ return unless $borrowernumber and $can; > > # Cancel reserves > my $patron = Koha::Patrons->find( $borrowernumber ); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharge.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharge.tt >index e0e403a730..4055737f31 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharge.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/discharge.tt >@@ -1,6 +1,7 @@ > [% USE raw %] > [% USE Asset %] > [% USE KohaDates %] >+[% USE Price %] > [% USE AuthorisedValues %] > [% USE Branches %] > [% PROCESS 'i18n.inc' %] >@@ -54,15 +55,13 @@ > [% UNLESS can_be_discharged %] > <p>Cannot edit discharge for following reasons: </p> > <ul> >- [% IF patron.checkouts.count %] >- <li>They have checked out items</li> >- [% END %] >- [% IF (patron.account.outstanding_debits.total_outstanding > 0) %] >- <li>They have unpaid fines</li> >- [% END %] >- [% IF patron.checkouts.count == 0 and (patron.account.outstanding_debits.total_outstanding == 0) %] >- <li> Patron cannot be discharged for an unknown reason, please contact a librarian for more information</li> >- [% END %] >+ [% IF discharge_problems.checkouts %] >+ <li>They have [% discharge_problems.checkouts | html %] item(s) checked out</li> >+ [% END %] >+ >+ [% IF discharge_problems.debt %] >+ <li>They have unpaid fines totaling [% discharge_problems.debt | $Price %]</li> >+ [% END %] > </ul> > [% ELSE %] > [% IF patron.holds.count %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-discharge.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-discharge.tt >index 752b58aea4..cc97779c62 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-discharge.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-discharge.tt >@@ -1,5 +1,6 @@ > [% USE raw %] > [% USE Koha %] >+[% USE Price %] > [% USE AdditionalContents %] > [% SET OpacNav = AdditionalContents.get( location => "OpacNav", lang => lang, library => logged_in_user.branchcode || default_branch, blocktitle => 0 ) %] > [% SET OpacNavBottom = AdditionalContents.get( location => "OpacNavBottom", lang => lang, library => logged_in_user.branchcode || default_branch, blocktitle => 0 ) %] >@@ -63,14 +64,18 @@ > [% UNLESS can_be_discharged %] > <div class="alert alert-info">You cannot be discharged for following reasons: > <ul> >- [% IF has_debt %] >- <li> You have unpaid fines. Please pay your fines before reapplying.</li> >+ [% IF discharge_problems.checkouts %] >+ <li> >+ You have [% discharge_problems.checkouts | html %] item(s) checked out. >+ Please return them before reapplying. >+ </li> > [% END %] >- [% IF has_checkouts %] >- <li> You have checked out items. Please return them before reapplying.</li> >- [% END %] >- [% IF !has_checkouts and !has_debt %] >- <li> Patron cannot be discharged for an unknown reason, please contact a librarian for more information</li> >+ >+ [% IF discharge_problems.debt %] >+ <li> >+ They have unpaid fines totaling [% discharge_problems.debt | $Price %]. >+ Please pay your fines before reapplying. >+ </li> > [% END %] > </ul> > </div> >diff --git a/members/discharge.pl b/members/discharge.pl >index 2742bad084..c16e4d8cd1 100755 >--- a/members/discharge.pl >+++ b/members/discharge.pl >@@ -61,16 +61,10 @@ my $logged_in_user = Koha::Patrons->find( $loggedinuser ); > my $patron = Koha::Patrons->find( $borrowernumber ); > output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); > >-my $can_be_discharged = Koha::Patron::Discharge::can_be_discharged({ >+my ( $can_be_discharged, $discharge_problems ) = Koha::Patron::Discharge::can_be_discharged({ > borrowernumber => $borrowernumber > }); > >- >-unless ($can_be_discharged) { >- my $has_checkout = $patron->checkouts->count; >- my $has_fine = $patron->account->outstanding_debits->total_outstanding; >-} >- > # Generating discharge if needed > if ( $op eq 'cud-discharge' && $input->param('discharge') and $can_be_discharged ) { > my $is_discharged = Koha::Patron::Discharge::is_discharged({ >@@ -111,8 +105,9 @@ my @validated_discharges = Koha::Patron::Discharge::get_validated({ > }); > > $template->param( >- patron => $patron, >- can_be_discharged => $can_be_discharged, >+ patron => $patron, >+ can_be_discharged => $can_be_discharged, >+ discharge_problems => $discharge_problems, > validated_discharges => \@validated_discharges, > ); > >diff --git a/opac/opac-discharge.pl b/opac/opac-discharge.pl >index 5e6869be44..7eeab30e9b 100755 >--- a/opac/opac-discharge.pl >+++ b/opac/opac-discharge.pl >@@ -43,14 +43,18 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ > type => "opac", > }); > >-my $can_be_discharged = Koha::Patron::Discharge::can_be_discharged( { borrowernumber => $loggedinuser } ); >+my ( $can_be_discharged, $discharge_problems ) = >+ Koha::Patron::Discharge::can_be_discharged( { borrowernumber => $loggedinuser } ); > my $patron = Koha::Patrons->find($loggedinuser); > my $has_pending_checkouts = $patron->checkouts->count; > my $has_debt = ( $patron->account->outstanding_debits->total_outstanding > 0 ); > >-$template->param( can_be_discharged => $can_be_discharged ); >-$template->param( has_checkouts => $has_pending_checkouts ); >-$template->param( has_debt => $has_debt ); >+$template->param( >+ can_be_discharged => $can_be_discharged, >+ discharge_problems => $discharge_problems, >+ has_checkouts => $has_pending_checkouts, >+ has_debt => $has_debt, >+); > > my $pending = Koha::Patron::Discharge::count({ > borrowernumber => $loggedinuser, >diff --git a/t/db_dependent/Patron/Borrower_Discharge.t b/t/db_dependent/Patron/Borrower_Discharge.t >index 8769f50f11..764bb6ccd9 100755 >--- a/t/db_dependent/Patron/Borrower_Discharge.t >+++ b/t/db_dependent/Patron/Borrower_Discharge.t >@@ -16,7 +16,7 @@ > > use Modern::Perl; > >-use Test::More tests => 25; >+use Test::More tests => 32; > > use Test::MockModule; > use Test::Warn; >@@ -83,7 +83,10 @@ $builder->build_sample_item( > ); > > AddIssue( $patron, $barcode ); >-is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }), 0, 'A patron with issues cannot be discharged' ); >+my ( $can, $problems ) = Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }); >+is( $can, 0, 'A patron with issues cannot be discharged' ); >+is( $problems->{checkouts}, 1, "Patron has checkouts" ); >+is( $problems->{debt}, undef, "Patron has no debt" ); > > is( Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernumber }), undef, 'No request done if patron has issues' ); > is( Koha::Patron::Discharge::discharge({ borrowernumber => $patron->borrowernumber }), undef, 'No discharge done if patron has issues' ); >@@ -99,14 +102,23 @@ $patron->account->add_debit( > } > ); > >+( $can, $problems ) = Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }); > is( >- Koha::Patron::Discharge::can_be_discharged( { borrowernumber => $patron->borrowernumber } ), 0, >+ $can, 0, > 'A patron with fines and checkouts cannot be discharged' > ); >+is( $problems->{checkouts}, 1, "Patron has checkouts" ); >+is( $problems->{debt}, 0.1, "Patron has debt" ); > > AddReturn( $barcode ); > >-is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }), 0, 'A patron with fines cannot be discharged' ); >+( $can, $problems ) = Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }); >+is( >+ $can, 0, >+ 'A patron with fines cannot be discharged' >+); >+is( $problems->{checkouts}, undef, "Patron has checkouts" ); >+is( $problems->{debt}, 0.1, "Patron has debt" ); > > > $patron->account->pay( >@@ -116,10 +128,12 @@ $patron->account->pay( > ); > > # Discharge possible without issue or fine >+( $can, $problems ) = Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }); > is( >- Koha::Patron::Discharge::can_be_discharged( { borrowernumber => $patron->borrowernumber } ), 1, >+ $can, 1, > 'A patron without issues or fines can be discharged' > ); >+is( scalar keys %$problems, 0, "No dishcharge problems found" ); > > is(Koha::Patron::Discharge::generate_as_pdf,undef,"Confirm failure when lacking borrower number"); > >-- >2.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14250
:
170136
|
170228
|
170581
|
170607
|
170662
|
170663
|
170664
|
170705
|
170706
|
170707
|
170708
|
171025
|
171026
|
171271
| 171272 |
171273
|
171274
|
171275
|
171276