From 5c93d9709c564d6dbed1e0194ac3dda19b0f88f9 Mon Sep 17 00:00:00 2001 From: Srdjan Date: Fri, 8 Jul 2016 14:09:35 +1200 Subject: [PATCH] Bug 16877: Redo GetBorrowercategoryList() to use Koha::Patron::Categories->search() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Followed plan from comment #2 * Check the "Category" list on eg Patron attribute type maint screens prove t/db_dependent/Members.t: Same results with and without patch (Test 1 fails, but not related to this patch) Signed-off-by: Marc VĂ©ron --- C4/Members.pm | 27 +++++++++++++-------------- t/db_dependent/Members.t | 6 +++++- tools/modborrowers.pl | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 383799d..68f5f77 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -42,6 +42,7 @@ use Text::Unaccent qw( unac_string ); use Koha::AuthUtils qw(hash_password); use Koha::Database; use Koha::List::Patron; +use Koha::Patron::Categories; our (@ISA,@EXPORT,@EXPORT_OK,$debug); @@ -1510,22 +1511,20 @@ If no category code provided, the function returns all the categories. =cut sub GetBorrowercategoryList { - my $no_branch_limit = @_ ? shift : 0; + my ($no_branch_limit) = @_; + my $branch_limit = $no_branch_limit ? 0 - : C4::Context->userenv ? C4::Context->userenv->{"branch"} : ""; - my $dbh = C4::Context->dbh; - my $query = "SELECT categories.* FROM categories"; - $query .= qq{ - LEFT JOIN categories_branches ON categories.categorycode = categories_branches.categorycode - WHERE branchcode = ? OR branchcode IS NULL GROUP BY description - } if $branch_limit; - $query .= " ORDER BY description"; - my $sth = $dbh->prepare( $query ); - $sth->execute( $branch_limit ? $branch_limit : () ); - my $data = $sth->fetchall_arrayref( {} ); - $sth->finish; - return $data; + : C4::Context->userenv && C4::Context->userenv->{"branch"}; + + + my %cond; + my %clause = (distinct => 1, order_by => 'description'); + if ($branch_limit) { + $clause{join} = 'categories_branches'; + $cond{'categories_branches.branchcode'} = [undef, $branch_limit]; + } + return Koha::Patron::Categories->search(\%cond, \%clause)->unblessed; } # sub getborrowercategory =head2 GetAge diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 797e9bc..21ee209 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 77; +use Test::More tests => 78; use Test::MockModule; use Data::Dumper; use C4::Context; @@ -77,6 +77,10 @@ C4::Context->set_userenv ( @USERENV ); my $userenv = C4::Context->userenv or BAIL_OUT("No userenv"); +# XXX maybe needs a real test +my $categories = GetBorrowercategoryList(); +ok(scalar( @$categories ), "GetBorrowercategoryList()"); + # Make a borrower for testing my %data = ( cardnumber => $CARDNUMBER, diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl index c776406..8b6297b 100755 --- a/tools/modborrowers.pl +++ b/tools/modborrowers.pl @@ -104,7 +104,7 @@ if ( $op eq 'show' ) { my @patron_attributes_values; my @patron_attributes_codes; my $patron_attribute_types = C4::Members::AttributeTypes::GetAttributeTypes_hashref('all'); - my $patron_categories = C4::Members::GetBorrowercategoryList; + my $patron_categories = C4::Members::GetBorrowercategoryList(); for ( values %$patron_attribute_types ) { my $attr_type = C4::Members::AttributeTypes->fetch( $_->{code} ); # TODO Repeatable attributes are not correctly managed and can cause data lost. -- 1.7.10.4