Bugzilla – Attachment 167734 Details for
Bug 28924
Allow checkout fine limit to be determined by patron category
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28924: Update all usages of the current sysprefs
Bug-28924-Update-all-usages-of-the-current-syspref.patch (text/plain), 20.02 KB, created by
Matt Blenkinsop
on 2024-06-14 15:15:39 UTC
(
hide
)
Description:
Bug 28924: Update all usages of the current sysprefs
Filename:
MIME Type:
Creator:
Matt Blenkinsop
Created:
2024-06-14 15:15:39 UTC
Size:
20.02 KB
patch
obsolete
>From c496b4caa8d8ac452e5fc34860fb2a0ef1278162 Mon Sep 17 00:00:00 2001 >From: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> >Date: Mon, 22 Jan 2024 15:55:19 +0000 >Subject: [PATCH] Bug 28924: Update all usages of the current sysprefs > >This patch updates all instances where the current noissuescharge sysprefs are used. They will now use the is_patron_inside_charge_limits method to handle the patron category level limits > >Signed-off-by: David Nind <david@davidnind.com> >--- > C4/Circulation.pm | 93 ++++++++++++++++++++++++------------------- > C4/Members.pm | 31 ++++++--------- > C4/SIP/ILS/Patron.pm | 23 +++++------ > circ/circulation.pl | 32 ++++++--------- > members/moremember.pl | 21 +++++----- > 5 files changed, 95 insertions(+), 105 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index d8b75fe858..ae0296f372 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -819,9 +819,9 @@ sub CanBookBeIssued { > # > # BORROWER STATUS > # >- if ( $patron->category->category_type eq 'X' && ( $item_object->barcode ) ) { >- >- # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1 . >+ my $patron_borrowing_status = $patron->can_borrow({ patron => $patron }); >+ if ( $patron->category->category_type eq 'X' && ( $item_object->barcode )) { >+ # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1 . > C4::Stats::UpdateStats( > { > branch => C4::Context->userenv->{'branch'}, >@@ -865,63 +865,72 @@ sub CanBookBeIssued { > my $non_issues_charges = $account->non_issues_charges; > my $other_charges = $balance - $non_issues_charges; > >- my $amountlimit = C4::Context->preference("noissuescharge"); > my $allowfineoverride = C4::Context->preference("AllowFineOverride"); > my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride"); > > # Check the debt of this patrons guarantees >- my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees"); >- $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); >+ my $no_issues_charge_guarantees = $patron_borrowing_status->{NoIssuesChargeGuarantees}->{limit}; >+ $no_issues_charge_guarantees = undef unless looks_like_number($no_issues_charge_guarantees); > if ( defined $no_issues_charge_guarantees ) { >- my @guarantees = map { $_->guarantee } $patron->guarantee_relationships->as_list; >- my $guarantees_non_issues_charges = 0; >- foreach my $g ( @guarantees ) { >- $guarantees_non_issues_charges += $g->account->non_issues_charges; >- } >- >- if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && !$allowfineoverride) { >- $issuingimpossible{DEBT_GUARANTEES} = $guarantees_non_issues_charges; >- } elsif ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && $allowfineoverride) { >- $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges; >- } elsif ( $allfinesneedoverride && $guarantees_non_issues_charges > 0 && $guarantees_non_issues_charges <= $no_issues_charge_guarantees && !$inprocess ) { >- $needsconfirmation{DEBT_GUARANTEES} = $guarantees_non_issues_charges; >+ if ( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit} && !$inprocess && !$allowfineoverride ) { >+ $issuingimpossible{DEBT_GUARANTEES} = $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}; >+ } elsif ( $patron_borrowing_status->{NoIssuesChargeGuarantees}->{overlimit} >+ && !$inprocess >+ && $allowfineoverride ) >+ { >+ $needsconfirmation{DEBT_GUARANTEES} = $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}; >+ } elsif ( $allfinesneedoverride >+ && $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge} > 0 >+ && !$inprocess ) >+ { >+ $needsconfirmation{DEBT_GUARANTEES} = $patron_borrowing_status->{NoIssuesChargeGuarantees}->{charge}; > } > } > > # Check the debt of this patrons guarantors *and* the guarantees of those guarantors >- my $no_issues_charge_guarantors = C4::Context->preference("NoIssuesChargeGuarantorsWithGuarantees"); >- $no_issues_charge_guarantors = undef unless looks_like_number( $no_issues_charge_guarantors ); >+ my $no_issues_charge_guarantors = $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; >+ $no_issues_charge_guarantors = undef unless looks_like_number($no_issues_charge_guarantors); > if ( defined $no_issues_charge_guarantors ) { >- my $guarantors_non_issues_charges = $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 1 }); >- >- if ( $guarantors_non_issues_charges > $no_issues_charge_guarantors && !$inprocess && !$allowfineoverride) { >- $issuingimpossible{DEBT_GUARANTORS} = $guarantors_non_issues_charges; >- } elsif ( $guarantors_non_issues_charges > $no_issues_charge_guarantors && !$inprocess && $allowfineoverride) { >- $needsconfirmation{DEBT_GUARANTORS} = $guarantors_non_issues_charges; >- } elsif ( $allfinesneedoverride && $guarantors_non_issues_charges > 0 && $guarantors_non_issues_charges <= $no_issues_charge_guarantors && !$inprocess ) { >- $needsconfirmation{DEBT_GUARANTORS} = $guarantors_non_issues_charges; >+ if ( $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit} >+ && !$inprocess >+ && !$allowfineoverride ) >+ { >+ $issuingimpossible{DEBT_GUARANTORS} = >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}; >+ } elsif ( $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit} >+ && !$inprocess >+ && $allowfineoverride ) >+ { >+ $needsconfirmation{DEBT_GUARANTORS} = >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}; >+ } elsif ( $allfinesneedoverride >+ && $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge} > 0 >+ && !$inprocess ) >+ { >+ $needsconfirmation{DEBT_GUARANTORS} = >+ $patron_borrowing_status->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}; > } > } > > if ( C4::Context->preference("IssuingInProcess") ) { >- if ( $non_issues_charges > $amountlimit && !$inprocess && !$allowfineoverride) { >- $issuingimpossible{DEBT} = $non_issues_charges; >- } elsif ( $non_issues_charges > $amountlimit && !$inprocess && $allowfineoverride) { >- $needsconfirmation{DEBT} = $non_issues_charges; >- } elsif ( $allfinesneedoverride && $non_issues_charges > 0 && $non_issues_charges <= $amountlimit && !$inprocess ) { >- $needsconfirmation{DEBT} = $non_issues_charges; >+ if ( $patron_borrowing_status->{noissuescharge}->{overlimit} && !$inprocess && !$allowfineoverride ) { >+ $issuingimpossible{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >+ } elsif ( $patron_borrowing_status->{noissuescharge}->{overlimit} && !$inprocess && $allowfineoverride ) { >+ $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >+ } elsif ( $allfinesneedoverride && $patron_borrowing_status->{noissuescharge}->{charge} > 0 && !$inprocess ) { >+ $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; > } >- } >- else { >- if ( $non_issues_charges > $amountlimit && $allowfineoverride ) { >- $needsconfirmation{DEBT} = $non_issues_charges; >- } elsif ( $non_issues_charges > $amountlimit && !$allowfineoverride) { >- $issuingimpossible{DEBT} = $non_issues_charges; >- } elsif ( $non_issues_charges > 0 && $allfinesneedoverride ) { >- $needsconfirmation{DEBT} = $non_issues_charges; >+ } else { >+ if ( $patron_borrowing_status->{noissuescharge}->{overlimit} && $allowfineoverride ) { >+ $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >+ } elsif ( $patron_borrowing_status->{noissuescharge}->{overlimit} && !$allowfineoverride ) { >+ $issuingimpossible{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >+ } elsif ( $patron_borrowing_status->{noissuescharge}->{charge} > 0 && $allfinesneedoverride ) { >+ $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; > } > } > >+ > if ($balance > 0 && $other_charges > 0) { > $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges ); > } >diff --git a/C4/Members.pm b/C4/Members.pm >index 8ecdad3e73..f7f6c383a6 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -135,13 +135,13 @@ sub patronflags { > my $dbh=C4::Context->dbh; > my $patron = Koha::Patrons->find( $patroninformation->{borrowernumber} ); > my $account = $patron->account; >- my $owing = $account->non_issues_charges; >- if ( $owing > 0 ) { >+ my $patron_charge_limits = $patron->is_patron_inside_charge_limits( { patron => $patron } ); >+ if ( $patron_charge_limits->{noissuescharge}->{charge} > 0 ) { > my %flaginfo; >- my $noissuescharge = C4::Context->preference("noissuescharge") || 5; >- $flaginfo{'message'} = sprintf 'Patron owes %.02f', $owing; >- $flaginfo{'amount'} = sprintf "%.02f", $owing; >- if ( $owing > $noissuescharge && !C4::Context->preference("AllowFineOverride") ) { >+ my $noissuescharge = $patron_charge_limits->{noissuescharge}->{limit} || 5; >+ $flaginfo{'message'} = sprintf 'Patron owes %.02f', $patron_charge_limits->{noissuescharge}->{charge}; >+ $flaginfo{'amount'} = sprintf "%.02f", $patron_charge_limits->{noissuescharge}->{charge}; >+ if ( $patron_charge_limits->{noissuescharge}->{overlimit} && !C4::Context->preference("AllowFineOverride") ) { > $flaginfo{'noissues'} = 1; > } > $flags{'CHARGES'} = \%flaginfo; >@@ -153,21 +153,14 @@ sub patronflags { > $flags{'CREDITS'} = \%flaginfo; > } > >- # Check the debt of the guarntees of this patron >- my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees"); >- $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); >+ # Check the debt of this patrons guarantees >+ my $no_issues_charge_guarantees = $patron_charge_limits->{NoIssuesChargeGuarantees}->{limit}; >+ $no_issues_charge_guarantees = undef unless looks_like_number($no_issues_charge_guarantees); > if ( defined $no_issues_charge_guarantees ) { >- my $p = Koha::Patrons->find( $patroninformation->{borrowernumber} ); >- my @guarantees = map { $_->guarantee } $p->guarantee_relationships->as_list; >- my $guarantees_non_issues_charges = 0; >- foreach my $g ( @guarantees ) { >- $guarantees_non_issues_charges += $g->account->non_issues_charges; >- } >- >- if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees ) { >+ if ( $patron_charge_limits->{NoIssuesChargeGuarantees}->{overlimit} ) { > my %flaginfo; >- $flaginfo{'message'} = sprintf 'patron guarantees owe %.02f', $guarantees_non_issues_charges; >- $flaginfo{'amount'} = $guarantees_non_issues_charges; >+ $flaginfo{'message'} = sprintf 'patron guarantees owe %.02f', $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}; >+ $flaginfo{'amount'} = $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}; > $flaginfo{'noissues'} = 1 unless C4::Context->preference("allowfineoverride"); > $flags{'CHARGES_GUARANTEES'} = \%flaginfo; > } >diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm >index f3dfea74bd..9c3cf69989 100644 >--- a/C4/SIP/ILS/Patron.pm >+++ b/C4/SIP/ILS/Patron.pm >@@ -99,24 +99,25 @@ sub new { > $dexpiry and $dexpiry =~ s/-//g; # YYYYMMDD > > # Get fines and add fines for guarantees (depends on preference NoIssuesChargeGuarantees) >- my $fines_amount = ($patron->account->balance > 0) ? $patron->account->non_issues_charges : 0; >+ my $patron_charge_limits = $patron->is_patron_inside_charge_limits( { patron => $patron } ); >+ my $fines_amount = $patron_charge_limits->{noissuescharge}->{charge}; > my $personal_fines_amount = $fines_amount; >- my $fee_limit = _fee_limit(); >- my $noissueschargeguarantorswithguarantees = C4::Context->preference('NoIssuesChargeGuarantorsWithGuarantees'); >+ my $fee_limit = $patron_charge_limits->{noissuescharge}->{limit} || 5; >+ my $noissueschargeguarantorswithguarantees = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; >+ my $noissueschargeguarantees = $patron_charge_limits->{NoIssuesChargeGuarantees}->{limit}; > my $fines_msg = ""; > my $fine_blocked = 0; >- my $noissueschargeguarantees = C4::Context->preference('NoIssuesChargeGuarantees'); >- if( $fines_amount > $fee_limit ){ >+ if( $patron_charge_limits->{noissuescharge}->{overlimit} ){ > $fine_blocked = 1; > $fines_msg .= " -- " . "Patron blocked by fines" if $fine_blocked; > } elsif ( $noissueschargeguarantorswithguarantees ) { >- $fines_amount += $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 0 }); >- $fine_blocked ||= $fines_amount > $noissueschargeguarantorswithguarantees; >+ $fines_amount = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}; >+ $fine_blocked = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}; > $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on related accounts" if $fine_blocked; > } elsif ( $noissueschargeguarantees ) { > if( $patron->guarantee_relationships->count ){ >- $fines_amount += $patron->relationships_debt({ include_guarantors => 0, only_this_guarantor => 1, include_this_patron => 0 }); >- $fine_blocked ||= $fines_amount > $noissueschargeguarantees; >+ $fines_amount += $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}; >+ $fine_blocked = $patron_charge_limits->{NoIssuesChargeGuarantees}->{overlimit}; > $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on guaranteed accounts" if $fine_blocked; > } > } >@@ -499,10 +500,6 @@ sub inet_privileges { > return $self->{inet} ? 'Y' : 'N'; > } > >-sub _fee_limit { >- return C4::Context->preference('noissuescharge') || 5; >-} >- > sub excessive_fees { > my $self = shift; > return ($self->fee_amount and $self->fee_amount > $self->fee_limit); >diff --git a/circ/circulation.pl b/circ/circulation.pl >index d1c2b3b0cf..9cc67e8faa 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -594,13 +594,13 @@ if ( $patron ) { > $template->param( is_anonymous => 1 ); > $noissues = 1; > } >- my $account = $patron->account; >- if( ( my $owing = $account->non_issues_charges ) > 0 ) { >- my $noissuescharge = C4::Context->preference("noissuescharge") || 5; # FIXME If noissuescharge == 0 then 5, why?? >- $noissues ||= ( not C4::Context->preference("AllowFineOverride") and ( $owing > $noissuescharge ) ); >+ my $patron_charge_limits = $patron->is_patron_inside_charge_limits( { patron => $patron } ); >+ if( $patron_charge_limits->{noissuescharge}->{charge} > 0 ) { >+ my $noissuescharge = $patron_charge_limits->{noissuescharge}->{limit} || 5; # FIXME If noissuescharge == 0 then 5, why?? >+ $noissues ||= ( not C4::Context->preference("AllowFineOverride") and $patron_charge_limits->{noissuescharge}->{overlimit} ); > $template->param( > charges => 1, >- chargesamount => $owing, >+ chargesamount => $patron_charge_limits->{noissuescharge}->{charge}, > ) > } elsif ( $balance < 0 ) { > $template->param( >@@ -610,30 +610,24 @@ if ( $patron ) { > } > > # Check the debt of this patrons guarantors *and* the guarantees of those guarantors >- my $no_issues_charge_guarantors = C4::Context->preference("NoIssuesChargeGuarantorsWithGuarantees"); >- if ( $no_issues_charge_guarantors ) { >- my $guarantors_non_issues_charges = $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 1 }); >- >- if ( $guarantors_non_issues_charges > $no_issues_charge_guarantors ) { >+ my $no_issues_charge_guarantors = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; >+ if ($no_issues_charge_guarantors) { >+ if ( $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit} ) { > $template->param( >- charges_guarantors_guarantees => $guarantors_non_issues_charges >+ noissues => 1, >+ charges_guarantors_guarantees => $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{charge} > ); > $noissues = 1 unless C4::Context->preference("allowfineoverride"); > } > } > >- my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees"); >+ my $no_issues_charge_guarantees = $patron_charge_limits->{NoIssuesChargeGuarantees}->{limit}; > $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); > if ( defined $no_issues_charge_guarantees ) { >- my $guarantees_non_issues_charges = 0; >- my $guarantees = $patron->guarantee_relationships->guarantees; >- while ( my $g = $guarantees->next ) { >- $guarantees_non_issues_charges += $g->account->non_issues_charges; >- } >- if ( $guarantees_non_issues_charges > $no_issues_charge_guarantees ) { >+ if ( $patron_charge_limits->{NoIssuesChargeGuarantees}->{overlimit} ) { > $template->param( > charges_guarantees => 1, >- chargesamount_guarantees => $guarantees_non_issues_charges, >+ chargesamount_guarantees => $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}, > ); > $noissues = 1 unless C4::Context->preference("allowfineoverride"); > } >diff --git a/members/moremember.pl b/members/moremember.pl >index c10926cd36..ba24fbfa2e 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -242,29 +242,26 @@ my $issues = $patron->checkouts; > my $balance = 0; > $balance = $patron->account->balance; > >-my $account = $patron->account; >-if( ( my $owing = $account->non_issues_charges ) > 0 ) { >- my $noissuescharge = C4::Context->preference("noissuescharge") || 5; # FIXME If noissuescharge == 0 then 5, why?? >+my $patron_charge_limits = $patron->is_patron_inside_charge_limits( { patron => $patron } ); >+if ( $patron_charge_limits->{noissuescharge}->{charge} > 0 ) { > $template->param( >- charges => 1, >- chargesamount => $owing, >- ) >+ charges => 1, >+ chargesamount => $patron_charge_limits->{noissuescharge}->{charge}, >+ ); > } elsif ( $balance < 0 ) { > $template->param( >- credits => 1, >+ credits => 1, > creditsamount => -$balance, > ); > } > > # Check the debt of this patrons guarantors *and* the guarantees of those guarantors >-my $no_issues_charge_guarantors = C4::Context->preference("NoIssuesChargeGuarantorsWithGuarantees"); >+my $no_issues_charge_guarantors = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; > if ( $no_issues_charge_guarantors ) { >- my $guarantors_non_issues_charges = $patron->relationships_debt({ include_guarantors => 1, only_this_guarantor => 0, include_this_patron => 1 }); >- >- if ( $guarantors_non_issues_charges > $no_issues_charge_guarantors ) { >+ if ( $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit} ) { > $template->param( > noissues => 1, >- charges_guarantors_guarantees => $guarantors_non_issues_charges >+ charges_guarantors_guarantees => $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{charge} > ); > } > } >-- >2.39.3 (Apple Git-146)
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 28924
:
165870
|
165871
|
165872
|
165873
|
165874
|
165875
|
165876
|
166042
|
166043
|
166044
|
166045
|
166046
|
166047
|
166048
|
166171
|
166172
|
166173
|
166174
|
166175
|
166176
|
166177
|
167719
|
167720
|
167721
|
167722
|
167723
|
167724
|
167725
|
167726
|
167727
|
167728
|
167729
|
167730
|
167731
|
167732
|
167733
|
167734
|
167735
|
167736
|
167737
|
167738
|
167739
|
167740
|
168728
|
168729
|
168730
|
168731
|
168732
|
168733
|
168734
|
168735
|
168736
|
168737
|
168738
|
168984
|
168985
|
168986
|
168987
|
168988
|
168989
|
168990
|
168991
|
168992
|
168993
|
168994
|
168995
|
169177
|
169428