From afb36c429eb77226e25d9d4fe91a7d6f9afe9ced Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 18 Mar 2008 17:49:34 -0500 Subject: [PATCH] refactor C4::Log::logaction This fix should resolve in whole or in part several bugs characterized by the error message 'Can't use string ("0") as a HASH ref while "strict refs" in use', including bugs 1101, 1899, and 1910. There are some possibilities for future work: [1] Dealing with an operator override, e.g., where a circ operator needs to get a supervisor to enter a login and password and escalate the original operator's privileges for a transaction, e.g., to forgive a fine. This is an enhancement, of course. [2] Creating a dummy operator to represent batch job runs; or alternatively, give each batch job an option to log its work under a specified user ID. Signed-off-by: Joshua Ferraro --- C4/Biblio.pm | 9 ++++----- C4/Circulation.pm | 4 ++-- C4/Items.pm | 13 ++++--------- C4/Letters.pm | 4 +--- C4/Log.pm | 17 +++++++++++++---- C4/Members.pm | 11 ++++------- C4/Overdues.pm | 1 - C4/Serials.pm | 12 ++++-------- 8 files changed, 32 insertions(+), 39 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 35de382894..1f6be62c8c 100755 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -239,8 +239,7 @@ sub AddBiblio { # now add the record $biblionumber = ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$biblionumber,"biblio") - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "ADD", $biblionumber, "biblio") if C4::Context->preference("CataloguingLog"); return ( $biblionumber, $biblioitemnumber ); } @@ -256,7 +255,7 @@ sub ModBiblio { my ( $record, $biblionumber, $frameworkcode ) = @_; if (C4::Context->preference("CataloguingLog")) { my $newrecord = GetMarcBiblio($biblionumber); - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","MODIFY",$biblionumber,"BEFORE=>".$newrecord->as_formatted); + logaction("CATALOGUING", "MODIFY", $biblionumber, "BEFORE=>".$newrecord->as_formatted); } my $dbh = C4::Context->dbh; @@ -388,8 +387,8 @@ sub DelBiblio { # from being generated by _koha_delete_biblioitems $error = _koha_delete_biblio( $dbh, $biblionumber ); - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","DELETE",$biblionumber,"") - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "DELETE", $biblionumber, "") if C4::Context->preference("CataloguingLog"); + return; } diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 1cd8202900..0add08eccf 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1026,7 +1026,7 @@ sub AddIssue { ); } - &logaction(C4::Context->userenv->{'number'},"CIRCULATION","ISSUE",$borrower->{'borrowernumber'},$biblio->{'biblionumber'}) + logaction("CIRCULATION", "ISSUE", $borrower->{'borrowernumber'}, $biblio->{'biblionumber'}) if C4::Context->preference("IssueLog"); return ($datedue); } @@ -1275,7 +1275,7 @@ sub AddReturn { $borrower->{'borrowernumber'} ); - &logaction(C4::Context->userenv->{'number'},"CIRCULATION","RETURN",$iteminformation->{borrowernumber},$iteminformation->{'biblionumber'}) + logaction("CIRCULATION", "RETURN", $iteminformation->{borrowernumber}, $iteminformation->{'biblionumber'}) if C4::Context->preference("ReturnLog"); #adding message if holdingbranch is non equal a userenv branch to return the document to homebranch diff --git a/C4/Items.pm b/C4/Items.pm index d0c0fb389a..a315e94392 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -231,8 +231,7 @@ sub AddItem { my $new_item_marc = _marc_from_item_hash($item, $frameworkcode, $unlinked_item_subfields); _add_item_field_to_biblio($new_item_marc, $item->{'biblionumber'}, $frameworkcode ); - logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$itemnumber,"item") - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "ADD", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); return ($item->{biblionumber}, $item->{biblioitemnumber}, $itemnumber); } @@ -327,8 +326,7 @@ sub AddItemBatchFromMarc { push @itemnumbers, $itemnumber; # FIXME not checking error $item->{'itemnumber'} = $itemnumber; - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$itemnumber,"item") - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "ADD", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); my $new_item_marc = _marc_from_item_hash($item, $frameworkcode, $unlinked_item_subfields); $item_field->replace_with($new_item_marc->field($itemtag)); @@ -444,10 +442,8 @@ sub ModItem { or die "FAILED _marc_from_item_hash($whole_item, $frameworkcode)"; _replace_item_field_in_biblio($new_item_marc, $biblionumber, $itemnumber, $frameworkcode); - (C4::Context->userenv eq '0') and die "userenv is '0', not hashref"; # logaction line would crash anyway ($new_item_marc eq '0') and die "$new_item_marc is '0', not hashref"; # logaction line would crash anyway - logaction(C4::Context->userenv->{'number'},"CATALOGUING","MODIFY",$itemnumber,$new_item_marc->as_formatted) - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "MODIFY", $itemnumber, $new_item_marc->as_formatted) if C4::Context->preference("CataloguingLog"); } =head2 ModItemTransfer @@ -537,8 +533,7 @@ sub DelItem { } } &ModBiblioMarc( $record, $biblionumber, $frameworkcode ); - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","DELETE",$itemnumber,"item") - if C4::Context->preference("CataloguingLog"); + logaction("CATALOGUING", "DELETE", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); } =head2 CheckItemPreSave diff --git a/C4/Letters.pm b/C4/Letters.pm index dffe2c202a..31033a8c6b 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -354,7 +354,6 @@ sub SendAlerts { } if ( C4::Context->preference("LetterLog") ) { logaction( - $userenv->{number}, "ACQUISITION", "Send Acquisition claim letter", "", @@ -419,8 +418,7 @@ sub SendAlerts { Message => "" . $innerletter->{content}, ); sendmail(%mail); - &logaction( - C4::Context->userenv->{'number'}, + logaction( "ACQUISITION", "CLAIM ISSUE", undef, diff --git a/C4/Log.pm b/C4/Log.pm index c7088c7edf..b4732acb70 100644 --- a/C4/Log.pm +++ b/C4/Log.pm @@ -52,16 +52,25 @@ The functions in this module perform various functions in order to log all the o =item logaction - &logaction($usernumber, $modulename, $actionname, $objectnumber, $infos); + &logaction($modulename, $actionname, $objectnumber, $infos); -Adds a record into action_logs table to report the different changes upon the database +Adds a record into action_logs table to report the different changes upon the database. +Each log entry includes the number of the user currently logged in. For batch +jobs, which operate without authenticating a user and setting up a session, the user +number is set to 0, which is the same as the superlibrarian's number. =cut #' sub logaction { - my ($usernumber,$modulename, $actionname, $objectnumber, $infos)=@_; - $usernumber='' unless $usernumber; + my ($modulename, $actionname, $objectnumber, $infos)=@_; + + # Get ID of logged in user. if called from a batch job, + # no user session exists and C4::Context->userenv() returns + # the scalar '0'. + my $userenv = C4::Context->userenv(); + my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : 0; + my $dbh = C4::Context->dbh; my $sth=$dbh->prepare("Insert into action_logs (timestamp,user,module,action,object,info) values (now(),?,?,?,?,?)"); $sth->execute($usernumber,$modulename,$actionname,$objectnumber,$infos); diff --git a/C4/Members.pm b/C4/Members.pm index 8d3861918b..f62baf77b4 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -633,7 +633,7 @@ sub ModMember { # is adult check guarantees; UpdateGuarantees(%data); } - &logaction(C4::Context->userenv->{'number'},"MEMBERS","MODIFY",$data{'borrowernumber'},"$query (executed w/ arg: $data{'borrowernumber'})") + logaction("MEMBERS", "MODIFY", $data{'borrowernumber'}, "$query (executed w/ arg: $data{'borrowernumber'})") if C4::Context->preference("BorrowersLog"); } @@ -729,8 +729,7 @@ sub AddMember { $data{'borrowernumber'} = $dbh->{'mysql_insertid'}; # unneeded w/ autoincrement ? # mysql_insertid is probably bad. not necessarily accurate and mysql-specific at best. - &logaction(C4::Context->userenv->{'number'},"MEMBERS","CREATE",$data{'borrowernumber'},"") - if C4::Context->preference("BorrowersLog"); + logaction("MEMBERS", "CREATE", $data{'borrowernumber'}, "") if C4::Context->preference("BorrowersLog"); # check for enrollment fee & add it if needed $sth = $dbh->prepare("SELECT enrolmentfee FROM categories WHERE categorycode=?"); @@ -783,8 +782,7 @@ sub changepassword { return 1; } - &logaction(C4::Context->userenv->{'number'},"MEMBERS","CHANGE PASS",$member,"") - if C4::Context->preference("BorrowersLog"); + logaction("MEMBERS", "CHANGE PASS", $member, "") if C4::Context->preference("BorrowersLog"); } @@ -1604,8 +1602,7 @@ sub DelMember { $sth = $dbh->prepare($query); $sth->execute($borrowernumber); $sth->finish; - &logaction(C4::Context->userenv->{'number'},"MEMBERS","DELETE",$borrowernumber,"") - if C4::Context->preference("BorrowersLog"); + logaction("MEMBERS", "DELETE", $borrowernumber, "") if C4::Context->preference("BorrowersLog"); return $sth->rows; } diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 9db5c6df7a..0457090c1d 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -507,7 +507,6 @@ sub UpdateFine { } # logging action &logaction( - C4::Context->userenv->{'number'}, "FINES", $type, $borrowernumber, diff --git a/C4/Serials.pm b/C4/Serials.pm index dce2c611d7..d86f90b3e3 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -1268,8 +1268,7 @@ sub ModSubscription { my $rows=$sth->rows; $sth->finish; - &logaction(C4::Context->userenv->{'number'},"SERIAL","MODIFY",$subscriptionid,"") - if C4::Context->preference("SubscriptionLog"); + logaction("SERIAL", "MODIFY", $subscriptionid, "") if C4::Context->preference("SubscriptionLog"); return $rows; } @@ -1382,8 +1381,7 @@ sub NewSubscription { format_date_in_iso($startdate) ); - &logaction(C4::Context->userenv->{'number'},"SERIAL","ADD",$subscriptionid,"") - if C4::Context->preference("SubscriptionLog"); + logaction("SERIAL", "ADD", $subscriptionid, "") if C4::Context->preference("SubscriptionLog"); #set serial flag on biblio if not already set. my ($null, ($bib)) = GetBiblio($biblionumber); @@ -1444,8 +1442,7 @@ sub ReNewSubscription { $sth->execute( format_date_in_iso($startdate), $numberlength, $weeklength, $monthlength, $subscriptionid ); - &logaction(C4::Context->userenv->{'number'},"SERIAL","RENEW",$subscriptionid,"") - if C4::Context->preference("SubscriptionLog"); + logaction("SERIAL", "RENEW", $subscriptionid, "") if C4::Context->preference("SubscriptionLog"); } =head2 NewIssue @@ -1797,8 +1794,7 @@ sub DelSubscription { "DELETE FROM subscriptionhistory WHERE subscriptionid=$subscriptionid"); $dbh->do("DELETE FROM serial WHERE subscriptionid=$subscriptionid"); - &logaction(C4::Context->userenv->{'number'},"SERIAL","DELETE",$subscriptionid,"") - if C4::Context->preference("SubscriptionLog"); + logaction("SERIAL", "DELETE", $subscriptionid, "") if C4::Context->preference("SubscriptionLog"); } =head2 DelIssue -- 2.39.2