Bug 10277 - Add C4::Context->IsSuperLibrarian()

The method of checking the logged in user for superlibrarian privileges
is obtuse ( $userenv && $userenv->{flags} % 2 != 1 ) to say the least.
The codebase is littered with these lines, with no explanation given. It
would be much better if we had one subroutine that returned a boolean
value to tell us if the logged in user is a superlibrarian or not.

Test Plan:
1) Apply this patch
2) Verify superlibrarian behavior remains unchanged

Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comments on second patch.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This commit is contained in:
Kyle M Hall 2013-05-15 11:04:07 -04:00 committed by Galen Charlton
parent 4a0f630f7f
commit fb4025b67b
23 changed files with 111 additions and 88 deletions

View File

@ -1696,7 +1696,7 @@ sub SearchOrders {
my $userenv = C4::Context->userenv;
if ( C4::Context->preference("IndependentBranches") ) {
if ( ( $userenv ) and ( $userenv->{flags} != 1 ) ) {
unless ( C4::Context->IsSuperLibrarian() ) {
$query .= q{
AND (
borrowers.branchcode = ?
@ -1892,11 +1892,10 @@ sub GetParcel {
my @query_params = ( $supplierid, $code, $datereceived );
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
if ( ($userenv) && ( $userenv->{flags} != 1 ) ) {
unless ( C4::Context->IsSuperLibrarian() ) {
$strsth .= " and (borrowers.branchcode = ?
or borrowers.branchcode = '')";
push @query_params, $userenv->{branch};
push @query_params, C4::Context->userenv->{branch};
}
}
$strsth .= " ORDER BY aqbasket.basketno";
@ -2111,8 +2110,7 @@ sub GetLateOrders {
$from .= ' AND ADDDATE(aqbasket.closedate, INTERVAL aqbooksellers.deliverytime DAY) <= CAST(now() AS date)';
}
if (C4::Context->preference("IndependentBranches")
&& C4::Context->userenv
&& C4::Context->userenv->{flags} != 1 ) {
&& !C4::Context->IsSuperLibrarian() ) {
$from .= ' AND borrowers.branchcode LIKE ? ';
push @query_params, C4::Context->userenv->{branch};
}
@ -2326,10 +2324,9 @@ sub GetHistory {
}
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
if ( $userenv && ($userenv->{flags} || 0) != 1 ) {
unless ( C4::Context->IsSuperLibrarian() ) {
$query .= " AND (borrowers.branchcode = ? OR borrowers.branchcode ='' ) ";
push @query_params, $userenv->{branch};
push @query_params, C4::Context->userenv->{branch};
}
}
$query .= " ORDER BY id";

View File

@ -146,11 +146,11 @@ sub GetBranches {
}
sub onlymine {
return
C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags} %2 != 1 &&
C4::Context->userenv->{branch} ;
return
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
}
# always returns a string for OK comparison via "eq" or "ne"

View File

@ -896,7 +896,7 @@ sub CanBookBeIssued {
}
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
unless ( C4::Context->IsSuperLibrarian() ) {
if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} ){
$issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
$issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};

View File

@ -105,6 +105,7 @@ use C4::Debug;
use POSIX ();
use DateTime::TimeZone;
use Module::Load::Conditional qw(can_load);
use Carp;
=head1 NAME
@ -1234,6 +1235,18 @@ sub tz {
}
=head2 IsSuperLibrarian
C4::Context->IsSuperlibrarian();
=cut
sub IsSuperLibrarian {
my $userenv = C4::Context->userenv
|| carp("C4::Context->userenv not defined!");
return $userenv->{flags} % 2 == 1;
}
1;
__END__

View File

@ -1303,7 +1303,7 @@ sub GetItemsInfo {
$datedue = $idata->{'date_due'};
if (C4::Context->preference("IndependentBranches")){
my $userenv = C4::Context->userenv;
if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
unless ( C4::Context->IsSuperLibrarian() ) {
$data->{'NOTSAMEBRANCH'} = 1 if ($idata->{'bcode'} ne $userenv->{branch});
}
}
@ -2265,7 +2265,7 @@ sub DelItemCheck {
if ($onloan){
$error = "book_on_loan"
}
elsif ( !( C4::Context->userenv->{flags} & 1 )
elsif ( !C4::Context->IsSuperLibrarian()
and C4::Context->preference("IndependentBranches")
and ( C4::Context->userenv->{branch} ne $item->{'homebranch'} ) )
{
@ -2747,7 +2747,7 @@ sub PrepareItemrecordDisplay {
#---- branch
if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) {
if ( ( C4::Context->preference("IndependentBranches") )
&& ( C4::Context->userenv && C4::Context->userenv->{flags} % 2 != 1 ) ) {
&& !C4::Context->IsSuperLibrarian() ) {
my $sth = $dbh->prepare( "SELECT branchcode,branchname FROM branches WHERE branchcode = ? ORDER BY branchname" );
$sth->execute( C4::Context->userenv->{branch} );
push @authorised_values, ""

View File

@ -262,7 +262,7 @@ sub Search {
if ( C4::Context->preference("IndependentBranches") ) { # && !$showallbranches){
if ( my $userenv = C4::Context->userenv ) {
my $branch = $userenv->{'branch'};
if ( ($userenv->{flags} % 2 !=1) && $branch ){
if ( !C4::Context->IsSuperLibrarian() && $branch ){
if (my $fr = ref $filter) {
if ( $fr eq "HASH" ) {
$filter->{branchcode} = $branch;
@ -2046,7 +2046,7 @@ sub GetBorrowersToExpunge {
my $filterbranch = $params->{'branchcode'} ||
((C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& C4::Context->userenv->{flags} % 2 !=1
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
: "");
@ -2112,7 +2112,7 @@ sub GetBorrowersWhoHaveNeverBorrowed {
my $filterbranch = shift ||
((C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& C4::Context->userenv->{flags} % 2 !=1
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
: "");
@ -2162,7 +2162,7 @@ sub GetBorrowersWithIssuesHistoryOlderThan {
my $filterbranch = shift ||
((C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& C4::Context->userenv->{flags} % 2 !=1
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch})
? C4::Context->userenv->{branch}
: "");

View File

@ -2837,7 +2837,7 @@ sub can_edit_subscription {
$userid ||= C4::Context->userenv->{'id'};
my $independent_branches = C4::Context->preference('IndependentBranches');
return 1 unless $independent_branches;
if( $flags % 2 == 1 # superlibrarian
if( C4::Context->IsSuperLibrarian()
or C4::Auth::haspermission( $userid, {serials => 'superserials'}),
or C4::Auth::haspermission( $userid, {serials => 'edit_subscription'}),
or not defined $subscription->{branchcode}

View File

@ -135,7 +135,7 @@ sub SearchSuggestion {
if ( C4::Context->preference('IndependentBranches') ) {
my $userenv = C4::Context->userenv;
if ($userenv) {
if ( ( $userenv->{flags} % 2 ) != 1 && !$suggestion->{branchcode} )
if ( !C4::Context->IsSuperLibrarian() && !$suggestion->{branchcode} )
{
push @sql_params, $$userenv{branch};
push @query, q{
@ -342,7 +342,7 @@ sub GetSuggestionByStatus {
if ( C4::Context->preference("IndependentBranches") || $branchcode ) {
my $userenv = C4::Context->userenv;
if ($userenv) {
unless ( $userenv->{flags} % 2 == 1 ) {
unless ( C4::Context->IsSuperLibrarian() ) {
push @sql_params, $userenv->{branch};
$query .= q{ AND (U1.branchcode = ? OR U1.branchcode ='') };
}
@ -352,7 +352,7 @@ sub GetSuggestionByStatus {
$query .= q{ AND (U1.branchcode = ? OR U1.branchcode ='') };
}
}
my $sth = $dbh->prepare($query);
$sth->execute(@sql_params);
my $results;
@ -390,7 +390,7 @@ sub CountSuggestion {
my $sth;
my $userenv = C4::Context->userenv;
if ( C4::Context->preference("IndependentBranches")
&& $userenv->{flags} % 2 != 1 )
&& !C4::Context->IsSuperLibrarian() )
{
my $query = q{
SELECT count(*)
@ -436,10 +436,10 @@ sub NewSuggestion {
Modify the suggestion according to the hash passed by ref.
The hash HAS to contain suggestionid
Data not defined is not updated unless it is a note or sort1
Data not defined is not updated unless it is a note or sort1
Send a mail to notify the user that did the suggestion.
Note that there is no function to modify a suggestion.
Note that there is no function to modify a suggestion.
=cut
@ -533,9 +533,9 @@ sub DelSuggestion {
=head2 DelSuggestionsOlderThan
&DelSuggestionsOlderThan($days)
Delete all suggestions older than TODAY-$days , that have be accepted or rejected.
=cut
sub DelSuggestionsOlderThan {

View File

@ -154,7 +154,7 @@ if ( $op eq 'delete_confirm' ) {
$template->param( delete_confirm => 1 );
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
unless ( $userenv->{flags} == 1 ) {
unless ( C4::Context->IsSuperLibrarian() ) {
my $validtest = ( $basket->{creationdate} eq '' )
|| ( $userenv->{branch} eq $basket->{branch} )
|| ( $userenv->{branch} eq '' )
@ -257,7 +257,7 @@ if ( $op eq 'delete_confirm' ) {
# get librarian branch...
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
unless ( $userenv->{flags} == 1 ) {
unless ( C4::Context->IsSuperLibrarian() ) {
my $validtest = ( $basket->{creationdate} eq '' )
|| ( $userenv->{branch} eq $basket->{branch} )
|| ( $userenv->{branch} eq '' )

View File

@ -229,10 +229,11 @@ for my $curr ( @rates ) {
}
# build branches list
my $onlymine=C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags}!=1 &&
C4::Context->userenv->{branch};
my $onlymine =
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
my $branches = GetBranches($onlymine);
my @branchloop;
foreach my $thisbranch ( sort {$branches->{$a}->{'branchname'} cmp $branches->{$b}->{'branchname'}} keys %$branches ) {

View File

@ -172,10 +172,11 @@ sub build_authorized_values_list {
#---- branch
if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) {
#Use GetBranches($onlymine)
my $onlymine=C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags} % 2 == 0 &&
C4::Context->userenv->{branch};
my $onlymine =
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
my $branches = GetBranches($onlymine);
my @branchloop;
foreach my $thisbranch ( sort keys %$branches ) {

View File

@ -756,10 +756,11 @@ my $i=0;
my $pref_itemcallnumber = C4::Context->preference('itemcallnumber');
my $onlymine = C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags}!=1 &&
C4::Context->userenv->{branch};
my $onlymine =
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
my $branch = $input->param('branch') || C4::Context->userenv->{branch};
my $branches = GetBranchesLoop($branch,$onlymine); # build once ahead of time, instead of multiple times later.

View File

@ -91,10 +91,11 @@ while (my ($itemtype, $description) =$req->fetchrow) {
itemtypename => $description,
};
}
my $onlymine=C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags} % 2 !=1 &&
C4::Context->userenv->{branch};
my $onlymine =
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
$branchfilter = C4::Context->userenv->{'branch'} if ($onlymine && !$branchfilter);

View File

@ -51,13 +51,13 @@ my $sql = q(
OR firstname LIKE ?
OR cardnumber LIKE ? )
);
if (C4::Context->preference("IndependentBranches")){
if ( C4::Context->userenv
&& (C4::Context->userenv->{flags} % 2) !=1
&& C4::Context->userenv->{'branch'}
){
$sql .= " AND borrowers.branchcode =" . $dbh->quote(C4::Context->userenv->{'branch'});
}
if ( C4::Context->preference("IndependentBranches")
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{'branch'} )
{
$sql .= " AND borrowers.branchcode ="
. $dbh->quote( C4::Context->userenv->{'branch'} );
}
$sql .= q( ORDER BY surname, firstname LIMIT 10);

View File

@ -68,7 +68,7 @@ if ($bor->{category_type} eq "S") {
if (C4::Context->preference("IndependentBranches")) {
my $userenv = C4::Context->userenv;
if (($userenv->{flags} % 2 != 1) && $bor->{'branchcode'}){
if ( !C4::Context->IsSuperLibrarian() && $bor->{'branchcode'}){
unless ($userenv->{branch} eq $bor->{'branchcode'}){
print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member&error=CANT_DELETE_OTHERLIBRARY");
exit;

View File

@ -313,7 +313,7 @@ if ($op eq 'save' || $op eq 'insert'){
}
if (C4::Context->preference("IndependentBranches")) {
if ($userenv && $userenv->{flags} % 2 != 1){
unless ( C4::Context->IsSuperLibrarian() ){
$debug and print STDERR " $newdata{'branchcode'} : ".$userenv->{flags}.":".$userenv->{branch};
unless (!$newdata{'branchcode'} || $userenv->{branch} eq $newdata{'branchcode'}){
push @errors, "ERROR_branch";
@ -450,7 +450,7 @@ if ($nok or !$nodouble){
}
if (C4::Context->preference("IndependentBranches")) {
my $userenv = C4::Context->userenv;
if ($userenv->{flags} % 2 != 1 && $data{'branchcode'}){
if ( !C4::Context->IsSuperLibrarian() && $data{'branchcode'} ) {
unless ($userenv->{branch} eq $data{'branchcode'}){
print $input->redirect("/cgi-bin/koha/members/members-home.pl");
exit;

View File

@ -213,11 +213,14 @@ $bor{'borrowernumber'} = $borrowernumber;
my $samebranch;
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
unless ( $userenv->{flags} % 2 == 1 ) {
if ( C4::Context->IsSuperLibrarian() ) {
$samebranch = 1;
}
else {
$samebranch = ( $data->{'branchcode'} eq $userenv->{branch} );
}
$samebranch = 1 if ( $userenv->{flags} % 2 == 1 );
}else{
}
else {
$samebranch = 1;
}
my $branchdetail = GetBranchDetail( $data->{'branchcode'});
@ -340,14 +343,17 @@ foreach (@$alerts) {
my $candeleteuser;
my $userenv = C4::Context->userenv;
if($userenv->{flags} % 2 == 1){
if ( C4::Context->IsSuperLibrarian() ) {
$candeleteuser = 1;
}elsif ( C4::Context->preference("IndependentBranches") ) {
}
elsif ( C4::Context->preference("IndependentBranches") ) {
$candeleteuser = ( $data->{'branchcode'} eq $userenv->{branch} );
}else{
if( C4::Auth::getuserflags( $userenv->{flags},$userenv->{number})->{borrowers} ) {
}
else {
if ( C4::Auth::getuserflags( $userenv->{flags}, $userenv->{number} )->{borrowers} ) {
$candeleteuser = 1;
}else{
}
else {
$candeleteuser = 0;
}
}

View File

@ -384,7 +384,7 @@ foreach my $biblionumber (@biblionumbers) {
if (! C4::Context->preference("canreservefromotherbranches")){
# cant reserve items so need to check if item homebranch and userenv branch match if not we cant reserve
my $userenv = C4::Context->userenv;
if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
unless ( C4::Context->IsSuperLibrarian ) {
$item->{cantreserve} = 1 if ( $item->{homebranch} ne $userenv->{branch} );
}
}

View File

@ -82,10 +82,13 @@ if (defined $member) {
my ($count,$results);
if (C4::Context->preference("IndependentBranches")){
if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
$$patron{branchcode}=C4::Context->userenv->{'branch'};
}
if ( C4::Context->preference("IndependentBranches") ) {
if ( C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{'branch'} )
{
$$patron{branchcode} = C4::Context->userenv->{'branch'};
}
}
$$patron{firstname}.="\%" if ($$patron{firstname});
$$patron{surname}.="\%" if ($$patron{surname});

View File

@ -125,16 +125,14 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') {
}
}
my $userenv = C4::Context->userenv;
my $onlymine =
C4::Context->preference('IndependentBranches')
&& $userenv
&& $userenv->{flags} % 2 != 1
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian
&& (
not C4::Auth::haspermission( $userenv->{id}, { serials => 'superserials' } )
not C4::Auth::haspermission( C4::Context->userenv->{id}, { serials => 'superserials' } )
)
&& $userenv->{branch};
&& C4::Context->userenv->{branch};
my $branches = GetBranches($onlymine);
my $branchloop;
for my $thisbranch (sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname} } keys %{$branches}) {

View File

@ -288,10 +288,11 @@ if(defined($returnsuggested) and $returnsuggested ne "noone")
#branch display management
my $branchfilter = ($displayby ne "branchcode") ? $input->param('branchcode') : '';
my $onlymine=C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags}!=1 &&
C4::Context->userenv->{branch};
my $onlymine =
C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch};
my $branches = GetBranches($onlymine);
my @branchloop;

View File

@ -120,13 +120,13 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
my $limit_ind_branch =
( C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !( C4::Context->userenv->{flags} & 1 )
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch} ) ? 1 : 0;
my $branch = $query->param("branch") || '';
if ( C4::Context->preference("IndependentBranches")
&& C4::Context->userenv
&& !( C4::Context->userenv->{flags} & 1 ) )
&& !C4::Context->IsSuperLibrarian() )
{
$branch = C4::Context->userenv->{'branch'};
}

View File

@ -57,10 +57,11 @@ $keydate =~ s/-/\//g;
my $branch= $input->param('branch') || C4::Context->userenv->{'branch'};
# Set all the branches.
my $onlymine=(C4::Context->preference('IndependentBranches') &&
C4::Context->userenv &&
C4::Context->userenv->{flags} % 2 !=1 &&
C4::Context->userenv->{branch}?1:0);
my $onlymine =
( C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
&& !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch} ? 1 : 0 );
if ( $onlymine ) {
$branch = C4::Context->userenv->{'branch'};
}