Attachment 55188 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.30 KB, created by Jonathan Druart on 2016-09-05 16:05:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-09-05 16:05:10 UTC

Size: 15.30 KB

patch

obsolete

>From 208694b54fbb252eee7f6e9c23a3584f4f88face 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               |  5 ++++-
> circ/circulation.pl                      |  3 ++-
> members/memberentry.pl                   |  4 +++-
> members/moremember.pl                    |  5 ++++-
> reports/borrowers_stats.pl               |  8 ++++++--
> suggestion/suggestion.pl                 |  9 +++++++--
> svc/checkouts                            | 23 +++++++++++++++++++----
> t/db_dependent/Koha.t                    |  6 ++----
> 12 files changed, 60 insertions(+), 52 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 5dbbba1..b776b56 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -35,14 +35,12 @@ use C4::Message;
> use C4::Debug;
> use C4::Branch; # GetBranches
> 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::AuthorisedValues;
> use Koha::DateUtils;
> use Koha::Calendar;
> use Koha::Items;
>@@ -890,7 +888,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 083df9c..4194adb 100644
>--- a/C4/Koha.pm
>+++ b/C4/Koha.pm
>@@ -57,7 +57,6 @@ BEGIN {
> 		&GetAuthorisedValues
> 		&GetAuthorisedValueCategories
> 		&GetKohaAuthorisedValues
>-    &GetAuthorisedValueByCode
> 		&GetNormalizedUPC
> 		&GetNormalizedISBN
> 		&GetNormalizedEAN
>@@ -1041,27 +1040,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 cdf0860..4997bb1 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::Branch;
>@@ -830,7 +829,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 ceb1184..6e9894e 100755
>--- a/admin/patron-attr-types.pl
>+++ b/admin/patron-attr-types.pl
>@@ -32,6 +32,8 @@ use C4::Koha;
> use C4::Members qw/GetBorrowercategoryList/;
> use C4::Members::AttributeTypes;
> 
>+use Koha::AuthorisedValues;
>+
> my $script_name = "/cgi-bin/koha/admin/patron-attr-types.pl";
> 
> our $input = new CGI;
>@@ -295,7 +297,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 58af3a7..558a579 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -607,7 +607,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 413db97..8c4e765 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -39,6 +39,7 @@ use C4::Log;
> use C4::Letters;
> use C4::Branch; # GetBranches
> use C4::Form::MessagingPreferences;
>+use Koha::AuthorisedValues;
> use Koha::Patron::Debarments;
> use Koha::Cities;
> use Koha::DateUtils;
>@@ -792,7 +793,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 98286a0..1a6005f 100755
>--- a/members/moremember.pl
>+++ b/members/moremember.pl
>@@ -51,6 +51,7 @@ use C4::Branch; # GetBranchName
> 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;
>@@ -290,7 +291,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 607f9d1..bdad12d 100755
>--- a/reports/borrowers_stats.pl
>+++ b/reports/borrowers_stats.pl
>@@ -25,12 +25,15 @@ use C4::Auth;
> use C4::Context;
> use C4::Branch; # GetBranches
> 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 Date::Calc qw(
>   Today
>   Add_Delta_YM
>@@ -549,7 +552,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 f00e53f..6b72bfa 100755
>--- a/suggestion/suggestion.pl
>+++ b/suggestion/suggestion.pl
>@@ -32,6 +32,7 @@ use C4::Members;
> use C4::Debug;
> 
> use Koha::DateUtils qw( dt_from_string );
>+use Koha::AuthorisedValues;
> use Koha::Acquisition::Currencies;
> 
> use URI::Escape;
>@@ -56,12 +57,16 @@ 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 (GetBranchName($criteriumvalue)) 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 e613126..bd03259 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;
>@@ -146,6 +146,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},
>@@ -153,7 +168,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
>         itemnotes            => $c->{itemnotes},
>         itemnotes_nonpublic  => $c->{itemnotes_nonpublic},
>         branchcode           => $c->{branchcode},
>@@ -190,8 +205,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 b8ea314..f3ff1b2 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 {
>@@ -48,9 +48,7 @@ subtest 'Authorized Values Tests' => sub {
> 
> # Tests
>     SKIP: {
>-        skip "INSERT failed", 4 unless $insert_success;
>-
>-        is ( GetAuthorisedValueByCode($data->{category}, $data->{authorised_value}), $data->{lib}, "GetAuthorisedValueByCode" );
>+        skip "INSERT failed", 3 unless $insert_success;
> 
>         my $sortdet=C4::Members::GetSortDetails("lost", "3");
>         is ($sortdet, "Lost and Paid For", "lost and paid works");
>-- 
>2.8.1

Actions: View | Diff | Splinter Review

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