Attachment 56245 Details for Bug 17252 – Bug 17252 - Koha::AuthorisedValues - Remove GetAuthorisedValueByCode

[patch] Bug 17252 - Koha::AuthorisedValues - Remove GetAuthorisedValueByCode

Bug-17252---KohaAuthorisedValues---Remove-GetAutho.patch (text/plain), 15.31 KB, created by Jonathan Druart on 2016-10-12 10:29:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-10-12 10:29:21 UTC

Size: 15.31 KB

patch

obsolete

>From 040a44b0561b3f205488963a8f472cf17cc748bf Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Tue, 30 Aug 2016 17:04:28 +0100
>Subject: [PATCH] Bug 17252 - Koha::AuthorisedValues - Remove
> GetAuthorisedValueByCode
>
>The subroutine C4::Koha::GetAuthorisedValueByCode returned the
>description (staff or opac) for a given authorised value.
>
>Note that we may need a unique key to ->find instead of ->search.
>
>Test plan:
>- Checkin an item that cannot be checked in because it's lost, the
>  message should display the AV description
>- Generate a letter with borrowers.streettype equals an ROADTYPE AV, the
>  description should be displayed.
>- Edit a patron attribute type, the AV dropdown list should be
>  displayed
>- Create the PA_CLASS AV category (see bug 7154) and make sure it
>  behaves as before when editing a patron
>- The checkout list should display descriptions for LOC, LOST and
>  DAMAGED
>---
> C4/Circulation.pm                        |  7 +++----
> C4/Koha.pm                               | 22 ----------------------
> C4/Letters.pm                            |  6 ++++--
> Koha/Template/Plugin/AuthorisedValues.pm | 14 ++++++--------
> admin/patron-attr-types.pl               |  4 +++-
> circ/circulation.pl                      |  3 ++-
> members/memberentry.pl                   |  4 +++-
> members/moremember.pl                    |  5 ++++-
> reports/borrowers_stats.pl               |  6 ++++--
> suggestion/suggestion.pl                 |  9 +++++++--
> svc/checkouts                            | 23 +++++++++++++++++++----
> t/db_dependent/Koha.t                    |  9 +--------
> 12 files changed, 56 insertions(+), 56 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 9d9a386..7993707 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -34,15 +34,13 @@ use C4::ItemCirculationAlertPreference;
> use C4::Message;
> use C4::Debug;
> use C4::Log; # logaction
>-use C4::Koha qw(
>-    GetAuthorisedValueByCode
>-);
> use C4::Overdues qw(CalcFine UpdateFine get_chargeable_units);
> use C4::RotatingCollections qw(GetCollectionItemBranches);
> use Algorithm::CheckDigits;
> 
> use Data::Dumper;
> use Koha::Account;
>+use Koha::AuthorisedValues;
> use Koha::DateUtils;
> use Koha::Calendar;
> use Koha::Items;
>@@ -901,7 +899,8 @@ sub CanBookBeIssued {
>         $issuingimpossible{RESTRICTED} = 1;
>     }
>     if ( $item->{'itemlost'} && C4::Context->preference("IssueLostItem") ne 'nothing' ) {
>-        my $code = GetAuthorisedValueByCode( 'LOST', $item->{'itemlost'} );
>+        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $item->{itemlost} });
>+        my $code = $av->count ? $av->next->lib : '';
>         $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/Koha.pm b/C4/Koha.pm
>index 42e868e..df7b7cc 100644
>--- a/C4/Koha.pm
>+++ b/C4/Koha.pm
>@@ -55,7 +55,6 @@ BEGIN {
> 		&GetAuthorisedValues
> 		&GetAuthorisedValueCategories
> 		&GetKohaAuthorisedValues
>-    &GetAuthorisedValueByCode
> 		&GetNormalizedUPC
> 		&GetNormalizedISBN
> 		&GetNormalizedEAN
>@@ -966,27 +965,6 @@ sub GetAuthorisedValueCategories {
>     return \@results;
> }
> 
>-=head2 GetAuthorisedValueByCode
>-
>-$authorised_value = GetAuthorisedValueByCode( $category, $authvalcode, $opac );
>-
>-Return the lib attribute from authorised_values from the row identified
>-by the passed category and code
>-
>-=cut
>-
>-sub GetAuthorisedValueByCode {
>-    my ( $category, $authvalcode, $opac ) = @_;
>-
>-    my $field = $opac ? 'lib_opac' : 'lib';
>-    my $dbh = C4::Context->dbh;
>-    my $sth = $dbh->prepare("SELECT $field FROM authorised_values WHERE category=? AND authorised_value =?");
>-    $sth->execute( $category, $authvalcode );
>-    while ( my $data = $sth->fetchrow_hashref ) {
>-        return $data->{ $field };
>-    }
>-}
>-
> =head2 GetKohaAuthorisedValues
> 
> Takes $kohafield, $fwcode as parameters.
>diff --git a/C4/Letters.pm b/C4/Letters.pm
>index 26f3e46..47eb203 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -28,7 +28,6 @@ use Carp;
> use Template;
> use Module::Load::Conditional qw(can_load);
> 
>-use C4::Koha qw(GetAuthorisedValueByCode);
> use C4::Members;
> use C4::Members::Attributes qw(GetBorrowerAttributes);
> use C4::Log;
>@@ -829,7 +828,10 @@ sub _parseletter {
>             #Therefore adding the test on biblio. This includes biblioitems,
>             #but excludes items. Removed unneeded global and lookahead.
> 
>-        $val = GetAuthorisedValueByCode ('ROADTYPE', $val, 0) if $table=~/^borrowers$/ && $field=~/^streettype$/;
>+        if ( $table=~/^borrowers$/ && $field=~/^streettype$/ ) {
>+            my $av = Koha::AuthorisedValues->search({ category => 'ROADTYPE', authorised_value => $val });
>+            $val = $av->count ? $av->next->lib : '';
>+        }
> 
>         # Dates replacement
>         my $replacedby   = defined ($val) ? $val : '';
>diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm
>index 62565e0..70751e8 100644
>--- a/Koha/Template/Plugin/AuthorisedValues.pm
>+++ b/Koha/Template/Plugin/AuthorisedValues.pm
>@@ -25,7 +25,12 @@ use C4::Koha;
> 
> sub GetByCode {
>     my ( $self, $category, $code, $opac ) = @_;
>-    return GetAuthorisedValueByCode( $category, $code, $opac );
>+    my $av = Koha::AuthorisedValues->search({ category => $category, authorised_value => $code });
>+    return $av->count
>+            ? $opac
>+                ? $av->next->opac_description
>+                : $av->next->lib
>+            : '';
> }
> 
> sub Get {
>@@ -59,13 +64,6 @@ Koha::Template::Plugin::AuthorisedValues - TT Plugin for authorised values
> In a template, you can get the description for an authorised value with
> the following TT code: [% AuthorisedValues.GetByCode( 'CATEGORY', 'AUTHORISED_VALUE_CODE', 'IS_OPAC' ) %]
> 
>-The parameters are identical to those used by the subroutine C4::Koha::GetAuthorisedValueByCode.
>-
>-sub GetByCode {
>-    my ( $self, $category, $code, $opac ) = @_;
>-    return GetAuthorisedValueByCode( $category, $code, $opac );
>-}
>-
> =head2 GetAuthValueDropbox
> 
> The parameters are identical to those used by the subroutine C4::Koha::GetAuthValueDropbox
>diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl
>index 72c13a9..2faa178 100755
>--- a/admin/patron-attr-types.pl
>+++ b/admin/patron-attr-types.pl
>@@ -30,6 +30,7 @@ use C4::Output;
> use C4::Koha;
> use C4::Members::AttributeTypes;
> 
>+use Koha::AuthorisedValues;
> use Koha::Libraries;
> use Koha::Patron::Categories;
> 
>@@ -298,7 +299,8 @@ sub patron_attribute_type_list {
>             $attr->{branches} = $attr_type->branches;
>             push @items, $attr;
>         }
>-        my $lib = GetAuthorisedValueByCode( 'PA_CLASS', $class ) || $class;
>+        my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
>+        my $lib = $av->count ? $av->next->lib : $class;
>         push @attributes_loop, {
>             class => $class,
>             items => \@items,
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index 787d501..a4bbdb6 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -609,7 +609,8 @@ my $relatives_issues_count =
>   Koha::Database->new()->schema()->resultset('Issue')
>   ->count( { borrowernumber => \@relatives } );
> 
>-my $roadtype = C4::Koha::GetAuthorisedValueByCode( 'ROADTYPE', $borrower->{streettype} );
>+my $av = Koha::AuthorisedValues->search({ category => 'ROADTYPE', authorised_value => $borrower->{streettype} });
>+my $roadtype = $av->count ? $av->next->lib : '';
> 
> $template->param(%$borrower);
> 
>diff --git a/members/memberentry.pl b/members/memberentry.pl
>index 8fc5d9b..36c8a94 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -38,6 +38,7 @@ use C4::Koha;
> use C4::Log;
> use C4::Letters;
> use C4::Form::MessagingPreferences;
>+use Koha::AuthorisedValues;
> use Koha::Patron::Debarments;
> use Koha::Cities;
> use Koha::DateUtils;
>@@ -795,7 +796,8 @@ sub patron_attributes_form {
>         }
>     }
>     while ( my ($class, @items) = each %items_by_class ) {
>-        my $lib = GetAuthorisedValueByCode( 'PA_CLASS', $class ) || $class;
>+        my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
>+        my $lib = $av->count ? $av->next->lib : $class;
>         push @attribute_loop, {
>             class => $class,
>             items => @items,
>diff --git a/members/moremember.pl b/members/moremember.pl
>index 00e2600..2c8bbc7 100755
>--- a/members/moremember.pl
>+++ b/members/moremember.pl
>@@ -51,6 +51,7 @@ use C4::Biblio;
> use C4::Form::MessagingPreferences;
> use List::MoreUtils qw/uniq/;
> use C4::Members::Attributes qw(GetBorrowerAttributes);
>+use Koha::AuthorisedValues;
> use Koha::Patron::Debarments qw(GetDebarments);
> use Koha::Patron::Images;
> use Module::Load;
>@@ -292,7 +293,9 @@ if (C4::Context->preference('ExtendedPatronAttributes')) {
>         for my $attr (@$attributes) {
>             push @items, $attr if $attr->{class} eq $class
>         }
>-        my $lib = GetAuthorisedValueByCode( 'PA_CLASS', $class ) || $class;
>+        my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
>+        my $lib = $av->count ? $av->next->lib : $class;
>+
>         push @attributes_loop, {
>             class => $class,
>             items => \@items,
>diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl
>index 2392871..795ca8e 100755
>--- a/reports/borrowers_stats.pl
>+++ b/reports/borrowers_stats.pl
>@@ -24,13 +24,14 @@ use List::MoreUtils qw/uniq/;
> use C4::Auth;
> use C4::Context;
> use C4::Koha;
>-use Koha::DateUtils;
> use C4::Acquisition;
> use C4::Output;
> use C4::Reports;
> use C4::Circulation;
> use C4::Members::AttributeTypes;
> 
>+use Koha::AuthorisedValues;
>+use Koha::DateUtils;
> use Koha::Libraries;
> use Koha::Patron::Categories;
> 
>@@ -533,7 +534,8 @@ sub patron_attributes_form {
> 
>     my @attribute_loop;
>     foreach my $class ( sort keys %items_by_class ) {
>-        my $lib = GetAuthorisedValueByCode( 'PA_CLASS', $class ) || $class;
>+        my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
>+        my $lib = $av->count ? $av->next->lib : $class;
>         push @attribute_loop, {
>             class => $class,
>             items => $items_by_class{$class},
>diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl
>index 472ab64..91ab741 100755
>--- a/suggestion/suggestion.pl
>+++ b/suggestion/suggestion.pl
>@@ -31,6 +31,7 @@ use C4::Members;
> use C4::Debug;
> 
> use Koha::DateUtils qw( dt_from_string );
>+use Koha::AuthorisedValues;
> use Koha::Acquisition::Currencies;
> use Koha::Libraries;
> 
>@@ -56,13 +57,17 @@ sub GetCriteriumDesc{
>     my ($criteriumvalue,$displayby)=@_;
>     if ($displayby =~ /status/i) {
>         unless ( grep { /$criteriumvalue/ } qw(ASKED ACCEPTED REJECTED CHECKED ORDERED AVAILABLE) ) {
>-            return GetAuthorisedValueByCode('SUGGEST_STATUS', $criteriumvalue ) || "Unknown";
>+            my $av = Koha::AuthorisedValues->search({ category => 'SUGGEST_STATUS', authorised_value => $criteriumvalue });
>+            return $av->count ? $av->next->lib : 'Unkown';
>         }
>         return ($criteriumvalue eq 'ASKED'?"Pending":ucfirst(lc( $criteriumvalue))) if ($displayby =~/status/i);
>     }
>     return Koha::Libraries->find($criteriumvalue)->branchname
>         if $displayby =~ /branchcode/;
>-    return GetAuthorisedValueByCode('SUGGEST_FORMAT', $criteriumvalue) || "Unknown" if ($displayby =~/itemtype/);
>+    if ( $displayby =~ /itemtype/ ) {
>+        my $av = Koha::AuthorisedValues->search({ category => 'SUGGEST_FORMAT', authorised_value => $criteriumvalue });
>+        return $av->count ? $av->next->lib : 'Unkown';
>+    }
>     if ($displayby =~/suggestedby/||$displayby =~/managedby/||$displayby =~/acceptedby/){
>         my $borr=C4::Members::GetMember(borrowernumber=>$criteriumvalue);
>         return "" unless $borr;
>diff --git a/svc/checkouts b/svc/checkouts
>index 119053c..7fae702 100755
>--- a/svc/checkouts
>+++ b/svc/checkouts
>@@ -26,10 +26,10 @@ use JSON qw(to_json);
> use C4::Auth qw(check_cookie_auth haspermission get_session);
> use C4::Biblio qw(GetMarcBiblio GetFrameworkCode GetRecordValue );
> use C4::Circulation qw(GetIssuingCharges CanBookBeRenewed GetRenewCount GetSoonestRenewDate);
>-use C4::Koha qw(GetAuthorisedValueByCode);
> use C4::Overdues qw(GetFine);
> use C4::Context;
> 
>+use Koha::AuthorisedValues;
> use Koha::DateUtils;
> 
> my $input = new CGI;
>@@ -149,6 +149,21 @@ while ( my $c = $sth->fetchrow_hashref() ) {
>       GetRenewCount( $c->{borrowernumber}, $c->{itemnumber} );
> 
>     my $itemtype = C4::Koha::getitemtypeinfo( $item_level_itypes ? $c->{itype} : $c->{itemtype} );
>+    my $location;
>+    if ( $c->{location} ) {
>+        my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $c->{location} });
>+        $location = $av->count ? $av->next->lib : '';
>+    }
>+    my $lost;
>+    if ( $c->{itemlost} ) {
>+        my $av = Koha::AuthorisedValues->search({ category => 'LOST', authorised_value => $c->{itemlost} });
>+        $lost = $av->count ? $av->next->lib : '';
>+    }
>+    my $damaged;
>+    if ( $c->{damaged} ) {
>+        my $av = Koha::AuthorisedValues->search({ category => 'DAMAGED', authorised_value => $c->{damaged} });
>+        $damaged = $av->count ? $av->next->lib : '';
>+    }
>     my $checkout = {
>         DT_RowId             => $c->{itemnumber} . '-' . $c->{borrowernumber},
>         title                => $c->{title},
>@@ -156,7 +171,7 @@ while ( my $c = $sth->fetchrow_hashref() ) {
>         barcode              => $c->{barcode},
>         itemtype             => $item_level_itypes ? $c->{itype} : $c->{itemtype},
>         itemtype_description => $itemtype->{translated_description},
>-        location             => $c->{location} ? GetAuthorisedValueByCode( 'LOC', $c->{location} ) : q{},
>+        location             => $location
>         homebranch           => $c->{homebranch},
>         itemnotes            => $c->{itemnotes},
>         itemnotes_nonpublic  => $c->{itemnotes_nonpublic},
>@@ -195,8 +210,8 @@ while ( my $c = $sth->fetchrow_hashref() ) {
>         ),
>         subtitle =>
>           GetRecordValue( 'subtitle', GetMarcBiblio( $c->{biblionumber} ), GetFrameworkCode( $c->{biblionumber} ) ),
>-        lost    => $c->{itemlost} ? GetAuthorisedValueByCode( 'LOST',    $c->{itemlost} ) : undef,
>-        damaged => $c->{damaged}  ? GetAuthorisedValueByCode( 'DAMAGED', $c->{damaged} )  : undef,
>+        lost    => $lost,
>+        damaged => $damaged,
>         borrower => {
>             surname    => $c->{surname},
>             firstname  => $c->{firstname},
>diff --git a/t/db_dependent/Koha.t b/t/db_dependent/Koha.t
>index 8392ef8..6f7f3ef 100644
>--- a/t/db_dependent/Koha.t
>+++ b/t/db_dependent/Koha.t
>@@ -9,7 +9,7 @@ use Koha::DateUtils qw(dt_from_string);
> use Koha::AuthorisedValue;
> use Koha::AuthorisedValueCategories;
> 
>-use Test::More tests => 9;
>+use Test::More tests => 8;
> use DateTime::Format::MySQL;
> 
> BEGIN {
>@@ -46,13 +46,6 @@ subtest 'Authorized Values Tests' => sub {
>     ok( $insert_success, "Insert data in database" );
> 
> 
>-# Tests
>-    SKIP: {
>-        skip "INSERT failed", 1 unless $insert_success;
>-
>-        is ( GetAuthorisedValueByCode($data->{category}, $data->{authorised_value}), $data->{lib}, "GetAuthorisedValueByCode" );
>-    }
>-
> # Clean up
>     if($insert_success){
>         my $query = "DELETE FROM authorised_values WHERE category=? AND authorised_value=? AND lib=? AND lib_opac=? AND imageurl=?;";
>-- 
>2.8.1

Actions: View | Diff | Splinter Review

Attachments on bug 17252: 55188 | 55519 | 56245 | 56282 | 56296 | 56398 | 56402 | 56403