Bugzilla – Attachment 184893 Details for
Bug 40554
Improve performance using a cache for AV descriptions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40554: find_description
Bug-40554-finddescription.patch (text/plain), 21.15 KB, created by
Jonathan Druart
on 2025-07-30 14:14:43 UTC
(
hide
)
Description:
Bug 40554: find_description
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2025-07-30 14:14:43 UTC
Size:
21.15 KB
patch
obsolete
>From 20e4e9e33b60be8249b26ae95de4aa33b35c8d3f Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 30 Jul 2025 15:54:58 +0200 >Subject: [PATCH] Bug 40554: find_description > >--- > C4/Circulation.pm | 4 +-- > C4/Search.pm | 15 ++++++----- > Koha/Acquisition/Bookseller/Issue.pm | 10 ++------ > Koha/ArticleRequest.pm | 8 +++--- > Koha/AuthorisedValues.pm | 31 +++++++++++++++++++++++ > Koha/ILL/Request.pm | 11 +++----- > Koha/Object/Mixin/AdditionalFields.pm | 6 ++--- > Koha/Patron/Attribute.pm | 12 +++++++-- > Koha/Preservation/Train/Item/Attribute.pm | 5 +--- > Koha/Suggestion.pm | 14 +++------- > Koha/Template/Plugin/AuthorisedValues.pm | 9 ++----- > Koha/Ticket.pm | 6 ++--- > Koha/Ticket/Update.pm | 6 ++--- > admin/patron-attr-types.pl | 3 +-- > circ/circulation.pl | 4 +-- > members/memberentry.pl | 3 +-- > members/moremember.pl | 3 +-- > opac/opac-memberentry.pl | 5 ++-- > reports/borrowers_stats.pl | 3 +-- > serials/subscription-detail.pl | 8 +++--- > t/db_dependent/Koha/Item.t | 2 +- > t/db_dependent/XSLT.t | 2 +- > 22 files changed, 91 insertions(+), 79 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 48185eb0224..0d75aa3e8ac 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1127,8 +1127,8 @@ sub CanBookBeIssued { > $issuingimpossible{RESTRICTED} = 1; > } > if ( $item_object->itemlost && C4::Context->preference("IssueLostItem") ne 'nothing' ) { >- my $av = Koha::AuthorisedValues->search( { category => 'LOST', authorised_value => $item_object->itemlost } ); >- my $code = $av->count ? $av->next->lib : ''; >+ my $code = Koha::AuthorisedValues->find_description( >+ { category => 'LOST', authorised_value => $item_object->itemlost } ); > $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' ); > $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' ); > } >diff --git a/C4/Search.pm b/C4/Search.pm >index a8c89b4998a..efc34647235 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -545,18 +545,21 @@ sub getRecords { > if ( $link_value =~ /location/ ) { > > # TODO Retrieve all authorised values at once, instead of 1 query per entry >- my $av = >- Koha::AuthorisedValues->search( { category => 'LOC', authorised_value => $one_facet } ); >- $facet_label_value = $av->count ? $av->next->opac_description : ''; >+ $facet_label_value = Koha::AuthorisedValues->find_description( >+ { category => 'LOC', authorised_value => $one_facet, opac => 1, default_to_blank => 1 } >+ ); > } > > # also, if it's a collection code, use the name instead of the code > if ( $link_value =~ /ccode/ ) { > > # TODO Retrieve all authorised values at once, instead of 1 query per entry >- my $av = Koha::AuthorisedValues->search( >- { category => 'CCODE', authorised_value => $one_facet } ); >- $facet_label_value = $av->count ? $av->next->opac_description : ''; >+ $facet_label_value = Koha::AuthorisedValues->find_description( >+ { >+ category => 'CCODE', authorised_value => $one_facet, opac => 1, >+ default_to_blank => 1 >+ } >+ ); > } > > # but we're down with the whole label being in the link's title. >diff --git a/Koha/Acquisition/Bookseller/Issue.pm b/Koha/Acquisition/Bookseller/Issue.pm >index 5ff3e133085..15681f009ff 100644 >--- a/Koha/Acquisition/Bookseller/Issue.pm >+++ b/Koha/Acquisition/Bookseller/Issue.pm >@@ -41,20 +41,14 @@ sub strings_map { > my $strings = {}; > > if ( defined $self->type ) { >- my $av = Koha::AuthorisedValues->search( >+ my $type_str = Koha::AuthorisedValues->find_description( > { > category => ISSUE_TYPE_CATEGORY, > authorised_value => $self->type, >+ opac => $params->{public} > } > ); > >- my $type_str = >- $av->count >- ? $params->{public} >- ? $av->next->opac_description >- : $av->next->lib >- : $self->type; >- > $strings->{type} = { > category => ISSUE_TYPE_CATEGORY, > str => $type_str, >diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm >index f1d5968754f..0916b577851 100644 >--- a/Koha/ArticleRequest.pm >+++ b/Koha/ArticleRequest.pm >@@ -292,13 +292,13 @@ sub notify { > my $status = $self->status; > my $reason = $self->notes; > if ( !defined $reason && $self->cancellation_reason ) { >- my $av = Koha::AuthorisedValues->search( >+ $reason = Koha::AuthorisedValues->find_description( > { > category => 'AR_CANCELLATION', >- authorised_value => $self->cancellation_reason >+ authorised_value => $self->cancellation_reason, >+ opac => 1, > } >- )->next; >- $reason = $av->lib_opac ? $av->lib_opac : $av->lib if $av; >+ ); > } > > require C4::Letters; >diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm >index 934b1e6a7b3..709bffb93cf 100644 >--- a/Koha/AuthorisedValues.pm >+++ b/Koha/AuthorisedValues.pm >@@ -20,6 +20,7 @@ package Koha::AuthorisedValues; > use Modern::Perl; > > use Koha::Database; >+use Koha::Caches; > > use Koha::AuthorisedValue; > use Koha::MarcSubfieldStructures; >@@ -37,6 +38,36 @@ Koha::AuthorisedValues - Koha Authorised value Object set class > > =cut > >+=head2 >+ >+ >+ >+=cut >+ >+sub find_description_by_code { >+ my ( $self, $params ) = @_; >+ my $category = $params->{category}; >+ my $value = $params->{value}; >+ my $opac = $params->{opac}; >+ my $default_to_blank = $params->{default_to_blank}; >+ >+ return unless $category || $value; >+ >+ # Is this cached already? >+ my $cache_key = "AuthorisedValues-$category-values-$value-$opac-$default_to_blank"; >+ my $cache = Koha::Caches->get_instance(); >+ my $result = $cache->get_from_cache($cache_key); >+ return $result if $result; >+ >+ my $av = $self->find( { category => $category, authorised_value => $value } ); >+ >+ my $description = $av ? $opac ? $av->opac_description : $av->lib : $default_to_blank ? q{} : $value; >+ >+ $cache->set_in_cache( $cache_key, $description, { expiry => 5 } ); >+ >+ return $description; >+} >+ > =head2 search_by_marc_field > > Missing POD for search_by_marc_field. >diff --git a/Koha/ILL/Request.pm b/Koha/ILL/Request.pm >index 4c8453d4dd5..0c8d629388b 100644 >--- a/Koha/ILL/Request.pm >+++ b/Koha/ILL/Request.pm >@@ -182,17 +182,14 @@ sub statusalias { > my ($self) = @_; > return unless $self->status_alias; > >- # We can't know which result is the right one if there are multiple >- # ILL_STATUS_ALIAS authorised values with the same authorised_value column value >- # so we just use the first >- return Koha::AuthorisedValues->search( >+ # FIXME We should use Koha::AuthorisedValues->find_description where this method is called >+ # to retrieve the value from the cache >+ return Koha::AuthorisedValues->find( > { > category => 'ILL_STATUS_ALIAS', > authorised_value => $self->get_column('status_alias'), > }, >- {}, >- $self->branchcode >- )->next; >+ ); > } > > =head3 illrequestattributes >diff --git a/Koha/Object/Mixin/AdditionalFields.pm b/Koha/Object/Mixin/AdditionalFields.pm >index 39334fd6841..166741951fa 100644 >--- a/Koha/Object/Mixin/AdditionalFields.pm >+++ b/Koha/Object/Mixin/AdditionalFields.pm >@@ -271,15 +271,13 @@ sub strings_map { > > foreach my $av_value ( @{ $afvs->{$af_id} } ) { > if ($av_cat) { >- my $av = Koha::AuthorisedValues->search( >+ $value_to_push = Koha::AuthorisedValues->find_description( > { > category => $av_cat, > authorised_value => $av_value, >+ opac => $params->{public}, > } > ); >- >- $value_to_push = >- $av->count ? $params->{public} ? $av->next->opac_description : $av->next->lib : $av_value; > } else { > $value_to_push = $av_value; > } >diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm >index d9055483d21..82fe4ac54f0 100644 >--- a/Koha/Patron/Attribute.pm >+++ b/Koha/Patron/Attribute.pm >@@ -110,6 +110,7 @@ Return undef if this attribute is not attached to an authorised value > > =cut > >+# FIXME Can this method be removed? > sub authorised_value { > my ($self) = @_; > >@@ -138,10 +139,17 @@ displayed instead of the code. > > sub description { > my ($self) = @_; >+ > if ( $self->type->authorised_value_category ) { >- my $av = $self->authorised_value; >- return $av ? $av->lib : ""; >+ return Koha::AuthorisedValues->find_description( >+ { >+ category => $self->type->authorised_value_category, >+ authorised_value => $self->attribute, >+ default_to_blank => 1, >+ } >+ ); > } >+ > return $self->attribute; > } > >diff --git a/Koha/Preservation/Train/Item/Attribute.pm b/Koha/Preservation/Train/Item/Attribute.pm >index 4b9474efe4b..227afe50100 100644 >--- a/Koha/Preservation/Train/Item/Attribute.pm >+++ b/Koha/Preservation/Train/Item/Attribute.pm >@@ -60,15 +60,12 @@ sub strings_map { > my ($self) = @_; > my $str = $self->value; > if ( $self->processing_attribute->type eq 'authorised_value' ) { >- my $av = Koha::AuthorisedValues->search( >+ $str = Koha::AuthorisedValues->find_description( > { > category => $self->processing_attribute->option_source, > authorised_value => $self->value, > } > ); >- if ( $av->count ) { >- $str = $av->next->lib || $self->value; >- } > } > > return { >diff --git a/Koha/Suggestion.pm b/Koha/Suggestion.pm >index f44342c687e..d2a20fa7f1c 100644 >--- a/Koha/Suggestion.pm >+++ b/Koha/Suggestion.pm >@@ -56,12 +56,12 @@ sub store { > my @status_constants = qw(ASKED CHECKED ACCEPTED REJECTED ORDERED AVAILABLE); > Koha::Exceptions::Suggestion::StatusForbidden->throw( STATUS => $self->STATUS ) > unless ( grep { $self->STATUS eq $_ } @status_constants ) >- || Koha::AuthorisedValues->search( >+ || Koha::AuthorisedValues->find( > { > category => 'SUGGEST_STATUS', > authorised_value => $self->STATUS > } >- )->count; >+ ); > > $self->branchcode(undef) if defined $self->branchcode && $self->branchcode eq ''; > unless ( $self->suggesteddate() ) { >@@ -312,14 +312,8 @@ sub strings_map { > }; > > foreach my $key ( keys %$required_strings ) { >- my $av = Koha::AuthorisedValues->search( >- { category => $required_strings->{$key}, authorised_value => $self->$key } ); >- my $status_str = >- $av->count >- ? $params->{public} >- ? $av->next->opac_description >- : $av->next->lib >- : $self->$key; >+ my $status_str = Koha::AuthorisedValues->find_description( >+ { category => $required_strings->{$key}, authorised_value => $self->$key, opac => $params->{public} } ); > > $strings->{$key} = { > category => $required_strings->{$key}, >diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm >index 5fe23da19c4..864cdd96133 100644 >--- a/Koha/Template/Plugin/AuthorisedValues.pm >+++ b/Koha/Template/Plugin/AuthorisedValues.pm >@@ -26,13 +26,8 @@ use Koha::AuthorisedValues; > > sub GetByCode { > my ( $self, $category, $code, $opac ) = @_; >- my $av = Koha::AuthorisedValues->search( { category => $category, authorised_value => $code } ); >- return >- $av->count >- ? $opac >- ? $av->next->opac_description >- : $av->next->lib >- : $code; >+ return Koha::AuthorisedValues->find_description_by_code( >+ { category => $category, authorised_value => $code, opac => $opac, } ); > } > > sub Get { >diff --git a/Koha/Ticket.pm b/Koha/Ticket.pm >index 18f9c672a39..b4366ce486a 100644 >--- a/Koha/Ticket.pm >+++ b/Koha/Ticket.pm >@@ -245,7 +245,7 @@ sub strings_map { > my $strings = {}; > > if ( defined $self->status ) { >- my $av = Koha::AuthorisedValues->search( >+ my $av = Koha::AuthorisedValues->find( > { > category => 'TICKET_STATUS', > authorised_value => $self->status, >@@ -253,8 +253,8 @@ sub strings_map { > ); > > # Fall back to TICKET_RESOLUTION as needed >- if ( !$av->count ) { >- $av = Koha::AuthorisedValues->search( >+ unless ($av) { >+ $av = Koha::AuthorisedValues->find( > { > category => 'TICKET_RESOLUTION', > authorised_value => $self->status, >diff --git a/Koha/Ticket/Update.pm b/Koha/Ticket/Update.pm >index 6ea262997aa..9fd7444e933 100644 >--- a/Koha/Ticket/Update.pm >+++ b/Koha/Ticket/Update.pm >@@ -91,7 +91,7 @@ sub strings_map { > my $strings = {}; > > if ( defined $self->status ) { >- my $av = Koha::AuthorisedValues->search( >+ my $av = Koha::AuthorisedValues->find( > { > category => 'TICKET_STATUS', > authorised_value => $self->status, >@@ -99,8 +99,8 @@ sub strings_map { > ); > > # Fall back to TICKET_RESOLUTION as needed >- if ( !$av->count ) { >- $av = Koha::AuthorisedValues->search( >+ unless ($av) { >+ $av = Koha::AuthorisedValues->find( > { > category => 'TICKET_RESOLUTION', > authorised_value => $self->status, >diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl >index f7e6e4daa90..ed8baa86f67 100755 >--- a/admin/patron-attr-types.pl >+++ b/admin/patron-attr-types.pl >@@ -266,8 +266,7 @@ sub patron_attribute_type_list { > next if $attr->class ne $class; > push @items, $attr; > } >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- my $lib = $av->count ? $av->next->lib : $class; >+ my $lib = Koha::AuthorisedValues->find_description( { category => 'PA_CLASS', authorised_value => $class } ); > push @attributes_loop, { > class => $class, > items => \@items, >diff --git a/circ/circulation.pl b/circ/circulation.pl >index 0a9906028e9..5b70d6ac936 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -740,8 +740,8 @@ my $relatives_issues_count = > Koha::Database->new()->schema()->resultset('Issue')->count( { borrowernumber => \@relatives } ); > > if ($patron) { >- my $av = Koha::AuthorisedValues->search( { category => 'ROADTYPE', authorised_value => $patron->streettype } ); >- my $roadtype = $av->count ? $av->next->lib : ''; >+ my $roadtype = Koha::AuthorisedValues->find_description( >+ { category => 'ROADTYPE', authorised_value => $patron->streettype, default_to_blank => 1 } ); > $template->param( > roadtype => $roadtype, > patron => $patron, >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 5f15a105556..857d5431846 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -988,8 +988,7 @@ sub patron_attributes_form { > } > } > for my $class ( sort keys %items_by_class ) { >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- my $lib = $av->count ? $av->next->lib : $class; >+ my $lib = Koha::AuthorisedValues->find_descriptoin( { category => 'PA_CLASS', authorised_value => $class } ); > push @attribute_loop, { > class => $class, > items => $items_by_class{$class}, >diff --git a/members/moremember.pl b/members/moremember.pl >index 4a8d789d1f2..62f320c429a 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -152,8 +152,7 @@ if ( C4::Context->preference('ExtendedPatronAttributes') ) { > for my $attr (@attributes) { > push @items, $attr if $attr->type->class eq $class; > } >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- my $lib = $av->count ? $av->next->lib : $class; >+ my $lib = Koha::AuthorisedValues->find_description( { category => 'PA_CLASS', authorised_value => $class } ); > > push @attributes_loop, { > class => $class, >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index 14ec05e7534..b4b0b01b382 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -740,9 +740,8 @@ sub GeneratePatronAttributesForm { > foreach my $class (@classes) { > next unless ( $items_by_class{$class} ); > >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- >- my $lib = $av->count ? $av->next->opac_description : $class; >+ my $lib = Koha::AuthorisedValues->find_description( >+ { category => 'PA_CLASS', authorised_value => $class, opac => 1 } ); > > push @class_loop, > { >diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl >index 98d675067c3..3dd525d4823 100755 >--- a/reports/borrowers_stats.pl >+++ b/reports/borrowers_stats.pl >@@ -524,8 +524,7 @@ sub patron_attributes_form { > > my @attribute_loop; > foreach my $class ( sort keys %items_by_class ) { >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- my $lib = $av->count ? $av->next->lib : $class; >+ my $lib = Koha::AuthorisedValues->find_description( { category => 'PA_CLASS', authorised_value => $class } ); > push @attribute_loop, { > class => $class, > items => $items_by_class{$class}, >diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl >index 747f0c282ac..0a5a288ebca 100755 >--- a/serials/subscription-detail.pl >+++ b/serials/subscription-detail.pl >@@ -119,10 +119,10 @@ for my $date (qw(startdate enddate firstacquidate histstartdate histenddate)) { > $subs->{$date} = output_pref( { str => $subs->{$date}, dateonly => 1 } ) > if $subs->{$date}; > } >-my $av = Koha::AuthorisedValues->search( { category => 'LOC', authorised_value => $subs->{location} } ); >-$subs->{location} = $av->count ? $av->next->lib : ''; >-$av = Koha::AuthorisedValues->search( { category => 'CCODE', authorised_value => $subs->{ccode} } ); >-$subs->{ccode} = $av->count ? $av->next->lib : ''; >+$subs->{location} = Koha::AuthorisedValues->find_description( >+ { category => 'LOC', authorised_value => $subs->{location}, default_to_blank => 1 } ); >+$subs->{ccode} = Koha::AuthorisedValues->find_description( >+ { category => 'CCODE', authorised_value => $subs->{ccode}, default_to_blank => 1 } ); > $subs->{abouttoexpire} = abouttoexpire( $subs->{subscriptionid} ); > $template->param( %{$subs} ); > $template->param( biblionumber_for_new_subscription => $subs->{bibnum} ); >diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t >index c505066d88c..d6bd00d0ba4 100755 >--- a/t/db_dependent/Koha/Item.t >+++ b/t/db_dependent/Koha/Item.t >@@ -2564,7 +2564,7 @@ subtest 'strings_map() tests' => sub { > } > ); > >- Koha::AuthorisedValues->search( { authorised_value => 3, category => 'LOST' } )->delete; >+ Koha::AuthorisedValues->find( { authorised_value => 3, category => 'LOST' } )->delete; > my $lost_av = $builder->build_object( > { > class => 'Koha::AuthorisedValues', >diff --git a/t/db_dependent/XSLT.t b/t/db_dependent/XSLT.t >index 1f58874f9e0..f39ff0d3c67 100755 >--- a/t/db_dependent/XSLT.t >+++ b/t/db_dependent/XSLT.t >@@ -120,7 +120,7 @@ subtest 'buildKohaItemsNamespace status tests' => sub { > like( $xml, qr{<status>reference</status>}, "reference if positive itemtype notforloan value" ); > Koha::ItemTypes->find( $item->itype )->notforloan(0)->store; > >- my $substatus = Koha::AuthorisedValues->search( { category => 'NOT_LOAN', authorised_value => -1 } )->next->lib; >+ my $substatus = Koha::AuthorisedValues->find_description( { category => 'NOT_LOAN', authorised_value => -1 } ); > $item->notforloan(-1)->store; > $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber, [] ); > like( $xml, qr{<status>reallynotforloan</status>}, "reallynotforloan if negative notforloan value" ); >-- >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 40554
: 184893 |
184894
|
184895