Bug 36526: Remove circular dependency from Koha::Objects

Koha::Objects depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPrefs, which depends on Koha::Objects

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Objects (YAML::XS and ZOOM for instance).
As Koha::Objects is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch removes uses of Koha::DateUtils from Koha::Objects.
It was only used in Koha::Objects::filter_by_last_update

filter_by_last_update now requires that the 'from' and 'to' parameters
must be DateTime objects. Previously it would also allow date and
datetime strings. This possibility was only used in two places:
* misc/cronjobs/cleanup_database.pl
* tools/cleanborrowers.pl

Now they call dt_from_string first and pass a DateTime object to
filter_by_last_update

Test plan:
1. Run `perl -cw Koha/Objects.pm`. It should only say:
   "Koha/Objects.pm syntax OK" without warnings
2. Run `prove t/db_dependent/Koha/Objects.t`
3. Verify that misc/cronjobs/cleanup_database.pl works as before,
   especially with the options --pseudo-transactions,
   --pseudo-transactions-from and --pseudo-transactions-to
4. Go to Tools » Batch patron deletion and anonymization, check "Verify
   you want to anonymize patron checkout history" and enter a date in
   the text input below. Then click Next and verify that the correct
   count of borrowers is shown. Click on the "Finish" button and verify
   that the circulation history has been correctly anonymized

See also bug 36432

Signed-off-by: Tadeusz Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 675c8263b7)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Julian Maurice 2024-04-04 16:26:00 +02:00 committed by Fridolin Somers
parent bdb4f0c1bb
commit aace5d0990
4 changed files with 26 additions and 34 deletions

View file

@ -25,7 +25,6 @@ use Class::Inspector;
use Koha::Database;
use Koha::Exceptions::Object;
use Koha::DateUtils qw( dt_from_string );
=head1 NAME
@ -233,7 +232,7 @@ sub update {
=head3 filter_by_last_update
my $filtered_objects = $objects->filter_by_last_update({
from => $date1, to => $date2,
from => $from_datetime, to => $to_datetime,
days|older_than => $days, min_days => $days, younger_than => $days,
});
@ -243,7 +242,7 @@ to get meaningful results.
Note: from, to and min_days are inclusive! And by nature days|older_than
and younger_than are exclusive.
The from and to parameters can be DateTime objects or date strings.
The from and to parameters must be DateTime objects.
=cut
@ -254,21 +253,24 @@ sub filter_by_last_update {
Koha::Exceptions::MissingParameter->throw("Please pass: days|from|to|older_than|younger_than")
unless grep { exists $params->{$_} } qw/days from to older_than younger_than min_days/;
foreach my $key (qw(from to)) {
if (exists $params->{$key} and ref $params->{$key} ne 'DateTime') {
Koha::Exceptions::WrongParameter->throw("'$key' parameter must be a DateTime object");
}
}
my $dtf = Koha::Database->new->schema->storage->datetime_parser;
foreach my $p ( qw/days older_than younger_than min_days/ ) {
next if !exists $params->{$p};
my $dt = Koha::DateUtils::dt_from_string();
my $days = $params->{$p};
my $operator = { days => '<', older_than => '<', min_days => '<=' }->{$p} // '>';
$dt->subtract( days => $params->{$p} )->truncate( to => 'day' );
$conditions->{$operator} = $dtf->format_datetime( $dt );
$conditions->{$operator} = \['DATE_SUB(CURDATE(), INTERVAL ? DAY)', $days];
}
if ( exists $params->{from} ) {
my $from = ref($params->{from}) ? $params->{from} : dt_from_string($params->{from});
$conditions->{'>='} = $dtf->format_datetime( $from );
$conditions->{'>='} = $dtf->format_datetime( $params->{from} );
}
if ( exists $params->{to} ) {
my $to = ref($params->{to}) ? $params->{to} : dt_from_string($params->{to});
$conditions->{'<='} = $dtf->format_datetime( $to );
$conditions->{'<='} = $dtf->format_datetime( $params->{to} );
}
return $self->search(

View file

@ -701,6 +701,9 @@ if ( defined $pPseudoTransactions or $pPseudoTransactionsFrom or $pPseudoTransac
$pPseudoTransactionsTo->set( hour => 23, minute => 59, second => 59 );
}
}
if ($pPseudoTransactionsFrom) {
$pPseudoTransactionsFrom = dt_from_string($pPseudoTransactionsFrom);
}
my $anonymized_transactions = Koha::PseudonymizedTransactions->filter_by_last_update(
{
timestamp_column_name => 'datetime',

View file

@ -1162,31 +1162,16 @@ subtest "filter_by_last_update" => sub {
)->count;
is( $count, 3, '3 patrons have been updated between D-4 and D-2' );
t::lib::Mocks::mock_preference( 'dateformat', 'metric' );
try {
throws_ok {
$count = $patrons->filter_by_last_update(
{ timestamp_column_name => 'updated_on', from => '1970-12-31' } )
->count;
}
catch {
ok(
$_->isa(
'No exception raised, from and to parameters can take an iso formatted date'
)
);
};
try {
} 'Koha::Exceptions::WrongParameter', 'from parameter must be a DateTime object';
throws_ok {
$count = $patrons->filter_by_last_update(
{ timestamp_column_name => 'updated_on', from => '31/12/1970' } )
{ timestamp_column_name => 'updated_on', to => '1970-12-31' } )
->count;
}
catch {
ok(
$_->isa(
'No exception raised, from and to parameters can take an metric formatted date (depending on dateformat syspref)'
)
);
};
} 'Koha::Exceptions::WrongParameter', 'to parameter must be a DateTime object';
subtest 'Parameters older_than, younger_than' => sub {
my $now = dt_from_string();

View file

@ -37,6 +37,8 @@ use CGI qw ( -utf8 );
use C4::Auth qw( get_template_and_user );
use C4::Output qw( output_html_with_http_headers );
use C4::Members qw( GetBorrowersToExpunge );
use Koha::DateUtils qw( dt_from_string );
use Koha::Old::Checkouts;
use Koha::Patron::Categories;
use Koha::Patrons;
@ -148,7 +150,7 @@ elsif ( $step == 3 ) {
my $rows = Koha::Old::Checkouts
->filter_by_anonymizable
->filter_by_last_update({
to => $last_issue_date, timestamp_column_name => 'returndate' })
to => dt_from_string($last_issue_date), timestamp_column_name => 'returndate' })
->anonymize;
$template->param(