From 20e4e9e33b60be8249b26ae95de4aa33b35c8d3f Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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{reference}, "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{reallynotforloan}, "reallynotforloan if negative notforloan value" ); -- 2.34.1