From 042e9f4853414eec409586471b01a1bf23ce35db Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 28 Jun 2013 16:07:13 +0200 Subject: [PATCH] Bug 10515: The prototype for GetBranchCategory and GetBranchCategories is not consistent 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 Signed-off-by: Kyle M Hall --- 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 d4e98b1..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({
    [% FOREACH categoryloo IN categoryloop %]
  1. - [% IF ( categoryloo.checked ) %] + [% IF categoryloo.selected %] [% ELSE %] 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.7.2.5