gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 23632: Remove C4::Logs::GetLogs 

Koha::ActionLogs->search must be used instead.
There is no call to this subroutine in our code, it should be removed.

Test plan:
Make sure the 3 test files still return green and that there is no more
occurrences of GetLogs in the codebase.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2020-08-06 11:28:53 +02:00
parent c933e25b6f
commit 807147fb7b
4 changed files with 49 additions and 185 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
C4/Log.pm
View File

@ -35,7 +35,7 @@ use vars qw(@ISA @EXPORT);
BEGIN {
require Exporter;
@ISA = qw(Exporter);
@EXPORT = qw(&logaction &cronlogaction &GetLogs);
@EXPORT = qw(&logaction &cronlogaction);
}
=head1 NAME
@ -120,81 +120,6 @@ sub cronlogaction {
logaction( 'CRONJOBS', 'Run', undef, $loginfo ) if C4::Context->preference('CronjobLog');
}
=item GetLogs
$logs = GetLogs($datefrom,$dateto,$user,\@modules,$action,$object,$info,$interfaces);
Return:
C<$logs> is a ref to a hash which contains all columns from action_logs
=cut
sub GetLogs {
my $datefrom = shift;
my $dateto = shift;
my $user = shift;
my $modules = shift;
my $action = shift;
my $object = shift;
my $info = shift;
my $interfaces = shift;
my $iso_datefrom = $datefrom ? output_pref({ dt => dt_from_string( $datefrom ), dateformat => 'iso', dateonly => 1 }) : undef;
my $iso_dateto = $dateto ? output_pref({ dt => dt_from_string( $dateto ), dateformat => 'iso', dateonly => 1 }) : undef;
$user ||= q{};
my $dbh = C4::Context->dbh;
my $query = "
SELECT *
FROM action_logs
WHERE 1
";
my @parameters;
$query .=
" AND DATE_FORMAT(timestamp, '%Y-%m-%d') >= \"" . $iso_datefrom . "\" "
if $iso_datefrom; #fix me - mysql specific
$query .=
" AND DATE_FORMAT(timestamp, '%Y-%m-%d') <= \"" . $iso_dateto . "\" "
if $iso_dateto;
if ( $user ne q{} ) {
$query .= " AND user = ? ";
push( @parameters, $user );
}
if ( $modules && scalar(@$modules) ) {
$query .=
" AND module IN (" . join( ",", map { "?" } @$modules ) . ") ";
push( @parameters, @$modules );
}
if ( $action && scalar(@$action) ) {
$query .= " AND action IN (" . join( ",", map { "?" } @$action ) . ") ";
push( @parameters, @$action );
}
if ($object) {
$query .= " AND object = ? ";
push( @parameters, $object );
}
if ($info) {
$query .= " AND info LIKE ? ";
push( @parameters, "%" . $info . "%" );
}
if ( $interfaces && scalar(@$interfaces) ) {
$query .=
" AND interface IN (" . join( ",", map { "?" } @$interfaces ) . ") ";
push( @parameters, @$interfaces );
}
my $sth = $dbh->prepare($query);
$sth->execute(@parameters);
my @logs;
while ( my $row = $sth->fetchrow_hashref ) {
push @logs, $row;
}
return \@logs;
}
1;
__END__

19
t/db_dependent/Illrequest/Logger.t
View File

@ -27,28 +27,9 @@ use t::lib::Mocks;
my $schema = Koha::Database->new->schema;
# A mock response from C4::Log::GetLogs()
my $logs = [
{
info => '{"log_origin": "core"}',
action => 'STATUS_CHANGE',
timestamp => '2018-10-02 11:12:22'
},
{
info => '{"log_origin": "core"}',
action => 'STATUS_CHANGE',
timestamp => '2018-10-02 11:12:12'
},
{
info => '{"log_origin": "core"}',
action => 'STATUS_CHANGE',
timestamp => '2018-10-02 11:12:32'
}
];
# Mock the modules we use
my $c4_log = Test::MockModule->new('C4::Log');
$c4_log->mock('logaction', sub { 1 });
$c4_log->mock('GetLogs', sub { return $logs; });
my $c4_tpl = Test::MockModule->new('C4::Templates');
$c4_tpl->mock('_get_template_file',
sub { return ('htdocs', 'theme', 'lang', 'base/'); });

4
t/db_dependent/Koha/Patron/Messages.t
View File

