From 39329802992a0b8172bc8144a4edad5bd2c88719 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Wed, 15 May 2013 11:04:07 -0400
Subject: [PATCH] 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
---
C4/Circulation.pm | 2 +-
C4/Context.pm | 13 +++++++++++++
C4/Items.pm | 4 ++--
C4/Members.pm | 8 ++++----
C4/Serials.pm | 4 ++--
C4/Suggestions.pm | 6 +++---
cataloguing/addbiblio.pl | 9 +++++----
circ/overdue.pl | 9 +++++----
members/deletemem.pl | 2 +-
members/memberentry.pl | 4 ++--
members/moremember.pl | 22 ++++++++++++++--------
reserve/request.pl | 2 +-
serials/member-search.pl | 11 +++++++----
serials/subscription-add.pl | 9 +++++----
serials/subscription-detail.pl | 11 ++++++-----
tools/holidays.pl | 9 +++++----
16 files changed, 76 insertions(+), 49 deletions(-)
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index b775b4f..188c75b 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -895,7 +895,7 @@ sub CanBookBeIssued {
}
if ( C4::Context->preference("IndependentBranches") ) {
my $userenv = C4::Context->userenv;
- if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
+ if ( C4::Context->IsSuperLibrarian() ) {
if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} ){
$issuingimpossible{ITEMNOTSAMEBRANCH} = 1;
$issuingimpossible{'itemhomebranch'} = $item->{C4::Context->preference("HomeOrHoldingBranch")};
diff --git a/C4/Context.pm b/C4/Context.pm
index 739a0ec..6bebfaf 100644
--- a/C4/Context.pm
+++ b/C4/Context.pm
@@ -106,6 +106,7 @@ use C4::Debug;
use POSIX ();
use DateTime::TimeZone;
use Module::Load::Conditional qw(can_load);
+use Carp;
=head1 NAME
@@ -1235,6 +1236,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__
diff --git a/C4/Items.pm b/C4/Items.pm
index 4a98712..7e942c4 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1259,7 +1259,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});
}
}
@@ -2694,7 +2694,7 @@ sub PrepareItemrecordDisplay {
#---- branch
if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) {
if ( ( C4::Context->preference("IndependentBranches") )
- && ( 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, ""
diff --git a/C4/Members.pm b/C4/Members.pm
index 7ac69df..37bfb3f 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -259,7 +259,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;
@@ -2066,7 +2066,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}
: "");
@@ -2132,7 +2132,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}
: "");
@@ -2182,7 +2182,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}
: "");
diff --git a/C4/Serials.pm b/C4/Serials.pm
index a1ce054..08bf633 100644
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -489,7 +489,7 @@ sub GetSubscriptionsFromBiblionumber {
$subs->{'cannotedit'} =
( C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
- && C4::Context->userenv->{flags} % 2 != 1
+ && !C4::Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch}
&& $subs->{branchcode}
&& ( C4::Context->userenv->{branch} ne $subs->{branchcode} ) );
@@ -629,7 +629,7 @@ sub GetSubscriptions {
$line->{'cannotedit'} =
( C4::Context->preference('IndependentBranches')
&& C4::Context->userenv
- && C4::Context->userenv->{flags} % 2 != 1
+ && !C4:Context->IsSuperLibrarian()
&& C4::Context->userenv->{branch}
&& $line->{branchcode}
&& ( C4::Context->userenv->{branch} ne $line->{branchcode} ) );
diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
index bf7878b..51a3d38 100644
--- a/C4/Suggestions.pm
+++ b/C4/Suggestions.pm
@@ -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 ='') };
}
@@ -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(*)
diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl
index cb40396..986c989 100755
--- a/cataloguing/addbiblio.pl
+++ b/cataloguing/addbiblio.pl
@@ -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 ) {
diff --git a/circ/overdue.pl b/circ/overdue.pl
index 4343592..80a9d7f 100755
--- a/circ/overdue.pl
+++ b/circ/overdue.pl
@@ -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);
diff --git a/members/deletemem.pl b/members/deletemem.pl
index ef6ba01..7dca073 100755
--- a/members/deletemem.pl
+++ b/members/deletemem.pl
@@ -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;
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 0bf3122..4f8a3e3 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -293,7 +293,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";
@@ -424,7 +424,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;
diff --git a/members/moremember.pl b/members/moremember.pl
index 7ed96d3..152e603 100755
--- a/members/moremember.pl
+++ b/members/moremember.pl
@@ -214,11 +214,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'});
@@ -341,14 +344,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;
}
}
diff --git a/reserve/request.pl b/reserve/request.pl
index c411ce9..a735b0c 100755
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -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} );
}
}
diff --git a/serials/member-search.pl b/serials/member-search.pl
index 17c19d5..4fa88a0 100755
--- a/serials/member-search.pl
+++ b/serials/member-search.pl
@@ -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});
diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl
index 1959b23..29734d4 100755
--- a/serials/subscription-add.pl
+++ b/serials/subscription-add.pl
@@ -120,10 +120,11 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') {
}
}
-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};
my $branches = GetBranches($onlymine);
my $branchloop;
for my $thisbranch (sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname} } keys %{$branches}) {
diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl
index fc57d89..93ab614 100755
--- a/serials/subscription-detail.pl
+++ b/serials/subscription-detail.pl
@@ -162,11 +162,12 @@ $template->param(
routing => C4::Context->preference("RoutingSerials"),
totalissues => $totalissues,
hemisphere => $hemisphere,
- cannotedit =>(C4::Context->preference('IndependentBranches') &&
- C4::Context->userenv &&
- C4::Context->userenv->{flags} % 2 !=1 &&
- C4::Context->userenv->{branch} && $subs->{branchcode} &&
- (C4::Context->userenv->{branch} ne $subs->{branchcode})),
+ cannotedit => ( C4::Context->preference('IndependentBranches')
+ && C4::Context->userenv
+ && !C4::Context->IsSuperLibrarian()
+ && C4::Context->userenv->{branch}
+ && $subs->{branchcode}
+ && ( C4::Context->userenv->{branch} ne $subs->{branchcode} ) ),
'periodicity' . $subs->{periodicity} => 1,
'arrival' . $subs->{dow} => 1,
'numberpattern' . $subs->{numberpattern} => 1,
diff --git a/tools/holidays.pl b/tools/holidays.pl
index 38c889d..376de5f 100755
--- a/tools/holidays.pl
+++ b/tools/holidays.pl
@@ -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'};
}
--
1.7.2.5