@@ -, +, @@ use placeholders for db queries --- C4/Acquisition.pm | 22 +++++++++++------- C4/Branch.pm | 25 +++++++------------- C4/Items.pm | 11 ++++++--- C4/Members.pm | 24 +++++++++++-------- C4/Serials.pm | 59 +++++++++++++++++++++++++++++------------------ C4/Suggestions.pm | 30 +++++++++++++----------- circ/pendingreserves.pl | 5 ++-- circ/reserveratios.pl | 5 ++-- t/db_dependent/Branch.t | 14 +---------- 9 files changed, 104 insertions(+), 91 deletions(-) --- a/C4/Acquisition.pm +++ a/C4/Acquisition.pm @@ -1883,9 +1883,12 @@ sub GetParcel { my @query_params = ( $supplierid, $code, $datereceived ); if ( C4::Context->preference("IndependentBranches") ) { unless ( C4::Context->IsSuperLibrarian() ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); - $strsth .= " AND ( borrowers.branchcode IN ( $branches ) OR borrowers.branchcode = '')"; + my @branches = GetIndependentGroupModificationRights(); + $strsth .= + " AND ( borrowers.branchcode IN ( " + . join( ',', ('?') x @branches ) + . " ) OR borrowers.branchcode = '')"; + push( @query_params, @branches ); } } $strsth .= " ORDER BY aqbasket.basketno"; @@ -2096,8 +2099,9 @@ sub GetLateOrders { } if (C4::Context->preference("IndependentBranches") && !C4::Context->IsSuperLibrarian() ) { - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - $from .= qq{ AND borrowers.branchcode IN ( $branches ) }; + my @branches = GetIndependentGroupModificationRights(); + $from .= "AND borrowers.branchcode IN ( " . join(',', ('?') x @branches) . " ) "; + push( @query_params, @branches ); } $from .= " AND orderstatus <> 'cancelled' "; my $query = "$select $from $having\nORDER BY latesince, basketno, borrowers.branchcode, supplier"; @@ -2311,9 +2315,11 @@ sub GetHistory { if ( C4::Context->preference("IndependentBranches") && !C4::Context->IsSuperLibrarian() ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); - $query .= qq{ AND ( borrowers.branchcode = ? OR borrowers.branchcode IN ( $branches ) ) }; + my @branches = GetIndependentGroupModificationRights(); + $query .= + " AND ( borrowers.branchcode = ? OR borrowers.branchcode IN ( " + . join( ',', ('?') x @branches ) . " ) ) "; + push( @query_params, @branches ); } $query .= " ORDER BY id"; my $sth = $dbh->prepare($query); --- a/C4/Branch.pm +++ a/C4/Branch.pm @@ -116,8 +116,9 @@ sub GetBranches { my $query = "SELECT * FROM branches"; my @bind_parameters; if ($onlymine && C4::Context->userenv && C4::Context->userenv->{branch}){ - my $branches = GetIndependentGroupModificationRights({ stringify => 1 }); - $query .= qq{ WHERE branchcode IN ( $branches ) }; + my @branches = GetIndependentGroupModificationRights(); + $query .= " WHERE branchcode IN ( " . join(',', ('?') x @branches) . " ) "; + push( @bind_parameters, @branches ); } $query .= " ORDER BY branchname"; $sth = $dbh->prepare($query); @@ -427,12 +428,11 @@ sub GetBranchesInCategory { =head2 GetIndependentGroupModificationRights GetIndependentGroupModificationRights( - { - branch => $this_branch, - for => $other_branch, - stringify => 1, - } - ); + { + branch => $this_branch, + for => $other_branch, + } + ); Returns a list of branches this branch shares a common independent group with. @@ -447,12 +447,6 @@ sub GetBranchesInCategory { If called in a scalar context, it returns a count of matching branchcodes. Returns 1 if - - If stringify param is passed, the return value will - be a string of the comma delimited branchcodes. This - is useful for "branchcode IN $branchcodes" clauses - in SQL queries. - $this_branch and $other_branch are equal for efficiency. So you can write: @@ -467,7 +461,6 @@ sub GetIndependentGroupModificationRights { my $this_branch = $params->{branch}; my $other_branch = $params->{for}; - my $stringify = $params->{stringify}; $this_branch ||= C4::Context->userenv->{branch}; @@ -501,8 +494,6 @@ sub GetIndependentGroupModificationRights { my $dbh = C4::Context->dbh; my @branchcodes = @{ $dbh->selectcol_arrayref( $sql, {}, @params ) }; - return join( ',', map { qq{'$_'} } @branchcodes ) if $stringify; - if ( wantarray() ) { if ( @branchcodes ) { return @branchcodes; --- a/C4/Items.pm +++ a/C4/Items.pm @@ -2759,9 +2759,14 @@ sub PrepareItemrecordDisplay { if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) { if ( ( C4::Context->preference("IndependentBranches") ) && !C4::Context->IsSuperLibrarian() ) { - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - my $sth = $dbh->prepare( "SELECT branchcode,branchname FROM branches WHERE branchcode IN ( $branches ) ORDER BY branchname" ); - $sth->execute(); + my @branches = + GetIndependentGroupModificationRights(); + my $sql = + "SELECT branchcode,branchname FROM branches WHERE branchcode IN (" + . join( ',', ('?') x @branches ) + . ") ORDER BY branchname"; + my $sth = $dbh->prepare($sql); + $sth->execute(@branches); push @authorised_values, "" unless ( $tagslib->{$tag}->{$subfield}->{mandatory} ); while ( my ( $branchcode, $branchname ) = $sth->fetchrow_array ) { --- a/C4/Members.pm +++ a/C4/Members.pm @@ -1314,19 +1314,25 @@ sub checkuniquemember { "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=? and dateofbirth=?" : "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=?"; + my @params; + if ($collectivity) { + push( @params, uc($surname) ); + } + elsif ($dateofbirth) { + push( @params, uc($surname), ucfirst($firstname), $dateofbirth ); + } + else { + push( @params, uc($surname), ucfirst($firstname) ); + } + if ( C4::Context->preference('IndependentBranches') ) { - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - $request .= " AND branchcode IN ( $branches )"; + my @branches = GetIndependentGroupModificationRights(); + $request .= " AND branchcode IN ( " . join(',', ('?') x @branches) ." )"; + push( @params, @branches ); } my $sth = $dbh->prepare($request); - if ($collectivity) { - $sth->execute( uc($surname) ); - } elsif($dateofbirth){ - $sth->execute( uc($surname), ucfirst($firstname), $dateofbirth ); - }else{ - $sth->execute( uc($surname), ucfirst($firstname)); - } + $sth->execute( @params ); my @data = $sth->fetchrow; ( $data[0] ) and return $data[0], $data[1]; return 0; --- a/C4/Serials.pm +++ a/C4/Serials.pm @@ -237,21 +237,25 @@ sub GetSerialInformation { subscription.*, subscription.subscriptionid as subsid |; + my @branches; if ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && C4::Context->userenv->{'flags'} % 2 != 1 && C4::Context->userenv->{'branch'} ) { - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - $query .= qq| - , ( ( subscription.branchcode NOT IN ( $branches ) ) AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL ) AS cannotedit - |; + @branches = GetIndependentGroupModificationRights(); + $query .= " + , ( + ( subscription.branchcode NOT IN ( " . join(',', ('?') x @branches) ." ) ) + AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL + ) AS cannotedit + "; } $query .= qq| FROM serial LEFT JOIN subscription ON subscription.subscriptionid=serial.subscriptionid WHERE serialid = ? |; my $rq = $dbh->prepare($query); - $rq->execute($serialid); + $rq->execute($serialid, @branches); my $data = $rq->fetchrow_hashref; # create item information if we have serialsadditems for this subscription @@ -354,17 +358,20 @@ sub GetSubscription { subscription.biblionumber as bibnum |; + my @branches; if ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && C4::Context->userenv->{'flags'} % 2 != 1 && C4::Context->userenv->{'branch'} ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); + @branches = GetIndependentGroupModificationRights(); - $query .= qq| - , ( ( subscription.branchcode NOT IN ( $branches ) ) AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL ) AS cannotedit - |; + $query .= " + , ( + ( subscription.branchcode NOT IN ( " . join(',', ('?') x @branches) ." ) ) + AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL + ) AS cannotedit + "; } $query .= qq| @@ -377,7 +384,7 @@ sub GetSubscription { $debug and warn "query : $query\nsubsid :$subscriptionid"; my $sth = $dbh->prepare($query); - $sth->execute($subscriptionid); + $sth->execute($subscriptionid, @branches); my $subscription = $sth->fetchrow_hashref; $subscription->{cannotedit} = not can_edit_subscription( $subscription ); return $subscription; @@ -412,17 +419,20 @@ sub GetFullSubscription { subscription.subscriptionid AS subscriptionid |; + my @branches; if ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && C4::Context->userenv->{'flags'} % 2 != 1 && C4::Context->userenv->{'branch'} ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); + @branches = GetIndependentGroupModificationRights(); - $query .= qq| - , ( ( subscription.branchcode NOT IN ( $branches ) ) AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL ) AS cannotedit - |; + $query .= " + , ( + ( subscription.branchcode NOT IN ( " . join(',', ('?') x @branches) . " ) ) + AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL + ) AS cannotedit + "; } $query .= qq| @@ -438,7 +448,7 @@ sub GetFullSubscription { |; $debug and warn "GetFullSubscription query: $query"; my $sth = $dbh->prepare($query); - $sth->execute($subscriptionid); + $sth->execute( $subscriptionid, @branches ); my $subscriptions = $sth->fetchall_arrayref( {} ); for my $subscription ( @$subscriptions ) { $subscription->{cannotedit} = not can_edit_subscription( $subscription ); @@ -595,17 +605,20 @@ sub GetFullSubscriptionsFromBiblionumber { subscription.subscriptionid AS subscriptionid |; + my @branches; if ( C4::Context->preference('IndependentBranches') && C4::Context->userenv && C4::Context->userenv->{'flags'} != 1 && C4::Context->userenv->{'branch'} ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); + @branches = GetIndependentGroupModificationRights(); - $query .= qq| - , ( ( subscription.branchcode NOT IN ( $branches ) ) AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL ) AS cannotedit - |; + $query .= " + , ( + ( subscription.branchcode NOT IN (" . join(',', ('?') x @branches) . ") ) + AND subscription.branchcode <> '' AND subscription.branchcode IS NOT NULL + ) AS cannotedit + "; } $query .= qq| @@ -620,7 +633,7 @@ sub GetFullSubscriptionsFromBiblionumber { serial.subscriptionid |; my $sth = $dbh->prepare($query); - $sth->execute($biblionumber); + $sth->execute($biblionumber, @branches); my $subscriptions = $sth->fetchall_arrayref( {} ); for my $subscription ( @$subscriptions ) { $subscription->{cannotedit} = not can_edit_subscription( $subscription ); --- a/C4/Suggestions.pm +++ a/C4/Suggestions.pm @@ -137,9 +137,10 @@ sub SearchSuggestion { && !C4::Context->IsSuperLibrarian() && !$suggestion->{branchcode} ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); - push( @query, qq{ AND (suggestions.branchcode IN ( $branches ) OR suggestions.branchcode='') } ); + my @branches = + GetIndependentGroupModificationRights(); + push( @query, " AND (suggestions.branchcode IN ( " . join(',', ('?') x @branches) ." ) OR suggestions.branchcode='') " ); + push( @sql_params, @branches ); } else { if ( defined $suggestion->{branchcode} && $suggestion->{branchcode} ) { @@ -343,12 +344,13 @@ sub GetSuggestionByStatus { && !C4::Context->IsSuperLibrarian() ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); + my @branches = + GetIndependentGroupModificationRights(); - $query .= qq{ - AND (U1.branchcode IN ( $branches ) OR U1.branchcode ='') - }; + $query .= " + AND (U1.branchcode IN ( " . join(',', ('?') x @branches) . " ) OR U1.branchcode ='') + "; + push( @sql_params, @branches ); } if ($branchcode) { @@ -396,22 +398,22 @@ sub CountSuggestion { if ( C4::Context->preference("IndependentBranches") && !C4::Context->IsSuperLibrarian() ) { - my $branches = - GetIndependentGroupModificationRights( { stringify => 1 } ); + my @branches = + GetIndependentGroupModificationRights(); - my $query = qq{ + my $query = " SELECT count(*) FROM suggestions LEFT JOIN borrowers ON borrowers.borrowernumber=suggestions.suggestedby WHERE STATUS=? AND ( - borrowers.branchcode IN ( $branches ) + borrowers.branchcode IN (" . join(',', ('?') x @branches) . ") OR borrowers.branchcode=? ) - }; + "; $sth = $dbh->prepare($query); - $sth->execute( $status, $userenv->{branch} ); + $sth->execute( $status, @branches, $userenv->{branch} ); } else { my $query = q{ --- a/circ/pendingreserves.pl +++ a/circ/pendingreserves.pl @@ -155,8 +155,9 @@ if ( $run_report ) { if (C4::Context->preference('IndependentBranches')){ - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - $strsth .= " AND items.holdingbranch IN ( $branches ) "; + my @branches = GetIndependentGroupModificationRights(); + $strsth .= " AND items.holdingbranch IN ( " . join(',', ('?') x @branches) . " ) "; + push( @query_params, @branches ); } $strsth .= " GROUP BY reserves.biblionumber ORDER BY biblio.title "; --- a/circ/reserveratios.pl +++ a/circ/reserveratios.pl @@ -114,8 +114,9 @@ my $strsth = "; if ( C4::Context->preference('IndependentBranches') ) { - my $branches = GetIndependentGroupModificationRights( { stringify => 1 } ); - $strsth .= " AND items.holdingbranch IN ( $branches ) "; + my @branches = GetIndependentGroupModificationRights(); + $strsth .= " AND items.holdingbranch IN (" . join(',', ('?') x @branches) . ") "; + push( @query_params, @branches ); } $strsth .= " GROUP BY reserves.biblionumber ORDER BY reservecount DESC"; --- a/t/db_dependent/Branch.t +++ a/t/db_dependent/Branch.t @@ -21,7 +21,7 @@ use Modern::Perl; use C4::Context; use Data::Dumper; -use Test::More tests => 70; +use Test::More tests => 66; use C4::Branch; @@ -351,9 +351,6 @@ is( scalar(@$loop), GetBranchesCount(), 'There is the right number of branches' my @branches_bra = GetIndependentGroupModificationRights({ branch => 'BRA' }); is_deeply( \@branches_bra, [ 'BRA' ], 'Library with no group only has rights for its own branch' ); -my $string = GetIndependentGroupModificationRights({ branch => 'BRA', stringify => 1 }); -ok( $string eq q{'BRA'}, "String returns correctly" ); - ok( GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRA' }), 'Boolean test for BRA rights to BRA returns true' ); ok( !GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRB' }), 'Boolean test for BRA rights to BRB returns false' ); ok( !GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRC' }), 'Boolean test for BRA rights to BRC returns false' ); @@ -378,12 +375,6 @@ ModBranch({ @branches_bra = GetIndependentGroupModificationRights({ branch => 'BRA' }); is_deeply( \@branches_bra, [ 'BRA', 'BRB' ], 'Libraries in LIBCATCODE returned correctly' ); -$string = GetIndependentGroupModificationRights({ branch => 'BRA', stringify => 1 }); -ok( $string eq q{'BRA','BRB'}, "String returns correctly" ); - -$string = GetIndependentGroupModificationRights({ branch => 'BRC', stringify => 1 }); -ok( $string eq q{'BRC'}, "String returns correctly" ); - ok( GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRA' }), 'Boolean test for BRA rights to BRA returns true' ); ok( GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRB'}), 'Boolean test for BRA rights to BRB returns true' ); ok( !GetIndependentGroupModificationRights({ branch => 'BRA', for => 'BRC'}), 'Boolean test for BRA rights to BRC returns false' ); @@ -413,9 +404,6 @@ ok( GetIndependentGroupModificationRights({ branch => 'BRC', for => 'BRC' }), 'B ok( GetIndependentGroupModificationRights({ branch => 'BRC', for => 'BRA'}), 'Boolean test for BRC rights to BRA returns true' ); ok( GetIndependentGroupModificationRights({ branch => 'BRC', for => 'BRA'}), 'Boolean test for BRC rights to BRB returns true' ); -$string = GetIndependentGroupModificationRights({ branch => 'BRA', stringify => 1 }); -ok( $string eq q{'BRA','BRB','BRC'}, "String returns correctly" ); - # End transaction $dbh->rollback; --