Bugzilla – Attachment 178598 Details for
Bug 38924
Introduce an organization level loan 'Quota' system for Koha
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38924: Move from get_patron_quota to Koha::Patron relations
Bug-38924-Move-from-getpatronquota-to-KohaPatron-r.patch (text/plain), 11.53 KB, created by
Martin Renvoize (ashimema)
on 2025-02-24 16:29:54 UTC
(
hide
)
Description:
Bug 38924: Move from get_patron_quota to Koha::Patron relations
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-02-24 16:29:54 UTC
Size:
11.53 KB
patch
obsolete
>From f99c28e3c9e8bdfece4f3485ca37db009ae94d35 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 10 Feb 2025 14:43:58 +0000 >Subject: [PATCH] Bug 38924: Move from get_patron_quota to Koha::Patron > relations > >Bug 38924: Deal with multiple quota options in QUOTA_EXCEEDED handling > >Bug 38924: Deal with using the same quota for renewals >--- > C4/Circulation.pm | 67 ++++++++++---------- > Koha/Patron.pm | 63 +++++++++++++++++++ > Koha/Patron/Quotas.pm | 108 +++------------------------------ > Koha/REST/V1/Patrons/Quotas.pm | 2 +- > circ/circulation.pl | 1 + > 5 files changed, 108 insertions(+), 133 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 5b9bc435d73..c9664352e83 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1356,25 +1356,27 @@ sub CanBookBeIssued { > } > > # CHECK FOR QUOTAS >- if ( my $quota = Koha::Patron::Quotas->get_patron_quota($patron->borrowernumber) ) { >- if (ref($quota) eq 'ARRAY') { >+ if ( my $quotas = $patron->all_quotas->filter_by_active ) { >+ if ( $quotas->count > 1 ) { >+ > # Multiple available quotas found - need confirmation from user >- $needsconfirmation{QUOTA_SELECT} = $quota; >- } >- elsif (!$quota->has_available_quota) { >- if ( C4::Context->preference("AllowQuotaOverride") ) { >- $needsconfirmation{QUOTA_EXCEEDED} = { >- available => $quota->available_quota, >- total => $quota->allocation, >- used => $quota->used, >- }; >- } >- else { >- $issuingimpossible{QUOTA_EXCEEDED} = { >- available => $quota->available_quota, >- total => $quota->allocation, >- used => $quota->used, >- }; >+ $needsconfirmation{QUOTA_SELECT} = $quotas; >+ } elsif ( $quotas->count == 1 ) { >+ my $quota = $quotas->next; >+ if ( !$quota->has_available_quota ) { >+ if ( C4::Context->preference("AllowQuotaOverride") ) { >+ $needsconfirmation{QUOTA_EXCEEDED} = { >+ available => $quota->available_quota, >+ total => $quota->allocation, >+ used => $quota->used, >+ }; >+ } else { >+ $issuingimpossible{QUOTA_EXCEEDED} = { >+ available => $quota->available_quota, >+ total => $quota->allocation, >+ used => $quota->used, >+ }; >+ } > } > } > } >@@ -1935,7 +1937,7 @@ sub AddIssue { > if ($selected_quota_id) { > $quota = Koha::Patron::Quotas->find($selected_quota_id); > } else { >- $quota = Koha::Patron::Quotas->get_patron_quota($patron->borrowernumber); >+ $quota = $patron->all_quotas->filter_by_active->single; > } > > if ($quota) { >@@ -3283,14 +3285,17 @@ sub CanBookBeRenewed { > } > > # # CHECK FOR QUOTAS >- if (my $quota_usage = Koha::Patron::Quota::Usages->find({ issue_id => $issue->issue_id })) { >- >- # Get current active quota for the patron >- if (my $active_quota = Koha::Patron::Quotas->get_active_quota($quota_usage->patron_id)) { >- >- if (!$active_quota->has_available_quota) { >- return (0, "QUOTA_EXCEEDED"); >+ if ( my $quota_usage = Koha::Patron::Quota::Usages->find( { issue_id => $issue->issue_id } ) ) { >+ >+ # Get current actives quota for the patron >+ if ( my $active_quotas = $patron->all_quotas->filter_by_active ) { >+ my $all_exceeded = 1; >+ while ( my $active_quota = $active_quotas->next ) { >+ if ( $active_quota->has_available_quota ) { >+ $all_exceeded = 0; >+ } > } >+ return ( 0, "QUOTA_EXCEEDED" ) if $all_exceeded; > } > } > >@@ -3409,12 +3414,12 @@ sub AddRenewal { > # Check quotas and record usage if needed > if (my $quota_usage = Koha::Patron::Quota::Usages->find({ issue_id => $issue->issue_id })) { > # Get current active quota for the patron >- if (my $active_quota = Koha::Patron::Quotas->get_active_quota($quota_usage->patron_id)) { >+ if (my $active_quota = Koha::Patron::Quotas->find($quota_usage->patron_quota_id)) { > # Update patron's used quota value >- $active_quota->add_usage({ >- patron_id => $patron->patron_id, >- issue_id => $issue->issue_id, >- }); >+ $active_quota->add_usage({ >+ patron_id => $patron->borrowernumber, >+ issue_id => $issue->issue_id, >+ }); > } > } > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index ec7f6704983..fc873fec2ed 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -595,6 +595,69 @@ sub is_guarantor { > return $self->guarantee_relationships()->count(); > } > >+=head3 quotas >+ >+Returns any quotas this patron may have >+ >+=cut >+ >+sub quotas { >+ my ($self) = @_; >+ my $quotas_rs = $self->_result->patron_quotas; >+ return Koha::Patron::Quotas->_new_from_dbic($quotas_rs); >+} >+ >+=head3 guarantors_quotas >+ >+Returns any guarantor quotas this patron may use >+ >+=cut >+ >+sub guarantors_quotas { >+ my ($self) = @_; >+ my $guarantors_query = >+ $self->_result->borrower_relationships_guarantors->search( {}, { columns => ['guarantor_id'] } )->as_query; >+ return Koha::Patron::Quotas->search( >+ { >+ patron_id => { '-in' => $guarantors_query }, >+ } >+ ); >+} >+ >+=head3 all_quotas >+ >+Returns a Koha::Patron::Quotas resultset containing: >+- Directly related quotas (first) >+- Guarantor quotas (second, if applicable) >+ >+=cut >+ >+sub all_quotas { >+ my ($self) = @_; >+ >+ # Step 1: Build a subquery to fetch the patron's own ID + guarantor IDs from borrower_relationships >+ my $guarantors_query = >+ $self->_result->borrower_relationships_guarantees->search( {}, { columns => ['guarantor_id'] } )->as_query; >+ >+ # Step 2: Fetch quotas where borrowernumber is either: >+ # - The patron's own borrowernumber >+ # - One of the guarantor_id values found in the subquery >+ return Koha::Patron::Quotas->search( >+ { >+ '-or' => [ >+ { patron_id => $self->id }, # The patron's own quotas >+ { patron_id => { '-in' => $guarantors_query } }, # Guarantor quotas if they exist >+ ], >+ }, >+ { >+ order_by => \[ >+ "CASE WHEN patron_id = ? THEN 0 ELSE 1 END, patron_id", >+ $self->id >+ ], >+ } >+ ); >+} >+ > =head3 relationships_debt > > Returns the amount owed by the patron's guarantors *and* the other guarantees of those guarantors >diff --git a/Koha/Patron/Quotas.pm b/Koha/Patron/Quotas.pm >index c8e2715a86b..fc55f26da23 100644 >--- a/Koha/Patron/Quotas.pm >+++ b/Koha/Patron/Quotas.pm >@@ -13,71 +13,6 @@ Koha::Patron::Quotas - Koha Patron Quota Object set class > > =head2 Class Methods > >-=head3 get_patron_quota >- >- my $quota = Koha::Patron::Quotas->get_patron_quota($patron_id); >- >-Searches for any applicable quota for the given patron. Returns: >-- First available quota found for patron or their guarantor (if UseGuarantorQuota enabled) >-- If no available quota found, returns the first found quota >-- Returns undef if no quota found >- >-=cut >- >-sub get_patron_quota { >- my ( $self, $patron_id ) = @_; >- >- my @available_quotas; >- my $first_found_quota = undef; >- >- # First check patron's own quota >- my $patron_quota = $self->search( >- { >- patron_id => $patron_id, >- start_date => { '<=' => \'CURRENT_DATE' }, >- end_date => { '>=' => \'CURRENT_DATE' } >- } >- )->single; >- >- if ( $patron_quota && $patron_quota->has_available_quota ) { >- push @available_quotas, $patron_quota; >- } >- $first_found_quota ||= $patron_quota if $patron_quota; >- >- # Then check all guarantor quotas if enabled >- if ( C4::Context->preference('UseGuarantorQuota') ) { >- my $patron = Koha::Patrons->find($patron_id); >- my @guarantors = $patron->guarantor_relationships->guarantors->as_list; >- >- foreach my $guarantor (@guarantors) { >- my $guarantor_quota = $self->search( >- { >- patron_id => $guarantor->borrowernumber, >- start_date => { '<=' => \'CURRENT_DATE' }, >- end_date => { '>=' => \'CURRENT_DATE' } >- } >- )->single; >- >- if ($guarantor_quota) { >- >- # Store first found quota in case we need it later >- $first_found_quota ||= $guarantor_quota; >- >- # Collect available guarantor quotas >- if ( $guarantor_quota->has_available_quota ) { >- push @available_quotas, $guarantor_quota; >- } >- } >- } >- } >- >- # Return single quota if only one available, array of quotas if multiple available, >- # first found quota if none available >- return $available_quotas[0] if @available_quotas == 1; >- return \@available_quotas if @available_quotas > 1; >- return $first_found_quota; >-} >- > =head3 create_quota > > Creates a new quota record for a patron >@@ -90,55 +25,26 @@ sub create_quota { > return Koha::Patron::Quota->new($params)->store; > } > >-=head3 search_by_patron >+=head3 filter_by_active > >-Finds all quotas for a patron >+ $quotas->filter_by_active; > >-=cut >+Returns the currently active quotas for the resultset. > >-sub search_by_patron { >- my ( $self, $patron_id ) = @_; >- return $self->search( { patron_id => $patron_id } ); >-} >- >-=head3 get_active_quotas >- >-Returns multiple active quota records >+Active means start_date <= NOW() and end_date >= NOW(). > > =cut > >-sub get_active_quotas { >+sub filter_by_active { > my ($self) = @_; > return $self->search( > { >- start_date => { '<=' => \'CURRENT_DATE' }, >- end_date => { '>=' => \'CURRENT_DATE' } >+ start_date => { '<=' => \'NOW()' }, >+ end_date => { '>=' => \'NOW()' }, > } > ); > } > >-=head2 get_active_quota >- >- my $active_quota = Koha::Patron::Quotas->get_active_quota($patron_id); >- >-Returns the currently active quota for a patron if one exists. >-Active means start_date <= NOW() and end_date >= NOW(). >-Returns undef if no active quota found. >- >-=cut >- >-sub get_active_quota { >- my ( $self, $patron_id ) = @_; >- >- return Koha::Patron::Quotas->search( >- { >- patron_id => $patron_id, >- start_date => { '<=' => \'CURRENT_DATE' }, >- end_date => { '>=' => \'CURRENT_DATE' } >- } >- )->single; >-} >- > =head2 Internal methods > > =head3 _type >diff --git a/Koha/REST/V1/Patrons/Quotas.pm b/Koha/REST/V1/Patrons/Quotas.pm >index 9067f16c76d..3d1ac6a879e 100644 >--- a/Koha/REST/V1/Patrons/Quotas.pm >+++ b/Koha/REST/V1/Patrons/Quotas.pm >@@ -34,7 +34,7 @@ sub list { > } > > if ($only_active) { >- $quotas_set = $quotas_set->get_active_quotas; >+ $quotas_set = $quotas_set->filter_by_active; > } > > return $c->render( >diff --git a/circ/circulation.pl b/circ/circulation.pl >index 6e62a6bdeb8..1403c2cb95b 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -320,6 +320,7 @@ if ($patron) { > { > $template->param( limited_category => 1 ); > } >+ > } > > # >-- >2.48.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 38924
:
178582
|
178583
|
178584
|
178585
|
178586
|
178587
|
178588
|
178589
|
178590
|
178591
|
178592
|
178593
|
178594
|
178595
|
178596
|
178597
| 178598 |
178599
|
178600
|
178601
|
178602
|
178603
|
178604
|
178605
|
178606
|
178607
|
178608