@ -22,7 +22,7 @@ use Modern::Perl;
use Test::More tests => 12;
use C4::Context;
use C4::Log;
use Koha::ActionLogs;
use Koha::Patron::Message;
use Koha::Patron::Messages;
use Koha::Patrons;
@ -102,6 +102,6 @@ is( get_nb_of_logactions(), $nb_of_logaction + 3, 'With BorrowersLog on, 1 new l
$schema->storage->txn_rollback;
sub get_nb_of_logactions {
return scalar( @{ C4::Log::GetLogs( undef, undef, undef, ['MEMBERS'] ) } );
return Koha::ActionLogs->search({ module => 'MEMBERS' })->count;
}

134
t/db_dependent/Log.t
View File

@ -15,13 +15,14 @@
# with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Test::More tests => 5;
use Test::More tests => 3;
use C4::Context;
use C4::Log;
use C4::Auth qw/checkpw/;
use Koha::Database;
use Koha::DateUtils;
use Koha::ActionLogs;
use t::lib::Mocks qw/mock_preference/; # to mock CronjobLog
use t::lib::TestBuilder;
@ -32,7 +33,7 @@ $schema->storage->txn_begin;
our $dbh = C4::Context->dbh;
subtest 'Existing tests' => sub {
plan tests => 6;
plan tests => 3;
my $success;
eval {
@ -45,33 +46,6 @@ subtest 'Existing tests' => sub {
};
ok($success, "logaction seemed to work");
eval {
# FIXME: US formatted date hardcoded into test for now
$success = scalar(@{GetLogs("","","",undef,undef,"","")});
} or do {
diag($@);
$success = 0;
};
ok($success, "GetLogs returns results for an open search");
eval {
# FIXME: US formatted date hardcoded into test for now
my $date = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } );
$success = scalar(@{GetLogs( $date, $date, "", undef, undef, "", "") } );
} or do {
diag($@);
$success = 0;
};
ok($success, "GetLogs accepts dates in an All-matching search");
eval {
$success = scalar(@{GetLogs("","","",["MEMBERS"],["MODIFY"],1,"")});
} or do {
diag($@);
$success = 0;
};
ok($success, "GetLogs seemed to find ".$success." like our test record in a tighter search");
# We want numbers to be the same between runs.
$dbh->do("DELETE FROM action_logs;");
@ -86,22 +60,6 @@ subtest 'Existing tests' => sub {
is($cronJobCount,1,"Cronjob logged as expected.");
};
subtest "GetLogs should return all logs if dates are not set" => sub {
plan tests => 2;
my $today = dt_from_string->add(minutes => -1);
my $yesterday = dt_from_string->add( days => -1 );
$dbh->do(q|
INSERT INTO action_logs (timestamp, user, module, action, object, info)
VALUES
(?, 42, 'CATALOGUING', 'MODIFY', 4242, 'Record 42 has been modified by patron 4242 yesterday'),
(?, 43, 'CATALOGUING', 'MODIFY', 4242, 'Record 43 has been modified by patron 4242 today')
|, undef, output_pref({dt =>$yesterday, dateformat => 'iso'}), output_pref({dt => $today, dateformat => 'iso'}));
my $logs = GetLogs( undef, undef, undef, ['CATALOGUING'], ['MODIFY'], 4242 );
is( scalar(@$logs), 2, 'GetLogs should return all logs regardless the dates' );
$logs = GetLogs( output_pref($today), undef, undef, ['CATALOGUING'], ['MODIFY'], 4242 );
is( scalar(@$logs), 1, 'GetLogs should return the logs for today' );
};
subtest 'logaction(): interface is correctly logged' => sub {
plan tests => 4;
@ -110,56 +68,29 @@ subtest 'logaction(): interface is correctly logged' => sub {
$dbh->do("DELETE FROM action_logs;");
C4::Context->interface( 'commandline' );
logaction( "MEMBERS", "MODIFY", 1, "test operation");
my $logs = GetLogs();
is( @{$logs}[0]->{ interface }, 'commandline', 'Interface correctly deduced (commandline)');
my $log = Koha::ActionLogs->search->next;
is( $log->interface, 'commandline', 'Interface correctly deduced (commandline)');
# No interface passed, using C4::Context->interface
$dbh->do("DELETE FROM action_logs;");
C4::Context->interface( 'opac' );
logaction( "MEMBERS", "MODIFY", 1, "test operation");
$logs = GetLogs();
is( @{$logs}[0]->{ interface }, 'opac', 'Interface correctly deduced (opac)');
$log = Koha::ActionLogs->search->next;
is( $log->interface, 'opac', 'Interface correctly deduced (opac)');
# Explicit interfaces
$dbh->do("DELETE FROM action_logs;");
C4::Context->interface( 'intranet' );
logaction( "MEMBERS", "MODIFY", 1, 'test info', 'intranet');
$logs = GetLogs();
is( @{$logs}[0]->{ interface }, 'intranet', 'Passed interface is respected (intranet)');
$log = Koha::ActionLogs->search->next;
is( $log->interface, 'intranet', 'Passed interface is respected (intranet)');
# Explicit interfaces
$dbh->do("DELETE FROM action_logs;");
C4::Context->interface( 'sip' );
logaction( "MEMBERS", "MODIFY", 1, 'test info', 'sip');
$logs = GetLogs();
is( @{$logs}[0]->{ interface }, 'sip', 'Passed interface is respected (sip)');
};
subtest 'GetLogs() respects interface filters' => sub {
plan tests => 5;
$dbh->do("DELETE FROM action_logs;");
logaction( 'MEMBERS', 'MODIFY', 1, 'opac info', 'opac');
logaction( 'MEMBERS', 'MODIFY', 1, 'sip info', 'sip');
logaction( 'MEMBERS', 'MODIFY', 1, 'intranet info', 'intranet');
logaction( 'MEMBERS', 'MODIFY', 1, 'commandline info', 'commandline');
my $logs = scalar @{ GetLogs() };
is( $logs, 4, 'If no filter on interfaces is passed, all logs are returned');
$logs = GetLogs(undef,undef,undef,undef,undef,undef,undef,['opac']);
is( @{$logs}[0]->{ interface }, 'opac', 'Interface correctly filtered (opac)');
$logs = GetLogs(undef,undef,undef,undef,undef,undef,undef,['sip']);
is( @{$logs}[0]->{ interface }, 'sip', 'Interface correctly filtered (sip)');
$logs = GetLogs(undef,undef,undef,undef,undef,undef,undef,['intranet']);
is( @{$logs}[0]->{ interface }, 'intranet', 'Interface correctly filtered (intranet)');
$logs = GetLogs(undef,undef,undef,undef,undef,undef,undef,['commandline']);
is( @{$logs}[0]->{ interface }, 'commandline', 'Interface correctly filtered (commandline)');
$log = Koha::ActionLogs->search->next;
is( $log->interface, 'sip', 'Passed interface is respected (sip)');
};
subtest 'GDPR logging' => sub {
@ -170,8 +101,15 @@ subtest 'GDPR logging' => sub {
t::lib::Mocks::mock_userenv({ patron => $patron });
logaction( 'AUTH', 'FAILURE', $patron->id, '', 'opac' );
my $logs = GetLogs( undef, undef, $patron->id, ['AUTH'], ['FAILURE'], $patron->id );
is( @$logs, 1, 'We should find one auth failure' );
my $logs = Koha::ActionLogs->search(
{
user => $patron->id,
module => 'AUTH',
action => 'FAILURE',
object => $patron->id,
}
);
is( $logs->count, 1, 'We should find one auth failure' );
t::lib::Mocks::mock_preference('AuthFailureLog', 1);
my $strong_password = 'N0tStr0ngAnyM0reN0w:)';
@ -179,18 +117,38 @@ subtest 'GDPR logging' => sub {
my @ret = checkpw( $dbh, $patron->userid, 'WrongPassword', undef, undef, 1);
is( $ret[0], 0, 'Authentication failed' );
# Look for auth failure but NOT on patron id, pass userid in info parameter
$logs = GetLogs( undef, undef, 0, ['AUTH'], ['FAILURE'], undef, $patron->userid );
is( @$logs, 1, 'We should find one auth failure with this userid' );
$logs = Koha::ActionLogs->search(
{
module => 'AUTH',
action => 'FAILURE',
info => { -like => '%'.$patron->userid.'%' },
}
);
is( $logs->count, 1, 'We should find one auth failure with this userid' );
t::lib::Mocks::mock_preference('AuthFailureLog', 0);
@ret = checkpw( $dbh, $patron->userid, 'WrongPassword', undef, undef, 1);
$logs = GetLogs( undef, undef, 0, ['AUTH'], ['FAILURE'], undef, $patron->userid );
is( @$logs, 1, 'Still only one failure with this userid' );
$logs = Koha::ActionLogs->search(
{
module => 'AUTH',
action => 'FAILURE',
info => { -like => '%'.$patron->userid.'%' },
}
);
is( $logs->count, 1, 'Still only one failure with this userid' );
t::lib::Mocks::mock_preference('AuthSuccessLog', 1);
@ret = checkpw( $dbh, $patron->userid, $strong_password, undef, undef, 1);
is( $ret[0], 1, 'Authentication succeeded' );
# Now we can look for patron id
$logs = GetLogs( undef, undef, $patron->id, ['AUTH'], ['SUCCESS'], $patron->id );
is( @$logs, 1, 'We expect only one auth success line for this patron' );
$logs = Koha::ActionLogs->search(
{
user => $patron->id,
module => 'AUTH',
action => 'SUCCESS',
object => $patron->id,
}
);
is( $logs->count, 1, 'We expect only one auth success line for this patron' );
};
$schema->storage->txn_rollback;