[Koha-patches] [PATCH] [SIGNED-OFF] Bug 10515: The prototype for GetBranchCategory and GetBranchCategories is not consistent
Srdjan
srdjan at catalyst.net.nz
Thu Jul 11 03:58:08 CEST 2013
From: Jonathan Druart <jonathan.druart at biblibre.com>
The prototype is not consistent, GetBranchCategory should return only 1 result
and GetBranchCategories should not have a categorycode argument.
This patch fixes that.
Test plan:
1/ Try to add/remove/modify a library.
2/ Add some groups
3/ Add these groups to a library
Signed-off-by: Srdjan <srdjan at catalyst.net.nz>
---
C4/Branch.pm | 63 ++++++++--------------
admin/branches.pl | 47 +++++++---------
catalogue/search.pl | 2 +-
.../prog/en/modules/admin/branches.tt | 2 +-
opac/opac-search.pl | 2 +-
5 files changed, 44 insertions(+), 72 deletions(-)
diff --git a/C4/Branch.pm b/C4/Branch.pm
index 5770f7b..5783de9 100644
--- a/C4/Branch.pm
+++ b/C4/Branch.pm
@@ -248,7 +248,7 @@ sub ModBranch {
}
# sort out the categories....
my @checkedcats;
- my $cats = GetBranchCategory();
+ my $cats = GetBranchCategories();
foreach my $cat (@$cats) {
my $code = $cat->{'categorycode'};
if ( $data->{$code} ) {
@@ -294,87 +294,66 @@ sub ModBranch {
$results = GetBranchCategory($categorycode);
-C<$results> is an ref to an array.
+C<$results> is an hashref
=cut
sub GetBranchCategory {
-
- # returns a reference to an array of hashes containing branches,
my ($catcode) = @_;
+ return unless $catcode;
+
my $dbh = C4::Context->dbh;
my $sth;
- # print DEBUG "GetBranchCategory: entry: catcode=".cvs($catcode)."\n";
- if ($catcode) {
- $sth =
- $dbh->prepare(
- "select * from branchcategories where categorycode = ?");
- $sth->execute($catcode);
- }
- else {
- $sth = $dbh->prepare("Select * from branchcategories");
- $sth->execute();
- }
- my @results;
- while ( my $data = $sth->fetchrow_hashref ) {
- push( @results, $data );
- }
- $sth->finish;
-
- # print DEBUG "GetBranchCategory: exit: returning ".cvs(\@results)."\n";
- return \@results;
+ $sth = $dbh->prepare(q{
+ SELECT *
+ FROM branchcategories
+ WHERE categorycode = ?
+ });
+ $sth->execute( $catcode );
+ return $sth->fetchrow_hashref;
}
=head2 GetBranchCategories
- my $categories = GetBranchCategories($branchcode,$categorytype,$show_in_pulldown,$selected_in_pulldown);
+ my $categories = GetBranchCategories($categorytype,$show_in_pulldown,$selected_in_pulldown);
Returns a list ref of anon hashrefs with keys eq columns of branchcategories table,
-i.e. categorycode, categorydescription, categorytype, categoryname.
-if $branchcode and/or $categorytype are passed, limit set to categories that
-$branchcode is a member of , and to $categorytype.
+i.e. categorydescription, categorytype, categoryname.
=cut
sub GetBranchCategories {
- my ( $branchcode, $categorytype, $show_in_pulldown, $selected_in_pulldown ) = @_;
+ my ( $categorytype, $show_in_pulldown, $selected_in_pulldown ) = @_;
my $dbh = C4::Context->dbh();
- my $query = "SELECT c.* FROM branchcategories c";
- my ( @where, @bind );
-
- if( $branchcode ) {
- $query .= ",branchrelations r, branches b ";
- push @where, "c.categorycode = r.categorycode AND r.branchcode = ? ";
- push @bind , $branchcode;
- }
+ my $query = "SELECT * FROM branchcategories ";
+ my ( @where, @bind );
if ( $categorytype ) {
- push @where, " c.categorytype = ? ";
+ push @where, " categorytype = ? ";
push @bind, $categorytype;
}
if ( defined( $show_in_pulldown ) ) {
- push( @where, " c.show_in_pulldown = ? " );
+ push( @where, " show_in_pulldown = ? " );
push( @bind, $show_in_pulldown );
}
$query .= " WHERE " . join(" AND ", @where) if(@where);
- $query .= " ORDER BY categorytype,c.categorycode";
+ $query .= " ORDER BY categorytype, categorycode";
my $sth=$dbh->prepare( $query);
$sth->execute(@bind);
my $branchcats = $sth->fetchall_arrayref({});
- $sth->finish();
if ( $selected_in_pulldown ) {
foreach my $bc ( @$branchcats ) {
- $bc->{'selected'} = 1 if ( $bc->{'categorycode'} eq $selected_in_pulldown );
+ $bc->{selected} = 1 if $bc->{categorycode} eq $selected_in_pulldown;
}
}
- return( $branchcats );
+ return $branchcats;
}
=head2 GetCategoryTypes
diff --git a/admin/branches.pl b/admin/branches.pl
index 026d983..9e6b14a 100755
--- a/admin/branches.pl
+++ b/admin/branches.pl
@@ -234,14 +234,31 @@ sub editbranchform {
my $data;
my $oldprinter = "";
+
+ # make the checkboxes.....
+ my $catinfo = GetBranchCategories();
+
if ($branchcode) {
$data = GetBranchInfo($branchcode);
$data = $data->[0];
+ if ( exists $data->{categories} ) {
+ # Set the selected flag for the categories of this branch
+ $catinfo = [
+ map {
+ my $catcode = $_->{categorycode};
+ if ( grep {/$catcode/} @{$data->{categories}} ){
+ $_->{selected} = 1;
+ }
+ $_;
+ } @{$catinfo}
+ ];
+ }
# get the old printer of the branch
$oldprinter = $data->{'branchprinter'} || '';
_branch_to_template($data, $innertemplate);
}
+ $innertemplate->param( categoryloop => $catinfo );
foreach my $thisprinter ( keys %$printers ) {
push @printerloop, {
@@ -252,29 +269,6 @@ sub editbranchform {
}
$innertemplate->param( printerloop => \@printerloop );
- # make the checkboxes.....
- #
- # We export a "categoryloop" array to the template, each element of which
- # contains separate 'categoryname', 'categorycode', 'codedescription', and
- # 'checked' fields. The $checked field is either empty or 1'
-
- my $catinfo = GetBranchCategory();
- my @categoryloop = ();
- foreach my $cat (@$catinfo) {
- my $checked;
- my $tmp = quotemeta( $cat->{'categorycode'} );
- if ( grep { /^$tmp$/ } @{ $data->{'categories'} } ) {
- $checked = 1;
- }
- push @categoryloop, {
- categoryname => $cat->{'categoryname'},
- categorycode => $cat->{'categorycode'},
- categorytype => $cat->{'categorytype'},
- codedescription => $cat->{'codedescription'},
- checked => $checked,
- };
- }
- $innertemplate->param( categoryloop => \@categoryloop );
for my $obsolete ( 'categoryname', 'categorycode', 'codedescription' ) {
$innertemplate->param(
@@ -291,7 +285,6 @@ sub editcatform {
my $data;
if ($categorycode) {
my $data = GetBranchCategory($categorycode);
- $data = $data->[0];
$innertemplate->param(
categorycode => $data->{'categorycode'},
categoryname => $data->{'categoryname'},
@@ -360,7 +353,7 @@ sub branchinfotable {
my $no_categories_p = 1;
my @categories;
foreach my $cat ( @{ $branch->{'categories'} } ) {
- my ($catinfo) = @{ GetBranchCategory($cat) };
+ my $catinfo = GetBranchCategory($cat);
push @categories, { 'categoryname' => $catinfo->{'categoryname'} };
$no_categories_p = 0;
}
@@ -375,8 +368,8 @@ sub branchinfotable {
}
my @branchcategories = ();
for my $ctype ( GetCategoryTypes() ) {
- my $catinfo = GetBranchCategories(undef,$ctype);
- my @categories;
+ my $catinfo = GetBranchCategories($ctype);
+ my @categories;
foreach my $cat (@$catinfo) {
push @categories, {
categoryname => $cat->{'categoryname'},
diff --git a/catalogue/search.pl b/catalogue/search.pl
index d4e98b18..f8dab56 100755
--- a/catalogue/search.pl
+++ b/catalogue/search.pl
@@ -237,7 +237,7 @@ my @branch_loop = map {
$branches->{$a}->{branchname} cmp $branches->{$b}->{branchname}
} keys %$branches;
-my $categories = GetBranchCategories(undef,'searchdomain');
+my $categories = GetBranchCategories('searchdomain');
$template->param(branchloop => \@branch_loop, searchdomainloop => $categories);
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt
index 902f809..86fb8ce 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt
@@ -107,7 +107,7 @@ tinyMCE.init({
<ol>
[% FOREACH categoryloo IN categoryloop %]
<li><label for="[% categoryloo.categorycode %]">[% categoryloo.categoryname %]: </label>
- [% IF ( categoryloo.checked ) %]
+ [% IF categoryloo.selected %]
<input type="checkbox" id="[% categoryloo.categorycode %]" name="[% categoryloo.categorycode %]" checked="checked" />
[% ELSE %]
<input type="checkbox" id="[% categoryloo.categorycode %]" name="[% categoryloo.categorycode %]" />
diff --git a/opac/opac-search.pl b/opac/opac-search.pl
index dfe651e..880317a 100755
--- a/opac/opac-search.pl
+++ b/opac/opac-search.pl
@@ -193,7 +193,7 @@ if (C4::Context->preference('TagsEnabled')) {
my $branches = GetBranches(); # used later in *getRecords, probably should be internalized by those functions after caching in C4::Branch is established
$template->param(
- searchdomainloop => GetBranchCategories(undef,'searchdomain'),
+ searchdomainloop => GetBranchCategories('searchdomain'),
);
# load the language limits (for search)
--
1.8.1.2
More information about the Koha-patches
mailing list